From d2e96f482d6621c1d7637208312f037466c0d113 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 24 Jun 2026 21:06:28 -0400 Subject: [PATCH] test(rollback): isolate resolution source in e2e + surface it in the rollback workflow The e2e harness could not distinguish which rollback target-resolution source won (live state vs the per-env history ring vs git-history). Surface the resolved source end-to-end: preflight --gha-output emits target_source, the generated rollback workflow logs 'rollback resolved from ' (ungated by dry_run), and the harness gains an ExpectSource assertion. Adds three e2e scenarios pinning each precedence path, validated under act+gitea. retry-on-conflict (#333) was already covered by CommitWithRetry unit tests. Closes #320. Signed-off-by: Joshua Temple --- e2e/harness/assert.go | 18 +++ e2e/harness/assert_test.go | 44 ++++++ e2e/harness/multistep.go | 5 +- e2e/harness/rollback_actions.go | 7 + .../rollback/rollback-source-git-history.yaml | 139 ++++++++++++++++++ .../rollback-source-previous-ring.yaml | 111 ++++++++++++++ .../rollback/rollback-source-state.yaml | 84 +++++++++++ internal/generate/rollback.go | 3 + internal/generate/rollback_test.go | 7 + internal/rollback/command_subcommands.go | 1 + internal/rollback/command_subcommands_test.go | 109 ++++++++++++++ 11 files changed, 527 insertions(+), 1 deletion(-) create mode 100644 e2e/scenarios/rollback/rollback-source-git-history.yaml create mode 100644 e2e/scenarios/rollback/rollback-source-previous-ring.yaml create mode 100644 e2e/scenarios/rollback/rollback-source-state.yaml diff --git a/e2e/harness/assert.go b/e2e/harness/assert.go index 6a19af6d..b5d87f95 100644 --- a/e2e/harness/assert.go +++ b/e2e/harness/assert.go @@ -483,3 +483,21 @@ func findJob(jobs map[string]*JobResultExtended, name string) *JobResultExtended return nil } + +// assertRollbackSource checks that the workflow logs contain the expected +// resolved-source marker emitted by the preflight "Report Resolved Source" +// step. The marker has the form "rollback resolved from ", where +// is one of "state", "previous-ring", or "git-history". +// +// It is called from executeRollback when RollbackStep.ExpectSource is set, so +// scenarios can assert WHICH precedence path the resolver chose, not just the +// resulting SHA. +func assertRollbackSource(t testingT, logs string, wantSource string) error { + t.Helper() + marker := fmt.Sprintf("rollback resolved from %s", wantSource) + if !strings.Contains(logs, marker) { + t.Errorf("rollback logs did not contain resolved-source marker %q", marker) + return fmt.Errorf("resolved-source marker %q not found in rollback logs", marker) + } + return nil +} diff --git a/e2e/harness/assert_test.go b/e2e/harness/assert_test.go index 563e833d..e10d3b55 100644 --- a/e2e/harness/assert_test.go +++ b/e2e/harness/assert_test.go @@ -406,3 +406,47 @@ func (m *mockT) Helper() {} var _ testingT = (*testing.T)(nil) var _ testingT = (*mockT)(nil) + +func TestAssertRollbackSource(t *testing.T) { + t.Run("marker present passes", func(t *testing.T) { + logs := "some output\nrollback resolved from previous-ring\nmore output\n" + mt := &mockT{} + err := assertRollbackSource(mt, logs, "previous-ring") + assert.NoError(t, err) + assert.False(t, mt.failed) + }) + + t.Run("marker absent fails", func(t *testing.T) { + logs := "some output\nrollback resolved from state\nmore output\n" + mt := &mockT{} + err := assertRollbackSource(mt, logs, "previous-ring") + assert.Error(t, err) + assert.True(t, mt.failed) + assert.Contains(t, mt.errors[0], "previous-ring") + }) + + t.Run("state source marker", func(t *testing.T) { + logs := "pre\nrollback resolved from state\npost\n" + mt := &mockT{} + err := assertRollbackSource(mt, logs, "state") + assert.NoError(t, err) + assert.False(t, mt.failed) + }) + + t.Run("git-history source marker", func(t *testing.T) { + logs := "pre\nrollback resolved from git-history\npost\n" + mt := &mockT{} + err := assertRollbackSource(mt, logs, "git-history") + assert.NoError(t, err) + assert.False(t, mt.failed) + }) + + t.Run("wrong source in logs fails", func(t *testing.T) { + logs := "rollback resolved from git-history\n" + mt := &mockT{} + err := assertRollbackSource(mt, logs, "state") + assert.Error(t, err) + assert.True(t, mt.failed) + assert.Contains(t, mt.errors[0], "state") + }) +} diff --git a/e2e/harness/multistep.go b/e2e/harness/multistep.go index 1a7b1e19..504a8ae0 100644 --- a/e2e/harness/multistep.go +++ b/e2e/harness/multistep.go @@ -201,13 +201,16 @@ type PromoteStep struct { // deployable. DryRun sets the dry_run input, which suppresses the deploy and // finalize jobs. ExpectFailure marks a run that is expected to conclude in // failure (for example a rollback whose preflight cannot resolve a target), -// mirroring PromoteStep.ExpectFailure. +// mirroring PromoteStep.ExpectFailure. ExpectSource, when non-empty, asserts the +// resolved-target source label that the preflight job echoes to its job log +// (one of "state", "previous-ring", or "git-history"). type RollbackStep struct { Environment string `yaml:"environment"` Target string `yaml:"target,omitempty"` Deployable string `yaml:"deployable,omitempty"` DryRun bool `yaml:"dry_run,omitempty"` ExpectFailure bool `yaml:"expect_failure,omitempty"` + ExpectSource string `yaml:"expect_source,omitempty"` } // VerifyStep defines a verify action: a read-only `cascade verify` run in the diff --git a/e2e/harness/rollback_actions.go b/e2e/harness/rollback_actions.go index d60c1c8c..5f51c34a 100644 --- a/e2e/harness/rollback_actions.go +++ b/e2e/harness/rollback_actions.go @@ -104,5 +104,12 @@ func (r *Runner) executeRollback(ctx context.Context, rollback *RollbackStep, co } r.t.Logf(" Rollback: workflow completed successfully") + + if rollback.ExpectSource != "" { + if err := assertRollbackSource(r.t, result.Logs, rollback.ExpectSource); err != nil { + return err + } + } + return nil } diff --git a/e2e/scenarios/rollback/rollback-source-git-history.yaml b/e2e/scenarios/rollback/rollback-source-git-history.yaml new file mode 100644 index 00000000..fd6054d7 --- /dev/null +++ b/e2e/scenarios/rollback/rollback-source-git-history.yaml @@ -0,0 +1,139 @@ +name: "Rollback preflight resolves target from git history (deployable-scoped)" +description: | + Isolates the Source="git-history" resolution path: proves that when the + rollback is scoped to a single deployable AND the --to value is not in the + current live state, the preflight job reports target_source=git-history in + its log output. + + The git-history path fires deterministically for a deployable-scoped rollback + because resolveTarget skips the deploy-history ring entirely when a deployable + is given (the ring is env-scoped only: its snapshots carry no per-deployable + data). With the ring skipped, resolution falls straight to step 3: manifest git + history, where it scans prior manifest commits for a matching per-deployable + SHA/version. + + prod is advanced through two published versions with a single deployable (api), + so the first version's per-deployable SHA lives only in an earlier manifest + commit - not in current live state (which is commit2) and not in the ring + (which the deployable-scoped path never consults). A dry-run rollback with + --deployable api --to commit1 forces this path: step 1 fails (live api sha = + commit2), step 2 is skipped (deployable-scoped), step 3 finds commit1 in the + manifest git log as api's recorded per-deployable SHA, resolves + Source="git-history". + + The dry_run flag suppresses the deploy and finalize jobs so the scenario is + fast and deterministic. The preflight job and its "Report Resolved Source" step + run unconditionally (they are never gated by dry_run), so the source marker + appears in logs for all three resolution paths under dry-run. + + The expect_source assertion on the rollback step is the distinguishing check: + it asserts the resolved-source marker in the workflow logs, proving that this + specific resolution path (Source="git-history") was taken end-to-end. + +config: + trunk_branch: main + environments: [dev, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: + # Single deployable so the rollback scope is unambiguous. Inner job id + # apideploy lets act key it distinctly; no actions/checkout needed because + # the step only echoes inputs (act cannot resolve checkout refs against the + # per-scenario gitea). + - name: api + workflow: .github/workflows/deploy-api.yaml + triggers: ["**"] + +steps: + - name: "Commit the first version source" + action: commit + commit: + message: "feat: first version" + files: + src/app.go: | + package main + func main() {} + .github/workflows/deploy-api.yaml: | + name: deploy-api + on: + workflow_call: + inputs: + environment: + required: false + type: string + sha: + required: false + type: string + jobs: + apideploy: + runs-on: ubuntu-latest + steps: + - run: echo "deployed env=${{ inputs.environment }} sha=${{ inputs.sha }}" + + - name: "Orchestrate the first commit into dev" + action: orchestrate + expect: + state: + dev: + sha: commit1 + + - name: "Promote the first version to prod (records api's per-deployable commit1 in manifest)" + action: promote + promote: + mode: cascade + target: prod + expect: + state: + prod: + sha: commit1 + deploys: + api: + sha: commit1 + + - name: "Commit the second version source" + action: commit + commit: + message: "feat: second version" + files: + src/app.go: | + package main + func main() { _ = 2 } + + - name: "Orchestrate the second commit into dev" + action: orchestrate + expect: + state: + dev: + sha: commit2 + + - name: "Promote the second version to prod (advances api past commit1)" + action: promote + promote: + mode: cascade + target: prod + expect: + state: + prod: + sha: commit2 + deploys: + api: + sha: commit2 + + # Dry-run rollback scoped to the api deployable, targeting commit1. Resolution: + # step 1 - live state: api.sha = commit2, no match. + # step 2 - deploy-history ring: SKIPPED because deployable != "" (ring is + # env-scoped only; per-deployable data is never captured there). + # step 3 - manifest git history: finds a prior manifest commit where + # api.sha = commit1_sha, returns Source="git-history". + # The "Report Resolved Source" step in the preflight job echoes + # "rollback resolved from git-history" unconditionally (not gated by dry_run). + - name: "Dry-run deployable-scoped rollback resolves from git history" + action: rollback + rollback: + environment: prod + deployable: api + target: commit1 + dry_run: true + expect_source: git-history diff --git a/e2e/scenarios/rollback/rollback-source-previous-ring.yaml b/e2e/scenarios/rollback/rollback-source-previous-ring.yaml new file mode 100644 index 00000000..f30f8194 --- /dev/null +++ b/e2e/scenarios/rollback/rollback-source-previous-ring.yaml @@ -0,0 +1,111 @@ +name: "Rollback preflight resolves target from previous-deploy ring" +description: | + Isolates the Source="previous-ring" resolution path: proves that when the + --to value is NOT the current live state but IS present in the deploy-history + ring (state..previous), the preflight job reports + target_source=previous-ring in its log output. + + prod is advanced through two published versions so the ring captures the first + commit as the N-1 entry. A dry-run rollback is then dispatched with --to set + to the first commit's SHA. The resolver checks live state first (no match: + live state is commit2), then finds a match in the ring, so the "Report + Resolved Source" step echoes "rollback resolved from previous-ring". The + dry_run flag suppresses the deploy and finalize jobs. + + The expect_source assertion on the rollback step is the distinguishing check: + it asserts the resolved-source marker in the workflow logs, proving that this + specific resolution path (Source="previous-ring") was taken end-to-end. + +config: + trunk_branch: main + environments: [dev, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: + - name: app + workflow: .github/workflows/deploy-app.yaml + triggers: ["**"] + +steps: + - name: "Commit the first version source" + action: commit + commit: + message: "feat: first version" + files: + src/app.go: | + package main + func main() {} + .github/workflows/deploy-app.yaml: | + name: deploy-app + on: + workflow_call: + inputs: + environment: + required: false + type: string + sha: + required: false + type: string + jobs: + appdeploy: + runs-on: ubuntu-latest + steps: + - run: echo "deployed env=${{ inputs.environment }} sha=${{ inputs.sha }}" + + - name: "Orchestrate the first commit into dev" + action: orchestrate + expect: + state: + dev: + sha: commit1 + + - name: "Promote the first version to prod (establishes the ring entry)" + action: promote + promote: + mode: cascade + target: prod + expect: + state: + prod: + sha: commit1 + + - name: "Commit the second version source" + action: commit + commit: + message: "feat: second version" + files: + src/app.go: | + package main + func main() { _ = 2 } + + - name: "Orchestrate the second commit into dev" + action: orchestrate + expect: + state: + dev: + sha: commit2 + + - name: "Promote the second version to prod (advances state past commit1)" + action: promote + promote: + mode: cascade + target: prod + expect: + state: + prod: + sha: commit2 + + # Dry-run rollback targeting commit1 which is now in the deploy-history ring + # (recorded when the second promotion advanced prod past it) but NOT the live + # state (commit2). The resolver skips live state, finds commit1 in the ring, + # and reports Source="previous-ring". The expect_source assertion verifies this + # path end-to-end. + - name: "Dry-run rollback targeting ring entry resolves from previous-ring" + action: rollback + rollback: + environment: prod + target: commit1 + dry_run: true + expect_source: previous-ring diff --git a/e2e/scenarios/rollback/rollback-source-state.yaml b/e2e/scenarios/rollback/rollback-source-state.yaml new file mode 100644 index 00000000..44413f36 --- /dev/null +++ b/e2e/scenarios/rollback/rollback-source-state.yaml @@ -0,0 +1,84 @@ +name: "Rollback preflight resolves target from live state" +description: | + Isolates the Source="state" resolution path: proves that when the --to value + matches the current live state of the environment, the preflight job reports + target_source=state in its log output. + + prod is advanced to a published commit via orchestrate+promote so the manifest + records a live state SHA. A dry-run rollback is then dispatched with --to set + to that exact SHA (the live state value). The resolver finds a match in live + state before consulting the ring or git history, so the "Report Resolved + Source" step echoes "rollback resolved from state". The dry_run flag suppresses + the deploy and finalize jobs so the scenario is fast and deterministic. + + The expect_source assertion on the rollback step is the distinguishing check: + it asserts the resolved-source marker in the workflow logs, proving that this + specific resolution path (Source="state") was taken end-to-end. + +config: + trunk_branch: main + environments: [dev, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["src/**"] + deploys: + - name: app + workflow: .github/workflows/deploy-app.yaml + triggers: ["**"] + +steps: + - name: "Commit the source" + action: commit + commit: + message: "feat: initial source" + files: + src/app.go: | + package main + func main() {} + .github/workflows/deploy-app.yaml: | + name: deploy-app + on: + workflow_call: + inputs: + environment: + required: false + type: string + sha: + required: false + type: string + jobs: + appdeploy: + runs-on: ubuntu-latest + steps: + - run: echo "deployed env=${{ inputs.environment }} sha=${{ inputs.sha }}" + + - name: "Orchestrate commit into dev" + action: orchestrate + expect: + state: + dev: + sha: commit1 + + - name: "Promote to prod to establish live state" + action: promote + promote: + mode: cascade + target: prod + expect: + state: + prod: + sha: commit1 + + # Dry-run rollback targeting the CURRENT live state SHA. The preflight + # resolver matches it against live state first (Source="state"), before + # consulting the ring or git history. The expect_source assertion verifies + # this path end-to-end by checking for "rollback resolved from state" in + # the workflow logs. + - name: "Dry-run rollback targeting live state SHA resolves from state" + action: rollback + rollback: + environment: prod + target: commit1 + dry_run: true + expect_source: state diff --git a/internal/generate/rollback.go b/internal/generate/rollback.go index 660f6e8c..766430d1 100644 --- a/internal/generate/rollback.go +++ b/internal/generate/rollback.go @@ -248,6 +248,7 @@ func (g *RollbackGenerator) writePreflightJob(sb *strings.Builder) { sb.WriteString(" target_env: ${{ steps.preflight.outputs.target_env }}\n") sb.WriteString(" target_sha: ${{ steps.preflight.outputs.target_sha }}\n") sb.WriteString(" target_version: ${{ steps.preflight.outputs.target_version }}\n") + sb.WriteString(" target_source: ${{ steps.preflight.outputs.target_source }}\n") sb.WriteString(" can_proceed: ${{ steps.preflight.outputs.can_proceed }}\n") sb.WriteString(" steps:\n") writeMintSteps(sb, g.config, " ", seamRelease) @@ -270,6 +271,8 @@ func (g *RollbackGenerator) writePreflightJob(sb *strings.Builder) { sb.WriteString(" - name: Fail if Cannot Proceed\n") sb.WriteString(" if: steps.preflight.outputs.can_proceed == 'false'\n") sb.WriteString(" run: exit 1\n") + sb.WriteString(" - name: Report Resolved Source\n") + sb.WriteString(" run: echo \"rollback resolved from ${{ steps.preflight.outputs.target_source }}\"\n") sb.WriteString("\n") } diff --git a/internal/generate/rollback_test.go b/internal/generate/rollback_test.go index 1d4370ba..659137e5 100644 --- a/internal/generate/rollback_test.go +++ b/internal/generate/rollback_test.go @@ -64,6 +64,13 @@ func TestRollbackGenerator_PreflightResolves(t *testing.T) { assert.Contains(t, content, "target_sha: ${{ steps.preflight.outputs.target_sha }}") assert.Contains(t, content, "target_env: ${{ steps.preflight.outputs.target_env }}") assert.Contains(t, content, "can_proceed: ${{ steps.preflight.outputs.can_proceed }}") + // target_source must be wired as a job output so downstream steps and the + // e2e harness can observe which resolution path the preflight chose. + assert.Contains(t, content, "target_source: ${{ steps.preflight.outputs.target_source }}") + // The "Report Resolved Source" step echoes the source to the job log so the + // e2e harness can assert it via log inspection. + assert.Contains(t, content, "Report Resolved Source") + assert.Contains(t, content, "rollback resolved from ${{ steps.preflight.outputs.target_source }}") } func TestRollbackGenerator_DeployJobsKeyedOnTargetSha(t *testing.T) { diff --git a/internal/rollback/command_subcommands.go b/internal/rollback/command_subcommands.go index 29cec3d9..2a26618f 100644 --- a/internal/rollback/command_subcommands.go +++ b/internal/rollback/command_subcommands.go @@ -99,6 +99,7 @@ func runPreflight(opts preflightOptions) error { w.Set("target_env", plan.Environment) w.Set("target_sha", plan.Target.SHA) w.Set("target_version", plan.Target.Version) + w.Set("target_source", plan.Target.Source) w.SetBool("can_proceed", true) return w.Flush() } diff --git a/internal/rollback/command_subcommands_test.go b/internal/rollback/command_subcommands_test.go index b913666d..4e1ea679 100644 --- a/internal/rollback/command_subcommands_test.go +++ b/internal/rollback/command_subcommands_test.go @@ -396,3 +396,112 @@ func TestRollbackFinalize_NoDeploysConfigured_StillApplies(t *testing.T) { t.Errorf("ref %q is not a rollback ref", prod.Ref) } } + +// TestRollbackPreflight_GHAOutput_EmitsTargetSource_State asserts that when +// the requested --to resolves against the live state (Source="state"), the +// preflight gha-output includes target_source=state. +func TestRollbackPreflight_GHAOutput_EmitsTargetSource_State(t *testing.T) { + path := ringManifest(t) + outFile := filepath.Join(t.TempDir(), "gha_output") + t.Setenv("GITHUB_OUTPUT", outFile) + + cmd := NewCommand() + cmd.SilenceUsage = true + cmd.SilenceErrors = true + // v2.0.0 is the live state version, so the resolver hits state first. + cmd.SetArgs([]string{ + "preflight", + "--config", path, + "--env", "prod", + "--to", "v2.0.0", + "--gha-output", + }) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + + data, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("read gha output: %v", err) + } + got := string(data) + if !strings.Contains(got, "target_source=state") { + t.Errorf("gha output missing target_source=state\n%s", got) + } + if !strings.Contains(got, "can_proceed=true") { + t.Errorf("can_proceed not true\n%s", got) + } +} + +// TestRollbackPreflight_GHAOutput_EmitsTargetSource_PreviousRing asserts that +// when the requested --to resolves against the deploy-history ring +// (Source="previous-ring"), the preflight gha-output includes +// target_source=previous-ring. +func TestRollbackPreflight_GHAOutput_EmitsTargetSource_PreviousRing(t *testing.T) { + path := ringManifest(t) + outFile := filepath.Join(t.TempDir(), "gha_output") + t.Setenv("GITHUB_OUTPUT", outFile) + + cmd := NewCommand() + cmd.SilenceUsage = true + cmd.SilenceErrors = true + // v1.9.0 is in the ring but not the live state; resolver hits previous-ring. + cmd.SetArgs([]string{ + "preflight", + "--config", path, + "--env", "prod", + "--to", "v1.9.0", + "--gha-output", + }) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + + data, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("read gha output: %v", err) + } + got := string(data) + if !strings.Contains(got, "target_source=previous-ring") { + t.Errorf("gha output missing target_source=previous-ring\n%s", got) + } + if !strings.Contains(got, "can_proceed=true") { + t.Errorf("can_proceed not true\n%s", got) + } +} + +// TestRollbackPreflight_GHAOutput_EmitsTargetSource_DefaultPreviousRing +// asserts that when no --to is given and the ring has a distinct N-1 entry, +// the default resolver picks it (Source="previous-ring") and the gha-output +// reflects that. +func TestRollbackPreflight_GHAOutput_EmitsTargetSource_DefaultPreviousRing(t *testing.T) { + path := ringManifest(t) + outFile := filepath.Join(t.TempDir(), "gha_output") + t.Setenv("GITHUB_OUTPUT", outFile) + + cmd := NewCommand() + cmd.SilenceUsage = true + cmd.SilenceErrors = true + // No --to: default resolves N-1 from the ring. + cmd.SetArgs([]string{ + "preflight", + "--config", path, + "--env", "prod", + "--gha-output", + }) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + + data, err := os.ReadFile(outFile) + if err != nil { + t.Fatalf("read gha output: %v", err) + } + got := string(data) + if !strings.Contains(got, "target_source=previous-ring") { + t.Errorf("gha output missing target_source=previous-ring\n%s", got) + } + if !strings.Contains(got, "can_proceed=true") { + t.Errorf("can_proceed not true\n%s", got) + } +}