From cd89667718e7f74ecd6333e036e234fcb6a26f5b Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Mon, 15 Jun 2026 23:14:49 -0400 Subject: [PATCH] fix: make workflow generation deterministic Signed-off-by: Joshua Temple --- internal/generate/determinism_test.go | 187 ++++++++++++++++++++++++++ internal/generate/generator.go | 19 ++- internal/generate/graph.go | 17 ++- 3 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 internal/generate/determinism_test.go diff --git a/internal/generate/determinism_test.go b/internal/generate/determinism_test.go new file mode 100644 index 00000000..2b6460e4 --- /dev/null +++ b/internal/generate/determinism_test.go @@ -0,0 +1,187 @@ +package generate + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stablekernel/cascade/internal/config" + "github.com/stretchr/testify/require" +) + +// writeDeterminismWorkflows lays down the reusable-workflow stubs referenced by +// the determinism manifest and returns the base directory the generators read. +func writeDeterminismWorkflows(t *testing.T) string { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, ".github/workflows"), 0o755)) + + imageBuild := ` +name: Image Build +on: + workflow_call: + inputs: + os: + type: string + arch: + type: string + outputs: + image: + value: test +` + bundleBuild := ` +name: Bundle Build +on: + workflow_call: + inputs: + image: + type: string + outputs: + bundle: + value: test +` + deployWorkflow := ` +name: Deploy +on: + workflow_call: + inputs: + environment: + type: string + bundle: + type: string +` + files := map[string]string{ + ".github/workflows/image-build.yaml": imageBuild, + ".github/workflows/bundle-build.yaml": bundleBuild, + ".github/workflows/deploy.yaml": deployWorkflow, + } + for path, body := range files { + require.NoError(t, os.WriteFile(filepath.Join(dir, path), []byte(body), 0o644)) + } + return dir +} + +// determinismConfig returns a representative multi-environment manifest with a +// matrix image build, a dependent bundle build, and a deploy that depends on the +// bundle. This produces multiple jobs, multi-entry needs: lists, and multi-entry +// if: skip-gate conditions, exercising every order-sensitive emission path. +func determinismConfig() *config.TrunkConfig { + return &config.TrunkConfig{ + TrunkBranch: "main", + Environments: []string{"dev", "staging", "prod", "canary"}, + Builds: []config.BuildConfig{ + { + Name: "image", + Workflow: ".github/workflows/image-build.yaml", + Triggers: []string{"src/**"}, + Matrix: &config.MatrixConfig{ + Dimensions: map[string][]string{ + "os": {"linux", "darwin", "windows"}, + "arch": {"amd64", "arm64"}, + }, + }, + }, + { + Name: "bundle", + Workflow: ".github/workflows/bundle-build.yaml", + Triggers: []string{"bundle/**"}, + DependsOn: []string{"image"}, + }, + // docs is an independent build root (no depends_on). Multiple + // roots are what surface toposort seed-order non-determinism: + // a single linear chain would be stable regardless of seed. + { + Name: "docs", + Workflow: ".github/workflows/bundle-build.yaml", + Triggers: []string{"docs/**"}, + }, + }, + Deploys: []config.DeployConfig{ + { + Name: "app", + Workflow: ".github/workflows/deploy.yaml", + Triggers: []string{"src/**"}, + DependsOn: []string{"bundle"}, + }, + // sidecar is an independent deploy root, again adding parallel + // nodes to the graph so seed order can diverge run to run. + { + Name: "sidecar", + Workflow: ".github/workflows/deploy.yaml", + Triggers: []string{"sidecar/**"}, + }, + }, + // External repos make this a primary repo, exercising external-update.yaml + // and the external-deploy branches of promote.yaml. Multiple repos and + // deploys give the external emission paths multiple entries to order. + External: []config.ExternalRepoConfig{ + { + Repo: "org/infra", + Deploys: []config.ExternalDeployConfig{ + {Name: "cdk", Workflow: "org/infra/.github/workflows/deploy.yaml@main", Triggers: []string{"infra/**"}}, + {Name: "dns", Workflow: "org/infra/.github/workflows/deploy.yaml@main", Triggers: []string{"dns/**"}}, + }, + }, + { + Repo: "org/data", + Deploys: []config.ExternalDeployConfig{ + {Name: "etl", Workflow: "org/data/.github/workflows/deploy.yaml@main", Triggers: []string{"etl/**"}}, + }, + }, + }, + } +} + +// generateAll produces every workflow file the determinism contract covers for +// the given config, keyed by a stable file label. It generates in a single +// process so repeated calls within one test exercise Go's per-range map-order +// randomization. +func generateAll(t *testing.T, cfg *config.TrunkConfig, dir string) map[string]string { + t.Helper() + out := make(map[string]string) + + orchestrate, err := NewGenerator(cfg, dir).Generate() + require.NoError(t, err) + out["orchestrate.yaml"] = orchestrate + + promote, err := NewPromoteGenerator(cfg, dir).Generate() + require.NoError(t, err) + out["promote.yaml"] = promote + + hotfix, err := NewHotfixGenerator(cfg, dir).Generate() + require.NoError(t, err) + out["cascade-hotfix.yaml"] = hotfix + + rollback, err := NewRollbackGenerator(cfg, dir).Generate() + require.NoError(t, err) + out["cascade-rollback.yaml"] = rollback + + external, err := NewExternalUpdateGenerator(cfg, dir).Generate() + require.NoError(t, err) + out["external-update.yaml"] = external + + return out +} + +// TestGeneration_Deterministic_MultiEnv regenerates the same multi-environment +// manifest many times in one process and asserts every generated file is +// byte-identical across all runs. Go randomizes map range order per range +// statement per process, so any emission that derives order from a Go map +// iteration would diverge across these iterations. +func TestGeneration_Deterministic_MultiEnv(t *testing.T) { + dir := writeDeterminismWorkflows(t) + cfg := determinismConfig() + + const runs = 20 + baseline := generateAll(t, cfg, dir) + require.NotEmpty(t, baseline) + + for i := 1; i < runs; i++ { + got := generateAll(t, cfg, dir) + require.Equal(t, len(baseline), len(got), "run %d produced a different set of files", i) + for name, want := range baseline { + require.Equal(t, want, got[name], + "run %d produced non-deterministic output for %s", i, name) + } + } +} diff --git a/internal/generate/generator.go b/internal/generate/generator.go index 5000a38b..71606b50 100644 --- a/internal/generate/generator.go +++ b/internal/generate/generator.go @@ -268,8 +268,10 @@ func (g *Generator) Validate() []string { return g.warnings } - // Check that dependents have inputs for dependency outputs - for _, node := range g.graph.Nodes { + // Check that dependents have inputs for dependency outputs. Iterate in + // declaration order so emitted warnings are stable across runs. + for _, jobID := range g.graph.Order { + node := g.graph.Nodes[jobID] deps := g.graph.GetDirectDependencies(node.JobID) declaredInputs := g.inputs[node.JobID] inputSet := make(map[string]bool) @@ -404,7 +406,9 @@ func (g *Generator) validateRequiredInputs() error { var errors []string - for _, node := range g.graph.Nodes { + // Iterate in declaration order so the validation error list is stable. + for _, jobID := range g.graph.Order { + node := g.graph.Nodes[jobID] requiredInputs := g.requiredInputs[node.JobID] if len(requiredInputs) == 0 { continue @@ -1728,10 +1732,13 @@ func (g *Generator) writeReleaseStep(sb *strings.Builder) { parts := strings.SplitN(g.config.Release.Tag, ".", 2) callbackName := parts[0] outputName := parts[1] - // Find the job ID for this callback name + // Find the job ID for this callback name. Iterate in declaration order + // (Order), not by ranging the Nodes map: a map range is randomized per + // process, so when two callbacks share a name across sections the break + // could pick either one run to run. var jobID string - for jid, info := range g.graph.Nodes { - if info.Name == callbackName { + for _, jid := range g.graph.Order { + if g.graph.Nodes[jid].Name == callbackName { jobID = jid break } diff --git a/internal/generate/graph.go b/internal/generate/graph.go index 75317354..838a026f 100644 --- a/internal/generate/graph.go +++ b/internal/generate/graph.go @@ -17,6 +17,13 @@ type DependencyGraph struct { // contribute a skip-gate to its if: condition. The job still runs when an // optional dep was skipped because its triggers didn't match (#18). OptionalEdges map[string][]string + + // Order lists every job ID in manifest declaration order (validate, then + // builds, then deploys, each in the order they appear in config). It is the + // deterministic seed for TopologicalSort: iterating Nodes (a map) directly + // would randomize emitted job order, needs: lists, and if: conditions across + // runs because Go randomizes map range order per process. + Order []string } // CallbackInfo holds information about a callback @@ -84,6 +91,7 @@ func BuildDependencyGraph(cfg *config.TrunkConfig) *DependencyGraph { Secrets: cfg.Validate.Secrets, } g.Edges[jobID] = nil + g.Order = append(g.Order, jobID) } // Add builds @@ -106,6 +114,7 @@ func BuildDependencyGraph(cfg *config.TrunkConfig) *DependencyGraph { PassthroughArtifact: b.PassthroughArtifact, Secrets: b.Secrets, } + g.Order = append(g.Order, jobID) // Resolve dependencies to job IDs var deps []string @@ -150,6 +159,7 @@ func BuildDependencyGraph(cfg *config.TrunkConfig) *DependencyGraph { SupportsDryRun: d.SupportsDryRun, Secrets: d.Secrets, } + g.Order = append(g.Order, jobID) // Resolve dependencies to job IDs var deps []string @@ -204,7 +214,12 @@ func (g *DependencyGraph) TopologicalSort() ([]string, error) { return nil } - for node := range g.Nodes { + // Seed the walk in manifest declaration order (Order), not by ranging + // g.Nodes: a map range is randomized per process, which would shuffle the + // emitted job order, needs: lists, and if: conditions run to run. With a + // stable seed the result is a deterministic topological order that follows + // declaration order wherever the dependency DAG leaves it free. + for _, node := range g.Order { if err := visit(node); err != nil { return nil, err }