Skip to content

Commit 6307920

Browse files
fix: merge provider credentials instead of overwriting (#163)
## Summary - The `writeCredentials` method in `internal/juju/juju.go` was replacing the entire `credentials["credentials"]` map on each loop iteration, rather than merging into it. - When multiple providers had credentials, only the last provider's credentials survived in the resulting `credentials.yaml`. - This fix retrieves the existing inner map via a type assertion and sets each provider's credentials as a new key, preserving all providers' credentials. Somewhat academic with only Google credentials right now, but a bug lying in wait, so might as well fix it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) but owned by me. --------- Co-authored-by: Claude Code <noreply@anthropic.com>
1 parent 86b1b21 commit 6307920

File tree

2 files changed

+89
-5
lines changed

2 files changed

+89
-5
lines changed

internal/juju/juju.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ func (j *JujuHandler) install() error {
127127
// writeCredentials iterates over any provided cloud credentials and authors Juju's
128128
// credentials.yaml
129129
func (j *JujuHandler) writeCredentials() error {
130-
credentials := map[string]any{"credentials": map[string]any{}}
130+
credMap := map[string]any{}
131+
credentials := map[string]any{"credentials": credMap}
131132
addedCredentials := false
132133

133134
// Iterate over the providers
@@ -138,10 +139,8 @@ func (j *JujuHandler) writeCredentials() error {
138139
}
139140

140141
// Set the credentials for the provider, under the credential name "concierge".
141-
credentials["credentials"] = map[string]any{
142-
p.CloudName(): map[string]any{
143-
"concierge": p.Credentials(),
144-
},
142+
credMap[p.CloudName()] = map[string]any{
143+
"concierge": p.Credentials(),
145144
}
146145
addedCredentials = true
147146
}

internal/juju/juju_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/canonical/concierge/internal/config"
1414
"github.com/canonical/concierge/internal/providers"
1515
"github.com/canonical/concierge/internal/system"
16+
"gopkg.in/yaml.v3"
1617
)
1718

1819
var fakeGoogleCreds = []byte(`auth-type: oauth2
@@ -152,6 +153,23 @@ func TestJujuHandlerCommandsPresets(t *testing.T) {
152153
}
153154
}
154155

156+
// mockProvider is a minimal Provider implementation for testing credential merging.
157+
type mockProvider struct {
158+
name string
159+
cloudName string
160+
credentials map[string]any
161+
}
162+
163+
func (m *mockProvider) Prepare() error { return nil }
164+
func (m *mockProvider) Restore() error { return nil }
165+
func (m *mockProvider) Name() string { return m.name }
166+
func (m *mockProvider) Bootstrap() bool { return false }
167+
func (m *mockProvider) CloudName() string { return m.cloudName }
168+
func (m *mockProvider) GroupName() string { return "" }
169+
func (m *mockProvider) Credentials() map[string]any { return m.credentials }
170+
func (m *mockProvider) ModelDefaults() map[string]string { return nil }
171+
func (m *mockProvider) BootstrapConstraints() map[string]string { return nil }
172+
155173
func TestJujuHandlerWithCredentialedProvider(t *testing.T) {
156174
expectedCredsFileContent := []byte(`credentials:
157175
google:
@@ -183,6 +201,73 @@ func TestJujuHandlerWithCredentialedProvider(t *testing.T) {
183201
}
184202
}
185203

204+
func TestJujuHandlerMergesMultipleCredentialedProviders(t *testing.T) {
205+
cfg := &config.Config{}
206+
207+
sys := system.NewMockSystem()
208+
209+
providerA := &mockProvider{
210+
name: "cloud-a",
211+
cloudName: "cloud-a",
212+
credentials: map[string]any{
213+
"auth-type": "userpass",
214+
"username": "alice",
215+
"password": "secret-a",
216+
},
217+
}
218+
providerB := &mockProvider{
219+
name: "cloud-b",
220+
cloudName: "cloud-b",
221+
credentials: map[string]any{
222+
"auth-type": "userpass",
223+
"username": "bob",
224+
"password": "secret-b",
225+
},
226+
}
227+
228+
handler := NewJujuHandler(cfg, sys, []providers.Provider{providerA, providerB})
229+
230+
err := handler.Prepare()
231+
if err != nil {
232+
t.Fatal(err.Error())
233+
}
234+
235+
credsFile := path.Join(os.TempDir(), ".local", "share", "juju", "credentials.yaml")
236+
content, ok := sys.CreatedFiles[credsFile]
237+
if !ok {
238+
t.Fatal("credentials.yaml was not created")
239+
}
240+
241+
// Parse the written YAML and verify both clouds are present.
242+
var parsed map[string]any
243+
if err := yaml.Unmarshal([]byte(content), &parsed); err != nil {
244+
t.Fatalf("failed to parse credentials.yaml: %v", err)
245+
}
246+
247+
credMap, ok := parsed["credentials"].(map[string]any)
248+
if !ok {
249+
t.Fatal("missing or invalid top-level 'credentials' key")
250+
}
251+
252+
if len(credMap) != 2 {
253+
t.Fatalf("expected 2 clouds in credentials, got %d: %v", len(credMap), credMap)
254+
}
255+
256+
for _, cloud := range []string{"cloud-a", "cloud-b"} {
257+
entry, ok := credMap[cloud]
258+
if !ok {
259+
t.Fatalf("credentials for %q missing — merge overwrote earlier entries", cloud)
260+
}
261+
cloudMap, ok := entry.(map[string]any)
262+
if !ok {
263+
t.Fatalf("credentials for %q is not a map", cloud)
264+
}
265+
if _, ok := cloudMap["concierge"]; !ok {
266+
t.Fatalf("credentials for %q missing 'concierge' key", cloud)
267+
}
268+
}
269+
}
270+
186271
func TestJujuRestoreNoKillController(t *testing.T) {
187272
system, handler, err := setupHandlerWithPreset("machine")
188273
if err != nil {

0 commit comments

Comments
 (0)