Skip to content

Commit 1ec3ba6

Browse files
pete-woodspete-woods
authored
Merge pull request #1237 from CircleCI-Public/config-atomic
Make config read/write atomic
2 parents 0e8ef33 + a08b2b0 commit 1ec3ba6

11 files changed

Lines changed: 263 additions & 69 deletions

File tree

‎Taskfile.yml‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ vars:
3030
tasks:
3131
build:
3232
desc: Build the circleci binary
33+
vars:
34+
DEST: '{{default "dist/circleci" .DEST}}'
3335
cmds:
34-
- go build -o dist/circleci .
36+
- go build -o "{{.DEST}}" .
3537

3638
test:
3739
desc: Run unit tests
@@ -112,10 +114,13 @@ tasks:
112114

113115
dev-install:
114116
desc: Build from source and install to ~/.local/bin
117+
vars:
118+
DEST: '${HOME}/.local/bin'
115119
cmds:
120+
- mkdir -p "{{.DEST}}"
116121
- task: build
117-
- mkdir -p ${HOME}/.local/bin
118-
- cp dist/circleci ${HOME}/.local/bin
122+
vars:
123+
DEST: '{{.DEST}}/circleci'
119124

120125
generate:
121126
desc: Run go generate

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ require (
1111
github.com/MakeNowJust/heredoc v1.0.0
1212
github.com/go-chi/chi/v5 v5.2.5
1313
github.com/go-chi/render v1.0.3
14+
github.com/gofrs/flock v0.13.0
1415
github.com/hashicorp/go-retryablehttp v0.7.8
1516
github.com/njayp/ophis v1.1.4
1617
github.com/spf13/cobra v1.10.2
@@ -108,7 +109,6 @@ require (
108109
github.com/gobwas/glob v0.2.3 // indirect
109110
github.com/godbus/dbus/v5 v5.2.2 // indirect
110111
github.com/godoc-lint/godoc-lint v0.11.2 // indirect
111-
github.com/gofrs/flock v0.13.0 // indirect
112112
github.com/golang/protobuf v1.5.3 // indirect
113113
github.com/golangci/asciicheck v0.5.0 // indirect
114114
github.com/golangci/dupl v0.0.0-20250308024227-f665c8d69b32 // indirect

‎internal/closer/closer.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2026 Circle Internet Services, Inc.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a copy
4+
// of this software and associated documentation files (the "Software"), to deal
5+
// in the Software without restriction, including without limitation the rights
6+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
7+
// copies of the Software, and to permit persons to whom the Software is
8+
// furnished to do so, subject to the following conditions:
9+
//
10+
// The above copyright notice and this permission notice shall be included in
11+
// all copies or substantial portions of the Software.
12+
//
13+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
14+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
15+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
16+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
17+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
18+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
19+
// SOFTWARE.
20+
//
21+
// SPDX-License-Identifier: MIT
22+
23+
package closer
24+
25+
import "io"
26+
27+
// ErrorHandler closes an io.Closer with error handling. If there's an
28+
// error during Close when `in` is `nil`, sets `in` to the value of
29+
// the Close error.
30+
func ErrorHandler(c io.Closer, in *error) {
31+
cerr := c.Close()
32+
if *in == nil {
33+
*in = cerr
34+
}
35+
}

‎internal/closer/closer_test.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) 2026 Circle Internet Services, Inc.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a copy
4+
// of this software and associated documentation files (the "Software"), to deal
5+
// in the Software without restriction, including without limitation the rights
6+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
7+
// copies of the Software, and to permit persons to whom the Software is
8+
// furnished to do so, subject to the following conditions:
9+
//
10+
// The above copyright notice and this permission notice shall be included in
11+
// all copies or substantial portions of the Software.
12+
//
13+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
14+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
15+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
16+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
17+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
18+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
19+
// SOFTWARE.
20+
//
21+
// SPDX-License-Identifier: MIT
22+
23+
package closer
24+
25+
import (
26+
"errors"
27+
"testing"
28+
29+
"gotest.tools/v3/assert"
30+
"gotest.tools/v3/assert/cmp"
31+
)
32+
33+
func TestErrorHandler(t *testing.T) {
34+
t.Run("with error", func(t *testing.T) {
35+
var errorSentinel = errors.New("error sentinel")
36+
37+
called := false
38+
closer := func() error {
39+
called = true
40+
return errorSentinel
41+
}
42+
var err error
43+
ErrorHandler(closerFunc(closer), &err)
44+
assert.Check(t, called)
45+
assert.Check(t, cmp.ErrorIs(err, errorSentinel))
46+
})
47+
48+
t.Run("no error", func(t *testing.T) {
49+
called := false
50+
closer := func() error {
51+
called = true
52+
return nil
53+
}
54+
var err error
55+
ErrorHandler(closerFunc(closer), &err)
56+
assert.Check(t, called)
57+
assert.Check(t, err)
58+
})
59+
}
60+
61+
type closerFunc func() error
62+
63+
func (f closerFunc) Close() error {
64+
return f()
65+
}

‎internal/cmd/cmdauth/logout.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,8 @@ func newLogoutCmd() *cobra.Command {
5252
}
5353

5454
func runLogout(ctx context.Context, secureStorage bool) error {
55-
cfg, err := config.Load(ctx, secureStorage)
55+
err := config.SetToken(ctx, "", secureStorage)
5656
if err != nil {
57-
return clierrors.New("settings.load_failed", "Failed to load settings", err.Error()).
58-
WithExitCode(clierrors.ExitGeneralError)
59-
}
60-
61-
cfg.Token = ""
62-
63-
if err := config.Save(ctx, cfg, secureStorage); err != nil {
6457
return clierrors.New("settings.save_failed", "Failed to save settings", err.Error()).
6558
WithExitCode(clierrors.ExitGeneralError)
6659
}

‎internal/cmd/cmdauth/token.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,6 @@ func newTokenCmd() *cobra.Command {
5555
}
5656

5757
func runToken(ctx context.Context, secureStorage bool) error {
58-
cfg, err := config.Load(ctx, secureStorage)
59-
if err != nil {
60-
return clierrors.New("settings.load_failed", "Failed to load settings", err.Error()).
61-
WithExitCode(clierrors.ExitGeneralError)
62-
}
63-
6458
p := tea.NewProgram(ui.NewTokenModel(),
6559
tea.WithContext(ctx),
6660
tea.WithInput(iostream.In(ctx)),
@@ -76,8 +70,8 @@ func runToken(ctx context.Context, secureStorage bool) error {
7670
return nil
7771
}
7872

79-
cfg.Token = m.Token()
80-
if err := config.Save(ctx, cfg, secureStorage); err != nil {
73+
err = config.SetToken(ctx, m.Token(), secureStorage)
74+
if err != nil {
8175
return clierrors.New("settings.save_failed", "Failed to save settings", err.Error()).
8276
WithExitCode(clierrors.ExitGeneralError)
8377
}

‎internal/cmd/settings/list.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ package settings
2525
import (
2626
"context"
2727
"fmt"
28+
"strings"
2829

2930
"github.com/MakeNowJust/heredoc"
3031
"github.com/spf13/cobra"
@@ -33,6 +34,7 @@ import (
3334
"github.com/CircleCI-Public/circleci-cli-v2/internal/config"
3435
clierrors "github.com/CircleCI-Public/circleci-cli-v2/internal/errors"
3536
"github.com/CircleCI-Public/circleci-cli-v2/internal/iostream"
37+
"github.com/CircleCI-Public/circleci-cli-v2/internal/mdtable"
3638
)
3739

3840
func newListCmd() *cobra.Command {
@@ -89,9 +91,15 @@ func runList(ctx context.Context, secureStorage bool, jsonOut bool) error {
8991
return cmdutil.WriteJSON(iostream.Out(ctx), out)
9092
}
9193

92-
iostream.Printf(ctx, "Config file: %s\n\n", path)
93-
iostream.Printf(ctx, "%-10s %s\n", "token", maskToken(cfg.EffectiveToken()))
94-
iostream.Printf(ctx, "%-10s %s\n", "host", cfg.EffectiveHost())
94+
var md strings.Builder
95+
_, _ = fmt.Fprintf(&md, "# Settings\n")
96+
_, _ = fmt.Fprintf(&md, "- Path: %s\n", path)
97+
_, _ = fmt.Fprintf(&md, "- Keyring: %t\n\n", secureStorage)
98+
table := mdtable.New("Name", "Value")
99+
table.Row("host", cfg.EffectiveHost())
100+
table.Row("token", maskToken(cfg.EffectiveToken()))
101+
md.WriteString(table.Render() + "\n")
102+
iostream.PrintMarkdown(ctx, md.String())
95103
return nil
96104
}
97105

‎internal/cmd/settings/set.go‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,25 +79,18 @@ func newSetCmd() *cobra.Command {
7979
return cmd
8080
}
8181

82-
func runSet(ctx context.Context, secureStorage bool, key, value string) error {
83-
cfg, err := config.Load(ctx, secureStorage)
84-
if err != nil {
85-
return clierrors.New("settings.load_failed", "Failed to load settings", err.Error()).
86-
WithExitCode(clierrors.ExitGeneralError)
87-
}
88-
82+
func runSet(ctx context.Context, secureStorage bool, key, value string) (err error) {
8983
switch key {
9084
case "token":
91-
cfg.Token = value
85+
err = config.SetToken(ctx, value, secureStorage)
9286
case "host":
93-
cfg.Host = value
87+
err = config.SetHost(ctx, value, secureStorage)
9488
default:
9589
return clierrors.New("settings.unknown_key", "Unknown setting", "Unknown setting key: "+key).
9690
WithSuggestions("Valid keys are: token, host").
9791
WithExitCode(clierrors.ExitBadArguments)
9892
}
99-
100-
if err := config.Save(ctx, cfg, secureStorage); err != nil {
93+
if err != nil {
10194
return clierrors.New("settings.save_failed", "Failed to save settings", err.Error()).
10295
WithExitCode(clierrors.ExitGeneralError)
10396
}

‎internal/cmdutil/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func IsSecureStorage(cmd *cobra.Command) bool {
5959
// Honors a --config path set by the root PersistentPreRunE via WithConfigPath.
6060
func LoadClient(ctx context.Context, cmd *cobra.Command) (*apiclient.Client, error) {
6161
configPath, _ := ctx.Value(configPathKey{}).(string)
62-
cfg, err := config.LoadFrom(configPath, ctx, IsSecureStorage(cmd))
62+
cfg, err := config.LoadFrom(ctx, configPath, IsSecureStorage(cmd))
6363
if err != nil {
6464
return nil, clierrors.New("config.load_failed", "Failed to load config", err.Error()).
6565
WithExitCode(clierrors.ExitGeneralError)

0 commit comments

Comments
 (0)