fix(config): sync external_routes and network_groups to viper before Save to prevent dotted-key corruption
- commit
- c3570537b20063616101a6ea43be359e4c36fccf
- author
- bnema · · 2026-02-24 21:51 UTC
- parent
- d7ba6d9
- signature
- unsigned
2 files · +60 -2
M internal/usecase/config/service.go +6 -1
| @@ -418,9 +418,14 @@ func (s *Service) Save(ctx context.Context) error { | |||
| 418 | 418 | s.mu.Lock() | |
| 419 | 419 | defer s.mu.Unlock() | |
| 420 | 420 | ||
| 421 | - | // Update viper with current config | |
| 421 | + | // Sync ALL mutable config sections back to Viper before writing. | |
| 422 | + | // Using explicit Set() ensures the values are stored as proper flat maps, | |
| 423 | + | // preventing viper from splitting dotted keys (e.g. "reg.example.com") into | |
| 424 | + | // nested TOML subtrees which corrupts the data on re-read. | |
| 422 | 425 | s.viper.Set("routes", s.config.Routes) | |
| 426 | + | s.viper.Set("external_routes", s.config.ExternalRoutes) | |
| 423 | 427 | s.viper.Set("attachments", s.config.Attachments) | |
| 428 | + | s.viper.Set("network_groups", s.config.NetworkGroups) | |
| 424 | 429 | ||
| 425 | 430 | // Record save time to debounce file watcher events | |
| 426 | 431 | atomic.StoreInt64(&s.lastSaveTime, time.Now().UnixNano()) | |
| @@ -418,9 +418,14 @@ func (s *Service) Save(ctx context.Context) error { | |||||
| 418 | s.mu.Lock() | 418 | s.mu.Lock() | ||
| 419 | defer s.mu.Unlock() | 419 | defer s.mu.Unlock() | ||
| 420 | 420 | ||||
| 421 | - | // Update viper with current config | 421 | + | // Sync ALL mutable config sections back to Viper before writing. |
| 422 | + | // Using explicit Set() ensures the values are stored as proper flat maps, | |||
| 423 | + | // preventing viper from splitting dotted keys (e.g. "reg.example.com") into | |||
| 424 | + | // nested TOML subtrees which corrupts the data on re-read. | |||
| 422 | s.viper.Set("routes", s.config.Routes) | 425 | s.viper.Set("routes", s.config.Routes) | ||
| 426 | + | s.viper.Set("external_routes", s.config.ExternalRoutes) | |||
| 423 | s.viper.Set("attachments", s.config.Attachments) | 427 | s.viper.Set("attachments", s.config.Attachments) | ||
| 428 | + | s.viper.Set("network_groups", s.config.NetworkGroups) | |||
| 424 | 429 | ||||
| 425 | // Record save time to debounce file watcher events | 430 | // Record save time to debounce file watcher events | ||
| 426 | atomic.StoreInt64(&s.lastSaveTime, time.Now().UnixNano()) | 431 | atomic.StoreInt64(&s.lastSaveTime, time.Now().UnixNano()) | ||
M internal/usecase/config/service_test.go +54 -1
| @@ -1135,7 +1135,7 @@ frontend = ["app1.example.com", "app2.example.com"] | |||
| 1135 | 1135 | err = v2.ReadInConfig() | |
| 1136 | 1136 | require.NoError(t, err) | |
| 1137 | 1137 | ||
| 1138 | - | // Verify network_groups is still in the file after save | |
| 1138 | + | // Verify network_groups is still intact and correct after save | |
| 1139 | 1139 | savedGroups := loadStringArrayMap(v2.Get("network_groups")) | |
| 1140 | 1140 | assert.Len(t, savedGroups, 1, "network_groups should still be present in saved config") | |
| 1141 | 1141 | assert.ElementsMatch(t, []string{"app1.example.com", "app2.example.com"}, savedGroups["frontend"]) | |
| @@ -1148,6 +1148,59 @@ frontend = ["app1.example.com", "app2.example.com"] | |||
| 1148 | 1148 | assert.Equal(t, 9999, v2.GetInt("server.port")) | |
| 1149 | 1149 | }) | |
| 1150 | 1150 | ||
| 1151 | + | // Regression test: external_routes keys contain dots (e.g. "reg.example.com"). | |
| 1152 | + | // Without explicitly calling viper.Set("external_routes", ...) before WriteConfig, | |
| 1153 | + | // viper splits dotted keys into nested subtrees, corrupting the data on re-read. | |
| 1154 | + | t.Run("external_routes with dotted domain keys are not corrupted on Save", func(t *testing.T) { | |
| 1155 | + | tmpDir := t.TempDir() | |
| 1156 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |
| 1157 | + | initialConfig := ` | |
| 1158 | + | [server] | |
| 1159 | + | port = 8080 | |
| 1160 | + | ||
| 1161 | + | [routes] | |
| 1162 | + | "app.example.com" = "myapp:latest" | |
| 1163 | + | ||
| 1164 | + | [external_routes] | |
| 1165 | + | "reg.example.com" = "localhost:5000" | |
| 1166 | + | ` | |
| 1167 | + | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | |
| 1168 | + | require.NoError(t, err) | |
| 1169 | + | ||
| 1170 | + | v := viper.New() | |
| 1171 | + | v.SetConfigFile(configFile) | |
| 1172 | + | err = v.ReadInConfig() | |
| 1173 | + | require.NoError(t, err) | |
| 1174 | + | ||
| 1175 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 1176 | + | svc := NewService(v, eventBus) | |
| 1177 | + | ctx := testContext() | |
| 1178 | + | ||
| 1179 | + | err = svc.Load(ctx) | |
| 1180 | + | require.NoError(t, err) | |
| 1181 | + | ||
| 1182 | + | // Verify external_routes loaded correctly in memory | |
| 1183 | + | extRoutes := svc.GetExternalRoutes() | |
| 1184 | + | require.Equal(t, "localhost:5000", extRoutes["reg.example.com"]) | |
| 1185 | + | ||
| 1186 | + | // Trigger Save | |
| 1187 | + | err = svc.AddRoute(ctx, domain.Route{Domain: "new.example.com", Image: "newapp:latest"}) | |
| 1188 | + | require.NoError(t, err) | |
| 1189 | + | ||
| 1190 | + | // Re-read the file with a fresh viper instance | |
| 1191 | + | v2 := viper.New() | |
| 1192 | + | v2.SetConfigFile(configFile) | |
| 1193 | + | err = v2.ReadInConfig() | |
| 1194 | + | require.NoError(t, err) | |
| 1195 | + | ||
| 1196 | + | // external_routes must survive Save with correct flat map structure. | |
| 1197 | + | // Without the fix, viper splits "reg.example.com" into nested TOML subtrees | |
| 1198 | + | // and GetString("external_routes.reg\\.example\\.com") returns empty string. | |
| 1199 | + | savedExtRoutes := loadStringMap(v2.Get("external_routes")) | |
| 1200 | + | assert.Equal(t, "localhost:5000", savedExtRoutes["reg.example.com"], | |
| 1201 | + | "external_routes must not be corrupted by viper's dot-path splitting on WriteConfig") | |
| 1202 | + | }) | |
| 1203 | + | ||
| 1151 | 1204 | t.Run("network_groups added in memory are written to disk on Save", func(t *testing.T) { | |
| 1152 | 1205 | // Setup: config file WITHOUT network_groups | |
| 1153 | 1206 | tmpDir := t.TempDir() | |
| @@ -1135,7 +1135,7 @@ frontend = ["app1.example.com", "app2.example.com"] | |||||
| 1135 | err = v2.ReadInConfig() | 1135 | err = v2.ReadInConfig() | ||
| 1136 | require.NoError(t, err) | 1136 | require.NoError(t, err) | ||
| 1137 | 1137 | ||||
| 1138 | - | // Verify network_groups is still in the file after save | 1138 | + | // Verify network_groups is still intact and correct after save |
| 1139 | savedGroups := loadStringArrayMap(v2.Get("network_groups")) | 1139 | savedGroups := loadStringArrayMap(v2.Get("network_groups")) | ||
| 1140 | assert.Len(t, savedGroups, 1, "network_groups should still be present in saved config") | 1140 | assert.Len(t, savedGroups, 1, "network_groups should still be present in saved config") | ||
| 1141 | assert.ElementsMatch(t, []string{"app1.example.com", "app2.example.com"}, savedGroups["frontend"]) | 1141 | assert.ElementsMatch(t, []string{"app1.example.com", "app2.example.com"}, savedGroups["frontend"]) | ||
| @@ -1148,6 +1148,59 @@ frontend = ["app1.example.com", "app2.example.com"] | |||||
| 1148 | assert.Equal(t, 9999, v2.GetInt("server.port")) | 1148 | assert.Equal(t, 9999, v2.GetInt("server.port")) | ||
| 1149 | }) | 1149 | }) | ||
| 1150 | 1150 | ||||
| 1151 | + | // Regression test: external_routes keys contain dots (e.g. "reg.example.com"). | |||
| 1152 | + | // Without explicitly calling viper.Set("external_routes", ...) before WriteConfig, | |||
| 1153 | + | // viper splits dotted keys into nested subtrees, corrupting the data on re-read. | |||
| 1154 | + | t.Run("external_routes with dotted domain keys are not corrupted on Save", func(t *testing.T) { | |||
| 1155 | + | tmpDir := t.TempDir() | |||
| 1156 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |||
| 1157 | + | initialConfig := ` | |||
| 1158 | + | [server] | |||
| 1159 | + | port = 8080 | |||
| 1160 | + | ||||
| 1161 | + | [routes] | |||
| 1162 | + | "app.example.com" = "myapp:latest" | |||
| 1163 | + | ||||
| 1164 | + | [external_routes] | |||
| 1165 | + | "reg.example.com" = "localhost:5000" | |||
| 1166 | + | ` | |||
| 1167 | + | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | |||
| 1168 | + | require.NoError(t, err) | |||
| 1169 | + | ||||
| 1170 | + | v := viper.New() | |||
| 1171 | + | v.SetConfigFile(configFile) | |||
| 1172 | + | err = v.ReadInConfig() | |||
| 1173 | + | require.NoError(t, err) | |||
| 1174 | + | ||||
| 1175 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 1176 | + | svc := NewService(v, eventBus) | |||
| 1177 | + | ctx := testContext() | |||
| 1178 | + | ||||
| 1179 | + | err = svc.Load(ctx) | |||
| 1180 | + | require.NoError(t, err) | |||
| 1181 | + | ||||
| 1182 | + | // Verify external_routes loaded correctly in memory | |||
| 1183 | + | extRoutes := svc.GetExternalRoutes() | |||
| 1184 | + | require.Equal(t, "localhost:5000", extRoutes["reg.example.com"]) | |||
| 1185 | + | ||||
| 1186 | + | // Trigger Save | |||
| 1187 | + | err = svc.AddRoute(ctx, domain.Route{Domain: "new.example.com", Image: "newapp:latest"}) | |||
| 1188 | + | require.NoError(t, err) | |||
| 1189 | + | ||||
| 1190 | + | // Re-read the file with a fresh viper instance | |||
| 1191 | + | v2 := viper.New() | |||
| 1192 | + | v2.SetConfigFile(configFile) | |||
| 1193 | + | err = v2.ReadInConfig() | |||
| 1194 | + | require.NoError(t, err) | |||
| 1195 | + | ||||
| 1196 | + | // external_routes must survive Save with correct flat map structure. | |||
| 1197 | + | // Without the fix, viper splits "reg.example.com" into nested TOML subtrees | |||
| 1198 | + | // and GetString("external_routes.reg\\.example\\.com") returns empty string. | |||
| 1199 | + | savedExtRoutes := loadStringMap(v2.Get("external_routes")) | |||
| 1200 | + | assert.Equal(t, "localhost:5000", savedExtRoutes["reg.example.com"], | |||
| 1201 | + | "external_routes must not be corrupted by viper's dot-path splitting on WriteConfig") | |||
| 1202 | + | }) | |||
| 1203 | + | ||||
| 1151 | t.Run("network_groups added in memory are written to disk on Save", func(t *testing.T) { | 1204 | t.Run("network_groups added in memory are written to disk on Save", func(t *testing.T) { | ||
| 1152 | // Setup: config file WITHOUT network_groups | 1205 | // Setup: config file WITHOUT network_groups | ||
| 1153 | tmpDir := t.TempDir() | 1206 | tmpDir := t.TempDir() | ||