detached 7b55ac6

fix(config,container): persist route updates/removals and fix env extraction image ref

commit
7b55ac690685537b4fa379f834d1d40c96466705
author
bnema · · 2026-01-16 21:54 UTC
parent
1280049
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +136 -13
Unified
UnifiedSplit
M internal/usecase/config/service.go +30 -10
@@ -217,7 +217,7 @@ func (s *Service) AddRoute(ctx context.Context, route domain.Route) error {
217217 return nil
218218 }
219219
220-// UpdateRoute updates an existing route.
220+// UpdateRoute updates an existing route and persists it.
221221 func (s *Service) UpdateRoute(ctx context.Context, route domain.Route) error {
222222 ctx = zerowrap.CtxWithFields(ctx, map[string]any{
223223 zerowrap.FieldLayer: "usecase",
@@ -226,20 +226,30 @@ func (s *Service) UpdateRoute(ctx context.Context, route domain.Route) error {
226226 })
227227 log := zerowrap.FromCtx(ctx)
228228
229+ // Store previous value for rollback
229230 s.mu.Lock()
230- defer s.mu.Unlock()
231-
232- if _, exists := s.config.Routes[route.Domain]; !exists {
231+ previousImage, exists := s.config.Routes[route.Domain]
232+ if !exists {
233+ s.mu.Unlock()
233234 return domain.ErrRouteNotFound
234235 }
235-
236236 s.config.Routes[route.Domain] = route.Image
237+ s.mu.Unlock()
238+
239+ // Persist to disk - rollback on failure
240+ if err := s.Save(ctx); err != nil {
241+ log.Warn().Err(err).Msg("failed to persist route update to disk, rolling back")
242+ s.mu.Lock()
243+ s.config.Routes[route.Domain] = previousImage
244+ s.mu.Unlock()
245+ return err
246+ }
237247
238248 log.Info().Str("image", route.Image).Msg("route updated")
239249 return nil
240250 }
241251
242-// RemoveRoute removes a route from the configuration.
252+// RemoveRoute removes a route from the configuration and persists it.
243253 func (s *Service) RemoveRoute(ctx context.Context, domainName string) error {
244254 ctx = zerowrap.CtxWithFields(ctx, map[string]any{
245255 zerowrap.FieldLayer: "usecase",
@@ -248,14 +258,24 @@ func (s *Service) RemoveRoute(ctx context.Context, domainName string) error {
248258 })
249259 log := zerowrap.FromCtx(ctx)
250260
261+ // Store previous value for rollback
251262 s.mu.Lock()
252- defer s.mu.Unlock()
253-
254- if _, exists := s.config.Routes[domainName]; !exists {
263+ previousImage, exists := s.config.Routes[domainName]
264+ if !exists {
265+ s.mu.Unlock()
255266 return domain.ErrRouteNotFound
256267 }
257-
258268 delete(s.config.Routes, domainName)
269+ s.mu.Unlock()
270+
271+ // Persist to disk - rollback on failure
272+ if err := s.Save(ctx); err != nil {
273+ log.Warn().Err(err).Msg("failed to persist route removal to disk, rolling back")
274+ s.mu.Lock()
275+ s.config.Routes[domainName] = previousImage
276+ s.mu.Unlock()
277+ return err
278+ }
259279
260280 log.Info().Msg("route removed")
261281 return nil
@@ -217,7 +217,7 @@ func (s *Service) AddRoute(ctx context.Context, route domain.Route) error {
217 return nil217 return nil
218 }218 }
219 219
220-// UpdateRoute updates an existing route.220+// UpdateRoute updates an existing route and persists it.
221 func (s *Service) UpdateRoute(ctx context.Context, route domain.Route) error {221 func (s *Service) UpdateRoute(ctx context.Context, route domain.Route) error {
222 ctx = zerowrap.CtxWithFields(ctx, map[string]any{222 ctx = zerowrap.CtxWithFields(ctx, map[string]any{
223 zerowrap.FieldLayer: "usecase",223 zerowrap.FieldLayer: "usecase",
@@ -226,20 +226,30 @@ func (s *Service) UpdateRoute(ctx context.Context, route domain.Route) error {
226 })226 })
227 log := zerowrap.FromCtx(ctx)227 log := zerowrap.FromCtx(ctx)
228 228
229+ // Store previous value for rollback
229 s.mu.Lock()230 s.mu.Lock()
230- defer s.mu.Unlock()231+ previousImage, exists := s.config.Routes[route.Domain]
231-232+ if !exists {
232- if _, exists := s.config.Routes[route.Domain]; !exists {233+ s.mu.Unlock()
233 return domain.ErrRouteNotFound234 return domain.ErrRouteNotFound
234 }235 }
235-
236 s.config.Routes[route.Domain] = route.Image236 s.config.Routes[route.Domain] = route.Image
237+ s.mu.Unlock()
238+
239+ // Persist to disk - rollback on failure
240+ if err := s.Save(ctx); err != nil {
241+ log.Warn().Err(err).Msg("failed to persist route update to disk, rolling back")
242+ s.mu.Lock()
243+ s.config.Routes[route.Domain] = previousImage
244+ s.mu.Unlock()
245+ return err
246+ }
237 247
238 log.Info().Str("image", route.Image).Msg("route updated")248 log.Info().Str("image", route.Image).Msg("route updated")
239 return nil249 return nil
240 }250 }
241 251
242-// RemoveRoute removes a route from the configuration.252+// RemoveRoute removes a route from the configuration and persists it.
243 func (s *Service) RemoveRoute(ctx context.Context, domainName string) error {253 func (s *Service) RemoveRoute(ctx context.Context, domainName string) error {
244 ctx = zerowrap.CtxWithFields(ctx, map[string]any{254 ctx = zerowrap.CtxWithFields(ctx, map[string]any{
245 zerowrap.FieldLayer: "usecase",255 zerowrap.FieldLayer: "usecase",
@@ -248,14 +258,24 @@ func (s *Service) RemoveRoute(ctx context.Context, domainName string) error {
248 })258 })
249 log := zerowrap.FromCtx(ctx)259 log := zerowrap.FromCtx(ctx)
250 260
261+ // Store previous value for rollback
251 s.mu.Lock()262 s.mu.Lock()
252- defer s.mu.Unlock()263+ previousImage, exists := s.config.Routes[domainName]
253-264+ if !exists {
254- if _, exists := s.config.Routes[domainName]; !exists {265+ s.mu.Unlock()
255 return domain.ErrRouteNotFound266 return domain.ErrRouteNotFound
256 }267 }
257-
258 delete(s.config.Routes, domainName)268 delete(s.config.Routes, domainName)
269+ s.mu.Unlock()
270+
271+ // Persist to disk - rollback on failure
272+ if err := s.Save(ctx); err != nil {
273+ log.Warn().Err(err).Msg("failed to persist route removal to disk, rolling back")
274+ s.mu.Lock()
275+ s.config.Routes[domainName] = previousImage
276+ s.mu.Unlock()
277+ return err
278+ }
259 279
260 log.Info().Msg("route removed")280 log.Info().Msg("route removed")
261 return nil281 return nil
M internal/usecase/config/service_test.go +16 -2
@@ -128,7 +128,14 @@ func TestService_AddRoute(t *testing.T) {
128128 }
129129
130130 func TestService_UpdateRoute(t *testing.T) {
131+ // Create temp config file for Save() to work
132+ tmpDir := t.TempDir()
133+ configFile := filepath.Join(tmpDir, "gordon.toml")
134+ err := os.WriteFile(configFile, []byte("[routes]\n\"app.example.com\" = \"myapp:v1\"\n"), 0600)
135+ require.NoError(t, err)
136+
131137 v := viper.New()
138+ v.SetConfigFile(configFile)
132139 v.Set("routes", map[string]interface{}{
133140 "app.example.com": "myapp:v1",
134141 })
@@ -144,7 +151,7 @@ func TestService_UpdateRoute(t *testing.T) {
144151 Image: "myapp:v2",
145152 }
146153
147- err := svc.UpdateRoute(ctx, route)
154+ err = svc.UpdateRoute(ctx, route)
148155
149156 assert.NoError(t, err)
150157
@@ -171,7 +178,14 @@ func TestService_UpdateRoute_NotFound(t *testing.T) {
171178 }
172179
173180 func TestService_RemoveRoute(t *testing.T) {
181+ // Create temp config file for Save() to work
182+ tmpDir := t.TempDir()
183+ configFile := filepath.Join(tmpDir, "gordon.toml")
184+ err := os.WriteFile(configFile, []byte("[routes]\n\"app.example.com\" = \"myapp:latest\"\n"), 0600)
185+ require.NoError(t, err)
186+
174187 v := viper.New()
188+ v.SetConfigFile(configFile)
175189 v.Set("routes", map[string]interface{}{
176190 "app.example.com": "myapp:latest",
177191 })
@@ -182,7 +196,7 @@ func TestService_RemoveRoute(t *testing.T) {
182196
183197 _ = svc.Load(ctx)
184198
185- err := svc.RemoveRoute(ctx, "app.example.com")
199+ err = svc.RemoveRoute(ctx, "app.example.com")
186200
187201 assert.NoError(t, err)
188202
@@ -128,7 +128,14 @@ func TestService_AddRoute(t *testing.T) {
128 }128 }
129 129
130 func TestService_UpdateRoute(t *testing.T) {130 func TestService_UpdateRoute(t *testing.T) {
131+ // Create temp config file for Save() to work
132+ tmpDir := t.TempDir()
133+ configFile := filepath.Join(tmpDir, "gordon.toml")
134+ err := os.WriteFile(configFile, []byte("[routes]\n\"app.example.com\" = \"myapp:v1\"\n"), 0600)
135+ require.NoError(t, err)
136+
131 v := viper.New()137 v := viper.New()
138+ v.SetConfigFile(configFile)
132 v.Set("routes", map[string]interface{}{139 v.Set("routes", map[string]interface{}{
133 "app.example.com": "myapp:v1",140 "app.example.com": "myapp:v1",
134 })141 })
@@ -144,7 +151,7 @@ func TestService_UpdateRoute(t *testing.T) {
144 Image: "myapp:v2",151 Image: "myapp:v2",
145 }152 }
146 153
147- err := svc.UpdateRoute(ctx, route)154+ err = svc.UpdateRoute(ctx, route)
148 155
149 assert.NoError(t, err)156 assert.NoError(t, err)
150 157
@@ -171,7 +178,14 @@ func TestService_UpdateRoute_NotFound(t *testing.T) {
171 }178 }
172 179
173 func TestService_RemoveRoute(t *testing.T) {180 func TestService_RemoveRoute(t *testing.T) {
181+ // Create temp config file for Save() to work
182+ tmpDir := t.TempDir()
183+ configFile := filepath.Join(tmpDir, "gordon.toml")
184+ err := os.WriteFile(configFile, []byte("[routes]\n\"app.example.com\" = \"myapp:latest\"\n"), 0600)
185+ require.NoError(t, err)
186+
174 v := viper.New()187 v := viper.New()
188+ v.SetConfigFile(configFile)
175 v.Set("routes", map[string]interface{}{189 v.Set("routes", map[string]interface{}{
176 "app.example.com": "myapp:latest",190 "app.example.com": "myapp:latest",
177 })191 })
@@ -182,7 +196,7 @@ func TestService_RemoveRoute(t *testing.T) {
182 196
183 _ = svc.Load(ctx)197 _ = svc.Load(ctx)
184 198
185- err := svc.RemoveRoute(ctx, "app.example.com")199+ err = svc.RemoveRoute(ctx, "app.example.com")
186 200
187 assert.NoError(t, err)201 assert.NoError(t, err)
188 202
M internal/usecase/container/autoroute.go +29 -1
@@ -132,10 +132,37 @@ func (h *AutoRouteHandler) buildImageName(name, reference string) string {
132132 return fmt.Sprintf("%s:%s", name, reference)
133133 }
134134
135+// buildFullImageRef constructs a fully qualified image reference with registry domain.
136+// This is needed for Docker operations since images are stored with the registry prefix.
137+func (h *AutoRouteHandler) buildFullImageRef(imageName string) string {
138+ if h.registryDomain == "" {
139+ return imageName
140+ }
141+
142+ // Don't prefix if already has registry domain
143+ if strings.HasPrefix(imageName, h.registryDomain+"/") {
144+ return imageName
145+ }
146+
147+ // Don't prefix if it's an external registry reference (contains dots and slashes)
148+ repoPart := strings.Split(imageName, ":")[0]
149+ if strings.Contains(repoPart, "@") {
150+ repoPart = strings.Split(imageName, "@")[0]
151+ }
152+ if strings.Contains(repoPart, ".") && strings.Contains(repoPart, "/") {
153+ return imageName
154+ }
155+
156+ return fmt.Sprintf("%s/%s", h.registryDomain, imageName)
157+}
158+
135159 // processRoutes processes each domain and creates/updates routes.
136160 func (h *AutoRouteHandler) processRoutes(ctx context.Context, domains []string, imageName string, labels *domain.ImageLabels) {
137161 log := zerowrap.FromCtx(ctx)
138162
163+ // Build full image reference for env extraction (Docker needs fully qualified name)
164+ fullImageRef := h.buildFullImageRef(imageName)
165+
139166 for _, routeDomain := range domains {
140167 routeDomain = strings.TrimSpace(routeDomain)
141168 if routeDomain == "" {
@@ -145,7 +172,8 @@ func (h *AutoRouteHandler) processRoutes(ctx context.Context, domains []string,
145172 h.createOrUpdateRoute(ctx, routeDomain, imageName)
146173
147174 if labels.EnvFile != "" && h.extractor != nil && h.envDir != "" {
148- if err := h.extractAndMergeEnvFile(ctx, imageName, routeDomain, labels.EnvFile); err != nil {
175+ // Use full image ref for extraction (Docker knows image by this name)
176+ if err := h.extractAndMergeEnvFile(ctx, fullImageRef, routeDomain, labels.EnvFile); err != nil {
149177 log.Warn().
150178 Err(err).
151179 Str("domain", routeDomain).
@@ -132,10 +132,37 @@ func (h *AutoRouteHandler) buildImageName(name, reference string) string {
132 return fmt.Sprintf("%s:%s", name, reference)132 return fmt.Sprintf("%s:%s", name, reference)
133 }133 }
134 134
135+// buildFullImageRef constructs a fully qualified image reference with registry domain.
136+// This is needed for Docker operations since images are stored with the registry prefix.
137+func (h *AutoRouteHandler) buildFullImageRef(imageName string) string {
138+ if h.registryDomain == "" {
139+ return imageName
140+ }
141+
142+ // Don't prefix if already has registry domain
143+ if strings.HasPrefix(imageName, h.registryDomain+"/") {
144+ return imageName
145+ }
146+
147+ // Don't prefix if it's an external registry reference (contains dots and slashes)
148+ repoPart := strings.Split(imageName, ":")[0]
149+ if strings.Contains(repoPart, "@") {
150+ repoPart = strings.Split(imageName, "@")[0]
151+ }
152+ if strings.Contains(repoPart, ".") && strings.Contains(repoPart, "/") {
153+ return imageName
154+ }
155+
156+ return fmt.Sprintf("%s/%s", h.registryDomain, imageName)
157+}
158+
135 // processRoutes processes each domain and creates/updates routes.159 // processRoutes processes each domain and creates/updates routes.
136 func (h *AutoRouteHandler) processRoutes(ctx context.Context, domains []string, imageName string, labels *domain.ImageLabels) {160 func (h *AutoRouteHandler) processRoutes(ctx context.Context, domains []string, imageName string, labels *domain.ImageLabels) {
137 log := zerowrap.FromCtx(ctx)161 log := zerowrap.FromCtx(ctx)
138 162
163+ // Build full image reference for env extraction (Docker needs fully qualified name)
164+ fullImageRef := h.buildFullImageRef(imageName)
165+
139 for _, routeDomain := range domains {166 for _, routeDomain := range domains {
140 routeDomain = strings.TrimSpace(routeDomain)167 routeDomain = strings.TrimSpace(routeDomain)
141 if routeDomain == "" {168 if routeDomain == "" {
@@ -145,7 +172,8 @@ func (h *AutoRouteHandler) processRoutes(ctx context.Context, domains []string,
145 h.createOrUpdateRoute(ctx, routeDomain, imageName)172 h.createOrUpdateRoute(ctx, routeDomain, imageName)
146 173
147 if labels.EnvFile != "" && h.extractor != nil && h.envDir != "" {174 if labels.EnvFile != "" && h.extractor != nil && h.envDir != "" {
148- if err := h.extractAndMergeEnvFile(ctx, imageName, routeDomain, labels.EnvFile); err != nil {175+ // Use full image ref for extraction (Docker knows image by this name)
176+ if err := h.extractAndMergeEnvFile(ctx, fullImageRef, routeDomain, labels.EnvFile); err != nil {
149 log.Warn().177 log.Warn().
150 Err(err).178 Err(err).
151 Str("domain", routeDomain).179 Str("domain", routeDomain).
M internal/usecase/container/autoroute_test.go +61 -0
@@ -825,3 +825,64 @@ func TestAutoRouteHandler_BuildImageName(t *testing.T) {
825825 })
826826 }
827827 }
828+
829+// buildFullImageRef tests
830+
831+func TestAutoRouteHandler_BuildFullImageRef(t *testing.T) {
832+ ctx := zerowrap.WithCtx(context.Background(), zerowrap.Default())
833+ configSvc := inmocks.NewMockConfigService(t)
834+ containerSvc := inmocks.NewMockContainerService(t)
835+ blobStorage := mocks.NewMockBlobStorage(t)
836+
837+ tests := []struct {
838+ name string
839+ registryDomain string
840+ imageName string
841+ expected string
842+ }{
843+ {
844+ name: "simple image with registry",
845+ registryDomain: "registry.example.com",
846+ imageName: "myapp:latest",
847+ expected: "registry.example.com/myapp:latest",
848+ },
849+ {
850+ name: "already has registry prefix",
851+ registryDomain: "registry.example.com",
852+ imageName: "registry.example.com/myapp:latest",
853+ expected: "registry.example.com/myapp:latest",
854+ },
855+ {
856+ name: "external registry reference",
857+ registryDomain: "registry.example.com",
858+ imageName: "docker.io/library/nginx:latest",
859+ expected: "docker.io/library/nginx:latest",
860+ },
861+ {
862+ name: "digest reference",
863+ registryDomain: "registry.example.com",
864+ imageName: "myapp@sha256:abc123def456",
865+ expected: "registry.example.com/myapp@sha256:abc123def456",
866+ },
867+ {
868+ name: "empty registry domain",
869+ registryDomain: "",
870+ imageName: "myapp:latest",
871+ expected: "myapp:latest",
872+ },
873+ {
874+ name: "image without tag",
875+ registryDomain: "registry.example.com",
876+ imageName: "myapp",
877+ expected: "registry.example.com/myapp",
878+ },
879+ }
880+
881+ for _, tt := range tests {
882+ t.Run(tt.name, func(t *testing.T) {
883+ handler := NewAutoRouteHandler(ctx, configSvc, containerSvc, blobStorage, tt.registryDomain)
884+ result := handler.buildFullImageRef(tt.imageName)
885+ assert.Equal(t, tt.expected, result)
886+ })
887+ }
888+}
@@ -825,3 +825,64 @@ func TestAutoRouteHandler_BuildImageName(t *testing.T) {
825 })825 })
826 }826 }
827 }827 }
828+
829+// buildFullImageRef tests
830+
831+func TestAutoRouteHandler_BuildFullImageRef(t *testing.T) {
832+ ctx := zerowrap.WithCtx(context.Background(), zerowrap.Default())
833+ configSvc := inmocks.NewMockConfigService(t)
834+ containerSvc := inmocks.NewMockContainerService(t)
835+ blobStorage := mocks.NewMockBlobStorage(t)
836+
837+ tests := []struct {
838+ name string
839+ registryDomain string
840+ imageName string
841+ expected string
842+ }{
843+ {
844+ name: "simple image with registry",
845+ registryDomain: "registry.example.com",
846+ imageName: "myapp:latest",
847+ expected: "registry.example.com/myapp:latest",
848+ },
849+ {
850+ name: "already has registry prefix",
851+ registryDomain: "registry.example.com",
852+ imageName: "registry.example.com/myapp:latest",
853+ expected: "registry.example.com/myapp:latest",
854+ },
855+ {
856+ name: "external registry reference",
857+ registryDomain: "registry.example.com",
858+ imageName: "docker.io/library/nginx:latest",
859+ expected: "docker.io/library/nginx:latest",
860+ },
861+ {
862+ name: "digest reference",
863+ registryDomain: "registry.example.com",
864+ imageName: "myapp@sha256:abc123def456",
865+ expected: "registry.example.com/myapp@sha256:abc123def456",
866+ },
867+ {
868+ name: "empty registry domain",
869+ registryDomain: "",
870+ imageName: "myapp:latest",
871+ expected: "myapp:latest",
872+ },
873+ {
874+ name: "image without tag",
875+ registryDomain: "registry.example.com",
876+ imageName: "myapp",
877+ expected: "registry.example.com/myapp",
878+ },
879+ }
880+
881+ for _, tt := range tests {
882+ t.Run(tt.name, func(t *testing.T) {
883+ handler := NewAutoRouteHandler(ctx, configSvc, containerSvc, blobStorage, tt.registryDomain)
884+ result := handler.buildFullImageRef(tt.imageName)
885+ assert.Equal(t, tt.expected, result)
886+ })
887+ }
888+}

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