detached bb7bc50

test: address Copilot review feedback

- Add unit tests for config.Reload() method (success, file not found, invalid syntax)
- Add unit tests for secrets.ListKeysWithAttachments() (validation, success, store errors)
- Fix extractServiceName() to handle edge cases more explicitly
commit
bb7bc50f8adf7ed954c241a99e553c14e0205362
author
brice · · 2026-01-18 08:11 UTC
parent
07eb07d
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
3 files · +223 -4
Split
UnifiedSplit
M internal/adapters/in/cli/secrets.go +11 -4
@@ -180,12 +180,19 @@ func extractServiceName(containerName string) string {
180180
181181 serviceName := parts[1]
182182 allParts := strings.Split(serviceName, "-")
183- if len(allParts) > 2 {
184- // Take last 2 parts as service name (e.g., "gitea-postgres")
185- return strings.Join(allParts[len(allParts)-2:], "-")
183+
184+ // Handle service names based on the number of segments explicitly
185+ if len(allParts) < 2 {
186+ // No additional segments; use the service name as-is.
187+ return serviceName
188+ }
189+ if len(allParts) == 2 {
190+ // Exactly two segments; the service name is already in the desired form.
191+ return strings.Join(allParts, "-")
186192 }
187193
188- return serviceName
194+ // More than two segments: take the last two as the short service name (e.g., "gitea-postgres").
195+ return strings.Join(allParts[len(allParts)-2:], "-")
189196 }
190197
191198 // getKeyPrefix returns the tree prefix for a key based on its position.
@@ -180,12 +180,19 @@ func extractServiceName(containerName string) string {
180 180
181 serviceName := parts[1]181 serviceName := parts[1]
182 allParts := strings.Split(serviceName, "-")182 allParts := strings.Split(serviceName, "-")
183- if len(allParts) > 2 {183+
184- // Take last 2 parts as service name (e.g., "gitea-postgres")184+ // Handle service names based on the number of segments explicitly
185- return strings.Join(allParts[len(allParts)-2:], "-")185+ if len(allParts) < 2 {
186+ // No additional segments; use the service name as-is.
187+ return serviceName
188+ }
189+ if len(allParts) == 2 {
190+ // Exactly two segments; the service name is already in the desired form.
191+ return strings.Join(allParts, "-")
186 }192 }
187 193
188- return serviceName194+ // More than two segments: take the last two as the short service name (e.g., "gitea-postgres").
195+ return strings.Join(allParts[len(allParts)-2:], "-")
189 }196 }
190 197
191 // getKeyPrefix returns the tree prefix for a key based on its position.198 // getKeyPrefix returns the tree prefix for a key based on its position.
M internal/usecase/config/service_test.go +109 -0
@@ -64,6 +64,115 @@ func TestService_Load(t *testing.T) {
6464 assert.False(t, preserve)
6565 }
6666
67+func TestService_Reload(t *testing.T) {
68+ t.Run("success - picks up config file changes", func(t *testing.T) {
69+ // Create temp config file
70+ tmpDir := t.TempDir()
71+ configFile := filepath.Join(tmpDir, "gordon.toml")
72+ initialConfig := `[server]
73+port = 8080
74+
75+[routes]
76+"app.example.com" = "myapp:v1"
77+`
78+ err := os.WriteFile(configFile, []byte(initialConfig), 0600)
79+ require.NoError(t, err)
80+
81+ v := viper.New()
82+ v.SetConfigFile(configFile)
83+ err = v.ReadInConfig()
84+ require.NoError(t, err)
85+
86+ eventBus := mocks.NewMockEventPublisher(t)
87+ svc := NewService(v, eventBus)
88+ ctx := testContext()
89+
90+ // Initial load
91+ err = svc.Load(ctx)
92+ require.NoError(t, err)
93+ assert.Equal(t, 8080, svc.GetServerPort())
94+ assert.Equal(t, "myapp:v1", svc.GetConfig().Routes["app.example.com"])
95+
96+ // Modify config file on disk
97+ updatedConfig := `[server]
98+port = 9090
99+
100+[routes]
101+"app.example.com" = "myapp:v2"
102+"new.example.com" = "newapp:latest"
103+`
104+ err = os.WriteFile(configFile, []byte(updatedConfig), 0600)
105+ require.NoError(t, err)
106+
107+ // Reload should pick up new values
108+ err = svc.Reload(ctx)
109+ require.NoError(t, err)
110+
111+ assert.Equal(t, 9090, svc.GetServerPort())
112+ assert.Equal(t, "myapp:v2", svc.GetConfig().Routes["app.example.com"])
113+ assert.Equal(t, "newapp:latest", svc.GetConfig().Routes["new.example.com"])
114+ })
115+
116+ t.Run("error - config file not found", func(t *testing.T) {
117+ tmpDir := t.TempDir()
118+ configFile := filepath.Join(tmpDir, "gordon.toml")
119+
120+ // Create config, load it, then delete it
121+ err := os.WriteFile(configFile, []byte("[server]\nport = 8080\n"), 0600)
122+ require.NoError(t, err)
123+
124+ v := viper.New()
125+ v.SetConfigFile(configFile)
126+ err = v.ReadInConfig()
127+ require.NoError(t, err)
128+
129+ eventBus := mocks.NewMockEventPublisher(t)
130+ svc := NewService(v, eventBus)
131+ ctx := testContext()
132+
133+ err = svc.Load(ctx)
134+ require.NoError(t, err)
135+
136+ // Delete the config file
137+ err = os.Remove(configFile)
138+ require.NoError(t, err)
139+
140+ // Reload should fail
141+ err = svc.Reload(ctx)
142+ assert.Error(t, err)
143+ assert.Contains(t, err.Error(), "failed to read config file")
144+ })
145+
146+ t.Run("error - invalid config syntax", func(t *testing.T) {
147+ tmpDir := t.TempDir()
148+ configFile := filepath.Join(tmpDir, "gordon.toml")
149+
150+ // Create valid config initially
151+ err := os.WriteFile(configFile, []byte("[server]\nport = 8080\n"), 0600)
152+ require.NoError(t, err)
153+
154+ v := viper.New()
155+ v.SetConfigFile(configFile)
156+ err = v.ReadInConfig()
157+ require.NoError(t, err)
158+
159+ eventBus := mocks.NewMockEventPublisher(t)
160+ svc := NewService(v, eventBus)
161+ ctx := testContext()
162+
163+ err = svc.Load(ctx)
164+ require.NoError(t, err)
165+
166+ // Write invalid TOML
167+ err = os.WriteFile(configFile, []byte("[server\nport = invalid syntax"), 0600)
168+ require.NoError(t, err)
169+
170+ // Reload should fail
171+ err = svc.Reload(ctx)
172+ assert.Error(t, err)
173+ })
174+}
175+
67176 func TestService_GetRoutes(t *testing.T) {
68177 v := viper.New()
69178 v.Set("routes", map[string]interface{}{
@@ -64,6 +64,115 @@ func TestService_Load(t *testing.T) {
64 assert.False(t, preserve)64 assert.False(t, preserve)
65 }65 }
66 66
67+func TestService_Reload(t *testing.T) {
68+ t.Run("success - picks up config file changes", func(t *testing.T) {
69+ // Create temp config file
70+ tmpDir := t.TempDir()
71+ configFile := filepath.Join(tmpDir, "gordon.toml")
72+ initialConfig := `[server]
73+port = 8080
74+
75+[routes]
76+"app.example.com" = "myapp:v1"
77+`
78+ err := os.WriteFile(configFile, []byte(initialConfig), 0600)
79+ require.NoError(t, err)
80+
81+ v := viper.New()
82+ v.SetConfigFile(configFile)
83+ err = v.ReadInConfig()
84+ require.NoError(t, err)
85+
86+ eventBus := mocks.NewMockEventPublisher(t)
87+ svc := NewService(v, eventBus)
88+ ctx := testContext()
89+
90+ // Initial load
91+ err = svc.Load(ctx)
92+ require.NoError(t, err)
93+ assert.Equal(t, 8080, svc.GetServerPort())
94+ assert.Equal(t, "myapp:v1", svc.GetConfig().Routes["app.example.com"])
95+
96+ // Modify config file on disk
97+ updatedConfig := `[server]
98+port = 9090
99+
100+[routes]
101+"app.example.com" = "myapp:v2"
102+"new.example.com" = "newapp:latest"
103+`
104+ err = os.WriteFile(configFile, []byte(updatedConfig), 0600)
105+ require.NoError(t, err)
106+
107+ // Reload should pick up new values
108+ err = svc.Reload(ctx)
109+ require.NoError(t, err)
110+
111+ assert.Equal(t, 9090, svc.GetServerPort())
112+ assert.Equal(t, "myapp:v2", svc.GetConfig().Routes["app.example.com"])
113+ assert.Equal(t, "newapp:latest", svc.GetConfig().Routes["new.example.com"])
114+ })
115+
116+ t.Run("error - config file not found", func(t *testing.T) {
117+ tmpDir := t.TempDir()
118+ configFile := filepath.Join(tmpDir, "gordon.toml")
119+
120+ // Create config, load it, then delete it
121+ err := os.WriteFile(configFile, []byte("[server]\nport = 8080\n"), 0600)
122+ require.NoError(t, err)
123+
124+ v := viper.New()
125+ v.SetConfigFile(configFile)
126+ err = v.ReadInConfig()
127+ require.NoError(t, err)
128+
129+ eventBus := mocks.NewMockEventPublisher(t)
130+ svc := NewService(v, eventBus)
131+ ctx := testContext()
132+
133+ err = svc.Load(ctx)
134+ require.NoError(t, err)
135+
136+ // Delete the config file
137+ err = os.Remove(configFile)
138+ require.NoError(t, err)
139+
140+ // Reload should fail
141+ err = svc.Reload(ctx)
142+ assert.Error(t, err)
143+ assert.Contains(t, err.Error(), "failed to read config file")
144+ })
145+
146+ t.Run("error - invalid config syntax", func(t *testing.T) {
147+ tmpDir := t.TempDir()
148+ configFile := filepath.Join(tmpDir, "gordon.toml")
149+
150+ // Create valid config initially
151+ err := os.WriteFile(configFile, []byte("[server]\nport = 8080\n"), 0600)
152+ require.NoError(t, err)
153+
154+ v := viper.New()
155+ v.SetConfigFile(configFile)
156+ err = v.ReadInConfig()
157+ require.NoError(t, err)
158+
159+ eventBus := mocks.NewMockEventPublisher(t)
160+ svc := NewService(v, eventBus)
161+ ctx := testContext()
162+
163+ err = svc.Load(ctx)
164+ require.NoError(t, err)
165+
166+ // Write invalid TOML
167+ err = os.WriteFile(configFile, []byte("[server\nport = invalid syntax"), 0600)
168+ require.NoError(t, err)
169+
170+ // Reload should fail
171+ err = svc.Reload(ctx)
172+ assert.Error(t, err)
173+ })
174+}
175+
67 func TestService_GetRoutes(t *testing.T) {176 func TestService_GetRoutes(t *testing.T) {
68 v := viper.New()177 v := viper.New()
69 v.Set("routes", map[string]interface{}{178 v.Set("routes", map[string]interface{}{
M internal/usecase/secrets/service_test.go +103 -0
@@ -9,6 +9,7 @@ import (
99 "github.com/stretchr/testify/assert"
1010 "github.com/stretchr/testify/mock"
1111
12+ "gordon/internal/boundaries/out"
1213 outmocks "gordon/internal/boundaries/out/mocks"
1314 )
1415
@@ -168,6 +169,108 @@ func TestService_ListKeys_Success(t *testing.T) {
168169 assert.Equal(t, expectedKeys, keys)
169170 }
170171
172+func TestService_ListKeysWithAttachments_ValidationErrors(t *testing.T) {
173+ store := outmocks.NewMockDomainSecretStore(t)
174+ svc := NewService(store, testLogger())
175+
176+ tests := []struct {
177+ name string
178+ domain string
179+ wantErr error
180+ }{
181+ {
182+ name: "empty domain",
183+ domain: "",
184+ wantErr: ErrDomainEmpty,
185+ },
186+ {
187+ name: "path traversal",
188+ domain: "../etc/passwd",
189+ wantErr: ErrDomainPathTraversal,
190+ },
191+ {
192+ name: "domain too long",
193+ domain: strings.Repeat("a", 300),
194+ wantErr: ErrDomainTooLong,
195+ },
196+ }
197+
198+ for _, tt := range tests {
199+ t.Run(tt.name, func(t *testing.T) {
200+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), tt.domain)
201+ assert.Nil(t, keys)
202+ assert.Nil(t, attachments)
203+ assert.ErrorIs(t, err, tt.wantErr)
204+ })
205+ }
206+}
207+
208+func TestService_ListKeysWithAttachments_Success(t *testing.T) {
209+ t.Run("returns both domain and attachment secrets", func(t *testing.T) {
210+ store := outmocks.NewMockDomainSecretStore(t)
211+ svc := NewService(store, testLogger())
212+
213+ expectedKeys := []string{"API_KEY", "DB_PASSWORD"}
214+ expectedAttachments := []out.AttachmentSecrets{
215+ {Service: "postgres", Keys: []string{"POSTGRES_PASSWORD"}},
216+ {Service: "redis", Keys: []string{"REDIS_PASSWORD", "REDIS_USER"}},
217+ }
218+
219+ store.EXPECT().ListKeys("app.example.com").Return(expectedKeys, nil)
220+ store.EXPECT().ListAttachmentKeys("app.example.com").Return(expectedAttachments, nil)
221+
222+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
223+ assert.NoError(t, err)
224+ assert.Equal(t, expectedKeys, keys)
225+ assert.Equal(t, expectedAttachments, attachments)
226+ })
227+
228+ t.Run("returns empty attachments when none exist", func(t *testing.T) {
229+ store := outmocks.NewMockDomainSecretStore(t)
230+ svc := NewService(store, testLogger())
231+
232+ expectedKeys := []string{"API_KEY"}
233+
234+ store.EXPECT().ListKeys("app.example.com").Return(expectedKeys, nil)
235+ store.EXPECT().ListAttachmentKeys("app.example.com").Return(nil, nil)
236+
237+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
238+ assert.NoError(t, err)
239+ assert.Equal(t, expectedKeys, keys)
240+ assert.Nil(t, attachments)
241+ })
242+
243+ t.Run("continues without attachments on attachment error", func(t *testing.T) {
244+ store := outmocks.NewMockDomainSecretStore(t)
245+ svc := NewService(store, testLogger())
246+
247+ expectedKeys := []string{"API_KEY"}
248+ attachmentErr := assert.AnError
249+
250+ store.EXPECT().ListKeys("app.example.com").Return(expectedKeys, nil)
251+ store.EXPECT().ListAttachmentKeys("app.example.com").Return(nil, attachmentErr)
252+
253+ // Should still succeed, just without attachments
254+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
255+ assert.NoError(t, err)
256+ assert.Equal(t, expectedKeys, keys)
257+ assert.Nil(t, attachments)
258+ })
259+}
260+
261+func TestService_ListKeysWithAttachments_StoreError(t *testing.T) {
262+ store := outmocks.NewMockDomainSecretStore(t)
263+ svc := NewService(store, testLogger())
264+
265+ storeErr := assert.AnError
266+ store.EXPECT().ListKeys("app.example.com").Return(nil, storeErr)
267+
268+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
269+ assert.Nil(t, keys)
270+ assert.Nil(t, attachments)
271+ assert.ErrorIs(t, err, storeErr)
272+}
273+
171274 func TestService_GetAll_ValidationErrors(t *testing.T) {
172275 store := outmocks.NewMockDomainSecretStore(t)
173276 svc := NewService(store, testLogger())
@@ -9,6 +9,7 @@ import (
9 "github.com/stretchr/testify/assert"9 "github.com/stretchr/testify/assert"
10 "github.com/stretchr/testify/mock"10 "github.com/stretchr/testify/mock"
11 11
12+ "gordon/internal/boundaries/out"
12 outmocks "gordon/internal/boundaries/out/mocks"13 outmocks "gordon/internal/boundaries/out/mocks"
13 )14 )
14 15
@@ -168,6 +169,108 @@ func TestService_ListKeys_Success(t *testing.T) {
168 assert.Equal(t, expectedKeys, keys)169 assert.Equal(t, expectedKeys, keys)
169 }170 }
170 171
172+func TestService_ListKeysWithAttachments_ValidationErrors(t *testing.T) {
173+ store := outmocks.NewMockDomainSecretStore(t)
174+ svc := NewService(store, testLogger())
175+
176+ tests := []struct {
177+ name string
178+ domain string
179+ wantErr error
180+ }{
181+ {
182+ name: "empty domain",
183+ domain: "",
184+ wantErr: ErrDomainEmpty,
185+ },
186+ {
187+ name: "path traversal",
188+ domain: "../etc/passwd",
189+ wantErr: ErrDomainPathTraversal,
190+ },
191+ {
192+ name: "domain too long",
193+ domain: strings.Repeat("a", 300),
194+ wantErr: ErrDomainTooLong,
195+ },
196+ }
197+
198+ for _, tt := range tests {
199+ t.Run(tt.name, func(t *testing.T) {
200+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), tt.domain)
201+ assert.Nil(t, keys)
202+ assert.Nil(t, attachments)
203+ assert.ErrorIs(t, err, tt.wantErr)
204+ })
205+ }
206+}
207+
208+func TestService_ListKeysWithAttachments_Success(t *testing.T) {
209+ t.Run("returns both domain and attachment secrets", func(t *testing.T) {
210+ store := outmocks.NewMockDomainSecretStore(t)
211+ svc := NewService(store, testLogger())
212+
213+ expectedKeys := []string{"API_KEY", "DB_PASSWORD"}
214+ expectedAttachments := []out.AttachmentSecrets{
215+ {Service: "postgres", Keys: []string{"POSTGRES_PASSWORD"}},
216+ {Service: "redis", Keys: []string{"REDIS_PASSWORD", "REDIS_USER"}},
217+ }
218+
219+ store.EXPECT().ListKeys("app.example.com").Return(expectedKeys, nil)
220+ store.EXPECT().ListAttachmentKeys("app.example.com").Return(expectedAttachments, nil)
221+
222+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
223+ assert.NoError(t, err)
224+ assert.Equal(t, expectedKeys, keys)
225+ assert.Equal(t, expectedAttachments, attachments)
226+ })
227+
228+ t.Run("returns empty attachments when none exist", func(t *testing.T) {
229+ store := outmocks.NewMockDomainSecretStore(t)
230+ svc := NewService(store, testLogger())
231+
232+ expectedKeys := []string{"API_KEY"}
233+
234+ store.EXPECT().ListKeys("app.example.com").Return(expectedKeys, nil)
235+ store.EXPECT().ListAttachmentKeys("app.example.com").Return(nil, nil)
236+
237+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
238+ assert.NoError(t, err)
239+ assert.Equal(t, expectedKeys, keys)
240+ assert.Nil(t, attachments)
241+ })
242+
243+ t.Run("continues without attachments on attachment error", func(t *testing.T) {
244+ store := outmocks.NewMockDomainSecretStore(t)
245+ svc := NewService(store, testLogger())
246+
247+ expectedKeys := []string{"API_KEY"}
248+ attachmentErr := assert.AnError
249+
250+ store.EXPECT().ListKeys("app.example.com").Return(expectedKeys, nil)
251+ store.EXPECT().ListAttachmentKeys("app.example.com").Return(nil, attachmentErr)
252+
253+ // Should still succeed, just without attachments
254+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
255+ assert.NoError(t, err)
256+ assert.Equal(t, expectedKeys, keys)
257+ assert.Nil(t, attachments)
258+ })
259+}
260+
261+func TestService_ListKeysWithAttachments_StoreError(t *testing.T) {
262+ store := outmocks.NewMockDomainSecretStore(t)
263+ svc := NewService(store, testLogger())
264+
265+ storeErr := assert.AnError
266+ store.EXPECT().ListKeys("app.example.com").Return(nil, storeErr)
267+
268+ keys, attachments, err := svc.ListKeysWithAttachments(context.Background(), "app.example.com")
269+ assert.Nil(t, keys)
270+ assert.Nil(t, attachments)
271+ assert.ErrorIs(t, err, storeErr)
272+}
273+
171 func TestService_GetAll_ValidationErrors(t *testing.T) {274 func TestService_GetAll_ValidationErrors(t *testing.T) {
172 store := outmocks.NewMockDomainSecretStore(t)275 store := outmocks.NewMockDomainSecretStore(t)
173 svc := NewService(store, testLogger())276 svc := NewService(store, testLogger())

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 7

Tags 106

↑↓ moveenter openesc close