diff --git a/cigen/render_gitlab.go b/cigen/render_gitlab.go index 55d5dfe2..1c5b109a 100644 --- a/cigen/render_gitlab.go +++ b/cigen/render_gitlab.go @@ -48,8 +48,8 @@ func renderGitLabWorkflow(p *CIPlan) (string, error) { // Global variables. Project-level CI/CD variables (secrets) are already // injected into every job's environment by GitLab, so we do NOT re-declare - // them as `NAME: $NAME` (a no-op that only obscures the pipeline). Only the - // wfctl version pin is a genuine pipeline variable. + // the plan-wide union globally. Only the wfctl version pin is a genuine + // pipeline variable. b.WriteString("variables:\n") fmt.Fprintf(&b, " WFCTL_VERSION: %q\n", version) b.WriteString("\n") @@ -100,8 +100,21 @@ func renderGitLabWorkflow(p *CIPlan) (string, error) { fmt.Fprintf(&b, " - job: %s\n", prevJob) b.WriteString(" artifacts: false\n") } + secrets := p.Secrets + if phase.Scoped { + secrets = phase.Secrets + } + if len(secrets) > 0 { + b.WriteString(" variables:\n") + for _, name := range sortedSecretNames(secrets) { + fmt.Fprintf(&b, " %s: $%s\n", name, name) + } + } b.WriteString(" script:\n") isLastPhase := i == len(p.Phases)-1 + if p.PlanGuard { + writeGitLabPlanGuard(&b, phase.ConfigPath) + } if isLastPhase && p.Migrations != nil { fmt.Fprintf(&b, " - %s\n", migrationsUpCommand(phase.ConfigPath, p.Migrations.Env)) } @@ -137,3 +150,16 @@ func renderGitLabWorkflow(p *CIPlan) (string, error) { return b.String(), nil } + +// writeGitLabPlanGuard refuses to apply when the plan includes a replace or +// destroy of a protected resource. The plan output stays in the job log (tee) +// and a destructive plan fails the job (exit 1) — no `|| true`. +func writeGitLabPlanGuard(b *strings.Builder, configPath string) { + b.WriteString(" - |\n") + fmt.Fprintf(b, " wfctl infra plan --config '%s' | tee plan-guard.txt\n", configPath) + b.WriteString(" if grep -Eq -- '^[[:space:]]*(- delete|-/\\+ replace)[[:space:]]' plan-guard.txt || \\\n") + b.WriteString(" grep -Eq -- 'Plan: .*([1-9][0-9]* to replace|[1-9][0-9]* to destroy)' plan-guard.txt; then\n") + b.WriteString(" echo 'Refusing apply: plan includes replace or destroy of a protected resource.' >&2\n") + b.WriteString(" exit 1\n") + b.WriteString(" fi\n") +} diff --git a/cigen/render_gitlab_test.go b/cigen/render_gitlab_test.go index 7710821c..8892e6a8 100644 --- a/cigen/render_gitlab_test.go +++ b/cigen/render_gitlab_test.go @@ -51,13 +51,14 @@ func TestRenderGitLabCI_NoRedundantSecretVars(t *testing.T) { t.Fatalf("RenderGitLabCI: %v", err) } content := files[".gitlab-ci.yml"] + globalVars := gitLabTopLevelBlock(content, "variables") // Project-level CI/CD variables (secrets) are auto-injected by GitLab into - // every job, so the renderer must NOT re-declare them as `NAME: $NAME` - // no-ops in the global variables block. + // every job, so the renderer must NOT re-declare the plan-wide union as + // `NAME: $NAME` no-ops in the global variables block. for _, s := range plan.Secrets { redundant := " " + s.Name + ": $" + s.Name - if strings.Contains(content, redundant) { + if strings.Contains(globalVars, redundant) { t.Errorf("expected no redundant `%s: $%s` declaration in variables block", s.Name, s.Name) } } @@ -97,6 +98,73 @@ func TestRenderGitLabCI_TwoPhaseNeeds(t *testing.T) { } } +func TestRenderGitLabCI_PlanGuardIsRealGate(t *testing.T) { + plan := richCIPlan() + + files, err := cigen.RenderGitLabCI(plan) + if err != nil { + t.Fatalf("RenderGitLabCI: %v", err) + } + content := files[".gitlab-ci.yml"] + + if !strings.Contains(content, "plan-guard.txt") { + t.Fatal("expected a plan guard when PlanGuard is set") + } + if strings.Contains(content, "|| true") { + t.Error("plan guard must not contain `|| true`") + } + if !strings.Contains(content, "exit 1") { + t.Error("plan guard must contain a failing-exit path") + } + if !strings.Contains(content, "to replace") || !strings.Contains(content, "to destroy") { + t.Error("plan guard should detect replace/destroy plans") + } + if !strings.Contains(content, "tee plan-guard.txt") { + t.Error("plan guard should keep plan output visible") + } + deploy := gitLabJobBlock(content, "apply-deploy") + guardIndex := strings.Index(deploy, "plan-guard.txt") + migrateIndex := strings.Index(deploy, "wfctl migrations up") + if guardIndex < 0 || migrateIndex < 0 { + t.Fatalf("expected plan guard and migrations in deploy job\n%s", deploy) + } + if guardIndex > migrateIndex { + t.Fatalf("plan guard must run before migrations\n%s", deploy) + } +} + +func TestRenderGitLabCI_ScopedPhase(t *testing.T) { + p := &cigen.CIPlan{ + DefaultBranch: "main", + Secrets: []cigen.SecretRef{{Name: "UNION_ONLY"}}, + Phases: []cigen.DeployPhase{ + {Name: "prereq", ConfigPath: "prereq.yaml", Scoped: true, Secrets: []cigen.SecretRef{{Name: "PREREQ_ONLY"}}}, + {Name: "deploy", ConfigPath: "deploy.yaml", Scoped: true, Secrets: []cigen.SecretRef{{Name: "DEPLOY_ONLY"}}}, + }, + } + + files, err := cigen.RenderGitLabCI(p) + if err != nil { + t.Fatalf("RenderGitLabCI: %v", err) + } + content := files[".gitlab-ci.yml"] + prereq := gitLabJobBlock(content, "apply-prereq") + deploy := gitLabJobBlock(content, "apply-deploy") + + if !strings.Contains(prereq, "PREREQ_ONLY") { + t.Errorf("prereq job must reference PREREQ_ONLY\n%s", prereq) + } + if strings.Contains(prereq, "DEPLOY_ONLY") || strings.Contains(prereq, "UNION_ONLY") { + t.Errorf("prereq job must not reference other phases' / union secrets\n%s", prereq) + } + if !strings.Contains(deploy, "DEPLOY_ONLY") { + t.Errorf("deploy job must reference DEPLOY_ONLY\n%s", deploy) + } + if strings.Contains(deploy, "PREREQ_ONLY") || strings.Contains(deploy, "UNION_ONLY") { + t.Errorf("deploy job must not reference other phases' / union secrets\n%s", deploy) + } +} + func TestRenderGitLabCI_NilPlan(t *testing.T) { _, err := cigen.RenderGitLabCI(nil) if err == nil { @@ -117,3 +185,30 @@ func TestRenderGitLabCI_NoDeprecatedOnlySyntax(t *testing.T) { t.Error(".gitlab-ci.yml uses deprecated 'only:' syntax") } } + +func gitLabJobBlock(content, jobName string) string { + start := strings.Index(content, jobName+":\n") + if start < 0 { + return "" + } + rest := content[start+len(jobName)+2:] + if next := strings.Index(rest, "\napply-"); next >= 0 { + return content[start : start+len(jobName)+2+next] + } + if next := strings.Index(rest, "\nsmoke:"); next >= 0 { + return content[start : start+len(jobName)+2+next] + } + return content[start:] +} + +func gitLabTopLevelBlock(content, name string) string { + start := strings.Index(content, name+":\n") + if start < 0 { + return "" + } + rest := content[start+len(name)+2:] + if next := strings.Index(rest, "\n\n"); next >= 0 { + return content[start : start+len(name)+2+next] + } + return content[start:] +} diff --git a/cmd/wfctl/main_test.go b/cmd/wfctl/main_test.go index 85c10eb8..92dc4d44 100644 --- a/cmd/wfctl/main_test.go +++ b/cmd/wfctl/main_test.go @@ -319,6 +319,115 @@ pipelines: } } +func TestRunValidateRejectsConditionalRoutesWithNonStringKeys(t *testing.T) { + dir := t.TempDir() + cfg := ` +modules: + - name: router + type: http.router +pipelines: + authz: + trigger: + type: mock + steps: + - name: route-by-authz + type: step.conditional + config: + field: authz.allowed + routes: + true: allow + false: deny + - name: allow + type: step.log + config: + message: allow + - name: deny + type: step.log + config: + message: deny +` + path := writeTestConfig(t, dir, "conditional.yaml", cfg) + + err := runValidate([]string{"--skip-unknown-types", "--allow-no-entry-points", path}) + if err == nil { + t.Fatal("expected validate to fail on non-string conditional route keys") + } + if !strings.Contains(err.Error(), "step.conditional") || + !strings.Contains(err.Error(), "routes") || + !strings.Contains(err.Error(), "'true'") { + t.Fatalf("expected actionable conditional route key error, got: %v", err) + } +} + +func TestRunValidateRejectsImportedConditionalRoutesWithNonStringKeys(t *testing.T) { + dir := t.TempDir() + imported := ` +pipelines: + imported: + steps: + - name: route-by-authz + type: step.conditional + config: + field: authz.allowed + routes: + true: allow + false: deny +` + writeTestConfig(t, dir, "imported.yaml", imported) + cfg := ` +imports: + - imported.yaml +modules: + - name: router + type: http.router +pipelines: {} +` + path := writeTestConfig(t, dir, "main.yaml", cfg) + + err := runValidate([]string{"--skip-unknown-types", "--allow-no-entry-points", path}) + if err == nil { + t.Fatal("expected validate to fail on imported non-string conditional route keys") + } + if !strings.Contains(err.Error(), "imported.yaml") || + !strings.Contains(err.Error(), "step.conditional") || + !strings.Contains(err.Error(), "'true'") { + t.Fatalf("expected actionable imported conditional route key error, got: %v", err) + } +} + +func TestRunValidateRejectsAliasedConditionalRoutesWithNonStringKeys(t *testing.T) { + dir := t.TempDir() + cfg := ` +shared: + routes: &routes + true: allow + false: deny + config: &condition + field: authz.allowed + routes: *routes +modules: + - name: router + type: http.router +pipelines: + authz: + steps: + - name: route-by-authz + type: step.conditional + config: *condition +` + path := writeTestConfig(t, dir, "conditional-alias.yaml", cfg) + + err := runValidate([]string{"--skip-unknown-types", "--allow-no-entry-points", path}) + if err == nil { + t.Fatal("expected validate to fail on aliased non-string conditional route keys") + } + if !strings.Contains(err.Error(), "step.conditional") || + !strings.Contains(err.Error(), "routes") || + !strings.Contains(err.Error(), "'true'") { + t.Fatalf("expected actionable aliased conditional route key error, got: %v", err) + } +} + func TestRunValidateMissingArg(t *testing.T) { err := runValidate([]string{}) if err == nil { diff --git a/cmd/wfctl/validate.go b/cmd/wfctl/validate.go index 4d5f345b..aec3cafa 100644 --- a/cmd/wfctl/validate.go +++ b/cmd/wfctl/validate.go @@ -150,6 +150,9 @@ func validateFile(cfgPath string, strict, skipUnknownTypes, allowNoEntryPoints, if isLikelyWfctlProjectManifest(cfgPath) { return fmt.Errorf("%s is a wfctl project manifest; use 'wfctl config validate %s' instead", cfgPath, cfgPath) } + if err := validateConditionalRouteKeySyntax(cfgPath); err != nil { + return err + } cfg, err := config.LoadFromFile(cfgPath) if err != nil { @@ -285,6 +288,130 @@ func validateFile(cfgPath string, strict, skipUnknownTypes, allowNoEntryPoints, return nil } +func validateConditionalRouteKeySyntax(cfgPath string) error { + return validateConditionalRouteKeySyntaxFile(cfgPath, make(map[string]bool)) +} + +func validateConditionalRouteKeySyntaxFile(cfgPath string, seen map[string]bool) error { + absPath, err := filepath.Abs(cfgPath) + if err != nil { + return fmt.Errorf("inspect conditional route keys: %w", err) + } + if seen[absPath] { + return nil + } + seen[absPath] = true + + data, err := os.ReadFile(cfgPath) + if err != nil { + return fmt.Errorf("inspect conditional route keys: %w", err) + } + var doc yaml.Node + if err := yaml.Unmarshal(data, &doc); err != nil { + return fmt.Errorf("inspect conditional route keys: %w", err) + } + if len(doc.Content) == 0 { + return nil + } + root := doc.Content[0] + pipelines := mappingValue(root, "pipelines") + if pipelines == nil || pipelines.Kind != yaml.MappingNode { + return nil + } + for i := 0; i+1 < len(pipelines.Content); i += 2 { + pipelineName := pipelines.Content[i].Value + pipeline := pipelines.Content[i+1] + if pipeline.Kind != yaml.MappingNode { + continue + } + steps := mappingValue(pipeline, "steps") + if steps == nil || steps.Kind != yaml.SequenceNode { + continue + } + for _, step := range steps.Content { + step = resolveYAMLAlias(step) + if step == nil || step.Kind != yaml.MappingNode || mappingScalarValue(step, "type") != "step.conditional" { + continue + } + stepName := mappingScalarValue(step, "name") + if stepName == "" { + stepName = "" + } + cfg := mappingValue(step, "config") + if cfg == nil || cfg.Kind != yaml.MappingNode { + continue + } + routes := mappingValue(cfg, "routes") + if routes == nil || routes.Kind != yaml.MappingNode { + continue + } + for j := 0; j+1 < len(routes.Content); j += 2 { + key := routes.Content[j] + if key.ShortTag() == "!!str" { + continue + } + return fmt.Errorf("pipeline %q step %q (type step.conditional): routes key %q is parsed as %s, not string; quote it as '%s'", + pipelineName, stepName, key.Value, strings.TrimPrefix(key.ShortTag(), "!!"), key.Value) + } + } + } + for _, imp := range importPathsFromNode(root) { + impPath := imp + if !filepath.IsAbs(impPath) { + impPath = filepath.Join(filepath.Dir(absPath), impPath) + } + if err := validateConditionalRouteKeySyntaxFile(impPath, seen); err != nil { + return fmt.Errorf("import %q: %w", imp, err) + } + } + return nil +} + +func importPathsFromNode(root *yaml.Node) []string { + imports := mappingValue(root, "imports") + if imports == nil || imports.Kind != yaml.SequenceNode { + return nil + } + paths := make([]string, 0, len(imports.Content)) + for _, item := range imports.Content { + if item.Kind == yaml.ScalarNode && item.ShortTag() == "!!str" { + paths = append(paths, item.Value) + } + } + return paths +} + +func mappingValue(n *yaml.Node, key string) *yaml.Node { + n = resolveYAMLAlias(n) + if n == nil || n.Kind != yaml.MappingNode { + return nil + } + for i := 0; i+1 < len(n.Content); i += 2 { + if n.Content[i].Value == key { + return resolveYAMLAlias(n.Content[i+1]) + } + } + return nil +} + +func mappingScalarValue(n *yaml.Node, key string) string { + v := mappingValue(n, key) + v = resolveYAMLAlias(v) + if v == nil || v.Kind != yaml.ScalarNode { + return "" + } + return v.Value +} + +func resolveYAMLAlias(n *yaml.Node) *yaml.Node { + seen := map[*yaml.Node]bool{} + for n != nil && n.Kind == yaml.AliasNode && n.Alias != nil && !seen[n] { + seen[n] = true + n = n.Alias + } + return n +} + // skipDirs are directory names that should be excluded from recursive scanning. var skipDirs = map[string]bool{ ".playwright-cli": true,