From 374760a8751cfcd2447f90d6e765d97b1814fedd Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Wed, 8 Jul 2026 09:20:58 -0400 Subject: [PATCH] feat(rollback): scope the git-history fallback to the component When a component rollback exhausts its recorded previous-version ring and falls through to reading prior deployments from git history, it now resolves each historical manifest through the component's own state.components.. rows instead of the flat state. node. A component can no longer recover a sibling's prior deployment from deep history. An empty component reads the flat history exactly as before, byte-identical. This completes the read-side scoping the recorded ring and state writes already had. Refs #294. Signed-off-by: Joshua Temple --- internal/rollback/coverage_test.go | 10 +- internal/rollback/history.go | 38 +++++- internal/rollback/history_component_test.go | 140 ++++++++++++++++++++ internal/rollback/history_test.go | 2 +- internal/rollback/rollback.go | 2 +- 5 files changed, 178 insertions(+), 14 deletions(-) create mode 100644 internal/rollback/history_component_test.go diff --git a/internal/rollback/coverage_test.go b/internal/rollback/coverage_test.go index eb1638b5..c7b143a1 100644 --- a/internal/rollback/coverage_test.go +++ b/internal/rollback/coverage_test.go @@ -349,26 +349,26 @@ func TestCommitAndPush_NoChangesIsNoOp(t *testing.T) { func TestExtractEnvState_EdgeCases(t *testing.T) { // Invalid YAML returns nil. - if s := extractEnvState([]byte("::: not yaml :::"), config.DefaultManifestKey, "prod"); s != nil { + if s := extractEnvState([]byte("::: not yaml :::"), config.DefaultManifestKey, "", "prod"); s != nil { t.Errorf("invalid yaml = %+v, want nil", s) } // Missing top-level key returns nil. - if s := extractEnvState([]byte("other:\n state: {}\n"), config.DefaultManifestKey, "prod"); s != nil { + if s := extractEnvState([]byte("other:\n state: {}\n"), config.DefaultManifestKey, "", "prod"); s != nil { t.Errorf("missing key = %+v, want nil", s) } // Env absent returns nil. missingEnv := []byte("ci:\n state:\n dev:\n sha: x\n") - if s := extractEnvState(missingEnv, config.DefaultManifestKey, "prod"); s != nil { + if s := extractEnvState(missingEnv, config.DefaultManifestKey, "", "prod"); s != nil { t.Errorf("absent env = %+v, want nil", s) } // Empty (zero-value) env state returns nil. emptyEnv := []byte("ci:\n state:\n prod: {}\n") - if s := extractEnvState(emptyEnv, config.DefaultManifestKey, "prod"); s != nil { + if s := extractEnvState(emptyEnv, config.DefaultManifestKey, "", "prod"); s != nil { t.Errorf("empty env state = %+v, want nil", s) } // A populated env state is returned, and an empty manifest key defaults. good := []byte("ci:\n state:\n prod:\n sha: prodsha9999999\n version: v1.9.0\n") - s := extractEnvState(good, "", "prod") + s := extractEnvState(good, "", "", "prod") if s == nil || s.SHA != "prodsha9999999" { t.Errorf("populated env state not returned: %+v", s) } diff --git a/internal/rollback/history.go b/internal/rollback/history.go index cf7d2b64..b9453214 100644 --- a/internal/rollback/history.go +++ b/internal/rollback/history.go @@ -17,10 +17,16 @@ import ( type gitHistoryReader struct { configPath string manifestKey string + // component, when non-empty, scopes historical reads to + // state.components.. so a component's rollback recovers only + // its own prior deployments, never a sibling's or the flat history. An empty + // value reads the flat state. history, byte-identical to the + // single-component behaviour. + component string } -func newGitHistoryReader(configPath, manifestKey string) *gitHistoryReader { - return &gitHistoryReader{configPath: configPath, manifestKey: manifestKey} +func newGitHistoryReader(configPath, manifestKey, component string) *gitHistoryReader { + return &gitHistoryReader{configPath: configPath, manifestKey: manifestKey, component: component} } // PriorStates returns historical EnvState snapshots for env from the manifest's @@ -64,7 +70,7 @@ func (g *gitHistoryReader) PriorStates(env string) ([]*config.EnvState, error) { if err != nil { continue // file may not exist at that revision } - state := extractEnvState(blob, g.manifestKey, env) + state := extractEnvState(blob, g.manifestKey, g.component, env) if state == nil { continue } @@ -81,8 +87,20 @@ func (g *gitHistoryReader) PriorStates(env string) ([]*config.EnvState, error) { // extractEnvState parses a manifest blob and returns the EnvState for env, or // nil when the manifest can't be parsed or the env isn't recorded. The manifest -// key is honoured so wrapped (ci:) manifests parse correctly. -func extractEnvState(blob []byte, manifestKey, env string) *config.EnvState { +// key is honoured so wrapped (ci:) manifests parse correctly. When component is +// non-empty the env is resolved from state.components.. via the +// same overlay path rollback and finalize use, so a component's history stays +// scoped to its own subtree; an empty component reads the flat state., +// byte-identical to the single-component behaviour. +func extractEnvState(blob []byte, manifestKey, component, env string) *config.EnvState { + if component != "" { + compState, err := config.ReadComponentState(blob, manifestKey, component) + if err != nil { + return nil + } + return meaningfulEnvState(compState[env]) + } + key := manifestKey if key == "" { key = config.DefaultManifestKey @@ -98,8 +116,14 @@ func extractEnvState(blob []byte, manifestKey, env string) *config.EnvState { if !ok { return nil } - state, ok := inner.State[env] - if !ok || state == nil { + return meaningfulEnvState(inner.State[env]) +} + +// meaningfulEnvState returns state when it records a deployment (a sha, version, +// or per-deployable entry) and nil otherwise, so an empty placeholder row is +// treated as absent history. +func meaningfulEnvState(state *config.EnvState) *config.EnvState { + if state == nil { return nil } if state.SHA == "" && state.Version == "" && len(state.Deploys) == 0 { diff --git a/internal/rollback/history_component_test.go b/internal/rollback/history_component_test.go new file mode 100644 index 00000000..5cda9c86 --- /dev/null +++ b/internal/rollback/history_component_test.go @@ -0,0 +1,140 @@ +package rollback + +import ( + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/stablekernel/cascade/internal/config" +) + +// componentManifestAt renders a two-component manifest whose recorded state lives +// entirely under state.components... Each component carries its own +// prod sha/version so a git-history rollback can be proven to resolve strictly +// from its own subtree and never a sibling's. +func componentManifestAt(checkoutSHA, checkoutVersion, billingSHA, billingVersion string) string { + return `ci: + config: + trunk_branch: main + environments: [dev, prod] + components: + checkout: + path: services/checkout + tag_prefix: checkout- + billing: + path: services/billing + tag_prefix: billing- + state: + components: + checkout: + prod: + sha: ` + checkoutSHA + ` + version: ` + checkoutVersion + ` + committed_at: "2026-04-01T11:00:00Z" + committed_by: alice + billing: + prod: + sha: ` + billingSHA + ` + version: ` + billingVersion + ` + committed_at: "2026-04-01T11:00:00Z" + committed_by: bob +` +} + +// TestGitHistoryReader_ComponentScoped_IsolatesSiblingHistory proves the +// component-aware git-history reader resolves prior deployments strictly from its +// own state.components.. subtree. The negative assertion is the +// point: checkout's reader must never surface billing's deeper history, and the +// empty-component (flat) reader stays byte-identical, reading only state.. +func TestGitHistoryReader_ComponentScoped_IsolatesSiblingHistory(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + dir := t.TempDir() + gitInit(t, dir) + + rel := "manifest.yaml" + // Old commit: each component records its own distinct prior deployment. + gitCommitFile(t, dir, rel, + componentManifestAt("checkoutold111", "checkout-v1.0.0", "billingdeep22", "billing-secret-v0.0.1"), + "checkout v1.0.0, billing v0.0.1") + // New commit: both advance, leaving the priors recoverable only from history. + gitCommitFile(t, dir, rel, + componentManifestAt("checkoutnew333", "checkout-v2.0.0", "billingnew444", "billing-v2.0.0"), + "checkout v2.0.0, billing v2.0.0") + + path := filepath.Join(dir, rel) + + // checkout's reader returns checkout's history and NEVER billing's. + checkoutStates, err := newGitHistoryReader(path, config.DefaultManifestKey, "checkout").PriorStates("prod") + require.NoError(t, err) + require.NotEmpty(t, checkoutStates, "component git-history must recover checkout's own subtree") + var sawCheckoutPrior bool + for _, s := range checkoutStates { + require.NotEqual(t, "billingdeep22", s.SHA, "checkout reader leaked a sibling deployment") + require.NotEqual(t, "billing-secret-v0.0.1", s.Version, "checkout reader leaked a sibling version") + require.NotEqual(t, "billingnew444", s.SHA, "checkout reader leaked a sibling deployment") + if s.SHA == "checkoutold111" && s.Version == "checkout-v1.0.0" { + sawCheckoutPrior = true + } + } + require.True(t, sawCheckoutPrior, "checkout's own prior deployment not recovered from history") + + // billing's reader returns billing's history and NEVER checkout's. + billingStates, err := newGitHistoryReader(path, config.DefaultManifestKey, "billing").PriorStates("prod") + require.NoError(t, err) + var sawBillingPrior bool + for _, s := range billingStates { + require.NotEqual(t, "checkoutold111", s.SHA, "billing reader leaked a sibling deployment") + if s.SHA == "billingdeep22" && s.Version == "billing-secret-v0.0.1" { + sawBillingPrior = true + } + } + require.True(t, sawBillingPrior, "billing's own prior deployment not recovered from history") + + // Empty-component reader is byte-identical to the pre-component behaviour: it + // reads only the flat state., which this components-only manifest never + // populates, so it recovers nothing (no cross-over into a component subtree). + flatStates, err := newGitHistoryReader(path, config.DefaultManifestKey, "").PriorStates("prod") + require.NoError(t, err) + require.Empty(t, flatStates, "flat reader must not descend into the components subtree") +} + +// TestGitHistoryReader_ComponentScoped_RollbackRecoversOwnPriorDeployment drives +// the full rollback flow: a component whose live manifest has advanced past a +// deployment recovers its prior sha/version from its OWN git history, and a +// target that exists only in a sibling's history is not resolvable under the +// component's scope. +func TestGitHistoryReader_ComponentScoped_RollbackRecoversOwnPriorDeployment(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + dir := t.TempDir() + gitInit(t, dir) + + rel := "manifest.yaml" + gitCommitFile(t, dir, rel, + componentManifestAt("checkoutold111", "checkout-v1.0.0", "billingdeep22", "billing-secret-v0.0.1"), + "checkout v1.0.0, billing v0.0.1") + gitCommitFile(t, dir, rel, + componentManifestAt("checkoutnew333", "checkout-v2.0.0", "billingnew444", "billing-v2.0.0"), + "checkout v2.0.0, billing v2.0.0") + + path := filepath.Join(dir, rel) + + rb, err := New(Options{ConfigPath: path, Actor: "oncall", Component: "checkout"}) + require.NoError(t, err) + + // checkout's own prior deployment resolves from git history. + plan, err := rb.Plan("prod", "checkout-v1.0.0", "") + require.NoError(t, err) + require.Equal(t, "checkoutold111", plan.Target.SHA) + require.Equal(t, "git-history", plan.Target.Source) + + // A version that lives ONLY in billing's history is not resolvable under + // checkout's scope: the sibling's history is never read. + _, err = rb.Plan("prod", "billing-secret-v0.0.1", "") + require.Error(t, err, "checkout rollback must not resolve a sibling's historical deployment") +} diff --git a/internal/rollback/history_test.go b/internal/rollback/history_test.go index 64640640..285f5463 100644 --- a/internal/rollback/history_test.go +++ b/internal/rollback/history_test.go @@ -122,7 +122,7 @@ func TestGitHistoryReader_RecoversPriorVersion_Subdir(t *testing.T) { path := filepath.Join(dir, rel) - reader := newGitHistoryReader(path, config.DefaultManifestKey) + reader := newGitHistoryReader(path, config.DefaultManifestKey, "") states, err := reader.PriorStates("prod") if err != nil { t.Fatalf("PriorStates: %v", err) diff --git a/internal/rollback/rollback.go b/internal/rollback/rollback.go index cad48850..c7bcdb54 100644 --- a/internal/rollback/rollback.go +++ b/internal/rollback/rollback.go @@ -137,7 +137,7 @@ func New(opts Options) (*Rollbacker, error) { history := opts.HistoryReader if history == nil { - history = newGitHistoryReader(configPath, key) + history = newGitHistoryReader(configPath, key, opts.Component) } // Resolve the effective environment ladder the eligibility and first-env guards