diff --git a/internal/promote/finalize.go b/internal/promote/finalize.go index 1c3c49e4..cbbd50c7 100644 --- a/internal/promote/finalize.go +++ b/internal/promote/finalize.go @@ -8,6 +8,7 @@ import ( "time" "github.com/stablekernel/cascade/internal/config" + "github.com/stablekernel/cascade/internal/hotfix" "github.com/stablekernel/cascade/internal/statewrite" ) @@ -161,14 +162,24 @@ func (f *Finalizer) runLifecycleCleanup() error { // so the rejoin is complete with no side effects to undo. continue } - if err := f.cleaner.DeleteEnvBranch(ev.env); err != nil { + if err := f.cleaner.DeleteEnvBranch(ev.component, ev.env); err != nil { fmt.Printf("Warning: rejoin cleanup for %s: deleting integration branch: %v\n", ev.env, err) } + // Collect hotfix tags under the rejoining branch's component grammar, so a + // component's cleanup sees only its own namespace. A resolve failure (for + // example a ref naming a component the manifest no longer declares) skips + // only the tag cleanup for this env rather than cross-matching a sibling's + // tags under the default grammar; the branch delete above already ran. + spec, err := resolveRejoinCleanupSpec(f.cicdFile, ev.component) + if err != nil { + fmt.Printf("Warning: rejoin cleanup for %s: resolving component tag grammar: %v\n", ev.env, err) + continue + } if err := f.cleaner.CleanHotfixReleases(CleanReleasesRequest{ Environment: ev.env, BaseVersion: ev.baseVersion, SHA: ev.sha, - Spec: resolveTagGrammar(f.cicdFile), + Spec: spec, }); err != nil { fmt.Printf("Warning: rejoin cleanup for %s: cleaning hotfix releases: %v\n", ev.env, err) } @@ -231,11 +242,20 @@ func (f *Finalizer) updateState() { // integration-branch and hotfix-release cleanup, which apply // only to hotfix divergences. rollbackOrigin := IsRollbackRef(state.Ref) + // Recover the component that owns the rejoining integration branch + // from the recorded ref (env//) before it is cleared, + // so the cleanup deletes the branch in the component's own namespace + // and collects tags under the component's grammar. A single-component + // ref (env/) recovers the empty component, keeping cleanup + // byte-identical; a rollback ref is not an env/* branch and recovers + // the empty component, but its cleanup is skipped by rollbackOrigin. + component, _, _ := hotfix.ParseEnvBranch(state.Ref) state.Ref = "" state.BaseSHA = "" state.Patches = nil f.pendingRejoins = append(f.pendingRejoins, rejoinEvent{ env: promo.Environment, + component: component, baseVersion: priorVersion, sha: priorSHA, rollbackOrigin: rollbackOrigin, diff --git a/internal/promote/rejoin.go b/internal/promote/rejoin.go index c64c72e4..3354b9d2 100644 --- a/internal/promote/rejoin.go +++ b/internal/promote/rejoin.go @@ -5,6 +5,7 @@ import ( "os" "time" + "github.com/stablekernel/cascade/internal/config" "github.com/stablekernel/cascade/internal/git" "github.com/stablekernel/cascade/internal/hotfix" "github.com/stablekernel/cascade/internal/release" @@ -37,8 +38,12 @@ type CleanReleasesRequest struct { // to provide one; the production implementation is wired only when finalize runs // in a repository with GitHub context. type LifecycleCleaner interface { - // DeleteEnvBranch deletes the env/ integration branch. - DeleteEnvBranch(env string) error + // DeleteEnvBranch deletes the integration branch for env within component: + // env//, or env/ for the default empty component. The + // component is recovered from the branch the rejoin is cleaning up so a + // component's branch is deleted in its own namespace and a sibling's branch is + // never cross-deleted. + DeleteEnvBranch(component, env string) error // CleanHotfixReleases deletes the hotfix tags and release drafts for the // rejoining environment's prior base version. CleanHotfixReleases(req CleanReleasesRequest) error @@ -49,7 +54,7 @@ type LifecycleCleaner interface { // for non-diverged promotions. type noopLifecycleCleaner struct{} -func (noopLifecycleCleaner) DeleteEnvBranch(string) error { return nil } +func (noopLifecycleCleaner) DeleteEnvBranch(string, string) error { return nil } func (noopLifecycleCleaner) CleanHotfixReleases(CleanReleasesRequest) error { return nil } // FinalizeOption customizes optional, additive Finalizer behavior. Required @@ -110,7 +115,13 @@ func withContentsClient(c statewrite.ContentsClient) FinalizeOption { // finalization, carrying the data the cleaner needs to remove its branch, tags, // and drafts. type rejoinEvent struct { - env string + env string + // component is the declared component that owns the rejoining integration + // branch, recovered from the recorded ref (env//) via + // hotfix.ParseEnvBranch. The default single-component form (env/) yields + // an empty component, keeping branch deletion and tag collection + // byte-identical to the pre-component behavior. + component string baseVersion string // sha is the commit the env pointed at while diverged (its hotfix merge SHA), // passed through to the release cleanup as a lookup fallback so a hotfix @@ -164,15 +175,39 @@ func newFinalizeCleaner() LifecycleCleaner { return newGitReleaseCleaner("origin", release.NewManager(repo, token)) } -// DeleteEnvBranch deletes the env/ branch on the configured remote. -func (c *gitReleaseCleaner) DeleteEnvBranch(env string) error { - branch := hotfix.EnvBranchPrefix + env +// DeleteEnvBranch deletes the integration branch for env within component on the +// configured remote. The branch name is composed with hotfix.EnvBranchName, so a +// named component targets env// and the default empty component +// targets env/, matching exactly what the hotfix finalize created. +func (c *gitReleaseCleaner) DeleteEnvBranch(component, env string) error { + branch := hotfix.EnvBranchName(component, env) if err := git.DeleteRemoteBranch(c.remote, branch); err != nil { return fmt.Errorf("deleting integration branch %s: %w", branch, err) } return nil } +// resolveRejoinCleanupSpec returns the tag grammar the divergence-end release +// cleanup collects hotfix tags under for the rejoining branch's component. The +// default (empty) component yields the manifest's permissive grammar, so a +// single-component rejoin collects tags byte-identically to the pre-component +// behavior. A named component yields that component's resolved grammar with its +// strict tag prefix, so the cleanup sees only that component's own hotfix tags +// and never cross-matches a sibling component's namespace. +func resolveRejoinCleanupSpec(f *config.CICDFile, component string) (taggrammar.Spec, error) { + if component == "" { + return resolveTagGrammar(f), nil + } + if f == nil || f.Config == nil { + return taggrammar.Spec{}, fmt.Errorf("component %q requested but manifest has no config block", component) + } + resolved, err := f.Config.ResolveComponent(component) + if err != nil { + return taggrammar.Spec{}, fmt.Errorf("resolving component %q tag grammar: %w", component, err) + } + return resolved.TagGrammarSpec(), nil +} + // CleanHotfixReleases deletes the hotfix release objects for the prior base // version and then the matching tags. The release object is removed FIRST and the // tag is deleted only when the release delete succeeded (or the release was diff --git a/internal/promote/rejoin_cleanup_test.go b/internal/promote/rejoin_cleanup_test.go index 728e80e8..a7ce4f75 100644 --- a/internal/promote/rejoin_cleanup_test.go +++ b/internal/promote/rejoin_cleanup_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/stablekernel/cascade/internal/hotfix" "github.com/stablekernel/cascade/internal/release" "github.com/stretchr/testify/require" ) @@ -130,8 +131,8 @@ type failingTagCleaner struct { failOnEnv string } -func (c *failingTagCleaner) DeleteEnvBranch(env string) error { - c.deletedBranches = append(c.deletedBranches, env) +func (c *failingTagCleaner) DeleteEnvBranch(component, env string) error { + c.deletedBranches = append(c.deletedBranches, hotfix.EnvBranchName(component, env)) return nil } @@ -161,7 +162,7 @@ func TestRunLifecycleCleanup_BestEffort_DoesNotStrandOtherEnvs(t *testing.T) { err := f.runLifecycleCleanup() require.NoError(t, err, "a cleanup failure for one env must not abort finalize") - require.Contains(t, cleaner.deletedBranches, "test") - require.Contains(t, cleaner.deletedBranches, "uat", "the second env's branch must still be cleaned after the first env's cleanup error") + require.Contains(t, cleaner.deletedBranches, "env/test") + require.Contains(t, cleaner.deletedBranches, "env/uat", "the second env's branch must still be cleaned after the first env's cleanup error") require.Contains(t, cleaner.cleaned, "uat", "the second env's hotfix releases must still be cleaned") } diff --git a/internal/promote/rejoin_component_test.go b/internal/promote/rejoin_component_test.go new file mode 100644 index 00000000..2c81320a --- /dev/null +++ b/internal/promote/rejoin_component_test.go @@ -0,0 +1,187 @@ +package promote + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stablekernel/cascade/internal/hotfix" + "github.com/stablekernel/cascade/internal/taggrammar" + "github.com/stretchr/testify/require" +) + +// divergedComponentManifest writes a manifest declaring two components (api and +// web, each with its own strict tag namespace) whose flat state carries two +// diverged envs on component-namespaced integration branches: "test" on +// env/api/test and "staging" on env/web/staging. It lets a rejoin cleanup prove +// it deletes the rejoining component's branch in its own namespace, never a +// sibling's, and collects tags under the component's strict grammar. +func divergedComponentManifest(t *testing.T) string { + t.Helper() + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "manifest.yaml") + initialConfig := `ci: + config: + trunk_branch: main + environments: [dev, test, staging, prod] + components: + api: + path: api + tag_prefix: api- + web: + path: web + tag_prefix: web- + state: + dev: + sha: trunkhead + version: api-1.4.0-rc.3 + test: + sha: apimerge + version: api-1.4.0-rc.2.hotfix.1 + ref: env/api/test + base_sha: apibase + patches: [apipatch] + staging: + sha: webmerge + version: web-2.1.0-rc.5.hotfix.1 + ref: env/web/staging + base_sha: webbase + patches: [webpatch] +` + require.NoError(t, os.WriteFile(configPath, []byte(initialConfig), 0644)) + return configPath +} + +// TestRejoin_Component_DeletesComponentNamespacedBranch proves the rejoin cleanup +// recovers the component from the recorded ref (env/api/test) and deletes the +// integration branch in that component's namespace, not the env-only env/test. +func TestRejoin_Component_DeletesComponentNamespacedBranch(t *testing.T) { + configPath := divergedComponentManifest(t) + cleaner := &recordingCleaner{} + + fin, err := NewFinalizer(configPath, "test", WithLifecycleCleaner(cleaner)) + require.NoError(t, err) + + // Trunk promotion into the diverged api "test" env (containment gate already + // passed in preflight): the env rejoins trunk and its component-namespaced + // branch must be cleaned up. + fin.SetPromotionResult(&PromotionResult{ + Promotions: []EnvPromotion{{ + Environment: "test", + SourceEnv: "dev", + SHA: "trunkhead", + Version: "api-1.4.0-rc.3", + }}, + }) + + require.NoError(t, fin.Run()) + + require.Equal(t, []string{"env/api/test"}, cleaner.deletedBranches, + "the rejoining component's branch must be deleted in its own namespace, not env/test") +} + +// TestRejoin_Component_NeverCrossDeletesSibling proves that rejoining one +// component's env never deletes a sibling component's integration branch. Only +// "test" (api) is promoted; "staging" (web) stays diverged and untouched. +func TestRejoin_Component_NeverCrossDeletesSibling(t *testing.T) { + configPath := divergedComponentManifest(t) + cleaner := &recordingCleaner{} + + fin, err := NewFinalizer(configPath, "test", WithLifecycleCleaner(cleaner)) + require.NoError(t, err) + + fin.SetPromotionResult(&PromotionResult{ + Promotions: []EnvPromotion{{ + Environment: "test", + SourceEnv: "dev", + SHA: "trunkhead", + Version: "api-1.4.0-rc.3", + }}, + }) + + require.NoError(t, fin.Run()) + + require.Equal(t, []string{"env/api/test"}, cleaner.deletedBranches, + "only the rejoining component's branch is deleted") + require.NotContains(t, cleaner.deletedBranches, "env/web/staging", + "a sibling component's branch must never be cross-deleted") +} + +// TestRejoin_Component_TagCollectionScopedToComponent proves the spec threaded +// into the hotfix-release cleanup is the rejoining component's strict grammar, so +// tag collection sees only that component's hotfix tags and never a sibling's. +func TestRejoin_Component_TagCollectionScopedToComponent(t *testing.T) { + configPath := divergedComponentManifest(t) + cleaner := &recordingCleaner{} + + fin, err := NewFinalizer(configPath, "test", WithLifecycleCleaner(cleaner)) + require.NoError(t, err) + + fin.SetPromotionResult(&PromotionResult{ + Promotions: []EnvPromotion{{ + Environment: "test", + SourceEnv: "dev", + SHA: "trunkhead", + Version: "api-1.4.0-rc.3", + }}, + }) + + require.NoError(t, fin.Run()) + + require.Len(t, cleaner.cleanedReleases, 1) + req := cleaner.cleanedReleases[0] + require.Equal(t, "test", req.Environment) + require.Equal(t, "api-1.4.0-rc.2.hotfix.1", req.BaseVersion, + "the base cleaned is the version the component env held while diverged") + + // Apply the threaded spec to a mixed tag list: only the api component's own + // hotfix tags for that rc base may be collected. A sibling web- tag on the + // same numeric base, and a default-grammar v-prefixed tag, must be excluded by + // the strict api- prefix. + mixed := []string{ + "api-1.4.0-rc.2.hotfix.1", + "api-1.4.0-rc.2.hotfix.2", + "web-1.4.0-rc.2.hotfix.1", + "v1.4.0-rc.2.hotfix.1", + "api-1.4.0-rc.3.hotfix.1", // different rc base + } + collected := hotfix.HotfixTagsForBase(req.Spec, req.BaseVersion, mixed) + require.Equal(t, []string{"api-1.4.0-rc.2.hotfix.1", "api-1.4.0-rc.2.hotfix.2"}, collected, + "tag collection must be scoped to the api component's own namespace and rc base") +} + +// TestRejoin_SingleComponent_ByteIdentical proves a single-component rejoin +// (ref env/, empty recovered component) deletes env/ and collects tags +// under the manifest's permissive default grammar exactly as before components +// existed. +func TestRejoin_SingleComponent_ByteIdentical(t *testing.T) { + configPath := divergedManifest(t) // ref env/test, version v1.4.0-rc.2.hotfix.1 + cleaner := &recordingCleaner{} + + fin, err := NewFinalizer(configPath, "test", WithLifecycleCleaner(cleaner)) + require.NoError(t, err) + + fin.SetPromotionResult(&PromotionResult{ + Promotions: []EnvPromotion{{ + Environment: "test", + SourceEnv: "dev", + SHA: "trunkhead", + Version: "v1.4.0-rc.3", + }}, + }) + + require.NoError(t, fin.Run()) + + require.Equal(t, []string{"env/test"}, cleaner.deletedBranches, + "a single-component rejoin deletes env/ exactly as before") + + require.Len(t, cleaner.cleanedReleases, 1) + req := cleaner.cleanedReleases[0] + require.Equal(t, taggrammar.Default(), req.Spec, + "single-component cleanup uses the permissive default grammar, unchanged") + + // The default grammar collects the v-prefixed hotfix tags for the rc base. + mixed := []string{"v1.4.0-rc.2.hotfix.1", "v1.4.0-rc.2.hotfix.2", "v1.4.0-rc.3.hotfix.1"} + collected := hotfix.HotfixTagsForBase(req.Spec, req.BaseVersion, mixed) + require.Equal(t, []string{"v1.4.0-rc.2.hotfix.1", "v1.4.0-rc.2.hotfix.2"}, collected) +} diff --git a/internal/promote/rejoin_test.go b/internal/promote/rejoin_test.go index a2c2efcc..53d78b34 100644 --- a/internal/promote/rejoin_test.go +++ b/internal/promote/rejoin_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stablekernel/cascade/internal/config" + "github.com/stablekernel/cascade/internal/hotfix" "github.com/stretchr/testify/require" ) @@ -17,8 +18,13 @@ type recordingCleaner struct { cleanedReleases []CleanReleasesRequest } -func (c *recordingCleaner) DeleteEnvBranch(env string) error { - c.deletedBranches = append(c.deletedBranches, env) +// DeleteEnvBranch records the fully composed integration branch name +// (hotfix.EnvBranchName(component, env)) rather than the bare env, so tests can +// assert the component was threaded through and the branch targeted its own +// namespace: env/ for the default component, env// for a +// named one. +func (c *recordingCleaner) DeleteEnvBranch(component, env string) error { + c.deletedBranches = append(c.deletedBranches, hotfix.EnvBranchName(component, env)) return nil } @@ -108,7 +114,7 @@ func TestRejoin_DeletesEnvBranch(t *testing.T) { require.NoError(t, fin.Run()) - require.Equal(t, []string{"test"}, cleaner.deletedBranches, + require.Equal(t, []string{"env/test"}, cleaner.deletedBranches, "the rejoined env's integration branch must be deleted exactly once") } @@ -167,7 +173,7 @@ func TestRejoin_PreservesOtherEnvsDivergence(t *testing.T) { require.Equal(t, []string{"patchY"}, uat.Patches) // Cleanup must only have touched the rejoined env. - require.Equal(t, []string{"test"}, cleaner.deletedBranches) + require.Equal(t, []string{"env/test"}, cleaner.deletedBranches) require.Len(t, cleaner.cleanedReleases, 1) require.Equal(t, "test", cleaner.cleanedReleases[0].Environment) } diff --git a/internal/promote/rollback_rejoin_test.go b/internal/promote/rollback_rejoin_test.go index 62f06138..8e0eb2bc 100644 --- a/internal/promote/rollback_rejoin_test.go +++ b/internal/promote/rollback_rejoin_test.go @@ -88,7 +88,7 @@ func TestFinalize_HotfixRejoin_DeletesBranch(t *testing.T) { // A hotfix-origin rejoin still deletes the integration branch and cleans // its hotfix releases exactly as before. - require.Equal(t, []string{"test"}, cleaner.deletedBranches, + require.Equal(t, []string{"env/test"}, cleaner.deletedBranches, "hotfix rejoin must delete the integration branch exactly once") require.Len(t, cleaner.cleanedReleases, 1, "hotfix rejoin must clean its hotfix releases")