diff --git a/e2e/scenarios/hotfix/hotfix-generation-threshold.yaml b/e2e/scenarios/hotfix/hotfix-generation-threshold.yaml index 918e9f47..8f516805 100644 --- a/e2e/scenarios/hotfix/hotfix-generation-threshold.yaml +++ b/e2e/scenarios/hotfix/hotfix-generation-threshold.yaml @@ -39,7 +39,7 @@ steps: - "target_env:" - "pull_request:" - "types: [closed]" - - "'env/**'" + - "'env/*'" - "format('hotfix-finalize-{0}', github.repository)" - "format('hotfix-{0}', github.event.inputs.target_env)" - " plan:" diff --git a/internal/generate/hotfix.go b/internal/generate/hotfix.go index a5264554..04367a15 100644 --- a/internal/generate/hotfix.go +++ b/internal/generate/hotfix.go @@ -112,6 +112,25 @@ func (g *HotfixGenerator) envBranchRef() string { return g.envBranchPrefix() + "${env}" } +// finalizeTriggerBranch returns the pull_request(closed) branch filter that gates +// the finalize chain, scoped to the component this workflow serves so a +// resolution PR into one component's env branch never fires a sibling's finalize. +// +// Single-component env branches are single-segment (env/staging), which env/* +// matches exactly; this is byte-identical to the historical single-component +// trigger. A named component's env branches are two-segment (env//, +// for example env/api/staging), which a GitHub Actions `*` glob will not match +// across the slash, so the component trigger is env//** - matching that +// component's env branches at any depth and no other component's. An unscoped +// env/** would match every component's env branches and fire spurious +// cross-component finalize runs, breaking per-component isolation. +func (g *HotfixGenerator) finalizeTriggerBranch() string { + if g.componentName != "" { + return "env/" + g.componentName + "/**" + } + return "env/*" +} + // hotfixBranchPrefix returns the throwaway cherry-pick branch name prefix the // apply lane pushes to: single-component yields "hotfix/" (byte-identical to the // historical form), a component yields "hotfix//" so two components @@ -238,12 +257,7 @@ func (g *HotfixGenerator) writeTriggers(sb *strings.Builder) { sb.WriteString(" pull_request:\n") sb.WriteString(" types: [closed]\n") sb.WriteString(" branches:\n") - // Double-star matches env branches at any depth. A GitHub Actions branch - // glob `*` stops at a slash, so a single-star `env/*` never matches a - // per-component branch like env/api/staging and the closed-PR finalize - // would never fire for a multi-component pipeline. `env/**` matches both - // the single-component branch (env/staging) and the per-component branch. - sb.WriteString(" - 'env/**'\n") + fmt.Fprintf(sb, " - '%s'\n", g.finalizeTriggerBranch()) sb.WriteString("\n") } diff --git a/internal/generate/hotfix_test.go b/internal/generate/hotfix_test.go index e28e3595..77daf33a 100644 --- a/internal/generate/hotfix_test.go +++ b/internal/generate/hotfix_test.go @@ -75,7 +75,10 @@ func TestHotfixGenerator_Triggers(t *testing.T) { assert.Contains(t, content, "pull_request:") assert.Contains(t, content, "types: [closed]") assert.Contains(t, content, "branches:") - assert.Contains(t, content, "'env/**'") + // Single-component env branches are single-segment (env/staging); the + // finalize trigger scopes to env/* and must not carry the broader env/**. + assert.Contains(t, content, "'env/*'") + assert.NotContains(t, content, "'env/**'") // Dispatch inputs. assert.Contains(t, content, "commit:") @@ -89,23 +92,40 @@ func TestHotfixGenerator_Triggers(t *testing.T) { assert.NotContains(t, content, "- dev") } -// TestHotfixGenerator_FinalizeTriggerMatchesNestedEnvBranches guards that the -// finalize pull_request trigger's branch filter matches multi-component env -// branches. Per-component env branches carry two path segments -// (env/api/staging), and a GitHub Actions `*` glob stops at a slash, so a -// single-star `env/*` filter never matches them and the closed-PR finalize -// never fires. A double-star `env/**` matches any depth, covering both the -// single-component branch (env/staging) and the per-component branch -// (env/api/staging). -func TestHotfixGenerator_FinalizeTriggerMatchesNestedEnvBranches(t *testing.T) { - gen := NewHotfixGenerator(threeEnvHotfixConfig(), "") - content, err := gen.Generate() - require.NoError(t, err) - - assert.Contains(t, content, " - 'env/**'", - "finalize trigger must use env/** so it matches per-component env branches like env/api/staging") - assert.NotContains(t, content, " - 'env/*'\n", - "finalize trigger must not use a single-star env/* filter, which a GitHub Actions glob will not match across the slash of env/api/staging") +// TestHotfixGenerator_FinalizeTriggerScopedToComponent guards that the finalize +// pull_request trigger's branch filter is scoped to the component the workflow +// serves, restoring per-component isolation. A single-component workflow's env +// branches are single-segment (env/staging), so its finalize trigger is env/*. +// A per-component workflow's env branches are two-segment (env/api/staging), so +// its finalize trigger is env//** - matching only its own component's +// env branches at any depth and never a sibling's. An unscoped env/** would fire +// every component's finalize on any component's resolution PR, spawning spurious +// cross-component runs. +func TestHotfixGenerator_FinalizeTriggerScopedToComponent(t *testing.T) { + single, err := NewHotfixGenerator(threeEnvHotfixConfig(), "").Generate() + require.NoError(t, err) + assert.Contains(t, single, " - 'env/*'", + "single-component finalize trigger must scope to env/*, matching env/staging") + assert.NotContains(t, single, "env/**", + "single-component finalize trigger must not use the broad env/** filter") + + api, err := NewHotfixGenerator(threeEnvHotfixConfig(), "", WithHotfixComponentName("api")).Generate() + require.NoError(t, err) + assert.Contains(t, api, " - 'env/api/**'", + "the api component finalize trigger must scope to env/api/**, matching env/api/staging") + assert.NotContains(t, api, " - 'env/**'", + "the api component finalize trigger must not use the unscoped env/** filter that matches sibling components") + assert.NotContains(t, api, "env/web/", + "the api component finalize trigger must not reference a sibling component's env namespace") + + web, err := NewHotfixGenerator(threeEnvHotfixConfig(), "", WithHotfixComponentName("web")).Generate() + require.NoError(t, err) + assert.Contains(t, web, " - 'env/web/**'", + "the web component finalize trigger must scope to env/web/**, matching env/web/staging") + assert.NotContains(t, web, " - 'env/**'", + "the web component finalize trigger must not use the unscoped env/** filter that matches sibling components") + assert.NotContains(t, web, "'env/api/", + "the web component finalize trigger must not reference a sibling component's env namespace") } // TestHotfixGenerator_CommitInputAcceptsMultiple guards that the dispatch diff --git a/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden b/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden index ae7d6e15..4b80dbf3 100644 --- a/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden +++ b/internal/generate/testdata/byte_identical_baseline/.github__workflows__cascade-hotfix.yaml.golden @@ -40,7 +40,7 @@ on: pull_request: types: [closed] branches: - - 'env/**' + - 'env/*' permissions: contents: read