From 2f946bcfaff904781c8c4ffb179669842a11c1d6 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 30 May 2026 17:24:26 -0400 Subject: [PATCH 1/4] feat(wfctl): interactive ci generate wizard When --platform is absent and stdin is a TTY, runCIGenerate drives an interactive wizard that: selects platform (github_actions|gitlab_ci), runner label (ubuntu-latest|self-hosted|custom), toggles smoke/migrations/ plan-guard from Analyze-derived defaults, surfaces plan.Warnings, and confirms write before emitting files. Non-TTY + no --platform errors with "specify --platform for non-interactive generation". The override logic lives in the pure applyWizardOverrides(plan, wizardChoices) function so it is fully unit-testable without a TTY; 7 new unit tests cover all toggles. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/wfctl/ci.go | 33 ++++++++-- cmd/wfctl/ci_test.go | 130 ++++++++++++++++++++++++++++++++++++ cmd/wfctl/ci_wizard.go | 145 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 304 insertions(+), 4 deletions(-) create mode 100644 cmd/wfctl/ci_wizard.go diff --git a/cmd/wfctl/ci.go b/cmd/wfctl/ci.go index 175fe6e2..cee8cff0 100644 --- a/cmd/wfctl/ci.go +++ b/cmd/wfctl/ci.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/GoCodeAlone/workflow/cigen" + "github.com/mattn/go-isatty" ) func runCI(args []string) error { @@ -80,6 +81,7 @@ func runCIGenerate(args []string) error { exitCode := fs.Bool("exit-code", false, "With --diff: exit 1 when files differ") write := fs.Bool("write", false, "Allow overwriting existing files") phaseConfig := fs.String("phase-config", "", "Prerequisite phase config path") + interactive := fs.Bool("interactive", false, "Force interactive wizard even when --platform is set") if err := fs.Parse(args); err != nil { return err } @@ -93,8 +95,13 @@ func runCIGenerate(args []string) error { *outputDir = *out } - if *platform == "" { - return fmt.Errorf("--platform is required (github_actions, gitlab_ci)") + // When --platform is absent, use the interactive wizard (TTY required). + // When stdin is not a TTY and --platform is also absent, fail clearly. + if *platform == "" && !*interactive { + if !isatty.IsTerminal(os.Stdin.Fd()) { + return fmt.Errorf("specify --platform for non-interactive generation (github_actions, gitlab_ci)") + } + // Fall through — wizard will be run after the plan is built. } // Build the plan @@ -125,16 +132,34 @@ func runCIGenerate(args []string) error { } } + // Wizard: run when --platform is absent or --interactive is explicitly set. + // The wizard populates choices which override plan fields and set the platform. + resolvedPlatform := *platform + if resolvedPlatform == "" || *interactive { + choices, wizErr := runCIWizard(plan) + if wizErr != nil { + return fmt.Errorf("ci generate: wizard: %w", wizErr) + } + applyWizardOverrides(plan, choices) + if resolvedPlatform == "" { + resolvedPlatform = choices.Platform + } + // Wizard controls the write flag when it ran (unless already set via CLI) + if !*write && choices.Write { + *write = true + } + } + // Render var files map[string]string var renderErr error - switch *platform { + switch resolvedPlatform { case "github_actions": files, renderErr = cigen.RenderGitHubActions(plan) case "gitlab_ci": files, renderErr = cigen.RenderGitLabCI(plan) default: - return fmt.Errorf("unsupported platform %q (supported: github_actions, gitlab_ci)", *platform) + return fmt.Errorf("unsupported platform %q (supported: github_actions, gitlab_ci)", resolvedPlatform) } if renderErr != nil { return renderErr diff --git a/cmd/wfctl/ci_test.go b/cmd/wfctl/ci_test.go index ef2b0e63..95f5cff7 100644 --- a/cmd/wfctl/ci_test.go +++ b/cmd/wfctl/ci_test.go @@ -6,6 +6,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/GoCodeAlone/workflow/cigen" ) func TestGenerateGitHubActions(t *testing.T) { @@ -254,6 +256,134 @@ func fileKeys(files map[string]string) []string { return keys } +// ─── Wizard unit tests (no TTY) ───────────────────────────────────────────── + +func TestApplyWizardOverrides_SmokeFalseDropsSmoke(t *testing.T) { + plan := &cigen.CIPlan{ + Runner: "ubuntu-latest", + Smoke: &cigen.SmokeSpec{URL: "https://example.com/healthz", Path: "/healthz"}, + } + choices := wizardChoices{ + Platform: "github_actions", + Runner: "ubuntu-latest", + Smoke: false, // operator toggled off + Migrations: false, + PlanGuard: false, + } + applyWizardOverrides(plan, choices) + if plan.Smoke != nil { + t.Error("expected plan.Smoke to be nil when choices.Smoke=false") + } +} + +func TestApplyWizardOverrides_SmokeTruePreservesSmoke(t *testing.T) { + smoke := &cigen.SmokeSpec{URL: "https://example.com/healthz", Path: "/healthz"} + plan := &cigen.CIPlan{ + Runner: "ubuntu-latest", + Smoke: smoke, + } + choices := wizardChoices{ + Platform: "github_actions", + Runner: "ubuntu-latest", + Smoke: true, + Migrations: false, + PlanGuard: false, + } + applyWizardOverrides(plan, choices) + if plan.Smoke == nil { + t.Error("expected plan.Smoke to be preserved when choices.Smoke=true") + } + if plan.Smoke.URL != smoke.URL { + t.Errorf("smoke URL changed unexpectedly: got %q, want %q", plan.Smoke.URL, smoke.URL) + } +} + +func TestApplyWizardOverrides_MigrationsFalseDropsMigrations(t *testing.T) { + plan := &cigen.CIPlan{ + Runner: "ubuntu-latest", + Migrations: &cigen.MigrationsSpec{DBEnv: "DB_URL", Source: "migrations"}, + } + choices := wizardChoices{ + Platform: "github_actions", + Runner: "ubuntu-latest", + Smoke: false, + Migrations: false, // operator toggled off + PlanGuard: false, + } + applyWizardOverrides(plan, choices) + if plan.Migrations != nil { + t.Error("expected plan.Migrations to be nil when choices.Migrations=false") + } +} + +func TestApplyWizardOverrides_RunnerOverrideApplies(t *testing.T) { + plan := &cigen.CIPlan{Runner: "ubuntu-latest"} + choices := wizardChoices{ + Platform: "github_actions", + Runner: "self-hosted", + Smoke: false, + Migrations: false, + PlanGuard: false, + } + applyWizardOverrides(plan, choices) + if plan.Runner != "self-hosted" { + t.Errorf("expected Runner=%q, got %q", "self-hosted", plan.Runner) + } +} + +func TestApplyWizardOverrides_PlanGuardToggle(t *testing.T) { + plan := &cigen.CIPlan{Runner: "ubuntu-latest", PlanGuard: true} + choices := wizardChoices{ + Platform: "github_actions", + Runner: "ubuntu-latest", + Smoke: false, + Migrations: false, + PlanGuard: false, // operator toggled off + } + applyWizardOverrides(plan, choices) + if plan.PlanGuard { + t.Error("expected PlanGuard=false after wizard override") + } + + // Toggle back on + choices.PlanGuard = true + applyWizardOverrides(plan, choices) + if !plan.PlanGuard { + t.Error("expected PlanGuard=true after wizard override") + } +} + +func TestApplyWizardOverrides_PlatformSelectsRenderer(t *testing.T) { + // Verify that the wizard choices struct carries the right platform string + // and that applyWizardOverrides does not overwrite plan fields with platform info + // (platform is used by the caller to select renderer, not stored in CIPlan). + plan := &cigen.CIPlan{Runner: "ubuntu-latest"} + ghaChoices := wizardChoices{Platform: "github_actions", Runner: "ubuntu-latest"} + glChoices := wizardChoices{Platform: "gitlab_ci", Runner: "ubuntu-latest"} + + applyWizardOverrides(plan, ghaChoices) + if ghaChoices.Platform != "github_actions" { + t.Errorf("platform should be github_actions, got %q", ghaChoices.Platform) + } + + applyWizardOverrides(plan, glChoices) + if glChoices.Platform != "gitlab_ci" { + t.Errorf("platform should be gitlab_ci, got %q", glChoices.Platform) + } +} + +func TestCIGenerateMissingPlatformNonTTY(t *testing.T) { + // When --platform is absent and stdin is not a TTY (test runner context), + // runCIGenerate must return an error that mentions --platform. + err := runCIGenerate([]string{}) + if err == nil { + t.Fatal("expected error when --platform is missing in non-TTY context") + } + if !strings.Contains(err.Error(), "--platform") { + t.Errorf("expected error to mention --platform, got: %v", err) + } +} + // ─── New ci plan + ci generate extended tests ──────────────────────────────── func TestRunCIPlan_StdoutJSON(t *testing.T) { diff --git a/cmd/wfctl/ci_wizard.go b/cmd/wfctl/ci_wizard.go new file mode 100644 index 00000000..3e92c21b --- /dev/null +++ b/cmd/wfctl/ci_wizard.go @@ -0,0 +1,145 @@ +package main + +import ( + "errors" + "fmt" + "strings" + + "github.com/GoCodeAlone/workflow/cigen" + "github.com/GoCodeAlone/workflow/cmd/wfctl/internal/prompt" +) + +// wizardChoices holds the user's selections from the interactive wizard. +// It is populated by runWizard (TTY path) and consumed by applyWizardOverrides. +// Tests inject it directly without touching a TTY. +type wizardChoices struct { + // Platform is one of "github_actions" or "gitlab_ci". + Platform string + // Runner is the CI runner label (e.g. "ubuntu-latest", "self-hosted"). + Runner string + // Smoke indicates whether to emit a smoke job. + Smoke bool + // Migrations indicates whether to emit a migrations step. + Migrations bool + // PlanGuard indicates whether to enforce plan-before-apply. + PlanGuard bool + // Write is true when the user confirmed writing the output. + Write bool +} + +// applyWizardOverrides mutates plan with the operator's wizard choices. +// It is a pure function: no I/O, no TTY dependency — designed for unit testing. +// Note: choices.Platform is used by the caller to pick the renderer; it is not +// stored in CIPlan (there is no platform field on the plan struct). +func applyWizardOverrides(plan *cigen.CIPlan, choices wizardChoices) { + if choices.Runner != "" { + plan.Runner = choices.Runner + } + + if !choices.Smoke { + plan.Smoke = nil + } + + if !choices.Migrations { + plan.Migrations = nil + } + + plan.PlanGuard = choices.PlanGuard +} + +// platformOptions is the ordered list of CI platforms for the wizard. +var platformOptions = []string{"github_actions", "gitlab_ci"} + +// runnerOptions is the ordered list of common runner labels for the wizard. +var runnerOptions = []string{"ubuntu-latest", "self-hosted", "other (type below)"} + +// runCIWizard drives the interactive wizard over a pre-analyzed plan. +// It prints warnings before asking the user to confirm writing. +// Returns wizardChoices populated from TTY input, or an error. +func runCIWizard(plan *cigen.CIPlan) (wizardChoices, error) { + choices := wizardChoices{ + // Seed defaults from what Analyze derived. + Smoke: plan.Smoke != nil, + Migrations: plan.Migrations != nil, + PlanGuard: plan.PlanGuard, + } + + // 1. Platform + platIdx, err := prompt.Select("Select CI platform", platformOptions) + if err != nil { + if errors.Is(err, prompt.ErrNotInteractive) { + return choices, fmt.Errorf("stdin is not a terminal; specify --platform for non-interactive generation") + } + return choices, fmt.Errorf("wizard: platform selection: %w", err) + } + choices.Platform = platformOptions[platIdx] + + // 2. Runner + runnerIdx, err := prompt.Select( + fmt.Sprintf("Select runner label (default: %s)", plan.Runner), + runnerOptions, + ) + if err != nil { + if errors.Is(err, prompt.ErrNotInteractive) { + return choices, fmt.Errorf("stdin is not a terminal") + } + return choices, fmt.Errorf("wizard: runner selection: %w", err) + } + + switch runnerIdx { + case 0: + choices.Runner = "ubuntu-latest" + case 1: + choices.Runner = "self-hosted" + default: + // "other" — ask for free-form input + customRunner, inputErr := prompt.Input("Runner label", false) + if inputErr != nil { + return choices, fmt.Errorf("wizard: runner input: %w", inputErr) + } + customRunner = strings.TrimSpace(customRunner) + if customRunner == "" { + customRunner = plan.Runner // keep the analyzed default + } + choices.Runner = customRunner + } + + // 3. Toggle: include smoke job? + smoke, err := prompt.Confirm("Include smoke job?", choices.Smoke) + if err != nil { + return choices, fmt.Errorf("wizard: smoke confirm: %w", err) + } + choices.Smoke = smoke + + // 4. Toggle: include migrations step? + mig, err := prompt.Confirm("Include migrations step?", choices.Migrations) + if err != nil { + return choices, fmt.Errorf("wizard: migrations confirm: %w", err) + } + choices.Migrations = mig + + // 5. Toggle: enable plan-guard? + pg, err := prompt.Confirm("Enable plan-guard (refuse apply on replace/destroy)?", choices.PlanGuard) + if err != nil { + return choices, fmt.Errorf("wizard: plan-guard confirm: %w", err) + } + choices.PlanGuard = pg + + // 6. Surface warnings + if len(plan.Warnings) > 0 { + fmt.Println("\nWarnings:") + for _, w := range plan.Warnings { + fmt.Printf(" ! %s\n", w) + } + fmt.Println() + } + + // 7. Confirm write + write, err := prompt.Confirm("Write generated files?", true) + if err != nil { + return choices, fmt.Errorf("wizard: write confirm: %w", err) + } + choices.Write = write + + return choices, nil +} From 84d069ff77a1c60d6e606a25af57e4001ebb6d56 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 30 May 2026 17:27:53 -0400 Subject: [PATCH 2/4] docs+test(cigen): multisite real-world regen evidence + honest gap analysis Copies gocodealone-multisite deploy.yaml + deploy.prereq.yaml verbatim into cigen/testdata/multisite/ and commits the REAL outputs of: wfctl-pr4 ci plan -c ... --phase-config ... --out plan.json wfctl-pr4 ci generate -c ... --platform github_actions --write diff -u .../infra.yml generated-infra.yml The real diff is 386 lines (249 removed from 283-line hand-written file, 100 added to produce 134-line generated file). GAP.md documents matched derivations (two-phase plan/apply, plan-guard, secrets env block, migrations, smoke URL, plugin install) and not-derivable features (hash-suffixed DB secret, SPACES casing alias, image wait loop, GHCR_CREDENTIALS derivation, phase selector inputs, concurrency, continue-on-error) sourced from the real diff and real plan.json warnings[]. Two real bugs noted: paths: filter uses testdata paths instead of repo-root paths when not run from project root; migrations step uses wrong subcommand. Live infra.yml NOT modified (ADR 0004). Co-Authored-By: Claude Opus 4.8 (1M context) --- cigen/analyze_test.go | 80 +++++ cigen/testdata/multisite/GAP.md | 316 +++++++++++++++++++ cigen/testdata/multisite/deploy.prereq.yaml | 102 ++++++ cigen/testdata/multisite/deploy.yaml | 184 +++++++++++ cigen/testdata/multisite/generated-infra.yml | 134 ++++++++ cigen/testdata/multisite/plan.json | 86 +++++ 6 files changed, 902 insertions(+) create mode 100644 cigen/testdata/multisite/GAP.md create mode 100644 cigen/testdata/multisite/deploy.prereq.yaml create mode 100644 cigen/testdata/multisite/deploy.yaml create mode 100644 cigen/testdata/multisite/generated-infra.yml create mode 100644 cigen/testdata/multisite/plan.json diff --git a/cigen/analyze_test.go b/cigen/analyze_test.go index 24455229..3490d94a 100644 --- a/cigen/analyze_test.go +++ b/cigen/analyze_test.go @@ -161,3 +161,83 @@ func TestAnalyze_ConfigPathAliasUsedVerbatim(t *testing.T) { "deploy.yaml", plan.Phases[len(plan.Phases)-1].ConfigPath) } } + +func TestAnalyze_MultisiteGolden(t *testing.T) { + // Golden test: load the REAL gocodealone-multisite configs (copied verbatim + // into testdata/multisite/) and assert Analyze produces the expected shape. + // The exact warning text is asserted from the plan.json warnings[] array + // produced by the real binary run during the Task 18 evidence session. + plan, err := cigen.Analyze( + []string{"testdata/multisite/deploy.yaml"}, + cigen.Options{ + PhaseConfig: "testdata/multisite/deploy.prereq.yaml", + }, + ) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + // Two phases: prereq then deploy + if len(plan.Phases) != 2 { + t.Fatalf("expected 2 phases, got %d", len(plan.Phases)) + } + if plan.Phases[0].Name != "prereq" { + t.Errorf("expected phase[0].Name=%q, got %q", "prereq", plan.Phases[0].Name) + } + if plan.Phases[1].Name != "deploy" { + t.Errorf("expected phase[1].Name=%q, got %q", "deploy", plan.Phases[1].Name) + } + + // Secrets union: deploy.yaml has 16 named secrets (14 entries + 2 provider keys) + // The real plan.json contains 16 secrets total. + if len(plan.Secrets) < 14 { + t.Errorf("expected ≥14 secrets in union, got %d", len(plan.Secrets)) + } + + // PluginInstall: iac.provider, iac.state, analytics.google_provider, infra.database → true + if !plan.PluginInstall { + t.Error("expected PluginInstall=true (iac.* + analytics.* modules present)") + } + + // PlanGuard: multisite-pg and gocodealone-multisite are both protected: true + if !plan.PlanGuard { + t.Error("expected PlanGuard=true (protected modules present)") + } + + // Migrations: ci.migrations[0].database.env = MULTISITE_DB_URL + if plan.Migrations == nil { + t.Fatal("expected Migrations to be non-nil") + } + if plan.Migrations.DBEnv != "MULTISITE_DB_URL" { + t.Errorf("Migrations.DBEnv = %q, want %q", plan.Migrations.DBEnv, "MULTISITE_DB_URL") + } + + // Smoke: infra.container_service with domain gocodealone.tech + health_check /healthz + if plan.Smoke == nil { + t.Fatal("expected Smoke to be non-nil") + } + if !strings.HasSuffix(plan.Smoke.URL, "/healthz") { + t.Errorf("Smoke.URL = %q, expected suffix /healthz", plan.Smoke.URL) + } + + // Warnings: must include a DB-url warning (state-derived) and a SPACES casing warning + if len(plan.Warnings) == 0 { + t.Fatal("expected non-empty Warnings") + } + dbWarn := false + casewarn := false + for _, w := range plan.Warnings { + if strings.Contains(w, "MULTISITE_DB_URL") && strings.Contains(w, "hash suffix") { + dbWarn = true + } + if strings.Contains(w, "SPACES_access_key") && strings.Contains(w, "upper-case") { + casewarn = true + } + } + if !dbWarn { + t.Errorf("expected warning mentioning MULTISITE_DB_URL hash suffix, got: %v", plan.Warnings) + } + if !casewarn { + t.Errorf("expected warning mentioning SPACES_access_key upper-case, got: %v", plan.Warnings) + } +} diff --git a/cigen/testdata/multisite/GAP.md b/cigen/testdata/multisite/GAP.md new file mode 100644 index 00000000..bb3ad977 --- /dev/null +++ b/cigen/testdata/multisite/GAP.md @@ -0,0 +1,316 @@ +# Multisite CI Generation Gap Analysis + +This document records an honest comparison between `wfctl ci generate` output +and the hand-written `.github/workflows/infra.yml` for `GoCodeAlone/gocodealone-multisite`. + +**The live `infra.yml` is NOT modified.** `generated-infra.yml` is a committed +validation artifact per ADR 0004 (demonstration-fidelity mandate). + +## How this was produced + +```sh +# 1. Real ci plan +wfctl-pr4 ci plan \ + -c cigen/testdata/multisite/deploy.yaml \ + --phase-config cigen/testdata/multisite/deploy.prereq.yaml \ + --out cigen/testdata/multisite/plan.json + +# 2. Real ci generate +wfctl-pr4 ci generate \ + -c cigen/testdata/multisite/deploy.yaml \ + --phase-config cigen/testdata/multisite/deploy.prereq.yaml \ + --platform github_actions \ + --out cigen/testdata/multisite \ + --write +# output: cigen/testdata/multisite/.github/workflows/multisite.yml +# renamed to: cigen/testdata/multisite/generated-infra.yml + +# 3. Real diff +diff -u .../infra.yml cigen/testdata/multisite/generated-infra.yml > /tmp/multisite.diff +``` + +## Real plan.json warnings[] array + +```json +[ + "secret \"MULTISITE_DB_URL\" is populated by IaC output — the real GitHub secret name may differ (e.g. include a resource hash suffix); verify the secret name matches what wfctl infra bootstrap writes", + "secret \"SPACES_access_key\" does not match ^[A-Z0-9_]+$ — the config casing is preserved as-is; you may need a GitHub-side alias if the platform normalises secret names to upper-case", + "secret \"SPACES_secret_key\" does not match ^[A-Z0-9_]+$ — the config casing is preserved as-is; you may need a GitHub-side alias if the platform normalises secret names to upper-case" +] +``` + +## Real diff summary + +- Hand-written `infra.yml`: 283 lines +- Generated `generated-infra.yml`: 134 lines +- Diff: 249 lines removed, 100 lines added (386-line diff total) + +Nearly all lines differ. The generated file is correct in structure and +logically sound, but is much simpler than the production hand-written file. + +--- + +## Matched (correctly derived from config) + +The following features ARE present in the generated output and match the +intent of the hand-written workflow, as confirmed by the diff: + +- **Two-phase plan/apply structure**: `plan`, `apply-prereq`, `apply-deploy` + jobs in the correct sequence with `needs: apply-prereq` chaining. +- **PR trigger + push-to-main trigger + workflow_dispatch**: all three trigger + types present in `on:`. +- **paths: filters on both PR and push**: derived from the two config file + paths supplied to Analyze. (Correct paths differ — see NOT derivable below.) +- **wfctl plugin install step**: present in plan and both apply jobs (derived + from `iac.*` and `analytics.*` module types → `plugin_install=true`). +- **Plan-guard in both apply jobs**: the `wfctl infra plan | tee plan-guard.txt` + + grep pattern that refuses destructive changes is present in both + `apply-prereq` and `apply-deploy`. Derived from `protected: true` on both + `multisite-pg` and `gocodealone-multisite` modules. +- **apply-job `env:` block with 1:1 secret name mapping**: all 16 secrets + from the union of both configs are present in the `env:` block on each + apply job using `${{ secrets.NAME }}` syntax. +- **Two-phase plugin install in plan job**: `Install plugins (prereq)` and + `Install plugins (deploy)` steps both present. +- **Migrations step in apply-deploy**: `Run migrations` step using + `wfctl ci run --config ... --phase migrate` derived from `ci.migrations`. +- **Smoke job**: derived from `infra.container_service` with + `health_check.http_path=/healthz` and `domain: gocodealone.tech`. URL + correctly computed as `https://gocodealone.tech/healthz`. +- **Post plan comment job**: GitHub script step that posts plan.md as PR comment. +- **permissions: contents: read + pull-requests: write**: at workflow level. + +--- + +## NOT derivable (stays hand-authored), with real warnings + +The following features are present in the hand-written `infra.yml` but NOT +produced by the generator. Where relevant, the real warnings[] from plan.json +explain the gap. + +### 1. Hash-suffixed DB secret name + +The hand-written workflow uses: +```yaml +MULTISITE_DB_URL: ${{ secrets.MULTISITE_PG__URI_6D4758EBCF22872E6C0D93190FB952E4 }} +``` + +The generator uses the config name verbatim: +```yaml +MULTISITE_DB_URL: ${{ secrets.MULTISITE_DB_URL }} +``` + +The plan.json warning explicitly flags this: +> "secret "MULTISITE_DB_URL" is populated by IaC output — the real GitHub +> secret name may differ (e.g. include a resource hash suffix); verify the +> secret name matches what wfctl infra bootstrap writes" + +The hash (`6D4758EBCF22872E6C0D93190FB952E4`) is a wfctl-infra-bootstrap output; +it is not derivable from the deploy config alone. + +### 2. SPACES_access_key → SPACES_ACCESS_KEY case normalisation + +The hand-written workflow passes secrets as: +```yaml +SPACES_access_key: ${{ secrets.SPACES_ACCESS_KEY }} +SPACES_secret_key: ${{ secrets.SPACES_SECRET_KEY }} +``` + +The generator uses the config casing verbatim: +```yaml +SPACES_access_key: ${{ secrets.SPACES_access_key }} +SPACES_secret_key: ${{ secrets.SPACES_secret_key }} +``` + +The plan.json warnings flag both: +> "secret "SPACES_access_key" does not match ^[A-Z0-9_]+$ — the config casing +> is preserved as-is; you may need a GitHub-side alias if the platform +> normalises secret names to upper-case" + +GitHub Actions silently normalises stored secret names to upper-case, so +`SPACES_access_key` on the left side of the env mapping must reference +`secrets.SPACES_ACCESS_KEY` on the right side when the secret was stored +under the upper-case name. The generator warns but cannot resolve this +automatically without inspecting the live GitHub secret store. + +### 3. Image wait loop (GHCR polling) + +The hand-written `apply-deploy` includes a step that polls the GHCR package +registry until the container image built by `build.yml` is available: +```yaml +- name: Wait for image + env: + GH_TOKEN: ${{ secrets.RELEASES_TOKEN || github.token }} + IMAGE_SHA: ${{ inputs.image_sha || github.sha }} + run: | + short_sha="${IMAGE_SHA:0:12}" + for i in $(seq 1 30); do + if gh api -X GET /orgs/.../versions --jq "..." | grep -qx "${short_sha}"; then ... +``` + +The generator has no knowledge of a separate `build.yml` workflow or that +this image must be waited for. Not derivable. + +### 4. GHCR_CREDENTIALS derivation step + +The hand-written workflow derives `GHCR_CREDENTIALS` dynamically from +`RELEASES_TOKEN`: +```yaml +- name: Derive GHCR_CREDENTIALS + run: printf 'GHCR_CREDENTIALS=%s:%s\n' "$actor" "$GH_TOKEN" >> "$GITHUB_ENV" +``` + +The generator simply passes `${{ secrets.GHCR_CREDENTIALS }}` as a static +secret. The config `deploy.yaml` declares `GHCR_CREDENTIALS` as a named +secret, so the generator is technically correct for the declared config — but +the runtime implementation is more sophisticated. Not derivable from config. + +### 5. workflow_dispatch phase selector inputs + +The hand-written workflow has typed `workflow_dispatch.inputs`: +```yaml +workflow_dispatch: + inputs: + phase: {type: choice, options: [auto, prereq, full]} + image_sha: {required: false, ...} +``` + +This drives conditional apply logic (`inputs.phase == 'prereq'`). The +generator emits `workflow_dispatch:` with no inputs. + +### 6. apply-deploy conditional includes apply-prereq skip logic + +The hand-written `apply-deploy` condition: +```yaml +always() && +(needs.apply-prereq.result == 'success' || needs.apply-prereq.result == 'skipped') && +((github.event_name == 'push' && ...) || + (github.event_name == 'workflow_dispatch' && (inputs.phase == 'full' || inputs.phase == 'auto'))) +``` + +The generator emits a simpler condition: +```yaml +if: "(github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'workflow_dispatch'" +``` + +The `always()` + `skipped` guard is not derivable. Also missing: the +phase-input-gated dispatch (`inputs.phase == 'full'`). + +### 7. continue-on-error: true on plan steps + +The hand-written plan job uses `continue-on-error: true` on both plan steps +so a fresh deploy (where `SPACES_*` secrets don't yet exist) can still pass. +The generator emits plan steps without `continue-on-error`. + +### 8. Per-step env blocks in plan + apply-prereq + +The hand-written workflow passes `DIGITALOCEAN_TOKEN`, `RELEASES_TOKEN`, +`SPACES_access_key`, `SPACES_secret_key` as per-step env on the plan steps +and apply-prereq step (not as job-level env). The generator places the full +secret set in the job-level `env:` block instead, which technically works +but differs in structure. This is a correctness delta, not just style. + +### 9. setup-go and SHA-pinned action references + +The hand-written workflow pins actions by SHA: +```yaml +- uses: GoCodeAlone/setup-wfctl@362fe9aaf4792e5adffa2b406ee39dcad31f54a9 +``` +and adds `actions/setup-go@v6` with `go-version-file: go.mod`. + +The generator uses tag references (`@v4`, `@v1`) without SHA pinning and +does not emit a Go setup step. Not derivable from config. + +### 10. analytics.google_ga4_ensure apply pipeline step + +The `deploy.yaml` has: +```yaml +pipelines: + apply: + steps: + - name: ensure_gocodealone_ga4 + type: step.analytics_google_ga4_ensure +``` + +The generator has no hook for `pipelines.apply` steps and emits no GA4 +provisioning step in CI. Not derivable without pipeline-step introspection. + +### 11. Concurrency group + +The hand-written workflow has: +```yaml +concurrency: + group: gocodealone-multisite-infra-${{ github.ref_name }}-... + cancel-in-progress: true +``` + +The generator emits no `concurrency:` block. Not derivable. + +### 12. Custom multi-route smoke matrix + +The hand-written smoke job tests 6 routes across 3 domains with retry loops. +The generator emits a single `curl --fail` against `https://gocodealone.tech/healthz`. +The smoke URL is correctly derived from the `infra.container_service` health +check path and primary domain, but the multi-domain retry matrix is not. + +### 13. Workflow name and global env vars + +The hand-written workflow is named `Infrastructure` with global env vars +(`MULTISITE_PUBLIC_URL`, `MULTISITE_WWW_URL`, `MULTISITE_ADMIN_URL`). +The generator derives the name from the config basename (`multisite`) and +emits no global env block. + +--- + +## What the generator got WRONG (not just incomplete) + +### paths: filter references testdata paths, not repo root paths + +The generated `paths:` filter lists: +```yaml +paths: + - 'cigen/testdata/multisite/deploy.prereq.yaml' + - 'cigen/testdata/multisite/deploy.yaml' +``` + +This is the actual filesystem path that was passed to Analyze, relativized +to the current working directory. When `ci generate` is run from a repo root +with `-c deploy.yaml`, the paths filter correctly resolves to `deploy.yaml`. +The testdata path is an artifact of running the binary from the workflow repo's +root with `cigen/testdata/multisite/deploy.yaml` as the argument — it is not +a bug in the logic, but demonstrates that the `--config` argument path is used +verbatim as the CI trigger path filter, which requires operators to run +`ci generate` from the target project root or use `--config-path-alias`. + +### Migration step uses wrong command + +Generated: +```yaml +run: wfctl ci run --config 'cigen/testdata/multisite/deploy.yaml' --phase migrate +``` + +Hand-written: +```yaml +run: wfctl migrations up --config deploy.yaml --env prod --format json +``` + +The generator emits `wfctl ci run --phase migrate` which is a different +subcommand from the actual `wfctl migrations up` used in production. The +`wfctl ci run --phase migrate` command exists but may not match the exact +operational semantics of `wfctl migrations up --env prod --format json`. +This is a real divergence in the generated command. + +--- + +## Verdict + +`wfctl ci generate` correctly derives the two-phase plan/apply structure, +plan-guard, secret inventory (by config name), smoke URL, migrations DBEnv, +plugin install, and trigger shape. It correctly warns about the DB hash-suffix +and SPACES casing gaps. It does NOT derive: hash-suffixed secret references, +image wait loops, GHCR credential derivation, phase-selector dispatch inputs, +action SHA pinning, apply pipeline steps, concurrency guards, per-step env +scoping, multi-route smoke matrix, or the `always()+skipped` dependency +condition. The generator is a useful starting scaffold; the 15+ hand-authored +additions are each justified by runtime or operational concerns not encodable +in the workflow config format alone. diff --git a/cigen/testdata/multisite/deploy.prereq.yaml b/cigen/testdata/multisite/deploy.prereq.yaml new file mode 100644 index 00000000..1ac1a564 --- /dev/null +++ b/cigen/testdata/multisite/deploy.prereq.yaml @@ -0,0 +1,102 @@ +# gocodealone-multisite — infrastructure prerequisites. +# +# Phase 1 of two-phase deploy. Creates the long-lived DO resources that +# the app depends on, but does NOT touch App Platform. +# +# Resources provisioned: +# - DigitalOcean Managed Postgres cluster (multisite-pg) +# - DigitalOcean Spaces bucket for IaC state backend +# - Generated secrets (HMAC, JWT, audit-sign, super-admin bootstrap) +# +# Run with: +# wfctl infra plan -c deploy.prereq.yaml +# wfctl infra apply -c deploy.prereq.yaml + +infra: + auto_bootstrap: true + +secretStores: + github-actions: + provider: github + config: + repo: GoCodeAlone/gocodealone-multisite + token_env: RELEASES_TOKEN + +secrets: + defaultStore: github-actions + provider: github + config: + repo: GoCodeAlone/gocodealone-multisite + token_env: RELEASES_TOKEN + generate: + - key: SPACES + type: provider_credential + source: digitalocean.spaces + name: multisite-deploy-key + - key: MULTISITE_INGEST_HMAC_SECRET + type: random_hex + length: 32 + - key: MULTISITE_JWT_SECRET + type: random_hex + length: 64 + - key: MULTISITE_AUDIT_SIGN_KEY + type: random_hex + length: 32 + - key: MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE + type: random_hex + length: 24 + entries: + - name: DIGITALOCEAN_TOKEN + description: DigitalOcean API token used by wfctl provider modules. + - name: RELEASES_TOKEN + description: GitHub token used by wfctl plugin install + GHCR pull credential derivation. + - name: GHCR_CREDENTIALS + description: GitHub Container Registry pull credential for DigitalOcean App Platform (username:token). + - name: SPACES_access_key + description: DO Spaces access key for the multisite IaC state backend. Generated by bootstrap. + - name: SPACES_secret_key + description: DO Spaces secret key for the multisite IaC state backend. Generated by bootstrap. + - name: MULTISITE_CONTENT_REPO_TOKEN + description: GitHub PAT with contents:read access to every content repo (gocodealone-website etc.). Set by operator. + - name: MULTISITE_OAUTH_GOOGLE_CLIENT_ID + description: Google OAuth client ID. Optional; required only if Google SSO enabled. + - name: MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET + description: Google OAuth client secret. Optional. + - name: MULTISITE_OAUTH_FACEBOOK_APP_ID + description: Facebook OAuth app ID. Optional. + - name: MULTISITE_OAUTH_FACEBOOK_APP_SECRET + description: Facebook OAuth app secret. Optional. + +modules: + - name: do-provider + type: iac.provider + config: + provider: digitalocean + region: nyc3 + token: ${DIGITALOCEAN_TOKEN} + spaces_access_key: ${SPACES_access_key} + spaces_secret_key: ${SPACES_secret_key} + + - name: iac-state + type: iac.state + # Private DO Spaces bucket for wfctl-managed resource IDs, config + # hashes, outputs, drift metadata. Survives failed mid-applies. + config: + backend: spaces + bucket: multisite-iac-state + prefix: prod/ + region: nyc3 + accessKey: ${SPACES_access_key} + secretKey: ${SPACES_secret_key} + + - name: multisite-pg + type: infra.database + protected: true + config: + provider: do-provider + engine: pg + version: "18" + size: db-s-1vcpu-1gb + num_nodes: 1 + region: nyc3 + adopt_existing: true diff --git a/cigen/testdata/multisite/deploy.yaml b/cigen/testdata/multisite/deploy.yaml new file mode 100644 index 00000000..b6972127 --- /dev/null +++ b/cigen/testdata/multisite/deploy.yaml @@ -0,0 +1,184 @@ +# gocodealone-multisite — application deploy spec. +# +# Phase 2 of two-phase deploy. Provisions the App Platform container +# service that runs the multisite-host binary built from this repo. +# +# Run with: +# wfctl infra plan -c deploy.yaml +# wfctl infra apply -c deploy.yaml --wait +# +# Image must be pushed to GHCR by CI before apply; IMAGE_SHA is +# provided as an env var by the GHA workflow. + +infra: + auto_bootstrap: true + +ci: + migrations: + - name: multisite + plugin: workflow-plugin-migrations + driver: golang-migrate + source_dir: migrations + database: + env: MULTISITE_DB_URL + validation: + forbid_dirty: true + +secretStores: + github-actions: + provider: github + config: + repo: GoCodeAlone/gocodealone-multisite + token_env: RELEASES_TOKEN + +secrets: + defaultStore: github-actions + provider: github + config: + repo: GoCodeAlone/gocodealone-multisite + token_env: RELEASES_TOKEN + entries: + - name: DIGITALOCEAN_TOKEN + description: DigitalOcean API token. + - name: RELEASES_TOKEN + description: GitHub token for wfctl plugin install + GHCR pull cred derivation. + - name: GHCR_CREDENTIALS + description: GHCR pull credential for DigitalOcean App Platform. + - name: SPACES_access_key + description: DO Spaces access key (wfctl state backend). + - name: SPACES_secret_key + description: DO Spaces secret key (wfctl state backend). + - name: MULTISITE_DB_URL + description: Managed Postgres URL — populated by deploy.prereq.yaml. + - name: MULTISITE_INGEST_HMAC_SECRET + description: HMAC secret shared with content repos for the release webhook. + - name: MULTISITE_JWT_SECRET + description: JWT signing key for the admin session. + - name: MULTISITE_AUDIT_SIGN_KEY + description: Audit-chain row signing key. + - name: MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE + description: One-shot bootstrap code redeemed for first super-admin. + - name: MULTISITE_CONTENT_REPO_TOKEN + description: GitHub PAT to fetch private content-repo release tarballs. + - name: MULTISITE_OAUTH_GOOGLE_CLIENT_ID + description: Google OAuth client ID (optional). + - name: MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET + description: Google OAuth client secret (optional). + - name: MULTISITE_OAUTH_FACEBOOK_APP_ID + description: Facebook OAuth app ID (optional). + - name: MULTISITE_OAUTH_FACEBOOK_APP_SECRET + description: Facebook OAuth app secret (optional). + - name: GOOGLE_ANALYTICS_ADMIN_CREDENTIALS_JSON + description: Google service-account JSON with Analytics Admin access for GA4 provisioning. + +modules: + - name: do-provider + type: iac.provider + config: + provider: digitalocean + region: nyc3 + token: ${DIGITALOCEAN_TOKEN} + spaces_access_key: ${SPACES_access_key} + spaces_secret_key: ${SPACES_secret_key} + + - name: iac-state + type: iac.state + config: + backend: spaces + bucket: multisite-iac-state + prefix: prod/ + region: nyc3 + accessKey: ${SPACES_access_key} + secretKey: ${SPACES_secret_key} + + - name: google-analytics + type: analytics.google_provider + config: + credentials_json: ${GOOGLE_ANALYTICS_ADMIN_CREDENTIALS_JSON} + allow_adc: false + analytics_account: accounts/395146029 + + # DB spec mirrors deploy.prereq.yaml but adds trusted_sources. The + # firewall rule references the gocodealone-multisite App Platform + # service below, which does not exist when prereq applies — only at + # deploy.yaml apply time. wfctl adopts the existing DB and updates + # its firewall (deferred-update) to whitelist the just-created app. + # See workflow-compute deploy.{prereq,}.yaml for the same pattern. + - name: multisite-pg + type: infra.database + protected: true + config: + provider: do-provider + engine: pg + version: "18" + size: db-s-1vcpu-1gb + num_nodes: 1 + region: nyc3 + adopt_existing: true + + - name: gocodealone-multisite + type: infra.container_service + protected: true + config: + provider: do-provider + region: nyc + image: ghcr.io/gocodealone/gocodealone-multisite:${IMAGE_SHA} + http_port: 8080 + health_check: + http_path: /healthz + routes: + - path: / + domains: + - domain: gocodealone.tech + type: PRIMARY + zone: gocodealone.tech + - domain: www.gocodealone.tech + type: ALIAS + zone: gocodealone.tech + - domain: admin.gocodealone.tech + type: ALIAS + zone: gocodealone.tech + provider_specific: + digitalocean: + registry_credentials: ${GHCR_CREDENTIALS} + env_vars: + MULTISITE_LISTEN_ADDR: ":8080" + MULTISITE_PREVIEW_SUBDOMAIN_BASE: preview.gocodealone.tech + MULTISITE_BUNDLE_ROOT: /var/lib/multisite/bundles + MULTISITE_DEFAULT_TENANT_SLUG: gocodealone + MULTISITE_DEFAULT_TENANT_LABEL: GoCodeAlone + MULTISITE_DEFAULT_TENANT_DOMAINS: gocodealone.tech,www.gocodealone.tech + MULTISITE_CONTENT_REPO_TENANT_MAP: GoCodeAlone/gocodealone-website:gocodealone + MULTISITE_ADMIN_HOST: admin.gocodealone.tech + MULTISITE_SUPER_ADMIN_EMAIL: admin@gocodealone.tech + MULTISITE_BOOTSTRAP_CONTENT_TENANT: gocodealone + MULTISITE_BOOTSTRAP_CONTENT_REPO: GoCodeAlone/gocodealone-website + MULTISITE_BOOTSTRAP_CONTENT_TAG: v0.1.0 + MULTISITE_BOOTSTRAP_CONTENT_TARBALL_URL: https://api.github.com/repos/GoCodeAlone/gocodealone-website/releases/assets/426414415 + MULTISITE_WEBAUTHN_RP_ID: gocodealone.tech + MULTISITE_WEBAUTHN_RP_ORIGIN: https://admin.gocodealone.tech + env_vars_secret: + MULTISITE_DB_URL: ${MULTISITE_DB_URL} + MULTISITE_INGEST_HMAC_SECRET: ${MULTISITE_INGEST_HMAC_SECRET} + MULTISITE_JWT_SECRET: ${MULTISITE_JWT_SECRET} + MULTISITE_AUDIT_SIGN_KEY: ${MULTISITE_AUDIT_SIGN_KEY} + MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE: ${MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE} + MULTISITE_CONTENT_REPO_TOKEN: ${MULTISITE_CONTENT_REPO_TOKEN} + MULTISITE_OAUTH_GOOGLE_CLIENT_ID: ${MULTISITE_OAUTH_GOOGLE_CLIENT_ID} + MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET: ${MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET} + MULTISITE_OAUTH_FACEBOOK_APP_ID: ${MULTISITE_OAUTH_FACEBOOK_APP_ID} + MULTISITE_OAUTH_FACEBOOK_APP_SECRET: ${MULTISITE_OAUTH_FACEBOOK_APP_SECRET} + +pipelines: + apply: + steps: + - name: ensure_gocodealone_ga4 + type: step.analytics_google_ga4_ensure + config: + provider: google-analytics + account: accounts/395146029 + property: properties/538139248 + property_name: GoCodeAlone + stream_name: gocodealone.tech + default_uri: https://gocodealone.tech + dry_run: false diff --git a/cigen/testdata/multisite/generated-infra.yml b/cigen/testdata/multisite/generated-infra.yml new file mode 100644 index 00000000..36b24e7b --- /dev/null +++ b/cigen/testdata/multisite/generated-infra.yml @@ -0,0 +1,134 @@ +name: multisite +on: + pull_request: + paths: + - 'cigen/testdata/multisite/deploy.prereq.yaml' + - 'cigen/testdata/multisite/deploy.yaml' + push: + branches: [main] + paths: + - 'cigen/testdata/multisite/deploy.prereq.yaml' + - 'cigen/testdata/multisite/deploy.yaml' + workflow_dispatch: +permissions: + contents: read + pull-requests: write +jobs: + plan: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install wfctl + uses: GoCodeAlone/setup-wfctl@v1 + with: + version: 'latest' + - name: Install plugins (prereq) + run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.prereq.yaml' + - name: Install plugins (deploy) + run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.yaml' + - name: Plan prereq + run: wfctl infra plan --config 'cigen/testdata/multisite/deploy.prereq.yaml' --format markdown >> plan.md + - name: Plan deploy + run: wfctl infra plan --config 'cigen/testdata/multisite/deploy.yaml' --format markdown >> plan.md + - name: Post plan comment + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const plan = fs.readFileSync('plan.md', 'utf8'); + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: plan + }); + apply-prereq: + if: "(github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'workflow_dispatch'" + runs-on: ubuntu-latest + env: + DIGITALOCEAN_TOKEN: ${{ secrets.DIGITALOCEAN_TOKEN }} + GHCR_CREDENTIALS: ${{ secrets.GHCR_CREDENTIALS }} + GOOGLE_ANALYTICS_ADMIN_CREDENTIALS_JSON: ${{ secrets.GOOGLE_ANALYTICS_ADMIN_CREDENTIALS_JSON }} + MULTISITE_AUDIT_SIGN_KEY: ${{ secrets.MULTISITE_AUDIT_SIGN_KEY }} + MULTISITE_CONTENT_REPO_TOKEN: ${{ secrets.MULTISITE_CONTENT_REPO_TOKEN }} + MULTISITE_DB_URL: ${{ secrets.MULTISITE_DB_URL }} + MULTISITE_INGEST_HMAC_SECRET: ${{ secrets.MULTISITE_INGEST_HMAC_SECRET }} + MULTISITE_JWT_SECRET: ${{ secrets.MULTISITE_JWT_SECRET }} + MULTISITE_OAUTH_FACEBOOK_APP_ID: ${{ secrets.MULTISITE_OAUTH_FACEBOOK_APP_ID }} + MULTISITE_OAUTH_FACEBOOK_APP_SECRET: ${{ secrets.MULTISITE_OAUTH_FACEBOOK_APP_SECRET }} + MULTISITE_OAUTH_GOOGLE_CLIENT_ID: ${{ secrets.MULTISITE_OAUTH_GOOGLE_CLIENT_ID }} + MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET: ${{ secrets.MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET }} + MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE: ${{ secrets.MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE }} + RELEASES_TOKEN: ${{ secrets.RELEASES_TOKEN }} + SPACES_access_key: ${{ secrets.SPACES_access_key }} + SPACES_secret_key: ${{ secrets.SPACES_secret_key }} + steps: + - uses: actions/checkout@v4 + - name: Install wfctl + uses: GoCodeAlone/setup-wfctl@v1 + with: + version: 'latest' + - name: Install plugins + run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.prereq.yaml' + - name: Plan guard + run: | + wfctl infra plan --config 'cigen/testdata/multisite/deploy.prereq.yaml' | tee plan-guard.txt + if grep -Eq -- '^[[:space:]]*(- delete|-/\+ replace)[[:space:]]' plan-guard.txt || \ + grep -Eq -- 'Plan: .*([1-9][0-9]* to replace|[1-9][0-9]* to destroy)' plan-guard.txt; then + echo "::error::Refusing apply: plan includes replace or destroy of a protected resource." >&2 + exit 1 + fi + - name: Apply prereq + run: wfctl infra apply --config 'cigen/testdata/multisite/deploy.prereq.yaml' --auto-approve + apply-deploy: + if: "(github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'workflow_dispatch'" + needs: apply-prereq + runs-on: ubuntu-latest + env: + DIGITALOCEAN_TOKEN: ${{ secrets.DIGITALOCEAN_TOKEN }} + GHCR_CREDENTIALS: ${{ secrets.GHCR_CREDENTIALS }} + GOOGLE_ANALYTICS_ADMIN_CREDENTIALS_JSON: ${{ secrets.GOOGLE_ANALYTICS_ADMIN_CREDENTIALS_JSON }} + MULTISITE_AUDIT_SIGN_KEY: ${{ secrets.MULTISITE_AUDIT_SIGN_KEY }} + MULTISITE_CONTENT_REPO_TOKEN: ${{ secrets.MULTISITE_CONTENT_REPO_TOKEN }} + MULTISITE_DB_URL: ${{ secrets.MULTISITE_DB_URL }} + MULTISITE_INGEST_HMAC_SECRET: ${{ secrets.MULTISITE_INGEST_HMAC_SECRET }} + MULTISITE_JWT_SECRET: ${{ secrets.MULTISITE_JWT_SECRET }} + MULTISITE_OAUTH_FACEBOOK_APP_ID: ${{ secrets.MULTISITE_OAUTH_FACEBOOK_APP_ID }} + MULTISITE_OAUTH_FACEBOOK_APP_SECRET: ${{ secrets.MULTISITE_OAUTH_FACEBOOK_APP_SECRET }} + MULTISITE_OAUTH_GOOGLE_CLIENT_ID: ${{ secrets.MULTISITE_OAUTH_GOOGLE_CLIENT_ID }} + MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET: ${{ secrets.MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET }} + MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE: ${{ secrets.MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE }} + RELEASES_TOKEN: ${{ secrets.RELEASES_TOKEN }} + SPACES_access_key: ${{ secrets.SPACES_access_key }} + SPACES_secret_key: ${{ secrets.SPACES_secret_key }} + steps: + - uses: actions/checkout@v4 + - name: Install wfctl + uses: GoCodeAlone/setup-wfctl@v1 + with: + version: 'latest' + - name: Install plugins + run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.yaml' + - name: Plan guard + run: | + wfctl infra plan --config 'cigen/testdata/multisite/deploy.yaml' | tee plan-guard.txt + if grep -Eq -- '^[[:space:]]*(- delete|-/\+ replace)[[:space:]]' plan-guard.txt || \ + grep -Eq -- 'Plan: .*([1-9][0-9]* to replace|[1-9][0-9]* to destroy)' plan-guard.txt; then + echo "::error::Refusing apply: plan includes replace or destroy of a protected resource." >&2 + exit 1 + fi + - name: Run migrations + run: wfctl ci run --config 'cigen/testdata/multisite/deploy.yaml' --phase migrate + env: + MULTISITE_DB_URL: ${{ secrets.MULTISITE_DB_URL }} + - name: Apply deploy + run: wfctl infra apply --config 'cigen/testdata/multisite/deploy.yaml' --auto-approve + smoke: + needs: apply-deploy + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + steps: + - name: Smoke test + run: | + curl --fail --max-time 30 'https://gocodealone.tech/healthz' diff --git a/cigen/testdata/multisite/plan.json b/cigen/testdata/multisite/plan.json new file mode 100644 index 00000000..a65fd59a --- /dev/null +++ b/cigen/testdata/multisite/plan.json @@ -0,0 +1,86 @@ +{ + "project": "multisite", + "wfctl_version": "latest", + "default_branch": "main", + "runner": "ubuntu-latest", + "plugin_install": true, + "secrets": [ + { + "name": "DIGITALOCEAN_TOKEN" + }, + { + "name": "GHCR_CREDENTIALS" + }, + { + "name": "GOOGLE_ANALYTICS_ADMIN_CREDENTIALS_JSON" + }, + { + "name": "MULTISITE_AUDIT_SIGN_KEY" + }, + { + "name": "MULTISITE_CONTENT_REPO_TOKEN" + }, + { + "name": "MULTISITE_DB_URL" + }, + { + "name": "MULTISITE_INGEST_HMAC_SECRET" + }, + { + "name": "MULTISITE_JWT_SECRET" + }, + { + "name": "MULTISITE_OAUTH_FACEBOOK_APP_ID" + }, + { + "name": "MULTISITE_OAUTH_FACEBOOK_APP_SECRET" + }, + { + "name": "MULTISITE_OAUTH_GOOGLE_CLIENT_ID" + }, + { + "name": "MULTISITE_OAUTH_GOOGLE_CLIENT_SECRET" + }, + { + "name": "MULTISITE_SUPER_ADMIN_BOOTSTRAP_CODE" + }, + { + "name": "RELEASES_TOKEN" + }, + { + "name": "SPACES_access_key" + }, + { + "name": "SPACES_secret_key" + } + ], + "phases": [ + { + "name": "prereq", + "config_path": "cigen/testdata/multisite/deploy.prereq.yaml" + }, + { + "name": "deploy", + "config_path": "cigen/testdata/multisite/deploy.yaml" + } + ], + "migrations": { + "db_env": "MULTISITE_DB_URL", + "source": "migrations" + }, + "smoke": { + "url": "https://gocodealone.tech/healthz", + "path": "/healthz" + }, + "plan_guard": true, + "triggers": { + "pr": true, + "push_main": true, + "dispatch": true + }, + "warnings": [ + "secret \"MULTISITE_DB_URL\" is populated by IaC output — the real GitHub secret name may differ (e.g. include a resource hash suffix); verify the secret name matches what wfctl infra bootstrap writes", + "secret \"SPACES_access_key\" does not match ^[A-Z0-9_]+$ — the config casing is preserved as-is; you may need a GitHub-side alias if the platform normalises secret names to upper-case", + "secret \"SPACES_secret_key\" does not match ^[A-Z0-9_]+$ — the config casing is preserved as-is; you may need a GitHub-side alias if the platform normalises secret names to upper-case" + ] +} From a5807b0458098ef6b538782b57c687210f2c8251 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 30 May 2026 17:41:01 -0400 Subject: [PATCH 3/4] fix(cigen): functional migrations step (wfctl migrations up); regen multisite evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generated migrations step emitted `wfctl ci run --config --phase migrate`, but `wfctl ci run` only accepts phases build|test|deploy and errors on anything else ("unknown phase: migrate") — the step would fail at runtime (looks-right-but-doesn't-run). Surfaced honestly in the multisite GAP.md. Fix: render_gha.go + render_gitlab.go now emit `wfctl migrations up --config ''` via a shared migrationsUpCommand helper. The DB-url secret is already wired into the apply job env: via the secrets union. A new MigrationsSpec.Env field, when set, appends `--env `; otherwise it is omitted. Adds two render tests asserting the step contains `wfctl migrations up --config` and does NOT contain `--phase migrate` (plus an --env variant). Also adds `--config-path-alias` / `--phase-config-alias` flags to `ci plan` and `ci generate` (wired to the existing cigen Options.ConfigPathAlias + new PhaseConfigAlias) so generated CI shows clean repo-relative config paths. Regenerated multisite evidence with the fixed binary + aliases: - plan.json: clean deploy.yaml / deploy.prereq.yaml phase paths (warnings unchanged: DB hash-suffix + 2x SPACES casing) - generated-infra.yml: migrations step now `wfctl migrations up --config 'deploy.yaml'`; paths: filters now `deploy.yaml` / `deploy.prereq.yaml` - diff vs live infra.yml: 384 lines (248 removed, 99 added) - GAP.md: migrations + paths moved out of "got WRONG" (both FIXED); added `--env prod --format json` as a not-derivable operational refinement; genuinely-not-derivable items retained. Live infra.yml NOT modified (ADR 0004). Co-Authored-By: Claude Opus 4.8 (1M context) --- cigen/analyze.go | 10 +- cigen/plan.go | 3 + cigen/render_gha.go | 19 +++- cigen/render_gha_test.go | 32 +++++- cigen/render_gitlab.go | 2 +- cigen/testdata/multisite/GAP.md | 107 +++++++++++-------- cigen/testdata/multisite/generated-infra.yml | 30 +++--- cigen/testdata/multisite/plan.json | 4 +- cmd/wfctl/ci.go | 10 +- cmd/wfctl/ci_plan.go | 12 ++- 10 files changed, 154 insertions(+), 75 deletions(-) diff --git a/cigen/analyze.go b/cigen/analyze.go index 7d7a8d63..7ef4490e 100644 --- a/cigen/analyze.go +++ b/cigen/analyze.go @@ -36,6 +36,10 @@ type Options struct { // so generated CI steps and path filters reference a real checkout path // rather than a deleted /tmp file. ConfigPathAlias string + // PhaseConfigAlias is the ConfigPathAlias equivalent for the prereq phase. + // When set, it is used verbatim as the prereq DeployPhase.ConfigPath + // instead of the relativized PhaseConfig filesystem path. + PhaseConfigAlias string } // Analyze reads the workflow config files in configs and derives a CIPlan. @@ -99,7 +103,11 @@ func Analyze(configs []string, opts Options) (*CIPlan, error) { } phaseConfigPath := opts.PhaseConfig if phaseConfigPath != "" { - phaseConfigPath = relativizeConfigPath(phaseConfigPath) + if opts.PhaseConfigAlias != "" { + phaseConfigPath = opts.PhaseConfigAlias + } else { + phaseConfigPath = relativizeConfigPath(phaseConfigPath) + } } plan.Phases = derivePhases(primaryConfigPath, phaseConfigPath) diff --git a/cigen/plan.go b/cigen/plan.go index e0785bfa..a9779a37 100644 --- a/cigen/plan.go +++ b/cigen/plan.go @@ -63,6 +63,9 @@ type MigrationsSpec struct { DBEnv string `json:"db_env"` // Source is the migrations source directory. Source string `json:"source,omitempty"` + // Env is the deploy environment name passed to `wfctl migrations up --env`. + // Empty when no environment is configured; the --env flag is then omitted. + Env string `json:"env,omitempty"` } // SmokeSpec describes the post-deploy smoke test. diff --git a/cigen/render_gha.go b/cigen/render_gha.go index ac510351..c4e70f36 100644 --- a/cigen/render_gha.go +++ b/cigen/render_gha.go @@ -205,11 +205,15 @@ func writeApplyJob(b *strings.Builder, jobName string, phase DeployPhase, needs b.WriteString(" fi\n") } - // Migrations step (only in the last phase) + // Migrations step (only in the last phase). Use `wfctl migrations up`, + // the real migration runner — `wfctl ci run --phase migrate` is NOT a valid + // phase (ci run only accepts build|test|deploy) and would fail at runtime. + // The DB-url secret is already present in the apply job's `env:` block via + // the secrets union, so the migration command can read it. isLastPhase := phase.Name == p.Phases[len(p.Phases)-1].Name if isLastPhase && p.Migrations != nil { b.WriteString(" - name: Run migrations\n") - fmt.Fprintf(b, " run: wfctl ci run --config '%s' --phase migrate\n", phase.ConfigPath) + fmt.Fprintf(b, " run: %s\n", migrationsUpCommand(phase.ConfigPath, p.Migrations.Env)) b.WriteString(" env:\n") fmt.Fprintf(b, " %s: ${{ secrets.%s }}\n", p.Migrations.DBEnv, p.Migrations.DBEnv) } @@ -218,6 +222,17 @@ func writeApplyJob(b *strings.Builder, jobName string, phase DeployPhase, needs fmt.Fprintf(b, " run: wfctl infra apply --config '%s' --auto-approve\n", phase.ConfigPath) } +// migrationsUpCommand builds the `wfctl migrations up` invocation. The deploy +// env is included as `--env ` only when MigrationsSpec.Env is set; +// otherwise it is omitted (the command defaults to the empty environment). +func migrationsUpCommand(configPath, env string) string { + cmd := fmt.Sprintf("wfctl migrations up --config '%s'", configPath) + if env != "" { + cmd += fmt.Sprintf(" --env %s", env) + } + return cmd +} + // writePhasePaths emits the paths filter for push/pull_request triggers. func writePhasePaths(b *strings.Builder, p *CIPlan) { b.WriteString(" paths:\n") diff --git a/cigen/render_gha_test.go b/cigen/render_gha_test.go index eae536db..22a9a5b5 100644 --- a/cigen/render_gha_test.go +++ b/cigen/render_gha_test.go @@ -76,8 +76,36 @@ func TestRenderGitHubActions_MigrationsStep(t *testing.T) { break } - if !strings.Contains(content, "migrate") { - t.Error("expected migrations step in output") + // Must emit the REAL migration runner. `wfctl ci run --phase migrate` is not + // a valid phase (ci run only accepts build|test|deploy) and would fail at + // runtime, so it must NOT appear. + if !strings.Contains(content, "wfctl migrations up --config") { + t.Errorf("expected migrations step to run 'wfctl migrations up --config', got:\n%s", content) + } + if strings.Contains(content, "--phase migrate") { + t.Error("migrations step must NOT use 'wfctl ci run --phase migrate' (not a valid phase)") + } +} + +func TestRenderGitHubActions_MigrationsStep_WithEnv(t *testing.T) { + plan := richCIPlan() + plan.Migrations.Env = "prod" + + files, err := cigen.RenderGitHubActions(plan) + if err != nil { + t.Fatalf("RenderGitHubActions: %v", err) + } + var content string + for _, c := range files { + content = c + break + } + + if !strings.Contains(content, "wfctl migrations up --config") { + t.Errorf("expected 'wfctl migrations up --config' in output, got:\n%s", content) + } + if !strings.Contains(content, "--env prod") { + t.Errorf("expected '--env prod' when Migrations.Env is set, got:\n%s", content) } } diff --git a/cigen/render_gitlab.go b/cigen/render_gitlab.go index b26a3270..55d5dfe2 100644 --- a/cigen/render_gitlab.go +++ b/cigen/render_gitlab.go @@ -103,7 +103,7 @@ func renderGitLabWorkflow(p *CIPlan) (string, error) { b.WriteString(" script:\n") isLastPhase := i == len(p.Phases)-1 if isLastPhase && p.Migrations != nil { - fmt.Fprintf(&b, " - wfctl ci run --config '%s' --phase migrate\n", phase.ConfigPath) + fmt.Fprintf(&b, " - %s\n", migrationsUpCommand(phase.ConfigPath, p.Migrations.Env)) } fmt.Fprintf(&b, " - wfctl infra apply --config '%s' --auto-approve\n", phase.ConfigPath) b.WriteString(" environment:\n") diff --git a/cigen/testdata/multisite/GAP.md b/cigen/testdata/multisite/GAP.md index bb3ad977..04d352fc 100644 --- a/cigen/testdata/multisite/GAP.md +++ b/cigen/testdata/multisite/GAP.md @@ -8,17 +8,26 @@ validation artifact per ADR 0004 (demonstration-fidelity mandate). ## How this was produced +The `--config-path-alias` / `--phase-config-alias` flags are used so the +committed artifact shows clean repo-relative `deploy.yaml` / `deploy.prereq.yaml` +paths instead of the testdata-relative paths the binary would otherwise emit +(it uses the `--config` argument verbatim as the CI trigger `paths:` filter). + ```sh # 1. Real ci plan wfctl-pr4 ci plan \ -c cigen/testdata/multisite/deploy.yaml \ --phase-config cigen/testdata/multisite/deploy.prereq.yaml \ + --config-path-alias deploy.yaml \ + --phase-config-alias deploy.prereq.yaml \ --out cigen/testdata/multisite/plan.json # 2. Real ci generate wfctl-pr4 ci generate \ -c cigen/testdata/multisite/deploy.yaml \ --phase-config cigen/testdata/multisite/deploy.prereq.yaml \ + --config-path-alias deploy.yaml \ + --phase-config-alias deploy.prereq.yaml \ --platform github_actions \ --out cigen/testdata/multisite \ --write @@ -43,7 +52,7 @@ diff -u .../infra.yml cigen/testdata/multisite/generated-infra.yml > /tmp/multis - Hand-written `infra.yml`: 283 lines - Generated `generated-infra.yml`: 134 lines -- Diff: 249 lines removed, 100 lines added (386-line diff total) +- Diff: 248 lines removed, 99 lines added (384-line diff total) Nearly all lines differ. The generated file is correct in structure and logically sound, but is much simpler than the production hand-written file. @@ -59,8 +68,8 @@ intent of the hand-written workflow, as confirmed by the diff: jobs in the correct sequence with `needs: apply-prereq` chaining. - **PR trigger + push-to-main trigger + workflow_dispatch**: all three trigger types present in `on:`. -- **paths: filters on both PR and push**: derived from the two config file - paths supplied to Analyze. (Correct paths differ — see NOT derivable below.) +- **paths: filters on both PR and push**: clean repo-relative `deploy.yaml` + and `deploy.prereq.yaml` paths (via `--config-path-alias` / `--phase-config-alias`). - **wfctl plugin install step**: present in plan and both apply jobs (derived from `iac.*` and `analytics.*` module types → `plugin_install=true`). - **Plan-guard in both apply jobs**: the `wfctl infra plan | tee plan-guard.txt` @@ -72,8 +81,15 @@ intent of the hand-written workflow, as confirmed by the diff: apply job using `${{ secrets.NAME }}` syntax. - **Two-phase plugin install in plan job**: `Install plugins (prereq)` and `Install plugins (deploy)` steps both present. -- **Migrations step in apply-deploy**: `Run migrations` step using - `wfctl ci run --config ... --phase migrate` derived from `ci.migrations`. +- **Migrations step in apply-deploy** (FUNCTIONAL — fixed): `Run migrations` + step runs `wfctl migrations up --config 'deploy.yaml'`, the REAL migration + runner, derived from `ci.migrations`. This is the same subcommand the + hand-written workflow uses (`wfctl migrations up --config deploy.yaml --env + prod --format json`); the generated form omits the operational `--env prod + --format json` flags (see "NOT derivable" below). A previous version of the + generator emitted `wfctl ci run --config ... --phase migrate`, which is NOT a + valid `ci run` phase (only build|test|deploy) and would have failed at + runtime; that defect is fixed. - **Smoke job**: derived from `infra.container_service` with `health_check.http_path=/healthz` and `domain: gocodealone.tech`. URL correctly computed as `https://gocodealone.tech/healthz`. @@ -260,57 +276,58 @@ The hand-written workflow is named `Infrastructure` with global env vars The generator derives the name from the config basename (`multisite`) and emits no global env block. ---- - -## What the generator got WRONG (not just incomplete) - -### paths: filter references testdata paths, not repo root paths +### 14. Migration `--env prod --format json` flags -The generated `paths:` filter lists: +The generated migrations step is now functional and uses the correct +subcommand: ```yaml -paths: - - 'cigen/testdata/multisite/deploy.prereq.yaml' - - 'cigen/testdata/multisite/deploy.yaml' +run: wfctl migrations up --config 'deploy.yaml' ``` -This is the actual filesystem path that was passed to Analyze, relativized -to the current working directory. When `ci generate` is run from a repo root -with `-c deploy.yaml`, the paths filter correctly resolves to `deploy.yaml`. -The testdata path is an artifact of running the binary from the workflow repo's -root with `cigen/testdata/multisite/deploy.yaml` as the argument — it is not -a bug in the logic, but demonstrates that the `--config` argument path is used -verbatim as the CI trigger path filter, which requires operators to run -`ci generate` from the target project root or use `--config-path-alias`. - -### Migration step uses wrong command - -Generated: -```yaml -run: wfctl ci run --config 'cigen/testdata/multisite/deploy.yaml' --phase migrate -``` - -Hand-written: +The hand-written form adds operational flags: ```yaml run: wfctl migrations up --config deploy.yaml --env prod --format json ``` -The generator emits `wfctl ci run --phase migrate` which is a different -subcommand from the actual `wfctl migrations up` used in production. The -`wfctl ci run --phase migrate` command exists but may not match the exact -operational semantics of `wfctl migrations up --env prod --format json`. -This is a real divergence in the generated command. +The `--env prod` flag selects a deploy-environment-scoped migration config; +the `deploy.yaml` config has a single top-level `ci.migrations` block with no +named environments, so the generator has no environment name to emit. (The +generator WILL emit `--env ` when `MigrationsSpec.Env` is set.) `--format +json` is a presentation choice. Neither is derivable from the base config. + +--- + +## What the generator got WRONG (not just incomplete) + +None remaining at command level. Both previously-documented defects are FIXED: + +- **Migration step command (FIXED)**: the generator previously emitted + `wfctl ci run --config ... --phase migrate`, but `wfctl ci run` only accepts + the phases `build|test|deploy` and errors on anything else (`unknown phase: + "migrate"`) — the generated step would have failed at runtime. It now emits + `wfctl migrations up --config ''`, the real migration runner. The + DB-url secret is already wired into the apply job's `env:` block via the + secrets union, so the command can read it. +- **paths: filter (FIXED in this artifact)**: the binary uses the `--config` + argument verbatim as the CI trigger `paths:` filter, so running it from the + workflow repo root with `cigen/testdata/multisite/deploy.yaml` would emit a + testdata-relative path. This artifact now uses `--config-path-alias deploy.yaml` + / `--phase-config-alias deploy.prereq.yaml` to produce clean repo-relative + paths. Operators generating CI for a real project should either run from the + project root (so `-c deploy.yaml` is already clean) or pass the alias flags. --- ## Verdict `wfctl ci generate` correctly derives the two-phase plan/apply structure, -plan-guard, secret inventory (by config name), smoke URL, migrations DBEnv, -plugin install, and trigger shape. It correctly warns about the DB hash-suffix -and SPACES casing gaps. It does NOT derive: hash-suffixed secret references, -image wait loops, GHCR credential derivation, phase-selector dispatch inputs, -action SHA pinning, apply pipeline steps, concurrency guards, per-step env -scoping, multi-route smoke matrix, or the `always()+skipped` dependency -condition. The generator is a useful starting scaffold; the 15+ hand-authored -additions are each justified by runtime or operational concerns not encodable -in the workflow config format alone. +plan-guard, secret inventory (by config name), smoke URL, a FUNCTIONAL +migrations step (`wfctl migrations up`), plugin install, and trigger shape. +It correctly warns about the DB hash-suffix and SPACES casing gaps. It does +NOT derive: hash-suffixed secret references, image wait loops, GHCR credential +derivation, phase-selector dispatch inputs, action SHA pinning, apply pipeline +steps, concurrency guards, per-step env scoping, multi-route smoke matrix, the +`always()+skipped` dependency condition, or the migration `--env prod --format +json` operational flags. The generator is a useful starting scaffold; the +14+ hand-authored additions are each justified by runtime or operational +concerns not encodable in the workflow config format alone. diff --git a/cigen/testdata/multisite/generated-infra.yml b/cigen/testdata/multisite/generated-infra.yml index 36b24e7b..44c74d19 100644 --- a/cigen/testdata/multisite/generated-infra.yml +++ b/cigen/testdata/multisite/generated-infra.yml @@ -2,13 +2,13 @@ name: multisite on: pull_request: paths: - - 'cigen/testdata/multisite/deploy.prereq.yaml' - - 'cigen/testdata/multisite/deploy.yaml' + - 'deploy.prereq.yaml' + - 'deploy.yaml' push: branches: [main] paths: - - 'cigen/testdata/multisite/deploy.prereq.yaml' - - 'cigen/testdata/multisite/deploy.yaml' + - 'deploy.prereq.yaml' + - 'deploy.yaml' workflow_dispatch: permissions: contents: read @@ -24,13 +24,13 @@ jobs: with: version: 'latest' - name: Install plugins (prereq) - run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.prereq.yaml' + run: wfctl plugin install --config 'deploy.prereq.yaml' - name: Install plugins (deploy) - run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.yaml' + run: wfctl plugin install --config 'deploy.yaml' - name: Plan prereq - run: wfctl infra plan --config 'cigen/testdata/multisite/deploy.prereq.yaml' --format markdown >> plan.md + run: wfctl infra plan --config 'deploy.prereq.yaml' --format markdown >> plan.md - name: Plan deploy - run: wfctl infra plan --config 'cigen/testdata/multisite/deploy.yaml' --format markdown >> plan.md + run: wfctl infra plan --config 'deploy.yaml' --format markdown >> plan.md - name: Post plan comment uses: actions/github-script@v7 with: @@ -70,17 +70,17 @@ jobs: with: version: 'latest' - name: Install plugins - run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.prereq.yaml' + run: wfctl plugin install --config 'deploy.prereq.yaml' - name: Plan guard run: | - wfctl infra plan --config 'cigen/testdata/multisite/deploy.prereq.yaml' | tee plan-guard.txt + wfctl infra plan --config 'deploy.prereq.yaml' | tee plan-guard.txt if grep -Eq -- '^[[:space:]]*(- delete|-/\+ replace)[[:space:]]' plan-guard.txt || \ grep -Eq -- 'Plan: .*([1-9][0-9]* to replace|[1-9][0-9]* to destroy)' plan-guard.txt; then echo "::error::Refusing apply: plan includes replace or destroy of a protected resource." >&2 exit 1 fi - name: Apply prereq - run: wfctl infra apply --config 'cigen/testdata/multisite/deploy.prereq.yaml' --auto-approve + run: wfctl infra apply --config 'deploy.prereq.yaml' --auto-approve apply-deploy: if: "(github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'workflow_dispatch'" needs: apply-prereq @@ -109,21 +109,21 @@ jobs: with: version: 'latest' - name: Install plugins - run: wfctl plugin install --config 'cigen/testdata/multisite/deploy.yaml' + run: wfctl plugin install --config 'deploy.yaml' - name: Plan guard run: | - wfctl infra plan --config 'cigen/testdata/multisite/deploy.yaml' | tee plan-guard.txt + wfctl infra plan --config 'deploy.yaml' | tee plan-guard.txt if grep -Eq -- '^[[:space:]]*(- delete|-/\+ replace)[[:space:]]' plan-guard.txt || \ grep -Eq -- 'Plan: .*([1-9][0-9]* to replace|[1-9][0-9]* to destroy)' plan-guard.txt; then echo "::error::Refusing apply: plan includes replace or destroy of a protected resource." >&2 exit 1 fi - name: Run migrations - run: wfctl ci run --config 'cigen/testdata/multisite/deploy.yaml' --phase migrate + run: wfctl migrations up --config 'deploy.yaml' env: MULTISITE_DB_URL: ${{ secrets.MULTISITE_DB_URL }} - name: Apply deploy - run: wfctl infra apply --config 'cigen/testdata/multisite/deploy.yaml' --auto-approve + run: wfctl infra apply --config 'deploy.yaml' --auto-approve smoke: needs: apply-deploy if: github.event_name == 'push' && github.ref == 'refs/heads/main' diff --git a/cigen/testdata/multisite/plan.json b/cigen/testdata/multisite/plan.json index a65fd59a..16903a4f 100644 --- a/cigen/testdata/multisite/plan.json +++ b/cigen/testdata/multisite/plan.json @@ -57,11 +57,11 @@ "phases": [ { "name": "prereq", - "config_path": "cigen/testdata/multisite/deploy.prereq.yaml" + "config_path": "deploy.prereq.yaml" }, { "name": "deploy", - "config_path": "cigen/testdata/multisite/deploy.yaml" + "config_path": "deploy.yaml" } ], "migrations": { diff --git a/cmd/wfctl/ci.go b/cmd/wfctl/ci.go index cee8cff0..d43e55fa 100644 --- a/cmd/wfctl/ci.go +++ b/cmd/wfctl/ci.go @@ -81,6 +81,8 @@ func runCIGenerate(args []string) error { exitCode := fs.Bool("exit-code", false, "With --diff: exit 1 when files differ") write := fs.Bool("write", false, "Allow overwriting existing files") phaseConfig := fs.String("phase-config", "", "Prerequisite phase config path") + configPathAlias := fs.String("config-path-alias", "", "Logical repo-relative path for the primary config in generated CI (default: relativized real path)") + phaseConfigAlias := fs.String("phase-config-alias", "", "Logical repo-relative path for the prereq config in generated CI") interactive := fs.Bool("interactive", false, "Force interactive wizard even when --platform is set") if err := fs.Parse(args); err != nil { return err @@ -121,9 +123,11 @@ func runCIGenerate(args []string) error { return err } opts := cigen.Options{ - WfctlVersion: ciGeneratedWfctlVersion(), - Runner: *runner, - PhaseConfig: *phaseConfig, + WfctlVersion: ciGeneratedWfctlVersion(), + Runner: *runner, + PhaseConfig: *phaseConfig, + ConfigPathAlias: *configPathAlias, + PhaseConfigAlias: *phaseConfigAlias, } var analyzeErr error plan, analyzeErr = cigen.Analyze([]string{configPath}, opts) diff --git a/cmd/wfctl/ci_plan.go b/cmd/wfctl/ci_plan.go index 9e08e679..afcf5a70 100644 --- a/cmd/wfctl/ci_plan.go +++ b/cmd/wfctl/ci_plan.go @@ -15,6 +15,8 @@ func runCIPlan(args []string) error { configFileShort := fs.String("c", "", "Workflow config file (shorthand for --config)") out := fs.String("out", "-", "Output file for the CIPlan JSON ('-' for stdout)") phaseConfig := fs.String("phase-config", "", "Prerequisite phase config path (creates a 2-phase plan)") + configPathAlias := fs.String("config-path-alias", "", "Logical repo-relative path for the primary config in generated CI (default: relativized real path)") + phaseConfigAlias := fs.String("phase-config-alias", "", "Logical repo-relative path for the prereq config in generated CI") wfctlVer := fs.String("wfctl-version", "", "Pin wfctl version in plan (default: latest)") branch := fs.String("branch", "", "Default branch name (default: main)") runner := fs.String("runner", "", "GitHub Actions runner label (default: ubuntu-latest)") @@ -44,10 +46,12 @@ Options: } opts := cigen.Options{ - WfctlVersion: *wfctlVer, - DefaultBranch: *branch, - Runner: *runner, - PhaseConfig: *phaseConfig, + WfctlVersion: *wfctlVer, + DefaultBranch: *branch, + Runner: *runner, + PhaseConfig: *phaseConfig, + ConfigPathAlias: *configPathAlias, + PhaseConfigAlias: *phaseConfigAlias, } plan, err := cigen.Analyze([]string{configPath}, opts) From b1f9b9c29449627ca532e148adb51c1e99e2eae9 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 30 May 2026 18:00:26 -0400 Subject: [PATCH 4/4] fix(cigen): drop redundant step-level env on migrations step; regen evidence + test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review follow-up for PR4. 1. IMPORTANT: render_gha.go emitted a step-level `env:` on the migrations step re-declaring ${{ secrets. }}, but deriveSecrets always adds the migrations DBEnv to the global secrets union (analyze.go), so it is already present in the apply job's job-level `env:` block. The step-level block was redundant and would have broken a minimal no-secrets plan. Removed it; the migration command still reads the DB secret via job-level env. (GitLab renderer relies on project CI/CD variables and had no step-level env.) 2. Added TestApplyWizardOverrides_MigrationsTruePreservesMigrations (symmetric to the existing Smoke-preserve test). Strengthened the GHA migrations test to assert the DBEnv is in the job-level env AND the step carries no step-level env block. 3. GAP.md: documented that the GLOBAL cross-phase secret union over-exposes secrets — e.g. MULTISITE_DB_URL appears in apply-prereq's env even though prereq runs no migrations. Honest known limitation; per-phase secret scoping is future work, NOT this PR's scope. Regenerated multisite evidence with the fixed binary (plan.json byte-identical; generated-infra.yml drops the 2 step-level env lines). Fresh regen matches the committed generated-infra.yml byte-for-byte. Live infra.yml NOT modified. Co-Authored-By: Claude Opus 4.8 (1M context) --- cigen/render_gha.go | 7 ++--- cigen/render_gha_test.go | 21 ++++++++++++++ cigen/testdata/multisite/GAP.md | 30 +++++++++++++++++--- cigen/testdata/multisite/generated-infra.yml | 2 -- cmd/wfctl/ci_test.go | 22 ++++++++++++++ 5 files changed, 72 insertions(+), 10 deletions(-) diff --git a/cigen/render_gha.go b/cigen/render_gha.go index c4e70f36..d267f046 100644 --- a/cigen/render_gha.go +++ b/cigen/render_gha.go @@ -208,14 +208,13 @@ func writeApplyJob(b *strings.Builder, jobName string, phase DeployPhase, needs // Migrations step (only in the last phase). Use `wfctl migrations up`, // the real migration runner — `wfctl ci run --phase migrate` is NOT a valid // phase (ci run only accepts build|test|deploy) and would fail at runtime. - // The DB-url secret is already present in the apply job's `env:` block via - // the secrets union, so the migration command can read it. + // No step-level `env:` is needed: deriveSecrets always adds the migrations + // DBEnv to the secrets union, so it is already in the apply job's job-level + // `env:` block above; re-declaring it here would be redundant. isLastPhase := phase.Name == p.Phases[len(p.Phases)-1].Name if isLastPhase && p.Migrations != nil { b.WriteString(" - name: Run migrations\n") fmt.Fprintf(b, " run: %s\n", migrationsUpCommand(phase.ConfigPath, p.Migrations.Env)) - b.WriteString(" env:\n") - fmt.Fprintf(b, " %s: ${{ secrets.%s }}\n", p.Migrations.DBEnv, p.Migrations.DBEnv) } fmt.Fprintf(b, " - name: Apply %s\n", phase.Name) diff --git a/cigen/render_gha_test.go b/cigen/render_gha_test.go index 22a9a5b5..7eba88d0 100644 --- a/cigen/render_gha_test.go +++ b/cigen/render_gha_test.go @@ -85,6 +85,27 @@ func TestRenderGitHubActions_MigrationsStep(t *testing.T) { if strings.Contains(content, "--phase migrate") { t.Error("migrations step must NOT use 'wfctl ci run --phase migrate' (not a valid phase)") } + + // The DB secret must still be available to the migrations step via the + // apply job's job-level `env:` block (deriveSecrets always adds DBEnv to + // the union). The migrations step must NOT re-declare it with a redundant + // step-level `env:`. + if !strings.Contains(content, " APP_DB_URL: ${{ secrets.APP_DB_URL }}") { + t.Errorf("expected DBEnv secret in job-level env block, got:\n%s", content) + } + migIdx := strings.Index(content, "- name: Run migrations") + if migIdx < 0 { + t.Fatalf("expected a 'Run migrations' step, got:\n%s", content) + } + // Slice from the migrations step to the next step (Apply) and assert no + // step-level env block appears inside it. + rest := content[migIdx:] + if nextIdx := strings.Index(rest[1:], "- name:"); nextIdx >= 0 { + rest = rest[:nextIdx+1] + } + if strings.Contains(rest, "env:") { + t.Errorf("migrations step must NOT carry a redundant step-level env: block, got:\n%s", rest) + } } func TestRenderGitHubActions_MigrationsStep_WithEnv(t *testing.T) { diff --git a/cigen/testdata/multisite/GAP.md b/cigen/testdata/multisite/GAP.md index 04d352fc..4b45a079 100644 --- a/cigen/testdata/multisite/GAP.md +++ b/cigen/testdata/multisite/GAP.md @@ -78,7 +78,8 @@ intent of the hand-written workflow, as confirmed by the diff: `multisite-pg` and `gocodealone-multisite` modules. - **apply-job `env:` block with 1:1 secret name mapping**: all 16 secrets from the union of both configs are present in the `env:` block on each - apply job using `${{ secrets.NAME }}` syntax. + apply job using `${{ secrets.NAME }}` syntax. NOTE: this union is GLOBAL + across phases — see "Known limitations: global-union secret over-exposure". - **Two-phase plugin install in plan job**: `Install plugins (prereq)` and `Install plugins (deploy)` steps both present. - **Migrations step in apply-deploy** (FUNCTIONAL — fixed): `Run migrations` @@ -297,6 +298,26 @@ json` is a presentation choice. Neither is derivable from the base config. --- +## Known limitations (this PR's scope) + +### Global-union secret over-exposure across phases + +`deriveSecrets` computes a single GLOBAL union of every secret referenced by +either config, and `RenderGitHubActions` emits that same full union as the +job-level `env:` block on EVERY apply job. As a consequence, the generated +`apply-prereq` job carries secrets it does not use — for example +`MULTISITE_DB_URL` appears in `apply-prereq`'s `env:` even though the prereq +phase runs no migrations and never reads that secret. + +This is honest over-exposure: it does not break the pipeline (the extra +secrets are simply unused in that job), but it widens the blast radius of each +job beyond the minimum it needs. The hand-written `infra.yml` scopes secrets +more tightly (per-step `env:` on the steps that actually use them — see item 8 +above). Per-phase / per-step secret scoping in the generator is future work +and is explicitly NOT in this PR's scope. + +--- + ## What the generator got WRONG (not just incomplete) None remaining at command level. Both previously-documented defects are FIXED: @@ -328,6 +349,7 @@ NOT derive: hash-suffixed secret references, image wait loops, GHCR credential derivation, phase-selector dispatch inputs, action SHA pinning, apply pipeline steps, concurrency guards, per-step env scoping, multi-route smoke matrix, the `always()+skipped` dependency condition, or the migration `--env prod --format -json` operational flags. The generator is a useful starting scaffold; the -14+ hand-authored additions are each justified by runtime or operational -concerns not encodable in the workflow config format alone. +json` operational flags. It also over-exposes secrets via a global union on +every apply job (see "Known limitations"). The generator is a useful starting +scaffold; the 14+ hand-authored additions are each justified by runtime or +operational concerns not encodable in the workflow config format alone. diff --git a/cigen/testdata/multisite/generated-infra.yml b/cigen/testdata/multisite/generated-infra.yml index 44c74d19..8652aa1f 100644 --- a/cigen/testdata/multisite/generated-infra.yml +++ b/cigen/testdata/multisite/generated-infra.yml @@ -120,8 +120,6 @@ jobs: fi - name: Run migrations run: wfctl migrations up --config 'deploy.yaml' - env: - MULTISITE_DB_URL: ${{ secrets.MULTISITE_DB_URL }} - name: Apply deploy run: wfctl infra apply --config 'deploy.yaml' --auto-approve smoke: diff --git a/cmd/wfctl/ci_test.go b/cmd/wfctl/ci_test.go index 95f5cff7..8d970a1e 100644 --- a/cmd/wfctl/ci_test.go +++ b/cmd/wfctl/ci_test.go @@ -316,6 +316,28 @@ func TestApplyWizardOverrides_MigrationsFalseDropsMigrations(t *testing.T) { } } +func TestApplyWizardOverrides_MigrationsTruePreservesMigrations(t *testing.T) { + mig := &cigen.MigrationsSpec{DBEnv: "DB_URL", Source: "migrations"} + plan := &cigen.CIPlan{ + Runner: "ubuntu-latest", + Migrations: mig, + } + choices := wizardChoices{ + Platform: "github_actions", + Runner: "ubuntu-latest", + Smoke: false, + Migrations: true, + PlanGuard: false, + } + applyWizardOverrides(plan, choices) + if plan.Migrations == nil { + t.Fatal("expected plan.Migrations to be preserved when choices.Migrations=true") + } + if plan.Migrations.DBEnv != mig.DBEnv { + t.Errorf("migrations DBEnv changed unexpectedly: got %q, want %q", plan.Migrations.DBEnv, mig.DBEnv) + } +} + func TestApplyWizardOverrides_RunnerOverrideApplies(t *testing.T) { plan := &cigen.CIPlan{Runner: "ubuntu-latest"} choices := wizardChoices{