From 433305e0aade8a93383ef031ab8c5729bb66005f Mon Sep 17 00:00:00 2001 From: eding Date: Sun, 23 Aug 2026 19:20:56 +0200 Subject: [PATCH] fix: require complete delivery for reconcile --- cmd/bootstrap.go | 14 +++-- internal/bootstrap/bootstrap.go | 8 +-- internal/bootstrap/bootstrap_test.go | 78 +++++++++++++++++++++------- internal/config/config.go | 41 +++++++++------ internal/config/config_test.go | 38 ++++++++++++-- internal/config/types.go | 4 +- 6 files changed, 135 insertions(+), 48 deletions(-) diff --git a/cmd/bootstrap.go b/cmd/bootstrap.go index 2fd28bf..16c4189 100644 --- a/cmd/bootstrap.go +++ b/cmd/bootstrap.go @@ -65,7 +65,8 @@ func runBootstrap(cmd *cobra.Command, args []string) error { if err != nil { return err } - if err := config.ValidateDelivery(cfg); err != nil { + cfg, err = config.ResolveDelivery(cfg) + if err != nil { return err } if err := bootstrap.EnsureTemplateRevisions(cfg); err != nil { @@ -81,7 +82,8 @@ func runBootstrap(cmd *cobra.Command, args []string) error { if err != nil { return err } - if err := config.ValidateDelivery(cfg); err != nil { + cfg, err = config.ResolveDelivery(cfg) + if err != nil { return err } authorization, err := bootstrap.NewWebhookAuthorization() @@ -130,7 +132,8 @@ func runBootstrap(cmd *cobra.Command, args []string) error { if err != nil { return err } - if err := config.ValidateDelivery(cfg); err != nil { + cfg, err = config.ResolveDelivery(cfg) + if err != nil { return err } if err := ensurePublishAppCheckoutClean(bootstrapPublishAppFrom); err != nil { @@ -192,7 +195,7 @@ func runBootstrap(cmd *cobra.Command, args []string) error { cfg, err = config.Resolve(cfg) } if err == nil && bootstrapPromptOperationalSecrets { - err = config.ValidateDelivery(cfg) + cfg, err = config.ResolveDelivery(cfg) } if err == nil && bootstrapPromptOperationalSecrets { var secrets map[string]map[string]string @@ -235,7 +238,8 @@ func runBootstrap(cmd *cobra.Command, args []string) error { return err } if bootstrapRegisterWebhook { - if err := config.ValidateDelivery(cfg); err != nil { + cfg, err = config.ResolveDelivery(cfg) + if err != nil { return err } if err := seedForgejoOperationalCredentials(cfg); err != nil { diff --git a/internal/bootstrap/bootstrap.go b/internal/bootstrap/bootstrap.go index c96a08e..27afee6 100644 --- a/internal/bootstrap/bootstrap.go +++ b/internal/bootstrap/bootstrap.go @@ -122,12 +122,14 @@ func (r Runner) Run() error { return err } r.Config = resolved - deliveryConfigured := r.Config.Delivery.Configured() - if r.RegisterWebhook || r.Config.Talos.AutoBootstrapFlux { - if err := config.ValidateDelivery(r.Config); err != nil { + if r.RegisterWebhook { + resolvedDelivery, err := config.ResolveDelivery(r.Config) + if err != nil { return err } + r.Config = resolvedDelivery } + deliveryConfigured := r.Config.Delivery.Configured() if err := preflight(r.Config); err != nil { return fmt.Errorf("preflight: %w", err) } diff --git a/internal/bootstrap/bootstrap_test.go b/internal/bootstrap/bootstrap_test.go index fb5b945..9f44193 100644 --- a/internal/bootstrap/bootstrap_test.go +++ b/internal/bootstrap/bootstrap_test.go @@ -243,6 +243,30 @@ func TestCopyTemplateBaseComponentsRefreshesCNPGAndPreservesGeneratedSecrets(t * } } +func TestPlatformCopyPreservesExistingDeliveryBases(t *testing.T) { + templateDir := t.TempDir() + repoDir := t.TempDir() + for _, component := range templateBaseComponents { + if err := os.MkdirAll(filepath.Join(templateDir, "base", component), 0755); err != nil { + t.Fatal(err) + } + } + legacy := filepath.Join(repoDir, "base", "tekton", "legacy-delivery.yaml") + if err := os.MkdirAll(filepath.Dir(legacy), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(legacy, []byte("generated: delivery\n"), 0644); err != nil { + t.Fatal(err) + } + if err := copyTemplateBaseComponents(templateDir, repoDir, false); err != nil { + t.Fatal(err) + } + content, err := os.ReadFile(legacy) + if err != nil || string(content) != "generated: delivery\n" { + t.Fatalf("platform reconciliation removed existing delivery output: %q, %v", content, err) + } +} + func TestCopyAndRenderCiliumBasesRefreshesTemplateWithoutLeavingPlaceholders(t *testing.T) { templateDir := t.TempDir() repoDir := t.TempDir() @@ -651,6 +675,41 @@ func TestWebhookTargetTimeoutExceedsExternalSecretRefreshInterval(t *testing.T) } } +func runnerTestConfig(workspace, ageKeyPath string) config.Config { + return config.Config{ + ClusterID: "test-cluster", + WorkspaceDir: workspace, + Git: config.GitConfig{Provider: "forgejo", BaseURL: "https://git.example.test", Username: "bot", Token: "token", Owner: "test-org", CloneParent: filepath.Join(workspace, "checkouts")}, + Flux: config.FluxConfig{RepoName: "cluster", Branch: "main", ClusterPath: "./clusters/test", ClusterDomain: "example.test", ManifestsRepo: "manifests"}, + Templates: config.TemplateConfig{TalosRepoURL: "https://git.example.test/talos.git", TalosRepoRef: "main", CICDRepoURL: "https://git.example.test/template.git", CICDRepoRef: "main", ManifestsRepoURL: "https://git.example.test/manifests.git", ManifestsRepoRef: "main"}, + Cilium: config.CiliumConfig{TrafficInterface: "eth1", LoadBalancerStart: "192.168.45.19", LoadBalancerEnd: "192.168.45.30"}, + DemocraticCSI: config.DemocraticCSIConfig{TrueNASAPIKey: "key", TrueNASHost: "truenas.example.test", TargetPortal: "truenas.example.test:3260", ShareHost: "truenas.example.test", DatasetParentNFS: "pool/kubernetes/nfs/v", DatasetSnapshotsNFS: "pool/kubernetes/nfs/s", AllowedNetworks: "192.168.45.0/24", NameSuffix: "-test", PortalGroup: "1", InitiatorGroup: "1"}, + Delivery: config.DeliveryConfig{AppName: "app", AppRepoURL: "https://git.example.test/app.git", AppRepoRef: "main", ProductionBranch: "production", ImageRepository: "registry.example.test/test/app", BuildOutputDirectory: "dist", BuildConfiguration: "production", WebhookHostname: "tekton.example.test", WebhookPath: "/"}, + SOPS: config.SOPSConfig{AgeKeyPath: ageKeyPath}, + Talos: config.TalosConfig{ + RepoDirName: "talos", TerraformDir: "terraform", GeneratedDir: "generated", ConfigFileName: "terraform.tfvars", + Proxmox: config.TalosProxmoxConfig{APIURL: "https://proxmox.example.test:8006", APITokenID: "id", APITokenSecret: "secret"}, + Cluster: config.TalosClusterConfig{Name: "test-cluster", Domain: "example.test"}, + Image: config.TalosImageConfig{TalosVersion: "v1.13.6", KubernetesVersion: "v1.33.4", SchematicID: "abcdefghijkl"}, + Nodes: []config.TalosNode{{Name: "cp-01", VMID: 100, Role: "controlplane", Networks: []config.TalosNetwork{{IP: "192.168.45.3", CIDR: "192.168.45.0/28", Gateway: "192.168.45.1", VLANID: 45}, {IP: "192.168.45.18", CIDR: "192.168.45.16/28", VLANID: 451}}}}, + }, + } +} + +func TestRunnerAutoBootstrapFluxIgnoresPartialLegacyDelivery(t *testing.T) { + originalPreflight := preflight + t.Cleanup(func() { preflight = originalPreflight }) + preflight = func(config.Config) error { return errors.New("reached preflight") } + + cfg := runnerTestConfig(t.TempDir(), "") + cfg.Delivery = config.DeliveryConfig{AppName: "legacy-app", AppRepoURL: "https://git.example.test/test-org/legacy-app.git", ProductionBranch: "production", ImageRepository: "registry.example.test/test-org/legacy-app"} + cfg.Talos.AutoBootstrapFlux = true + err := (Runner{Config: cfg}).Run() + if err == nil || !strings.Contains(err.Error(), "reached preflight") { + t.Fatalf("platform reconcile validated partial delivery before preflight: %v", err) + } +} + func TestRunnerRegisterWebhookSkipsTemplateRevisions(t *testing.T) { originalPreflight := preflight originalGit := runGit @@ -692,24 +751,7 @@ func TestRunnerRegisterWebhookSkipsTemplateRevisions(t *testing.T) { return nil } - cfg := config.Config{ - ClusterID: "test-cluster", - WorkspaceDir: workspace, - Git: config.GitConfig{Provider: "forgejo", BaseURL: "https://git.example.test", Username: "bot", Token: "token", Owner: "test-org", CloneParent: filepath.Join(workspace, "checkouts")}, - Flux: config.FluxConfig{RepoName: "cluster", Branch: "main", ClusterPath: "./clusters/test", ClusterDomain: "example.test", ManifestsRepo: "manifests"}, - Templates: config.TemplateConfig{TalosRepoURL: "https://git.example.test/talos.git", TalosRepoRef: "main", CICDRepoURL: "https://git.example.test/template.git", CICDRepoRef: "main", ManifestsRepoURL: "https://git.example.test/manifests.git", ManifestsRepoRef: "main"}, - Cilium: config.CiliumConfig{TrafficInterface: "eth1", LoadBalancerStart: "192.168.45.19", LoadBalancerEnd: "192.168.45.30"}, - DemocraticCSI: config.DemocraticCSIConfig{TrueNASAPIKey: "key", TrueNASHost: "truenas.example.test", TargetPortal: "truenas.example.test:3260", ShareHost: "truenas.example.test", DatasetParentNFS: "pool/kubernetes/nfs/v", DatasetSnapshotsNFS: "pool/kubernetes/nfs/s", AllowedNetworks: "192.168.45.0/24", NameSuffix: "-test", PortalGroup: "1", InitiatorGroup: "1"}, - Delivery: config.DeliveryConfig{AppName: "app", AppRepoURL: "https://git.example.test/app.git", AppRepoRef: "main", ProductionBranch: "production", ImageRepository: "registry.example.test/test/app", WebhookHostname: "tekton.example.test", WebhookPath: "/"}, - SOPS: config.SOPSConfig{AgeKeyPath: ageKeyPath}, - Talos: config.TalosConfig{ - RepoDirName: "talos", TerraformDir: "terraform", GeneratedDir: "generated", ConfigFileName: "terraform.tfvars", - Proxmox: config.TalosProxmoxConfig{APIURL: "https://proxmox.example.test:8006", APITokenID: "id", APITokenSecret: "secret"}, - Cluster: config.TalosClusterConfig{Name: "test-cluster", Domain: "example.test"}, - Image: config.TalosImageConfig{TalosVersion: "v1.13.6", KubernetesVersion: "v1.33.4", SchematicID: "abcdefghijkl"}, - Nodes: []config.TalosNode{{Name: "cp-01", VMID: 100, Role: "controlplane", Networks: []config.TalosNetwork{{IP: "192.168.45.3", CIDR: "192.168.45.0/28", Gateway: "192.168.45.1", VLANID: 45}, {IP: "192.168.45.18", CIDR: "192.168.45.16/28", VLANID: 451}}}}, - }, - } + cfg := runnerTestConfig(workspace, ageKeyPath) partial := cfg partial.Delivery.ImageRepository = "" preflight = func(config.Config) error { diff --git a/internal/config/config.go b/internal/config/config.go index 7555a2c..8a98945 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -100,6 +100,12 @@ func Resolve(cfg Config) (Config, error) { return cfg, Validate(cfg) } +// ResolveDelivery applies delivery defaults for an explicitly requested app operation. +func ResolveDelivery(cfg Config) (Config, error) { + applyDeliveryDefaults(&cfg) + return cfg, ValidateDelivery(cfg) +} + func applyDefaults(cfg *Config) { if cfg.ClusterID == "" { cfg.ClusterID = cfg.Talos.Cluster.Name @@ -158,23 +164,6 @@ func applyDefaults(cfg *Config) { if cfg.DemocraticCSI.InitiatorGroup == "" { cfg.DemocraticCSI.InitiatorGroup = "1" } - if cfg.Delivery.Configured() { - if cfg.Delivery.AppRepoRef == "" { - cfg.Delivery.AppRepoRef = cfg.Flux.Branch - } - if cfg.Delivery.BuildOutputDirectory == "" { - cfg.Delivery.BuildOutputDirectory = "dist" - } - if cfg.Delivery.BuildConfiguration == "" { - cfg.Delivery.BuildConfiguration = "production" - } - if cfg.Delivery.WebhookHostname == "" && cfg.Flux.ClusterDomain != "" { - cfg.Delivery.WebhookHostname = "tekton." + cfg.Flux.ClusterDomain - } - if cfg.Delivery.WebhookPath == "" { - cfg.Delivery.WebhookPath = "/" - } - } if cfg.Talos.RepoDirName == "" { cfg.Talos.RepoDirName = "maidn-talos-proxmox" } @@ -219,6 +208,24 @@ func applyDefaults(cfg *Config) { } } +func applyDeliveryDefaults(cfg *Config) { + if cfg.Delivery.AppRepoRef == "" { + cfg.Delivery.AppRepoRef = cfg.Flux.Branch + } + if cfg.Delivery.BuildOutputDirectory == "" { + cfg.Delivery.BuildOutputDirectory = "dist" + } + if cfg.Delivery.BuildConfiguration == "" { + cfg.Delivery.BuildConfiguration = "production" + } + if cfg.Delivery.WebhookHostname == "" && cfg.Flux.ClusterDomain != "" { + cfg.Delivery.WebhookHostname = "tekton." + cfg.Flux.ClusterDomain + } + if cfg.Delivery.WebhookPath == "" { + cfg.Delivery.WebhookPath = "/" + } +} + func Validate(cfg Config) error { if !regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`).MatchString(cfg.ClusterID) { return errors.New("clusterId must be a lowercase DNS label") diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 85b8b30..6b89f06 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -17,7 +17,7 @@ func validConfig(t *testing.T) Config { Templates: TemplateConfig{TalosRepoURL: "https://git.example.test/talos.git", TalosRepoRef: "main", CICDRepoURL: "https://git.example.test/template.git", CICDRepoRef: "main", ManifestsRepoURL: "https://git.example.test/manifests.git", ManifestsRepoRef: "main"}, Cilium: CiliumConfig{TrafficInterface: "eth1", LoadBalancerStart: "192.168.45.19", LoadBalancerEnd: "192.168.45.30"}, DemocraticCSI: DemocraticCSIConfig{TrueNASAPIKey: "api-key", TrueNASHost: "truenas.example.test", TargetPortal: "truenas.example.test:3260", ShareHost: "truenas.example.test", DatasetParentNFS: "pool/kubernetes/nfs/v", DatasetSnapshotsNFS: "pool/kubernetes/nfs/s", AllowedNetworks: "192.168.45.0/24", NameSuffix: "-test", PortalGroup: "1", InitiatorGroup: "1"}, - Delivery: DeliveryConfig{AppName: "web-ui", AppRepoURL: "https://git.example.test/test-org/web-ui.git", AppRepoRef: "main", ProductionBranch: "production", ImageRepository: "registry.example.test/test-org/web-ui", WebhookHostname: "tekton.example.test", WebhookPath: "/"}, + Delivery: DeliveryConfig{AppName: "web-ui", AppRepoURL: "https://git.example.test/test-org/web-ui.git", AppRepoRef: "main", ProductionBranch: "production", ImageRepository: "registry.example.test/test-org/web-ui", BuildOutputDirectory: "dist", BuildConfiguration: "production", WebhookHostname: "tekton.example.test", WebhookPath: "/"}, Talos: TalosConfig{ RepoDirName: "talos", TerraformDir: "terraform", GeneratedDir: "generated", ConfigFileName: "terraform.tfvars", Proxmox: TalosProxmoxConfig{APIURL: "https://proxmox.example.test:8006", APITokenID: "id", APITokenSecret: "secret"}, @@ -32,7 +32,8 @@ func TestResolveDeliveryStates(t *testing.T) { platformOnly := validConfig(t) platformOnly.Delivery = DeliveryConfig{} partialLegacy := platformOnly - partialLegacy.Delivery.AppName = "legacy-app" + partialLegacy.Delivery = DeliveryConfig{AppName: "legacy-app", AppRepoURL: "https://git.example.test/test-org/legacy-app.git", ProductionBranch: "production", ImageRepository: "registry.example.test/test-org/legacy-app"} + partialLegacy.Talos.AutoBootstrapFlux = true for _, test := range []struct { name string @@ -58,6 +59,30 @@ func TestResolveDeliveryStates(t *testing.T) { } } +func TestResolveDeliveryAppliesDefaultsOnlyForExplicitAppOperations(t *testing.T) { + cfg := validConfig(t) + cfg.Delivery.AppRepoRef = "" + cfg.Delivery.BuildOutputDirectory = "" + cfg.Delivery.BuildConfiguration = "" + cfg.Delivery.WebhookHostname = "" + cfg.Delivery.WebhookPath = "" + + platform, err := Resolve(cfg) + if err != nil { + t.Fatal(err) + } + if platform.Delivery.Configured() { + t.Fatal("platform resolution treated incomplete delivery as configured") + } + delivery, err := ResolveDelivery(platform) + if err != nil { + t.Fatal(err) + } + if !delivery.Delivery.Configured() || delivery.Delivery.AppRepoRef != "main" || delivery.Delivery.BuildOutputDirectory != "dist" || delivery.Delivery.BuildConfiguration != "production" || delivery.Delivery.WebhookURL() != "https://tekton.example.test/" { + t.Fatalf("ResolveDelivery() did not apply the complete delivery contract: %#v", delivery.Delivery) + } +} + func TestValidateDeliveryRequiresCompleteConfig(t *testing.T) { cfg := validConfig(t) cfg.Delivery.ImageRepository = "" @@ -99,7 +124,14 @@ func TestValidateRequiresDistinctValidProductionBranch(t *testing.T) { } func TestResolveDefaultsWebhookEndpoint(t *testing.T) { - cfg, err := Resolve(validConfig(t)) + input := validConfig(t) + input.Delivery.WebhookHostname = "" + input.Delivery.WebhookPath = "" + platform, err := Resolve(input) + if err != nil { + t.Fatal(err) + } + cfg, err := ResolveDelivery(platform) if err != nil { t.Fatal(err) } diff --git a/internal/config/types.go b/internal/config/types.go index e77bbaf..9b7aa81 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -40,9 +40,9 @@ type DeliveryConfig struct { WebhookPath string `yaml:"webhookPath"` } -// Configured reports whether the non-default app-delivery settings are complete. +// Configured reports whether the app-delivery rendering contract is complete. func (c DeliveryConfig) Configured() bool { - return c.AppName != "" && c.AppRepoURL != "" && c.ProductionBranch != "" && c.ImageRepository != "" + return c.AppName != "" && c.AppRepoURL != "" && c.AppRepoRef != "" && c.ProductionBranch != "" && c.ImageRepository != "" && c.BuildOutputDirectory != "" && c.BuildConfiguration != "" && c.WebhookHostname != "" && c.WebhookPath != "" } func (c DeliveryConfig) WebhookURL() string { -- 2.43.7