diff --git a/pipeline/frontend/yaml/compiler/dag.go b/pipeline/frontend/yaml/compiler/dag.go index f536de68a..9969e4210 100644 --- a/pipeline/frontend/yaml/compiler/dag.go +++ b/pipeline/frontend/yaml/compiler/dag.go @@ -75,6 +75,16 @@ func (c dagCompiler) compileByDependsOn() ([]*backend_types.Stage, error) { } func dfsVisit(steps map[string]*dagCompilerStep, name string, visited map[string]struct{}, path []string) error { + step, exists := steps[name] + if !exists { + // `name` is an optional dependency on a step that was filtered out. convertDAGToStages + // drops those edges, but it resolves one step at a time and runs this walk in the same + // loop, so the walk can reach a step whose dependsOn is not resolved yet and follow an + // edge to a filtered-out step. There is nothing to traverse, and a missing *required* + // dependency is still reported by convertDAGToStages when it reaches that step. + return nil + } + if _, ok := visited[name]; ok { return &ErrStepDependencyCycle{path: path} } @@ -82,7 +92,7 @@ func dfsVisit(steps map[string]*dagCompilerStep, name string, visited map[string visited[name] = struct{}{} path = append(path, name) - for _, dep := range steps[name].dependsOn { + for _, dep := range step.dependsOn { if err := dfsVisit(steps, dep.Name, visited, path); err != nil { return err } diff --git a/pipeline/frontend/yaml/compiler/dag_test.go b/pipeline/frontend/yaml/compiler/dag_test.go index 30b479da3..8f3e981c6 100644 --- a/pipeline/frontend/yaml/compiler/dag_test.go +++ b/pipeline/frontend/yaml/compiler/dag_test.go @@ -162,6 +162,42 @@ func TestOptionalStepDependency(t *testing.T) { assert.Len(t, stages, 2, "should produce 2 stages (build then deploy)") }) + t.Run("missing optional step dep on a step that has dependents", func(t *testing.T) { + // convertDAGToStages resolves one step at a time and runs the cycle check in the + // same loop, so if it reaches `notify` before `deploy`, the walk follows deploy's + // still-unresolved edge to the absent `lint`. Map iteration order decides, so run + // this enough times to cover both orders. + for range 100 { + steps := map[string]*dagCompilerStep{ + "build": { + position: 0, + name: "build", + step: &backend_types.Step{Name: "build"}, + }, + "deploy": { + position: 1, + name: "deploy", + step: &backend_types.Step{Name: "deploy"}, + dependsOn: constraint.DependsOn{ + {Name: "build"}, + {Name: "lint", Optional: true}, + }, + }, + "notify": { + position: 2, + name: "notify", + step: &backend_types.Step{Name: "notify"}, + dependsOn: constraint.DependsOn{ + {Name: "deploy"}, + }, + }, + } + stages, err := convertDAGToStages(steps) + assert.NoError(t, err) + assert.Len(t, stages, 3, "build, then deploy, then notify") + } + }) + t.Run("missing required step dep still errors", func(t *testing.T) { steps := map[string]*dagCompilerStep{ "deploy": {