diff --git a/e2e/scenarios/52-component-orchestrate-isolation.yaml b/e2e/scenarios/52-component-orchestrate-isolation.yaml new file mode 100644 index 00000000..4c9ac17b --- /dev/null +++ b/e2e/scenarios/52-component-orchestrate-isolation.yaml @@ -0,0 +1,93 @@ +name: "Per-Component Orchestrate Seed Isolation" +description: | + Proves two components seed the same first environment independently, each + recording its version under only its own state.components.. subtree, + so neither overwrites the other's seed row. The manifest declares two components + (api, web), each owning a path subtree with its own strict tag namespace, sharing + the dev to prod ladder on its own version line. Generation fans the orchestrate + lane out to one orchestrate-.yaml per component. + + api and web are seeded, then each is orchestrated to dev in turn with NO + interleaving promotion between them. Before, an orchestrate recorded its built + version into the shared flat state.dev row, so web's orchestrate would overwrite + api's seed and the two could not both be observed at dev at once; a scenario had + to promote api out of dev before touching web to avoid the clobber. Here both + orchestrate to dev back to back, and the proof is that after web's orchestrate, + api's dev seed still reads its own api-0.1.0-rc.0 line under state.components.api, + coexisting with web's web-0.1.0-rc.0 under state.components.web: the seed write is + scoped per component, not shared and flat. + +config: + trunk_branch: main + environments: [dev, prod] + builds: + - name: app + workflow: build.yaml + triggers: ["services/**"] + deploys: + - name: app + workflow: deploy.yaml + triggers: ["services/**"] + components: + api: + path: services/api + tag_prefix: api- + web: + path: services/web + tag_prefix: web- + +steps: + - name: "Seed both component subtrees" + action: commit + commit: + message: "feat: seed component sources" + files: + services/api/main.go: | + package main + + func main() {} + services/web/main.go: | + package main + + func main() {} + + # Confirm the multi-component generate then verify roundtrip is drift-free, so + # the per-component workflows executed below are the pristine generated output. + - name: "Regenerate the per-component set and confirm no drift" + action: verify + verify: + regenerate: true + expect_exit: 0 + + # Cut api's dev prerelease on its own version line. The seed lands under + # state.components.api.dev, not the shared flat state.dev. + - name: "Orchestrate api to seed its dev prerelease" + action: orchestrate + orchestrate: + component: api + expect: + state: + api-dev: + component: api + env: dev + version: "api-0.1.0-rc.0" + + # Cut web's dev prerelease WITHOUT first promoting api out of dev. web writes + # only state.components.web.dev; api's seed row must survive untouched. Before the + # per-component seed write, web's orchestrate would have overwritten the shared + # flat state.dev and api's seed would be gone. + - name: "Orchestrate web to seed its dev prerelease" + action: orchestrate + orchestrate: + component: web + expect: + state: + web-dev: + component: web + env: dev + version: "web-0.1.0-rc.0" + api-dev: + component: api + env: dev + version: "api-0.1.0-rc.0" + unchanged: true diff --git a/internal/config/readcomponentstate_test.go b/internal/config/readcomponentstate_test.go new file mode 100644 index 00000000..e5578ebd --- /dev/null +++ b/internal/config/readcomponentstate_test.go @@ -0,0 +1,81 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +const twoComponentStateManifest = `ci: + config: + environments: [dev, prod] + state: + components: + api: + dev: + sha: apidevsha + version: api-0.1.0-rc.0 + prod: + sha: apiprodsha + version: api-0.1.0 + web: + dev: + sha: webdevsha + version: web-0.1.0-rc.0 +` + +// TestReadComponentState_ReadsOnlyNamedComponent proves the reader returns every +// env row a single component owns and ignores its siblings, so a flat-map +// consumer can overlay one component's seed without pulling in another's. +func TestReadComponentState_ReadsOnlyNamedComponent(t *testing.T) { + got, err := ReadComponentState([]byte(twoComponentStateManifest), "ci", "api") + require.NoError(t, err) + require.Len(t, got, 2, "api owns dev and prod rows") + require.Equal(t, "apidevsha", got["dev"].SHA) + require.Equal(t, "api-0.1.0-rc.0", got["dev"].Version) + require.Equal(t, "apiprodsha", got["prod"].SHA) + require.Equal(t, "api-0.1.0", got["prod"].Version) + _, hasWebLeak := got["web"] + require.False(t, hasWebLeak, "reader must not leak a sibling component's rows") +} + +// TestReadComponentState_MissingComponentOrSubtree proves absent inputs yield a +// nil map and no error, so an overlay is a clean no-op when a component has no +// recorded state yet or the manifest declares no components at all. +func TestReadComponentState_MissingComponentOrSubtree(t *testing.T) { + got, err := ReadComponentState([]byte(twoComponentStateManifest), "ci", "billing") + require.NoError(t, err) + require.Nil(t, got, "an undeclared component yields no rows") + + const flat = `ci: + config: + environments: [dev] + state: + dev: + sha: devsha +` + got, err = ReadComponentState([]byte(flat), "ci", "api") + require.NoError(t, err) + require.Nil(t, got, "a manifest with no components subtree yields no rows") +} + +// TestReadComponentState_RoundTripsWriteScopedState proves the reader observes +// exactly what the component-scoped writer records: writing a component's env row +// then reading it back yields the same SHA and version, so overlay-then-write is +// a faithful round trip. +func TestReadComponentState_RoundTripsWriteScopedState(t *testing.T) { + const base = `ci: + config: + environments: [dev, prod] +` + out, err := WriteScopedState([]byte(base), "ci", + StateWrite{Component: "api", Env: "dev", State: &EnvState{SHA: "s1", Version: "api-0.1.0-rc.0"}}, + ) + require.NoError(t, err) + + got, err := ReadComponentState(out, "ci", "api") + require.NoError(t, err) + require.Len(t, got, 1) + require.Equal(t, "s1", got["dev"].SHA) + require.Equal(t, "api-0.1.0-rc.0", got["dev"].Version) +} diff --git a/internal/config/statemerge.go b/internal/config/statemerge.go index d763195b..81c51e97 100644 --- a/internal/config/statemerge.go +++ b/internal/config/statemerge.go @@ -382,6 +382,37 @@ func deleteMappingKey(m *yaml.Node, key string) { } } +// ReadComponentState reads the recorded per-env state a single component owns +// under state.components.., returning an env-keyed map of the +// same EnvState shape the flat state. rows use. It is the read counterpart +// to the component-scoped WriteScopedState: a consumer that works against the +// flat state map (promotion, orchestration) can overlay a component's seed into +// its working map so every existing state. lookup transparently sees that +// component's rows, without teaching each lookup about the components subtree. +// +// A manifest with no components subtree, or no rows for the named component, +// yields a nil map and no error. manifestKey defaults to DefaultManifestKey when +// empty. YAML parse failures are wrapped with %w. Sibling components are ignored; +// only the named component's rows are returned. +func ReadComponentState(current []byte, manifestKey, component string) (map[string]*EnvState, error) { + if manifestKey == "" { + manifestKey = DefaultManifestKey + } + var doc map[string]struct { + State struct { + Components map[string]map[string]*EnvState `yaml:"components"` + } `yaml:"state"` + } + if err := yaml.Unmarshal(current, &doc); err != nil { + return nil, fmt.Errorf("parsing component state: %w", err) + } + section, ok := doc[manifestKey] + if !ok { + return nil, nil + } + return section.State.Components[component], nil +} + // valueNode marshals v through YAML and returns the resulting value node, so a // typed value can be spliced into the document tree. func valueNode(v any) (*yaml.Node, error) { diff --git a/internal/generate/component_workflows_test.go b/internal/generate/component_workflows_test.go index 3159f9f7..d67909ac 100644 --- a/internal/generate/component_workflows_test.go +++ b/internal/generate/component_workflows_test.go @@ -129,6 +129,71 @@ func TestOrchestrateTargets_Components_FanOut(t *testing.T) { } } +// updateManifestStep extracts the Update Manifest step body from a generated +// orchestrate workflow so state-write assertions stay focused on the yq edits. +func updateManifestStep(t *testing.T, workflow string) string { + t.Helper() + idx := strings.Index(workflow, "- name: Update Manifest") + require.GreaterOrEqual(t, idx, 0, "Update Manifest step missing") + return workflow[idx:] +} + +// TestOrchestrate_Components_ScopeSeedStateWrite proves each per-component +// orchestrate workflow records its seeded env under +// state.components.. in the Update Manifest step, and never writes the +// shared flat state. row. This is the seed-state isolation: two components +// orchestrating the same env write disjoint subtrees, so neither overwrites the +// other's row (the coupling that previously forced scenarios to interleave). +func TestOrchestrate_Components_ScopeSeedStateWrite(t *testing.T) { + cfg := twoComponentConfig() + + targets, err := orchestrateTargets(cfg, "", ".github/workflows/orchestrate.yaml", nil, false) + require.NoError(t, err) + + byPath := map[string]string{} + for _, tg := range targets { + content, gerr := tg.Gen.Generate() + require.NoError(t, gerr) + byPath[tg.Path] = content + } + + api := updateManifestStep(t, byPath[".github/workflows/orchestrate-api.yaml"]) + web := updateManifestStep(t, byPath[".github/workflows/orchestrate-web.yaml"]) + + // api seeds only its own subtree. + require.Contains(t, api, "state.components.api.$ENVIRONMENT.sha", + "api orchestrate must record its seed under state.components.api") + require.Contains(t, api, "state.components.api.$ENVIRONMENT.version") + require.NotContains(t, api, "state.components.web", + "api orchestrate must not touch web's subtree (isolation)") + + // The flat env row must not be written by a component orchestrate: a bare + // state.$ENVIRONMENT would let two components clobber the same row. + require.NotContains(t, api, "state.$ENVIRONMENT.sha", + "component orchestrate must not write the shared flat state. row") + + // web seeds only its own subtree. + require.Contains(t, web, "state.components.web.$ENVIRONMENT.sha", + "web orchestrate must record its seed under state.components.web") + require.NotContains(t, web, "state.components.api", + "web orchestrate must not touch api's subtree (isolation)") +} + +// TestOrchestrate_SingleComponent_SeedStateWriteStaysFlat proves a manifest with +// no components: block keeps writing the flat state. row, byte-identical to +// the historical behavior (no state.components. path leaks in). +func TestOrchestrate_SingleComponent_SeedStateWriteStaysFlat(t *testing.T) { + cfg := &config.TrunkConfig{TrunkBranch: "main", Environments: []string{"dev", "prod"}} + workflow, err := NewGenerator(cfg, "").Generate() + require.NoError(t, err) + step := updateManifestStep(t, workflow) + + require.Contains(t, step, "state.$ENVIRONMENT.sha", + "single-component orchestrate must keep the flat state. write") + require.NotContains(t, step, "state.components.", + "single-component orchestrate must not emit any component-scoped state path") +} + // TestGenerator_SingleComponent_NoComponentFlag proves the single-component // orchestrate workflow emits no --component flag, keeping its setup invocation // byte-identical to the pre-component generator. diff --git a/internal/generate/generator.go b/internal/generate/generator.go index 2f9631a3..df27778c 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -1684,19 +1684,32 @@ func (g *Generator) writeManifestUpdateStep(sb *strings.Builder, sorted []string sb.WriteString(" apply_state_edits() {\n") sb.WriteString(" TIMESTAMP=$(date -u +%Y-%m-%dT%H:%M:%SZ)\n") + // A per-component orchestrate scopes its seed under state.components.. + // so two components seeding the same environment write disjoint subtrees and + // never overwrite each other's row. The single-component workflow keeps the + // flat state. path (empty segment), byte-identical to the historical + // output. The name is a generation-time literal validated by the component + // grammar, matching how deploy/build names are already interpolated into these + // yq paths. + stateComp := "" + if g.componentName != "" { + stateComp = "components." + g.componentName + "." + } + if len(g.config.Environments) > 0 { + envSel := ".$MANIFEST_KEY.state." + stateComp + "$ENVIRONMENT" sb.WriteString(" # Update environment-level state (committed, not deployed)\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.version = \\\"$VERSION\\\"\" \"$MANIFEST_FILE\"\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.committed_at = \\\"$TIMESTAMP\\\"\" \"$MANIFEST_FILE\"\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.committed_by = \\\"${{ github.actor }}\\\"\" \"$MANIFEST_FILE\"\n") + fmt.Fprintf(sb, " yq eval -i \"%s.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n", envSel) + fmt.Fprintf(sb, " yq eval -i \"%s.version = \\\"$VERSION\\\"\" \"$MANIFEST_FILE\"\n", envSel) + fmt.Fprintf(sb, " yq eval -i \"%s.committed_at = \\\"$TIMESTAMP\\\"\" \"$MANIFEST_FILE\"\n", envSel) + fmt.Fprintf(sb, " yq eval -i \"%s.committed_by = \\\"${{ github.actor }}\\\"\" \"$MANIFEST_FILE\"\n", envSel) for _, d := range g.config.Deploys { envName := strings.ToUpper(strings.ReplaceAll(d.Name, "-", "_")) fmt.Fprintf(sb, " if [[ \"$%s_RESULT\" == \"success\" ]]; then\n", envName) - fmt.Fprintf(sb, " yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.deploys.%s.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n", d.Name) - fmt.Fprintf(sb, " yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.deploys.%s.deployed_at = \\\"$TIMESTAMP\\\"\" \"$MANIFEST_FILE\"\n", d.Name) - fmt.Fprintf(sb, " yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.deploys.%s.deployed_by = \\\"${{ github.actor }}\\\"\" \"$MANIFEST_FILE\"\n", d.Name) + fmt.Fprintf(sb, " yq eval -i \"%s.deploys.%s.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n", envSel, d.Name) + fmt.Fprintf(sb, " yq eval -i \"%s.deploys.%s.deployed_at = \\\"$TIMESTAMP\\\"\" \"$MANIFEST_FILE\"\n", envSel, d.Name) + fmt.Fprintf(sb, " yq eval -i \"%s.deploys.%s.deployed_by = \\\"${{ github.actor }}\\\"\" \"$MANIFEST_FILE\"\n", envSel, d.Name) sb.WriteString(" fi\n") } @@ -1706,8 +1719,8 @@ func (g *Generator) writeManifestUpdateStep(sb *strings.Builder, sorted []string if out == "artifact_id" { envName := strings.ToUpper(strings.ReplaceAll(b.Name, "-", "_")) fmt.Fprintf(sb, " if [[ -n \"$BUILD_ARTIFACT_%s\" ]]; then\n", envName) - fmt.Fprintf(sb, " yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.builds.%s.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n", b.Name) - fmt.Fprintf(sb, " yq eval -i \".$MANIFEST_KEY.state.$ENVIRONMENT.builds.%s.artifact_id = \\\"$BUILD_ARTIFACT_%s\\\"\" \"$MANIFEST_FILE\"\n", b.Name, envName) + fmt.Fprintf(sb, " yq eval -i \"%s.builds.%s.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n", envSel, b.Name) + fmt.Fprintf(sb, " yq eval -i \"%s.builds.%s.artifact_id = \\\"$BUILD_ARTIFACT_%s\\\"\" \"$MANIFEST_FILE\"\n", envSel, b.Name, envName) sb.WriteString(" fi\n") break } @@ -1715,11 +1728,12 @@ func (g *Generator) writeManifestUpdateStep(sb *strings.Builder, sorted []string } } else { // No environments - update state under 'prerelease' key (consistent with orchestrator.DefaultStateKey) + preSel := ".$MANIFEST_KEY.state." + stateComp + "prerelease" sb.WriteString(" # Update state under prerelease key (no environments)\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.prerelease.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.prerelease.version = \\\"$VERSION\\\"\" \"$MANIFEST_FILE\"\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.prerelease.committed_at = \\\"$TIMESTAMP\\\"\" \"$MANIFEST_FILE\"\n") - sb.WriteString(" yq eval -i \".$MANIFEST_KEY.state.prerelease.committed_by = \\\"${{ github.actor }}\\\"\" \"$MANIFEST_FILE\"\n") + fmt.Fprintf(sb, " yq eval -i \"%s.sha = \\\"$HEAD_SHA\\\"\" \"$MANIFEST_FILE\"\n", preSel) + fmt.Fprintf(sb, " yq eval -i \"%s.version = \\\"$VERSION\\\"\" \"$MANIFEST_FILE\"\n", preSel) + fmt.Fprintf(sb, " yq eval -i \"%s.committed_at = \\\"$TIMESTAMP\\\"\" \"$MANIFEST_FILE\"\n", preSel) + fmt.Fprintf(sb, " yq eval -i \"%s.committed_by = \\\"${{ github.actor }}\\\"\" \"$MANIFEST_FILE\"\n", preSel) } sb.WriteString(" }\n") diff --git a/internal/orchestrate/command.go b/internal/orchestrate/command.go index 680c96b8..e7c2f0e5 100644 --- a/internal/orchestrate/command.go +++ b/internal/orchestrate/command.go @@ -197,8 +197,12 @@ func runFinalize(version, deployResults, buildResults string) error { log.Info("%sRunning in dry-run mode", log.DryRunPrefix()) } - // Create orchestrator - orch, err := NewOrchestrator(configPath, manifestKey, environment) + // Create orchestrator. WithComponent("") is a no-op, so the single-component + // path is unchanged; a per-component generated workflow passes --component so + // the seeded state is recorded under state.components.. rather + // than the shared flat state., keeping two components from overwriting + // each other's seed row. + orch, err := NewOrchestrator(configPath, manifestKey, environment, WithComponent(component)) if err != nil { return fmt.Errorf("initializing orchestrator: %w", err) } diff --git a/internal/orchestrate/component_seed_test.go b/internal/orchestrate/component_seed_test.go new file mode 100644 index 00000000..cab1c669 --- /dev/null +++ b/internal/orchestrate/component_seed_test.go @@ -0,0 +1,210 @@ +package orchestrate + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stablekernel/cascade/internal/config" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// componentSeedManifest is a two-component manifest whose recorded state lives +// entirely under state.components.., the component-scoped form. It +// carries a sibling component ("web") the writes below never address, so every +// test asserts it survives verbatim. +const componentSeedManifest = `ci: + config: + environments: [dev, prod] + components: + api: + path: services/api + tag_prefix: api- + web: + path: services/web + tag_prefix: web- + state: + components: + web: + dev: + sha: websha + version: web-0.1.0 + committed_by: someone +` + +// readManifestTree parses raw manifest bytes into a generic tree so tests can +// assert on the exact serialized shape, including keys the typed model ignores. +func readManifestTree(t *testing.T, data []byte) map[string]any { + t.Helper() + var m map[string]any + require.NoError(t, yaml.Unmarshal(data, &m)) + return m +} + +// componentEnvRow digs out ci.state.components.. from a parsed +// manifest, failing the test when any level is missing. +func componentEnvRow(t *testing.T, m map[string]any, comp, env string) map[string]any { + t.Helper() + ci, ok := m["ci"].(map[string]any) + require.True(t, ok, "ci block present") + state, ok := ci["state"].(map[string]any) + require.True(t, ok, "ci.state present") + comps, ok := state["components"].(map[string]any) + require.True(t, ok, "ci.state.components present") + c, ok := comps[comp].(map[string]any) + require.True(t, ok, "component %s present", comp) + leaf, ok := c[env].(map[string]any) + require.True(t, ok, "component %s env %s present", comp, env) + return leaf +} + +// TestWriteConfig_Component_WritesScopedNodeAndKeepsSibling proves the orchestrate +// state-write path records the seeded env under state.components.. and +// leaves an unaddressed sibling component byte-intact. A regression to a flat +// WriteManifestState (whole-state-node rebuild) would drop state.components.web +// and write a flat state.dev instead. +func TestWriteConfig_Component_WritesScopedNodeAndKeepsSibling(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "manifest.yaml") + require.NoError(t, os.WriteFile(path, []byte(componentSeedManifest), 0o644)) + + o, err := NewOrchestrator(path, "ci", "dev", WithComponent("api")) + require.NoError(t, err) + + // Model the in-memory state Finalize would carry for its component's env. + o.cicdFile.State["dev"] = &config.EnvState{ + SHA: "apisha", + Version: "api-0.1.0", + CommittedBy: "orchestrator", + } + require.NoError(t, o.writeConfig()) + + out, err := os.ReadFile(path) + require.NoError(t, err) + m := readManifestTree(t, out) + + // api's dev seed landed under its own subtree. + got := componentEnvRow(t, m, "api", "dev") + require.Equal(t, "apisha", got["sha"]) + require.Equal(t, "api-0.1.0", got["version"]) + require.Equal(t, "orchestrator", got["committed_by"]) + + // The sibling web component is preserved verbatim. + sib := componentEnvRow(t, m, "web", "dev") + require.Equal(t, "websha", sib["sha"]) + require.Equal(t, "web-0.1.0", sib["version"]) + require.Equal(t, "someone", sib["committed_by"]) + + // No flat state. leaked alongside the component form. + ci := m["ci"].(map[string]any) + state := ci["state"].(map[string]any) + _, hasFlatDev := state["dev"] + require.False(t, hasFlatDev, "component orchestrate must not write a flat state.dev node") +} + +// TestWriteConfig_SingleComponent_ByteIdentical proves an empty component takes +// the exact original WriteManifestState path, so a single-component manifest +// round-trips byte-for-byte identical to the historical single-component behavior. +func TestWriteConfig_SingleComponent_ByteIdentical(t *testing.T) { + const flat = `ci: + config: + environments: [dev, prod] + state: + dev: + sha: devsha + version: v1.0.0 + prod: {} +` + dir := t.TempDir() + + // Path A: the orchestrator writeConfig with no component set. + pathA := filepath.Join(dir, "a.yaml") + require.NoError(t, os.WriteFile(pathA, []byte(flat), 0o644)) + oA, err := NewOrchestrator(pathA, "ci", "dev") + require.NoError(t, err) + oA.cicdFile.State["dev"].SHA = "newsha" + oA.cicdFile.State["dev"].Version = "v1.1.0" + require.NoError(t, oA.writeConfig()) + gotA, err := os.ReadFile(pathA) + require.NoError(t, err) + + // Path B: the direct WriteManifestState reference, mirroring the exact + // in-memory mutation, serialized the historical way. Both must be identical. + pathB := filepath.Join(dir, "b.yaml") + require.NoError(t, os.WriteFile(pathB, []byte(flat), 0o644)) + oB, err := NewOrchestrator(pathB, "ci", "dev") + require.NoError(t, err) + oB.cicdFile.State["dev"].SHA = "newsha" + oB.cicdFile.State["dev"].Version = "v1.1.0" + ref, err := config.WriteManifestState([]byte(flat), config.DefaultManifestKey, oB.cicdFile.State, oB.cicdFile.LatestRelease) + require.NoError(t, err) + + require.Equal(t, string(ref), string(gotA), "single-component orchestrate must be byte-identical to WriteManifestState") +} + +// TestWriteConfig_Component_ReapplyPreservesUnmodeledSibling models a concurrent +// sibling orchestrate landing between this component's read and its write (the +// git rebase-retry re-applies the state edit against the winner's committed +// bytes). writeConfig re-derives only this component's leaf from its in-memory +// state and re-reads the on-disk file, so a sibling subtree that gained an +// unmodeled key on the winner's commit survives the re-apply verbatim. A +// regression to a whole-state-node rebuild would delete the sibling on re-apply. +func TestWriteConfig_Component_ReapplyPreservesUnmodeledSibling(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "manifest.yaml") + require.NoError(t, os.WriteFile(path, []byte(componentSeedManifest), 0o644)) + + o, err := NewOrchestrator(path, "ci", "dev", WithComponent("api")) + require.NoError(t, err) + o.cicdFile.State["dev"] = &config.EnvState{SHA: "apisha", Version: "api-0.1.0", CommittedBy: "orchestrator"} + require.NoError(t, o.writeConfig()) + + // A concurrent web orchestrate wins the race: rewrite the on-disk file to + // carry its freshly committed subtree, including a key this binary does not + // model, exactly as a rebase would leave the tree before the re-apply. + winner := `ci: + config: + environments: [dev, prod] + components: + api: + path: services/api + tag_prefix: api- + web: + path: services/web + tag_prefix: web- + state: + components: + api: + dev: + sha: apisha + version: api-0.1.0 + committed_by: orchestrator + web: + dev: + sha: webnewsha + version: web-0.2.0 + committed_by: web-bot + unmodeled_key: keep-me +` + require.NoError(t, os.WriteFile(path, []byte(winner), 0o644)) + + // Re-apply this component's edit on top of the winner's tree. + require.NoError(t, o.writeConfig()) + + out, err := os.ReadFile(path) + require.NoError(t, err) + m := readManifestTree(t, out) + + // api's own leaf is intact. + api := componentEnvRow(t, m, "api", "dev") + require.Equal(t, "apisha", api["sha"]) + require.Equal(t, "api-0.1.0", api["version"]) + + // The winner's web subtree survived verbatim, including the unmodeled key. + web := componentEnvRow(t, m, "web", "dev") + require.Equal(t, "webnewsha", web["sha"]) + require.Equal(t, "web-0.2.0", web["version"]) + require.Equal(t, "web-bot", web["committed_by"]) + require.Equal(t, "keep-me", web["unmodeled_key"], "unmodeled sibling key must survive the re-apply") +} diff --git a/internal/orchestrate/orchestrator.go b/internal/orchestrate/orchestrator.go index b6e0aca0..98211d77 100644 --- a/internal/orchestrate/orchestrator.go +++ b/internal/orchestrate/orchestrator.go @@ -590,7 +590,7 @@ func (o *Orchestrator) writeConfig() error { return fmt.Errorf("failed to read config: %w", err) } - data, err := config.WriteManifestState(current, key, o.cicdFile.State, o.cicdFile.LatestRelease) + data, err := o.serializeState(current, key) if err != nil { return fmt.Errorf("failed to marshal config: %w", err) } @@ -603,6 +603,39 @@ func (o *Orchestrator) writeConfig() error { return nil } +// serializeState rewrites current with the orchestrator's owned state. In the +// single-component form (component == "") it reconciles the whole flat state node +// via WriteManifestState, byte-identical to the historical behavior. In the +// component-scoped form it node-patches only state.components.. +// through WriteScopedState, so a sibling component present in current survives +// verbatim, including keys this binary does not model. +func (o *Orchestrator) serializeState(current []byte, key string) ([]byte, error) { + if o.component != "" { + return config.WriteScopedState(current, key, o.componentStateWrites()...) + } + return config.WriteManifestState(current, key, o.cicdFile.State, o.cicdFile.LatestRelease) +} + +// componentStateWrites builds the component-scoped write the orchestrator owns +// from its in-memory state: one state directive addressing +// state.components.. for the orchestrated environment. It is +// re-appliable: the rebase-retry loop reruns writeConfig against the re-fetched +// trunk bytes on a rejected push, and each call deterministically re-derives the +// same owned leaf, so a concurrent sibling component's subtree is never rebuilt +// or dropped. A missing env state yields no write (a nil State on a StateWrite +// means delete), so an unexpected miss never becomes an accidental node delete. +func (o *Orchestrator) componentStateWrites() []config.StateWrite { + st := o.cicdFile.State[o.environment] + if st == nil { + return nil + } + return []config.StateWrite{{ + Component: o.component, + Env: o.environment, + State: st, + }} +} + // commitAndPush commits and pushes state changes. func (o *Orchestrator) commitAndPush(version string) error { // Check if there are changes to commit diff --git a/internal/promote/command_preflight.go b/internal/promote/command_preflight.go index a0fe59db..f09d1323 100644 --- a/internal/promote/command_preflight.go +++ b/internal/promote/command_preflight.go @@ -60,6 +60,14 @@ func runPreflight(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to parse config: %w", err) } + // A component-scoped preflight reads its source and target env state from + // state.components..; overlay those rows into the working flat + // state map so the source-env deployment check and version comparison resolve + // the component's seed. A no-op for a single-component (empty) preflight. + if err := overlayComponentState(cicdFile, configPath, componentName); err != nil { + return err + } + // Parse and validate mode // Mode can be "default" for sequential promotion, or a cascade target like "dev-to-prod" var mode PromotionMode diff --git a/internal/promote/component_source_read_test.go b/internal/promote/component_source_read_test.go new file mode 100644 index 00000000..4d3ea300 --- /dev/null +++ b/internal/promote/component_source_read_test.go @@ -0,0 +1,87 @@ +package promote + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// componentSourceManifest records a component's dev seed only under +// state.components.api.dev, exactly as a per-component orchestrate now writes it. +// No flat state.dev row exists, so a component promotion that read the flat map +// would see "no deployments". +const componentSourceManifest = `ci: + config: + environments: [dev, prod] + state: + components: + api: + dev: + sha: apidevsha + version: api-0.1.0-rc.0 + web: + dev: + sha: webdevsha + version: web-0.1.0-rc.0 +` + +// TestNewPromoter_Component_OverlaysComponentSourceState proves a component-scoped +// promotion reads its source-env seed from state.components..: the +// dev-to-prod cascade for api finds api's dev deployment (recorded only under the +// component subtree) and promotes it, rather than failing with "source +// environment 'dev' has no deployments". This is the read counterpart to the +// per-component orchestrate seed write; without the overlay the two paths are +// coupled through the flat state.dev row and a component promotion regresses. +func TestNewPromoter_Component_OverlaysComponentSourceState(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "manifest.yaml") + require.NoError(t, os.WriteFile(path, []byte(componentSourceManifest), 0o644)) + + p, err := NewPromoter(PromoterOptions{ + ConfigPath: path, + DryRun: true, + Actor: "test-actor", + Component: "api", + }) + require.NoError(t, err) + + result, err := p.Promote(ModeCascade, "dev-to-prod") + require.NoError(t, err) + require.True(t, result.Success, "component promotion must succeed reading its own dev seed; error: %s", result.Error) + require.NotEmpty(t, result.Promotions) + + // The promotion carries api's own dev seed SHA forward, proving the source + // read resolved the component subtree rather than a missing flat row. The + // promoted version is api's release line (the rc suffix is dropped on the + // promotion to prod, standard promotion semantics). + last := result.Promotions[len(result.Promotions)-1] + require.Equal(t, "prod", last.Environment) + require.Equal(t, "apidevsha", last.SHA) + require.Equal(t, "api-0.1.0", last.Version) +} + +// TestNewPromoter_Component_IgnoresSiblingSourceState proves the overlay is scoped +// to the promoting component: api's promotion never reads web's dev seed, so a +// sibling's version line cannot leak into api's promotion. +func TestNewPromoter_Component_IgnoresSiblingSourceState(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "manifest.yaml") + require.NoError(t, os.WriteFile(path, []byte(componentSourceManifest), 0o644)) + + p, err := NewPromoter(PromoterOptions{ + ConfigPath: path, + DryRun: true, + Actor: "test-actor", + Component: "api", + }) + require.NoError(t, err) + + result, err := p.Promote(ModeCascade, "dev-to-prod") + require.NoError(t, err) + require.True(t, result.Success) + last := result.Promotions[len(result.Promotions)-1] + require.NotEqual(t, "webdevsha", last.SHA, "api promotion must not read web's dev seed") + require.NotEqual(t, "web-0.1.0-rc.0", last.Version) +} diff --git a/internal/promote/promote.go b/internal/promote/promote.go index dfaf4450..38255f4f 100644 --- a/internal/promote/promote.go +++ b/internal/promote/promote.go @@ -92,6 +92,16 @@ func NewPromoter(opts PromoterOptions, options ...Option) (*Promoter, error) { return nil, fmt.Errorf("failed to parse config: %w", err) } + // For a component-scoped promotion the recorded source and target env state + // lives under state.components.., not the flat state. + // rows the promotion reads. Overlay the component's own rows into the working + // state map so every existing State[env] lookup transparently sees that + // component's seed. The write path stays component-scoped, so the overlaid + // rows never leak back into the manifest as flat state. + if err := overlayComponentState(cicdFile, opts.ConfigPath, opts.Component); err != nil { + return nil, err + } + actor := opts.Actor if actor == "" { actor = "github-actions[bot]" @@ -110,6 +120,35 @@ func NewPromoter(opts PromoterOptions, options ...Option) (*Promoter, error) { }, nil } +// overlayComponentState overlays a component's recorded per-env state, read from +// the manifest at configPath under state.components.., into the +// working flat state map. Every State[env] lookup in preflight and promotion then +// sees that component's seed without teaching each lookup about the components +// subtree. It is a no-op when component is empty, keeping the single-component +// path byte-identical. It is the read counterpart to the component-scoped state +// writes: those keep the persisted form component-scoped, so an overlaid row is +// never round-tripped back to the manifest as a flat state. node. +func overlayComponentState(cicdFile *config.CICDFile, configPath, component string) error { + if component == "" { + return nil + } + raw, err := os.ReadFile(configPath) + if err != nil { + return fmt.Errorf("failed to read config for component state: %w", err) + } + compState, err := config.ReadComponentState(raw, config.DefaultManifestKey, component) + if err != nil { + return fmt.Errorf("failed to read component state: %w", err) + } + if cicdFile.State == nil { + cicdFile.State = make(map[string]*config.EnvState) + } + for env, st := range compState { + cicdFile.State[env] = st + } + return nil +} + // Promote executes a promotion and returns the result // mode: "default" for sequential single-step, "cascade" for atomic multi-step // target: for cascade mode, the target (e.g., "dev-to-prod")