fix: safe config save with backup, surgical write, and verification
Save() now bypasses viper.WriteConfig() and performs a surgical read-modify-write on the TOML file, updating only the 4 mutable sections (routes, external_routes, attachments, network_groups). Creates a .bak backup before every write and verifies critical fields survived after writing. Restores from backup on corruption. Close #101
2 files · +267 -18
M internal/usecase/config/service.go +131 -12
| @@ -4,6 +4,7 @@ package config | |||
| 4 | 4 | import ( | |
| 5 | 5 | "context" | |
| 6 | 6 | "fmt" | |
| 7 | + | "os" | |
| 7 | 8 | "strings" | |
| 8 | 9 | "sync" | |
| 9 | 10 | "sync/atomic" | |
| @@ -11,6 +12,7 @@ import ( | |||
| 11 | 12 | ||
| 12 | 13 | "github.com/bnema/zerowrap" | |
| 13 | 14 | "github.com/fsnotify/fsnotify" | |
| 15 | + | "github.com/pelletier/go-toml/v2" | |
| 14 | 16 | "github.com/spf13/viper" | |
| 15 | 17 | ||
| 16 | 18 | "github.com/bnema/gordon/internal/boundaries/out" | |
| @@ -454,27 +456,144 @@ func (s *Service) Save(ctx context.Context) error { | |||
| 454 | 456 | s.mu.Lock() | |
| 455 | 457 | defer s.mu.Unlock() | |
| 456 | 458 | ||
| 457 | - | // Sync ALL mutable config sections back to Viper before writing. | |
| 458 | - | // Using explicit Set() ensures the values are stored as proper flat maps, | |
| 459 | - | // preventing viper from splitting dotted keys (e.g. "reg.example.com") into | |
| 460 | - | // nested TOML subtrees which corrupts the data on re-read. | |
| 461 | - | s.viper.Set("routes", s.config.Routes) | |
| 462 | - | s.viper.Set("external_routes", s.config.ExternalRoutes) | |
| 463 | - | s.viper.Set("attachments", s.config.Attachments) | |
| 464 | - | s.viper.Set("network_groups", s.config.NetworkGroups) | |
| 459 | + | configFile := s.viper.ConfigFileUsed() | |
| 460 | + | if configFile == "" { | |
| 461 | + | return log.WrapErr(fmt.Errorf("no config file path"), "cannot save config") | |
| 462 | + | } | |
| 465 | 463 | ||
| 466 | - | // Record save time to debounce file watcher events | |
| 467 | - | atomic.StoreInt64(&s.lastSaveTime, time.Now().UnixNano()) | |
| 464 | + | if err := backupConfigFile(configFile); err != nil { | |
| 465 | + | log.Warn().Err(err).Msg("failed to create config backup") | |
| 466 | + | } | |
| 467 | + | ||
| 468 | + | snapshot := s.snapshotCriticalFields() | |
| 468 | 469 | ||
| 469 | - | // Write config to disk | |
| 470 | - | if err := s.viper.WriteConfig(); err != nil { | |
| 470 | + | if err := s.writeConfigSurgical(configFile); err != nil { | |
| 471 | 471 | return log.WrapErr(err, "failed to write config") | |
| 472 | 472 | } | |
| 473 | 473 | ||
| 474 | + | atomic.StoreInt64(&s.lastSaveTime, time.Now().UnixNano()) | |
| 475 | + | ||
| 476 | + | if err := s.viper.ReadInConfig(); err != nil { | |
| 477 | + | return log.WrapErr(err, "failed to re-read config after save") | |
| 478 | + | } | |
| 479 | + | ||
| 480 | + | if err := s.verifyCriticalFields(snapshot); err != nil { | |
| 481 | + | log.Error().Err(err).Msg("config corruption detected after save, restoring backup") | |
| 482 | + | if restoreErr := restoreConfigBackup(configFile); restoreErr != nil { | |
| 483 | + | log.Error().Err(restoreErr).Msg("CRITICAL: failed to restore config backup") | |
| 484 | + | } | |
| 485 | + | _ = s.viper.ReadInConfig() | |
| 486 | + | return log.WrapErr(err, "config verification failed after save") | |
| 487 | + | } | |
| 488 | + | ||
| 474 | 489 | log.Info().Msg("configuration saved to disk") | |
| 475 | 490 | return nil | |
| 476 | 491 | } | |
| 477 | 492 | ||
| 493 | + | type configSnapshot struct { | |
| 494 | + | values map[string]any | |
| 495 | + | } | |
| 496 | + | ||
| 497 | + | func (s *Service) snapshotCriticalFields() configSnapshot { | |
| 498 | + | snap := configSnapshot{values: make(map[string]any)} | |
| 499 | + | for _, key := range s.viper.AllKeys() { | |
| 500 | + | snap.values[key] = s.viper.Get(key) | |
| 501 | + | } | |
| 502 | + | return snap | |
| 503 | + | } | |
| 504 | + | ||
| 505 | + | func (s *Service) verifyCriticalFields(snap configSnapshot) error { | |
| 506 | + | for key, oldVal := range snap.values { | |
| 507 | + | oldStr, ok := oldVal.(string) | |
| 508 | + | if !ok || oldStr == "" { | |
| 509 | + | continue | |
| 510 | + | } | |
| 511 | + | ||
| 512 | + | newVal := s.viper.GetString(key) | |
| 513 | + | if newVal != oldStr { | |
| 514 | + | return fmt.Errorf("config key %q changed from %q to %q after save", key, oldStr, newVal) | |
| 515 | + | } | |
| 516 | + | } | |
| 517 | + | ||
| 518 | + | return nil | |
| 519 | + | } | |
| 520 | + | ||
| 521 | + | func (s *Service) writeConfigSurgical(configFile string) error { | |
| 522 | + | data, err := os.ReadFile(configFile) | |
| 523 | + | if err != nil { | |
| 524 | + | if os.IsNotExist(err) { | |
| 525 | + | s.viper.Set("routes", s.config.Routes) | |
| 526 | + | s.viper.Set("external_routes", s.config.ExternalRoutes) | |
| 527 | + | s.viper.Set("attachments", s.config.Attachments) | |
| 528 | + | s.viper.Set("network_groups", s.config.NetworkGroups) | |
| 529 | + | return s.viper.WriteConfigAs(configFile) | |
| 530 | + | } | |
| 531 | + | return fmt.Errorf("failed to read config file: %w", err) | |
| 532 | + | } | |
| 533 | + | ||
| 534 | + | var config map[string]any | |
| 535 | + | if len(data) > 0 { | |
| 536 | + | if err := toml.Unmarshal(data, &config); err != nil { | |
| 537 | + | return fmt.Errorf("failed to parse config file: %w", err) | |
| 538 | + | } | |
| 539 | + | } | |
| 540 | + | if config == nil { | |
| 541 | + | config = make(map[string]any) | |
| 542 | + | } | |
| 543 | + | ||
| 544 | + | config["routes"] = s.config.Routes | |
| 545 | + | config["external_routes"] = s.config.ExternalRoutes | |
| 546 | + | config["attachments"] = s.config.Attachments | |
| 547 | + | config["network_groups"] = s.config.NetworkGroups | |
| 548 | + | ||
| 549 | + | out, err := toml.Marshal(config) | |
| 550 | + | if err != nil { | |
| 551 | + | return fmt.Errorf("failed to marshal config: %w", err) | |
| 552 | + | } | |
| 553 | + | ||
| 554 | + | tmpFile := configFile + ".tmp" | |
| 555 | + | if err := os.WriteFile(tmpFile, out, 0600); err != nil { | |
| 556 | + | return fmt.Errorf("failed to write temp config: %w", err) | |
| 557 | + | } | |
| 558 | + | if err := os.Rename(tmpFile, configFile); err != nil { | |
| 559 | + | _ = os.Remove(tmpFile) | |
| 560 | + | return fmt.Errorf("failed to rename temp config: %w", err) | |
| 561 | + | } | |
| 562 | + | ||
| 563 | + | return nil | |
| 564 | + | } | |
| 565 | + | ||
| 566 | + | func backupConfigFile(configFile string) error { | |
| 567 | + | src, err := os.ReadFile(configFile) | |
| 568 | + | if err != nil { | |
| 569 | + | if os.IsNotExist(err) { | |
| 570 | + | return nil | |
| 571 | + | } | |
| 572 | + | return fmt.Errorf("failed to read config for backup: %w", err) | |
| 573 | + | } | |
| 574 | + | ||
| 575 | + | backupPath := configFile + ".bak" | |
| 576 | + | if err := os.WriteFile(backupPath, src, 0600); err != nil { | |
| 577 | + | return fmt.Errorf("failed to write backup config: %w", err) | |
| 578 | + | } | |
| 579 | + | ||
| 580 | + | return nil | |
| 581 | + | } | |
| 582 | + | ||
| 583 | + | func restoreConfigBackup(configFile string) error { | |
| 584 | + | backupPath := configFile + ".bak" | |
| 585 | + | src, err := os.ReadFile(backupPath) | |
| 586 | + | if err != nil { | |
| 587 | + | return fmt.Errorf("failed to read backup: %w", err) | |
| 588 | + | } | |
| 589 | + | ||
| 590 | + | if err := os.WriteFile(configFile, src, 0600); err != nil { | |
| 591 | + | return fmt.Errorf("failed to restore backup: %w", err) | |
| 592 | + | } | |
| 593 | + | ||
| 594 | + | return nil | |
| 595 | + | } | |
| 596 | + | ||
| 478 | 597 | // Watch starts watching for configuration changes. | |
| 479 | 598 | func (s *Service) Watch(ctx context.Context, onChange func()) error { | |
| 480 | 599 | log := zerowrap.FromCtx(ctx) | |
| @@ -4,6 +4,7 @@ package config | |||||
| 4 | import ( | 4 | import ( | ||
| 5 | "context" | 5 | "context" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | "os" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | "sync" | 9 | "sync" | ||
| 9 | "sync/atomic" | 10 | "sync/atomic" | ||
| @@ -11,6 +12,7 @@ import ( | |||||
| 11 | 12 | ||||
| 12 | "github.com/bnema/zerowrap" | 13 | "github.com/bnema/zerowrap" | ||
| 13 | "github.com/fsnotify/fsnotify" | 14 | "github.com/fsnotify/fsnotify" | ||
| 15 | + | "github.com/pelletier/go-toml/v2" | |||
| 14 | "github.com/spf13/viper" | 16 | "github.com/spf13/viper" | ||
| 15 | 17 | ||||
| 16 | "github.com/bnema/gordon/internal/boundaries/out" | 18 | "github.com/bnema/gordon/internal/boundaries/out" | ||
| @@ -454,27 +456,144 @@ func (s *Service) Save(ctx context.Context) error { | |||||
| 454 | s.mu.Lock() | 456 | s.mu.Lock() | ||
| 455 | defer s.mu.Unlock() | 457 | defer s.mu.Unlock() | ||
| 456 | 458 | ||||
| 457 | - | // Sync ALL mutable config sections back to Viper before writing. | 459 | + | configFile := s.viper.ConfigFileUsed() |
| 458 | - | // Using explicit Set() ensures the values are stored as proper flat maps, | 460 | + | if configFile == "" { |
| 459 | - | // preventing viper from splitting dotted keys (e.g. "reg.example.com") into | 461 | + | return log.WrapErr(fmt.Errorf("no config file path"), "cannot save config") |
| 460 | - | // nested TOML subtrees which corrupts the data on re-read. | 462 | + | } |
| 461 | - | s.viper.Set("routes", s.config.Routes) | |||
| 462 | - | s.viper.Set("external_routes", s.config.ExternalRoutes) | |||
| 463 | - | s.viper.Set("attachments", s.config.Attachments) | |||
| 464 | - | s.viper.Set("network_groups", s.config.NetworkGroups) | |||
| 465 | 463 | ||||
| 466 | - | // Record save time to debounce file watcher events | 464 | + | if err := backupConfigFile(configFile); err != nil { |
| 467 | - | atomic.StoreInt64(&s.lastSaveTime, time.Now().UnixNano()) | 465 | + | log.Warn().Err(err).Msg("failed to create config backup") |
| 466 | + | } | |||
| 467 | + | ||||
| 468 | + | snapshot := s.snapshotCriticalFields() | |||
| 468 | 469 | ||||
| 469 | - | // Write config to disk | 470 | + | if err := s.writeConfigSurgical(configFile); err != nil { |
| 470 | - | if err := s.viper.WriteConfig(); err != nil { | |||
| 471 | return log.WrapErr(err, "failed to write config") | 471 | return log.WrapErr(err, "failed to write config") | ||
| 472 | } | 472 | } | ||
| 473 | 473 | ||||
| 474 | + | atomic.StoreInt64(&s.lastSaveTime, time.Now().UnixNano()) | |||
| 475 | + | ||||
| 476 | + | if err := s.viper.ReadInConfig(); err != nil { | |||
| 477 | + | return log.WrapErr(err, "failed to re-read config after save") | |||
| 478 | + | } | |||
| 479 | + | ||||
| 480 | + | if err := s.verifyCriticalFields(snapshot); err != nil { | |||
| 481 | + | log.Error().Err(err).Msg("config corruption detected after save, restoring backup") | |||
| 482 | + | if restoreErr := restoreConfigBackup(configFile); restoreErr != nil { | |||
| 483 | + | log.Error().Err(restoreErr).Msg("CRITICAL: failed to restore config backup") | |||
| 484 | + | } | |||
| 485 | + | _ = s.viper.ReadInConfig() | |||
| 486 | + | return log.WrapErr(err, "config verification failed after save") | |||
| 487 | + | } | |||
| 488 | + | ||||
| 474 | log.Info().Msg("configuration saved to disk") | 489 | log.Info().Msg("configuration saved to disk") | ||
| 475 | return nil | 490 | return nil | ||
| 476 | } | 491 | } | ||
| 477 | 492 | ||||
| 493 | + | type configSnapshot struct { | |||
| 494 | + | values map[string]any | |||
| 495 | + | } | |||
| 496 | + | ||||
| 497 | + | func (s *Service) snapshotCriticalFields() configSnapshot { | |||
| 498 | + | snap := configSnapshot{values: make(map[string]any)} | |||
| 499 | + | for _, key := range s.viper.AllKeys() { | |||
| 500 | + | snap.values[key] = s.viper.Get(key) | |||
| 501 | + | } | |||
| 502 | + | return snap | |||
| 503 | + | } | |||
| 504 | + | ||||
| 505 | + | func (s *Service) verifyCriticalFields(snap configSnapshot) error { | |||
| 506 | + | for key, oldVal := range snap.values { | |||
| 507 | + | oldStr, ok := oldVal.(string) | |||
| 508 | + | if !ok || oldStr == "" { | |||
| 509 | + | continue | |||
| 510 | + | } | |||
| 511 | + | ||||
| 512 | + | newVal := s.viper.GetString(key) | |||
| 513 | + | if newVal != oldStr { | |||
| 514 | + | return fmt.Errorf("config key %q changed from %q to %q after save", key, oldStr, newVal) | |||
| 515 | + | } | |||
| 516 | + | } | |||
| 517 | + | ||||
| 518 | + | return nil | |||
| 519 | + | } | |||
| 520 | + | ||||
| 521 | + | func (s *Service) writeConfigSurgical(configFile string) error { | |||
| 522 | + | data, err := os.ReadFile(configFile) | |||
| 523 | + | if err != nil { | |||
| 524 | + | if os.IsNotExist(err) { | |||
| 525 | + | s.viper.Set("routes", s.config.Routes) | |||
| 526 | + | s.viper.Set("external_routes", s.config.ExternalRoutes) | |||
| 527 | + | s.viper.Set("attachments", s.config.Attachments) | |||
| 528 | + | s.viper.Set("network_groups", s.config.NetworkGroups) | |||
| 529 | + | return s.viper.WriteConfigAs(configFile) | |||
| 530 | + | } | |||
| 531 | + | return fmt.Errorf("failed to read config file: %w", err) | |||
| 532 | + | } | |||
| 533 | + | ||||
| 534 | + | var config map[string]any | |||
| 535 | + | if len(data) > 0 { | |||
| 536 | + | if err := toml.Unmarshal(data, &config); err != nil { | |||
| 537 | + | return fmt.Errorf("failed to parse config file: %w", err) | |||
| 538 | + | } | |||
| 539 | + | } | |||
| 540 | + | if config == nil { | |||
| 541 | + | config = make(map[string]any) | |||
| 542 | + | } | |||
| 543 | + | ||||
| 544 | + | config["routes"] = s.config.Routes | |||
| 545 | + | config["external_routes"] = s.config.ExternalRoutes | |||
| 546 | + | config["attachments"] = s.config.Attachments | |||
| 547 | + | config["network_groups"] = s.config.NetworkGroups | |||
| 548 | + | ||||
| 549 | + | out, err := toml.Marshal(config) | |||
| 550 | + | if err != nil { | |||
| 551 | + | return fmt.Errorf("failed to marshal config: %w", err) | |||
| 552 | + | } | |||
| 553 | + | ||||
| 554 | + | tmpFile := configFile + ".tmp" | |||
| 555 | + | if err := os.WriteFile(tmpFile, out, 0600); err != nil { | |||
| 556 | + | return fmt.Errorf("failed to write temp config: %w", err) | |||
| 557 | + | } | |||
| 558 | + | if err := os.Rename(tmpFile, configFile); err != nil { | |||
| 559 | + | _ = os.Remove(tmpFile) | |||
| 560 | + | return fmt.Errorf("failed to rename temp config: %w", err) | |||
| 561 | + | } | |||
| 562 | + | ||||
| 563 | + | return nil | |||
| 564 | + | } | |||
| 565 | + | ||||
| 566 | + | func backupConfigFile(configFile string) error { | |||
| 567 | + | src, err := os.ReadFile(configFile) | |||
| 568 | + | if err != nil { | |||
| 569 | + | if os.IsNotExist(err) { | |||
| 570 | + | return nil | |||
| 571 | + | } | |||
| 572 | + | return fmt.Errorf("failed to read config for backup: %w", err) | |||
| 573 | + | } | |||
| 574 | + | ||||
| 575 | + | backupPath := configFile + ".bak" | |||
| 576 | + | if err := os.WriteFile(backupPath, src, 0600); err != nil { | |||
| 577 | + | return fmt.Errorf("failed to write backup config: %w", err) | |||
| 578 | + | } | |||
| 579 | + | ||||
| 580 | + | return nil | |||
| 581 | + | } | |||
| 582 | + | ||||
| 583 | + | func restoreConfigBackup(configFile string) error { | |||
| 584 | + | backupPath := configFile + ".bak" | |||
| 585 | + | src, err := os.ReadFile(backupPath) | |||
| 586 | + | if err != nil { | |||
| 587 | + | return fmt.Errorf("failed to read backup: %w", err) | |||
| 588 | + | } | |||
| 589 | + | ||||
| 590 | + | if err := os.WriteFile(configFile, src, 0600); err != nil { | |||
| 591 | + | return fmt.Errorf("failed to restore backup: %w", err) | |||
| 592 | + | } | |||
| 593 | + | ||||
| 594 | + | return nil | |||
| 595 | + | } | |||
| 596 | + | ||||
| 478 | // Watch starts watching for configuration changes. | 597 | // Watch starts watching for configuration changes. | ||
| 479 | func (s *Service) Watch(ctx context.Context, onChange func()) error { | 598 | func (s *Service) Watch(ctx context.Context, onChange func()) error { | ||
| 480 | log := zerowrap.FromCtx(ctx) | 599 | log := zerowrap.FromCtx(ctx) | ||
M internal/usecase/config/service_test.go +136 -6
| @@ -951,12 +951,12 @@ func TestNormalizeBootstrapImage(t *testing.T) { | |||
| 951 | 951 | expected string | |
| 952 | 952 | wantErr string | |
| 953 | 953 | }{ | |
| 954 | - | {name: "bare name prepends registry", image: "pitlane", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | |
| 955 | - | {name: "bare name strips tag", image: "pitlane:v1", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | |
| 956 | - | {name: "qualified image unchanged", image: "reg.bnema.dev/pitlane", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | |
| 957 | - | {name: "qualified image strips tag", image: "reg.bnema.dev/pitlane:abc123", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | |
| 958 | - | {name: "other registry strips tag", image: "other.registry.com/myapp:v2", registryDomain: "reg.bnema.dev", expected: "other.registry.com/myapp"}, | |
| 959 | - | {name: "empty image", image: "", registryDomain: "reg.bnema.dev", wantErr: "image is required"}, | |
| 954 | + | {name: "bare name prepends registry", image: "myapp", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, | |
| 955 | + | {name: "bare name strips tag", image: "myapp:v1", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, | |
| 956 | + | {name: "qualified image unchanged", image: "reg.example.com/myapp", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, | |
| 957 | + | {name: "qualified image strips tag", image: "reg.example.com/myapp:abc123", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, | |
| 958 | + | {name: "other registry strips tag", image: "other.registry.com/myapp:v2", registryDomain: "reg.example.com", expected: "other.registry.com/myapp"}, | |
| 959 | + | {name: "empty image", image: "", registryDomain: "reg.example.com", wantErr: "image is required"}, | |
| 960 | 960 | {name: "empty registry domain", image: "pitlane", registryDomain: "", wantErr: "registry domain is not configured"}, | |
| 961 | 961 | } | |
| 962 | 962 | ||
| @@ -1155,6 +1155,136 @@ func TestService_FindRoutesByImage(t *testing.T) { | |||
| 1155 | 1155 | } | |
| 1156 | 1156 | ||
| 1157 | 1157 | func TestSavePreservesAllConfigFields(t *testing.T) { | |
| 1158 | + | t.Run("with defaults does not corrupt config", func(t *testing.T) { | |
| 1159 | + | tmpDir := t.TempDir() | |
| 1160 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |
| 1161 | + | initialConfig := ` | |
| 1162 | + | [server] | |
| 1163 | + | port = 8088 | |
| 1164 | + | registry_port = 5000 | |
| 1165 | + | registry_domain = "reg.example.com" | |
| 1166 | + | tls_enabled = true | |
| 1167 | + | tls_port = 8443 | |
| 1168 | + | data_dir = "/tmp/test-gordon" | |
| 1169 | + | runtime = "auto" | |
| 1170 | + | ||
| 1171 | + | [auth] | |
| 1172 | + | enabled = true | |
| 1173 | + | secrets_backend = "pass" | |
| 1174 | + | username = "testadmin" | |
| 1175 | + | password_hash = "test/auth/password_hash" | |
| 1176 | + | token_secret = "test/auth/token_secret" | |
| 1177 | + | token_expiry = "720h" | |
| 1178 | + | type = "password" | |
| 1179 | + | ||
| 1180 | + | [auto_route] | |
| 1181 | + | enabled = true | |
| 1182 | + | ||
| 1183 | + | [network_isolation] | |
| 1184 | + | enabled = true | |
| 1185 | + | network_prefix = "gordon" | |
| 1186 | + | dns_suffix = ".internal" | |
| 1187 | + | ||
| 1188 | + | [backups] | |
| 1189 | + | enabled = true | |
| 1190 | + | schedule = "daily" | |
| 1191 | + | storage_dir = "~/.gordon/backups" | |
| 1192 | + | ||
| 1193 | + | [routes] | |
| 1194 | + | "app.example.com" = "reg.example.com/myapp:latest" | |
| 1195 | + | ||
| 1196 | + | [attachments] | |
| 1197 | + | "app.example.com" = ["postgres:15"] | |
| 1198 | + | ||
| 1199 | + | [network_groups] | |
| 1200 | + | backend = ["app.example.com"] | |
| 1201 | + | ` | |
| 1202 | + | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | |
| 1203 | + | require.NoError(t, err) | |
| 1204 | + | ||
| 1205 | + | v := viper.New() | |
| 1206 | + | v.SetConfigFile(configFile) | |
| 1207 | + | v.SetDefault("server.port", 80) | |
| 1208 | + | v.SetDefault("server.tls_port", 443) | |
| 1209 | + | v.SetDefault("auth.secrets_backend", "") | |
| 1210 | + | v.SetDefault("auto_route.enabled", false) | |
| 1211 | + | v.SetDefault("network_isolation.enabled", false) | |
| 1212 | + | v.SetDefault("backups.enabled", false) | |
| 1213 | + | ||
| 1214 | + | err = v.ReadInConfig() | |
| 1215 | + | require.NoError(t, err) | |
| 1216 | + | ||
| 1217 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 1218 | + | svc := NewService(v, eventBus) | |
| 1219 | + | ctx := testContext() | |
| 1220 | + | ||
| 1221 | + | err = svc.Load(ctx) | |
| 1222 | + | require.NoError(t, err) | |
| 1223 | + | ||
| 1224 | + | err = svc.AddRoute(ctx, domain.Route{Domain: "new.example.com", Image: "newapp:latest"}) | |
| 1225 | + | require.NoError(t, err) | |
| 1226 | + | ||
| 1227 | + | v2 := viper.New() | |
| 1228 | + | v2.SetConfigFile(configFile) | |
| 1229 | + | err = v2.ReadInConfig() | |
| 1230 | + | require.NoError(t, err) | |
| 1231 | + | ||
| 1232 | + | assert.Equal(t, 8088, v2.GetInt("server.port")) | |
| 1233 | + | assert.Equal(t, 8443, v2.GetInt("server.tls_port")) | |
| 1234 | + | assert.Equal(t, "reg.example.com", v2.GetString("server.registry_domain")) | |
| 1235 | + | assert.Equal(t, "auto", v2.GetString("server.runtime")) | |
| 1236 | + | assert.Equal(t, "pass", v2.GetString("auth.secrets_backend")) | |
| 1237 | + | assert.Equal(t, "testadmin", v2.GetString("auth.username")) | |
| 1238 | + | assert.Equal(t, "test/auth/password_hash", v2.GetString("auth.password_hash")) | |
| 1239 | + | assert.Equal(t, "test/auth/token_secret", v2.GetString("auth.token_secret")) | |
| 1240 | + | assert.Equal(t, "password", v2.GetString("auth.type")) | |
| 1241 | + | assert.True(t, v2.GetBool("auto_route.enabled")) | |
| 1242 | + | assert.True(t, v2.GetBool("network_isolation.enabled")) | |
| 1243 | + | assert.Equal(t, ".internal", v2.GetString("network_isolation.dns_suffix")) | |
| 1244 | + | assert.True(t, v2.GetBool("backups.enabled")) | |
| 1245 | + | assert.Equal(t, "~/.gordon/backups", v2.GetString("backups.storage_dir")) | |
| 1246 | + | ||
| 1247 | + | savedRoutes := loadStringMap(v2.Get("routes")) | |
| 1248 | + | assert.Equal(t, "newapp:latest", savedRoutes["new.example.com"]) | |
| 1249 | + | assert.Equal(t, "reg.example.com/myapp:latest", savedRoutes["app.example.com"]) | |
| 1250 | + | ||
| 1251 | + | _, err = os.Stat(configFile + ".bak") | |
| 1252 | + | assert.NoError(t, err) | |
| 1253 | + | }) | |
| 1254 | + | ||
| 1255 | + | t.Run("backup created before write", func(t *testing.T) { | |
| 1256 | + | tmpDir := t.TempDir() | |
| 1257 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |
| 1258 | + | initialConfig := ` | |
| 1259 | + | [server] | |
| 1260 | + | port = 9999 | |
| 1261 | + | ||
| 1262 | + | [routes] | |
| 1263 | + | "app.example.com" = "myapp:latest" | |
| 1264 | + | ` | |
| 1265 | + | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | |
| 1266 | + | require.NoError(t, err) | |
| 1267 | + | ||
| 1268 | + | v := viper.New() | |
| 1269 | + | v.SetConfigFile(configFile) | |
| 1270 | + | err = v.ReadInConfig() | |
| 1271 | + | require.NoError(t, err) | |
| 1272 | + | ||
| 1273 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 1274 | + | svc := NewService(v, eventBus) | |
| 1275 | + | ctx := testContext() | |
| 1276 | + | err = svc.Load(ctx) | |
| 1277 | + | require.NoError(t, err) | |
| 1278 | + | ||
| 1279 | + | err = svc.AddRoute(ctx, domain.Route{Domain: "new.example.com", Image: "newapp:latest"}) | |
| 1280 | + | require.NoError(t, err) | |
| 1281 | + | ||
| 1282 | + | backupData, err := os.ReadFile(configFile + ".bak") | |
| 1283 | + | require.NoError(t, err) | |
| 1284 | + | assert.Contains(t, string(backupData), "port = 9999") | |
| 1285 | + | assert.NotContains(t, string(backupData), "new.example.com") | |
| 1286 | + | }) | |
| 1287 | + | ||
| 1158 | 1288 | t.Run("network_groups are persisted to disk", func(t *testing.T) { | |
| 1159 | 1289 | // Setup: config file with auth, server, and network_groups | |
| 1160 | 1290 | tmpDir := t.TempDir() | |
| @@ -951,12 +951,12 @@ func TestNormalizeBootstrapImage(t *testing.T) { | |||||
| 951 | expected string | 951 | expected string | ||
| 952 | wantErr string | 952 | wantErr string | ||
| 953 | }{ | 953 | }{ | ||
| 954 | - | {name: "bare name prepends registry", image: "pitlane", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | 954 | + | {name: "bare name prepends registry", image: "myapp", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, |
| 955 | - | {name: "bare name strips tag", image: "pitlane:v1", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | 955 | + | {name: "bare name strips tag", image: "myapp:v1", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, |
| 956 | - | {name: "qualified image unchanged", image: "reg.bnema.dev/pitlane", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | 956 | + | {name: "qualified image unchanged", image: "reg.example.com/myapp", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, |
| 957 | - | {name: "qualified image strips tag", image: "reg.bnema.dev/pitlane:abc123", registryDomain: "reg.bnema.dev", expected: "reg.bnema.dev/pitlane"}, | 957 | + | {name: "qualified image strips tag", image: "reg.example.com/myapp:abc123", registryDomain: "reg.example.com", expected: "reg.example.com/myapp"}, |
| 958 | - | {name: "other registry strips tag", image: "other.registry.com/myapp:v2", registryDomain: "reg.bnema.dev", expected: "other.registry.com/myapp"}, | 958 | + | {name: "other registry strips tag", image: "other.registry.com/myapp:v2", registryDomain: "reg.example.com", expected: "other.registry.com/myapp"}, |
| 959 | - | {name: "empty image", image: "", registryDomain: "reg.bnema.dev", wantErr: "image is required"}, | 959 | + | {name: "empty image", image: "", registryDomain: "reg.example.com", wantErr: "image is required"}, |
| 960 | {name: "empty registry domain", image: "pitlane", registryDomain: "", wantErr: "registry domain is not configured"}, | 960 | {name: "empty registry domain", image: "pitlane", registryDomain: "", wantErr: "registry domain is not configured"}, | ||
| 961 | } | 961 | } | ||
| 962 | 962 | ||||
| @@ -1155,6 +1155,136 @@ func TestService_FindRoutesByImage(t *testing.T) { | |||||
| 1155 | } | 1155 | } | ||
| 1156 | 1156 | ||||
| 1157 | func TestSavePreservesAllConfigFields(t *testing.T) { | 1157 | func TestSavePreservesAllConfigFields(t *testing.T) { | ||
| 1158 | + | t.Run("with defaults does not corrupt config", func(t *testing.T) { | |||
| 1159 | + | tmpDir := t.TempDir() | |||
| 1160 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |||
| 1161 | + | initialConfig := ` | |||
| 1162 | + | [server] | |||
| 1163 | + | port = 8088 | |||
| 1164 | + | registry_port = 5000 | |||
| 1165 | + | registry_domain = "reg.example.com" | |||
| 1166 | + | tls_enabled = true | |||
| 1167 | + | tls_port = 8443 | |||
| 1168 | + | data_dir = "/tmp/test-gordon" | |||
| 1169 | + | runtime = "auto" | |||
| 1170 | + | ||||
| 1171 | + | [auth] | |||
| 1172 | + | enabled = true | |||
| 1173 | + | secrets_backend = "pass" | |||
| 1174 | + | username = "testadmin" | |||
| 1175 | + | password_hash = "test/auth/password_hash" | |||
| 1176 | + | token_secret = "test/auth/token_secret" | |||
| 1177 | + | token_expiry = "720h" | |||
| 1178 | + | type = "password" | |||
| 1179 | + | ||||
| 1180 | + | [auto_route] | |||
| 1181 | + | enabled = true | |||
| 1182 | + | ||||
| 1183 | + | [network_isolation] | |||
| 1184 | + | enabled = true | |||
| 1185 | + | network_prefix = "gordon" | |||
| 1186 | + | dns_suffix = ".internal" | |||
| 1187 | + | ||||
| 1188 | + | [backups] | |||
| 1189 | + | enabled = true | |||
| 1190 | + | schedule = "daily" | |||
| 1191 | + | storage_dir = "~/.gordon/backups" | |||
| 1192 | + | ||||
| 1193 | + | [routes] | |||
| 1194 | + | "app.example.com" = "reg.example.com/myapp:latest" | |||
| 1195 | + | ||||
| 1196 | + | [attachments] | |||
| 1197 | + | "app.example.com" = ["postgres:15"] | |||
| 1198 | + | ||||
| 1199 | + | [network_groups] | |||
| 1200 | + | backend = ["app.example.com"] | |||
| 1201 | + | ` | |||
| 1202 | + | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | |||
| 1203 | + | require.NoError(t, err) | |||
| 1204 | + | ||||
| 1205 | + | v := viper.New() | |||
| 1206 | + | v.SetConfigFile(configFile) | |||
| 1207 | + | v.SetDefault("server.port", 80) | |||
| 1208 | + | v.SetDefault("server.tls_port", 443) | |||
| 1209 | + | v.SetDefault("auth.secrets_backend", "") | |||
| 1210 | + | v.SetDefault("auto_route.enabled", false) | |||
| 1211 | + | v.SetDefault("network_isolation.enabled", false) | |||
| 1212 | + | v.SetDefault("backups.enabled", false) | |||
| 1213 | + | ||||
| 1214 | + | err = v.ReadInConfig() | |||
| 1215 | + | require.NoError(t, err) | |||
| 1216 | + | ||||
| 1217 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 1218 | + | svc := NewService(v, eventBus) | |||
| 1219 | + | ctx := testContext() | |||
| 1220 | + | ||||
| 1221 | + | err = svc.Load(ctx) | |||
| 1222 | + | require.NoError(t, err) | |||
| 1223 | + | ||||
| 1224 | + | err = svc.AddRoute(ctx, domain.Route{Domain: "new.example.com", Image: "newapp:latest"}) | |||
| 1225 | + | require.NoError(t, err) | |||
| 1226 | + | ||||
| 1227 | + | v2 := viper.New() | |||
| 1228 | + | v2.SetConfigFile(configFile) | |||
| 1229 | + | err = v2.ReadInConfig() | |||
| 1230 | + | require.NoError(t, err) | |||
| 1231 | + | ||||
| 1232 | + | assert.Equal(t, 8088, v2.GetInt("server.port")) | |||
| 1233 | + | assert.Equal(t, 8443, v2.GetInt("server.tls_port")) | |||
| 1234 | + | assert.Equal(t, "reg.example.com", v2.GetString("server.registry_domain")) | |||
| 1235 | + | assert.Equal(t, "auto", v2.GetString("server.runtime")) | |||
| 1236 | + | assert.Equal(t, "pass", v2.GetString("auth.secrets_backend")) | |||
| 1237 | + | assert.Equal(t, "testadmin", v2.GetString("auth.username")) | |||
| 1238 | + | assert.Equal(t, "test/auth/password_hash", v2.GetString("auth.password_hash")) | |||
| 1239 | + | assert.Equal(t, "test/auth/token_secret", v2.GetString("auth.token_secret")) | |||
| 1240 | + | assert.Equal(t, "password", v2.GetString("auth.type")) | |||
| 1241 | + | assert.True(t, v2.GetBool("auto_route.enabled")) | |||
| 1242 | + | assert.True(t, v2.GetBool("network_isolation.enabled")) | |||
| 1243 | + | assert.Equal(t, ".internal", v2.GetString("network_isolation.dns_suffix")) | |||
| 1244 | + | assert.True(t, v2.GetBool("backups.enabled")) | |||
| 1245 | + | assert.Equal(t, "~/.gordon/backups", v2.GetString("backups.storage_dir")) | |||
| 1246 | + | ||||
| 1247 | + | savedRoutes := loadStringMap(v2.Get("routes")) | |||
| 1248 | + | assert.Equal(t, "newapp:latest", savedRoutes["new.example.com"]) | |||
| 1249 | + | assert.Equal(t, "reg.example.com/myapp:latest", savedRoutes["app.example.com"]) | |||
| 1250 | + | ||||
| 1251 | + | _, err = os.Stat(configFile + ".bak") | |||
| 1252 | + | assert.NoError(t, err) | |||
| 1253 | + | }) | |||
| 1254 | + | ||||
| 1255 | + | t.Run("backup created before write", func(t *testing.T) { | |||
| 1256 | + | tmpDir := t.TempDir() | |||
| 1257 | + | configFile := filepath.Join(tmpDir, "gordon.toml") | |||
| 1258 | + | initialConfig := ` | |||
| 1259 | + | [server] | |||
| 1260 | + | port = 9999 | |||
| 1261 | + | ||||
| 1262 | + | [routes] | |||
| 1263 | + | "app.example.com" = "myapp:latest" | |||
| 1264 | + | ` | |||
| 1265 | + | err := os.WriteFile(configFile, []byte(initialConfig), 0600) | |||
| 1266 | + | require.NoError(t, err) | |||
| 1267 | + | ||||
| 1268 | + | v := viper.New() | |||
| 1269 | + | v.SetConfigFile(configFile) | |||
| 1270 | + | err = v.ReadInConfig() | |||
| 1271 | + | require.NoError(t, err) | |||
| 1272 | + | ||||
| 1273 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 1274 | + | svc := NewService(v, eventBus) | |||
| 1275 | + | ctx := testContext() | |||
| 1276 | + | err = svc.Load(ctx) | |||
| 1277 | + | require.NoError(t, err) | |||
| 1278 | + | ||||
| 1279 | + | err = svc.AddRoute(ctx, domain.Route{Domain: "new.example.com", Image: "newapp:latest"}) | |||
| 1280 | + | require.NoError(t, err) | |||
| 1281 | + | ||||
| 1282 | + | backupData, err := os.ReadFile(configFile + ".bak") | |||
| 1283 | + | require.NoError(t, err) | |||
| 1284 | + | assert.Contains(t, string(backupData), "port = 9999") | |||
| 1285 | + | assert.NotContains(t, string(backupData), "new.example.com") | |||
| 1286 | + | }) | |||
| 1287 | + | ||||
| 1158 | t.Run("network_groups are persisted to disk", func(t *testing.T) { | 1288 | t.Run("network_groups are persisted to disk", func(t *testing.T) { | ||
| 1159 | // Setup: config file with auth, server, and network_groups | 1289 | // Setup: config file with auth, server, and network_groups | ||
| 1160 | tmpDir := t.TempDir() | 1290 | tmpDir := t.TempDir() | ||