From 14bb21bf4fa84f09907c85c9c1263f4a39f6453b Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Fri, 28 Aug 2026 14:59:15 +0100 Subject: [PATCH] feat(pipeline): let a stage declare it consumes no sibling's output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stage graph is fixed on purpose, and stays fixed. But *dependency* is a claim about the repository, and a repository can be right that it does not have one. pipeline: ci: stages: [preflight, build, test, end2end] independent_stages: [end2end] A stage named here drops its sibling dependencies and depends on preflight alone. That is all it can do — it cannot add a dependency, invent an edge, or change the order stages render in. Removing an edge you do not have is not rearranging the pipeline. preflight is never dropped, because it produces no artefact: it is the gate deciding whether a stage runs at all, and the rendered `if:` reads its outputs, so losing it from `needs:` would leave a condition that can never be true. `preflight` itself cannot be declared independent, having no sibling dependency to drop; saying so is rejected rather than ignored. Detaching a stage does NOT remove it from ci-gate. A failing end2end still blocks the merge; only its start time moves. There is a test pinning that, because acquiring a silent hole in branch protection as a side effect of a performance tweak is the worst way this could go wrong. wardnet is the case it was added for. Its end2end suite rebuilds the daemon from source inside Docker and consumes nothing the build stage produces — its previous hand-rolled pipeline ran the two concurrently. Serialising them added about eleven minutes to every daemon pull request, 24 to 35, to prove an edge that does not exist. The field is omitempty and default rendering is untouched: rendering every managed file on v1.5.0 and on this branch and diffing leaves them byte-identical, comments included. Unlike #58 there is no fleet-wide comment churn, and gt's own repo stays compliant without a re-render. --- docs/pipeline-design.md | 34 +++++++++++++ internal/repogov/pipeline.go | 24 +++++++-- internal/repospec/spec.go | 74 ++++++++++++++++++++++++++- tests/repogov_pipeline_test.go | 92 ++++++++++++++++++++++++++++++++++ 4 files changed, 217 insertions(+), 7 deletions(-) diff --git a/docs/pipeline-design.md b/docs/pipeline-design.md index 4afd6ff..0b7c40c 100644 --- a/docs/pipeline-design.md +++ b/docs/pipeline-design.md @@ -458,6 +458,40 @@ answer for almost every stage, a widened stage names the one scope it needs, and the rendered orchestrator says which stage holds what — so the grant is reviewable in the diff rather than implied by a template. +### Independent stages + +Ordering is fixed, but *dependency* is a claim about the repository, and a +repository can be right that it does not have one. + +```yaml +pipeline: + ci: + stages: [preflight, build, test, end2end] + # end2end rebuilds the daemon from source inside Docker and consumes + # nothing build produces. + independent_stages: [end2end] +``` + +A stage named here drops its sibling dependencies and depends on preflight +alone. That is the only thing it can do: it cannot add a dependency, invent an +edge, or change the order stages render in. The fixed graph survives, because +removing an edge you do not have is not rearranging the pipeline. + +The dependency on preflight is never removed. preflight produces no artefact — +it is the gate deciding whether a stage runs at all, and the rendered `if:` +reads its outputs, so dropping it from `needs:` would leave a condition that is +never true. `preflight` itself cannot be declared independent; it has no +sibling dependency to drop. + +Detaching a stage does **not** remove it from `ci-gate`. A failing end2end +still blocks the merge; only its start time moves. + +wardnet is the case this was added for. Its end2end suite rebuilds the daemon +from source, so it consumes nothing the build stage produces, and its previous +hand-rolled pipeline ran the two concurrently. Serialising them added about +eleven minutes to every daemon pull request — 24 minutes to 35 — to prove an +edge that does not exist. + ## Migration Per repo: move build and test jobs out of the existing `ci.yml` into diff --git a/internal/repogov/pipeline.go b/internal/repogov/pipeline.go index 50041a3..024ad3c 100644 --- a/internal/repogov/pipeline.go +++ b/internal/repogov/pipeline.go @@ -120,12 +120,16 @@ var cdWiring = map[string]stageWiring{ // decided whether it needs to run at all. func buildStages( enabled []string, order []string, wiring map[string]stageWiring, root, guard string, - baseline map[string]string, grants repospec.StagePermissions, + baseline map[string]string, grants repospec.StagePermissions, independent []string, ) ([]stageJob, error) { present := map[string]bool{} for _, s := range enabled { present[s] = true } + detached := map[string]bool{} + for _, s := range independent { + detached[s] = true + } hasPreflight := present["preflight"] jobs := make([]stageJob, 0, len(enabled)) @@ -143,9 +147,17 @@ func buildStages( needs := []string{root} for _, dep := range w.after { - if present[dep] { - needs = append(needs, dep) + if !present[dep] { + continue + } + // A stage declared independent consumes no sibling's output, so + // only preflight survives — it produces no artefact, and the + // gating `if:` below reads its outputs, which requires it in + // `needs:`. + if detached[name] && dep != repospec.StagePreflight { + continue } + needs = append(needs, dep) } var conds []string @@ -343,7 +355,8 @@ const checkoutRef = "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # func buildCIData(in Input, shared templateData) (ciData, error) { stages, err := buildStages( in.Spec.Pipeline.CI.Stages, repospec.CIStages, ciWiring, "attest", attestGuard, - ciBaselinePermissions, in.Spec.Pipeline.CI.StagePermissions) + ciBaselinePermissions, in.Spec.Pipeline.CI.StagePermissions, + in.Spec.Pipeline.CI.IndependentStages) if err != nil { return ciData{}, err } @@ -388,7 +401,8 @@ func buildCDData(in Input, shared templateData) (cdData, error) { const root = "verify-attestation" stages, err := buildStages( in.Spec.Pipeline.CD.Stages, repospec.CDStages, cdWiring, root, "", - cdBaselinePermissions, in.Spec.Pipeline.CD.StagePermissions) + cdBaselinePermissions, in.Spec.Pipeline.CD.StagePermissions, + in.Spec.Pipeline.CD.IndependentStages) if err != nil { return cdData{}, err } diff --git a/internal/repospec/spec.go b/internal/repospec/spec.go index eb4daa0..21ab796 100644 --- a/internal/repospec/spec.go +++ b/internal/repospec/spec.go @@ -160,6 +160,9 @@ type PipelineCI struct { // StagePermissions grants a named stage scopes beyond the CI baseline. // See StagePermissions for why this exists and what it cannot do. StagePermissions StagePermissions `yaml:"stage_permissions,omitempty" json:"stage_permissions,omitempty"` + // IndependentStages names stages that consume no sibling stage's output. + // See the type for what it can and cannot express. + IndependentStages []string `yaml:"independent_stages,omitempty" json:"independent_stages,omitempty"` } type PipelineCD struct { @@ -171,6 +174,8 @@ type PipelineCD struct { Tags []string `yaml:"tags" json:"tags"` // StagePermissions grants a named stage scopes beyond the CD baseline. StagePermissions StagePermissions `yaml:"stage_permissions,omitempty" json:"stage_permissions,omitempty"` + // IndependentStages names stages that consume no sibling stage's output. + IndependentStages []string `yaml:"independent_stages,omitempty" json:"independent_stages,omitempty"` } // StagePermissions maps a stage name to the scopes its orchestrator job is @@ -188,6 +193,26 @@ type PipelineCD struct { // so the grant is reviewable in the diff rather than implied by a template. type StagePermissions map[string]map[string]string +// IndependentStages, in a pipeline spec, is the set of stages that consume no +// sibling stage's output and so need not wait for one. +// +// This is a statement about the repository, not a reordering knob. The stage +// graph stays fixed: nothing here can add a dependency, invent an edge, or +// change the order stages render in. The only thing it can do is remove a +// dependency the repository does not actually have, dropping that stage back +// to depending on preflight alone. +// +// The dependency on preflight is never removed, because preflight produces no +// artefact — it is the gate deciding whether a stage runs at all, and the +// rendered `if:` reads its outputs. +// +// wardnet is the case this was added for. Its end2end suite rebuilds the +// daemon from source inside Docker, so it consumes nothing the build stage +// produces, and under its previous hand-rolled pipeline the two ran +// concurrently. Making it wait for build added about eleven minutes to every +// daemon pull request while proving nothing — a real cost paid for an edge +// that does not exist. + type ConventionalCommits struct { Enabled bool `yaml:"enabled" json:"enabled"` Scope string `yaml:"scope" json:"scope"` @@ -275,10 +300,15 @@ const ( // CIStages and CDStages are the stage vocabularies, in the order the // orchestrators wire them. var ( - CIStages = []string{"preflight", "build", "test", "end2end"} - CDStages = []string{"preflight", "publish", "deploy", "verify"} + CIStages = []string{StagePreflight, "build", "test", "end2end"} + CDStages = []string{StagePreflight, "publish", "deploy", "verify"} ) +// StagePreflight is the gate every other stage reads its run- output +// from. It is the one stage that never depends on a sibling, and the one that +// can never be declared independent. +const StagePreflight = "preflight" + // PermissionScopes is the GITHUB_TOKEN scope vocabulary, and PermissionLevels // the values each may take. // @@ -583,6 +613,16 @@ func validatePipeline(p Pipeline) error { ); err != nil { return err } + if err := validateIndependentStages( + "pipeline.ci.independent_stages", p.CI.Enabled, p.CI.IndependentStages, p.CI.Stages, + ); err != nil { + return err + } + if err := validateIndependentStages( + "pipeline.cd.independent_stages", p.CD.Enabled, p.CD.IndependentStages, p.CD.Stages, + ); err != nil { + return err + } if p.CD.Enabled && len(p.CD.Tags) == 0 { return fmt.Errorf("pipeline.cd.tags cannot be empty when CD is enabled; nothing would ever trigger it") } @@ -649,6 +689,36 @@ func validateStagePermissions( return nil } +// validateIndependentStages rejects a declaration that cannot mean anything: +// one naming a stage the pipeline does not run, a duplicate, or `preflight`, +// which has no sibling dependency to drop in the first place. +func validateIndependentStages(field string, enabled bool, got, stages []string) error { + if len(got) == 0 { + return nil + } + if !enabled { + return fmt.Errorf("%s: cannot name stages while the pipeline is disabled", field) + } + seen := map[string]bool{} + for i, stage := range got { + if !contains(stages, stage) { + return fmt.Errorf( + "%s[%d]: stage %q is not enabled (enabled: %s)", + field, i, stage, strings.Join(stages, ", ")) + } + if seen[stage] { + return fmt.Errorf("%s[%d]: duplicate stage %q", field, i, stage) + } + if stage == StagePreflight { + return fmt.Errorf( + "%s[%d]: %q has no stage dependency to drop; remove it", + field, i, stage) + } + seen[stage] = true + } + return nil +} + func validateSettings(s Settings) error { m := s.Merge if !m.Squash && !m.MergeCommit && !m.Rebase { diff --git a/tests/repogov_pipeline_test.go b/tests/repogov_pipeline_test.go index 89ac0f2..d2df3d9 100644 --- a/tests/repogov_pipeline_test.go +++ b/tests/repogov_pipeline_test.go @@ -313,6 +313,20 @@ func TestPipelineValidation(t *testing.T) { "build": {"security-events": "write"}, } }, "pipeline is disabled"}, + {"independent stage that is not enabled", func(s *repospec.Spec) { + s.Pipeline.CI.Stages = []string{"preflight", "build"} + s.Pipeline.CI.IndependentStages = []string{"end2end"} + }, "is not enabled"}, + {"independent preflight", func(s *repospec.Spec) { + s.Pipeline.CI.IndependentStages = []string{"preflight"} + }, "no stage dependency to drop"}, + {"duplicate independent stage", func(s *repospec.Spec) { + s.Pipeline.CI.IndependentStages = []string{"end2end", "end2end"} + }, "duplicate stage"}, + {"independent stages while ci is disabled", func(s *repospec.Spec) { + s.Pipeline.CI.Enabled = false + s.Pipeline.CI.IndependentStages = []string{"end2end"} + }, "pipeline is disabled"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -860,3 +874,81 @@ func TestStagePermissionsRenderDeterministically(t *testing.T) { } } } + +// The reason this exists: wardnet's end2end suite rebuilds the daemon from +// source and consumes nothing build produces, so waiting on build added ~11 +// minutes to every daemon PR to prove an edge that does not exist. +func TestIndependentStageDropsSiblingDependenciesButNotPreflight(t *testing.T) { + spec := repospec.Default() + spec.Pipeline.CI.IndependentStages = []string{"end2end"} + + jobs := workflowJobs(t, pipelineFiles(t, spec)[".github/workflows/ci-orchestration.yml"]) + + e2e, ok := jobs["end2end"] + if !ok { + t.Fatalf("no end2end job; jobs = %v", jobs) + } + want := []string{"attest", "preflight"} + if !reflect.DeepEqual(e2e.Needs, want) { + t.Errorf("end2end needs = %v, want %v", e2e.Needs, want) + } + + // preflight must survive, or the gating condition references the outputs + // of a job this one does not wait for and is never true. + if !strings.Contains(e2e.If, "needs.preflight.outputs.run-end2end") { + t.Errorf("end2end lost its preflight gate; if = %q", e2e.If) + } + + // Independence is per-stage: test still consumes build's artefacts. + if test, ok := jobs["test"]; !ok { + t.Error("no test job") + } else if !contains(test.Needs, "build") { + t.Errorf("test needs = %v, want it to still wait on build", test.Needs) + } +} + +// Detaching a stage must not drop it from the gate. If it did, a failing +// end2end would stop blocking the merge — the exact defect branch protection +// exists to prevent, arriving as a side effect of a performance tweak. +func TestIndependentStageIsStillGated(t *testing.T) { + spec := repospec.Default() + spec.Pipeline.CI.IndependentStages = []string{"end2end"} + + jobs := workflowJobs(t, pipelineFiles(t, spec)[".github/workflows/ci-orchestration.yml"]) + gate, ok := jobs[repospec.GateCheckJob] + if !ok { + t.Fatalf("no %s job", repospec.GateCheckJob) + } + if !contains(gate.Needs, "end2end") { + t.Errorf("%s needs = %v, want end2end among them", repospec.GateCheckJob, gate.Needs) + } +} + +// Declaring nothing must render exactly what it rendered before, so this +// cannot quietly reshape the seventeen repos that do not use it. +func TestStagesKeepTheirDependenciesByDefault(t *testing.T) { + jobs := workflowJobs(t, pipelineFiles(t, repospec.Default())[".github/workflows/ci-orchestration.yml"]) + + for _, tc := range []struct{ stage, dep string }{ + {"test", "build"}, + {"end2end", "build"}, + {"build", "preflight"}, + } { + j, ok := jobs[tc.stage] + if !ok { + t.Fatalf("no %s job", tc.stage) + } + if !contains(j.Needs, tc.dep) { + t.Errorf("%s needs = %v, want %q among them", tc.stage, j.Needs, tc.dep) + } + } +} + +func contains(haystack []string, needle string) bool { + for _, h := range haystack { + if h == needle { + return true + } + } + return false +}