feat(linter): rules:needs: validation (GL044)
Add GL044 to validate jobs listed in rules:needs: overrides (GitLab CI 16.4+). rules:needs: lets a specific rule override the job's top-level needs: list; any referenced job must exist in the pipeline. Unknown jobs produce an error; optional: true entries produce a warning, matching the GL027 behaviour for top-level needs:. Cross-pipeline needs and skipped jobs (when a context is active) are excluded from checking. Implementation: - model.Rule gains a Needs []any field (yaml:"needs") - checkRulesNeeds(p, skipped) added to needs.go; wired into Lint - GL044 / RuleRulesNeedsUnknown added to rules.go and explain.go - 6 unit tests + 2 testdata fixtures; task validate updated Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -816,4 +816,41 @@ my-job:
|
||||
my-job:
|
||||
script: echo hi`,
|
||||
},
|
||||
|
||||
RuleRulesNeedsUnknown: {
|
||||
Title: "'rules:needs:' references unknown job",
|
||||
Severity: Error,
|
||||
Description: "A job listed in a rule's 'needs:' override does not exist in " +
|
||||
"the pipeline. 'rules:needs:' (GitLab CI 16.4+) overrides the top-level " +
|
||||
"'needs:' list when that specific rule matches. GitLab will refuse to " +
|
||||
"create the pipeline if any referenced job is missing.",
|
||||
Example: `stages: [build, test]
|
||||
|
||||
build:
|
||||
stage: build
|
||||
script: make
|
||||
|
||||
test:
|
||||
stage: test
|
||||
script: make test
|
||||
rules:
|
||||
- if: $CI_COMMIT_BRANCH == "main"
|
||||
needs: [build, lint] # lint does not exist`,
|
||||
Fix: `stages: [build, test]
|
||||
|
||||
lint:
|
||||
stage: build
|
||||
script: make lint
|
||||
|
||||
build:
|
||||
stage: build
|
||||
script: make
|
||||
|
||||
test:
|
||||
stage: test
|
||||
script: make test
|
||||
rules:
|
||||
- if: $CI_COMMIT_BRANCH == "main"
|
||||
needs: [build, lint]`,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -62,6 +62,7 @@ func Lint(p *model.Pipeline, skipped map[string]bool) []Finding {
|
||||
findings = append(findings, checkWorkflow(p)...)
|
||||
findings = append(findings, checkJobs(p)...)
|
||||
findings = append(findings, checkNeeds(p, skipped)...)
|
||||
findings = append(findings, checkRulesNeeds(p, skipped)...)
|
||||
findings = append(findings, checkDependencies(p, skipped)...)
|
||||
findings = append(findings, checkVariableRefs(p)...)
|
||||
findings = append(findings, checkRulesIfReachability(p)...)
|
||||
|
||||
@@ -85,6 +85,44 @@ func checkNeeds(p *model.Pipeline, skipped map[string]bool) []Finding {
|
||||
return findings
|
||||
}
|
||||
|
||||
// checkRulesNeeds validates rules:needs: entries across all jobs. Each entry
|
||||
// in a rule's needs: list must reference a job that exists in the pipeline.
|
||||
// Cross-pipeline needs (maps with a "pipeline" key) are ignored. Skipped jobs
|
||||
// are excluded from checking (same semantics as top-level needs: via GL027).
|
||||
func checkRulesNeeds(p *model.Pipeline, skipped map[string]bool) []Finding {
|
||||
var findings []Finding
|
||||
for name, job := range p.Jobs {
|
||||
if skipped[name] {
|
||||
continue
|
||||
}
|
||||
for i, rule := range job.Rules {
|
||||
if len(rule.Needs) == 0 {
|
||||
continue
|
||||
}
|
||||
for _, entry := range parseNeedEntries(rule.Needs) {
|
||||
if _, exists := p.Jobs[entry.job]; !exists {
|
||||
sev := Error
|
||||
if entry.optional {
|
||||
sev = Warning
|
||||
}
|
||||
findings = append(findings, Finding{
|
||||
Severity: sev,
|
||||
Rule: RuleRulesNeedsUnknown,
|
||||
Job: name,
|
||||
File: job.File,
|
||||
Line: job.Line,
|
||||
Message: fmt.Sprintf(
|
||||
"rules[%d].needs: references unknown job %q",
|
||||
i, entry.job,
|
||||
),
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return findings
|
||||
}
|
||||
|
||||
// parseNeedEntries extracts needs entries from a needs: list, preserving the
|
||||
// optional flag. Each element is a plain string (job name) or a map with a
|
||||
// "job" key. Cross-pipeline needs (maps with a "pipeline" key) are skipped.
|
||||
|
||||
@@ -31,6 +31,129 @@ func TestCheckNeeds_SkippedJob(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckRulesNeeds_UnknownJob verifies that an unknown job in rules:needs: produces GL044.
|
||||
func TestCheckRulesNeeds_UnknownJob(t *testing.T) {
|
||||
p := &model.Pipeline{
|
||||
Stages: []string{"build", "test"},
|
||||
Jobs: map[string]model.Job{
|
||||
"build-job": {
|
||||
Name: "build-job",
|
||||
Stage: "build",
|
||||
Script: []any{"make"},
|
||||
},
|
||||
"test-job": {
|
||||
Name: "test-job",
|
||||
Stage: "test",
|
||||
Script: []any{"make test"},
|
||||
Rules: []model.Rule{
|
||||
{
|
||||
If: `$CI_COMMIT_BRANCH == "main"`,
|
||||
Needs: []any{"build-job", "nonexistent"},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
findings := checkRulesNeeds(p, nil)
|
||||
var got bool
|
||||
for _, f := range findings {
|
||||
if f.Rule == RuleRulesNeedsUnknown && f.Severity == Error {
|
||||
got = true
|
||||
}
|
||||
}
|
||||
if !got {
|
||||
t.Errorf("expected GL044 error for unknown rules:needs: job; got %v", findings)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckRulesNeeds_KnownJob verifies no finding when all rules:needs: jobs exist.
|
||||
func TestCheckRulesNeeds_KnownJob(t *testing.T) {
|
||||
p := &model.Pipeline{
|
||||
Stages: []string{"build", "test"},
|
||||
Jobs: map[string]model.Job{
|
||||
"build-job": {Name: "build-job", Stage: "build", Script: []any{"make"}},
|
||||
"test-job": {
|
||||
Name: "test-job",
|
||||
Stage: "test",
|
||||
Script: []any{"make test"},
|
||||
Rules: []model.Rule{{Needs: []any{"build-job"}}},
|
||||
},
|
||||
},
|
||||
}
|
||||
if findings := checkRulesNeeds(p, nil); len(findings) != 0 {
|
||||
t.Errorf("expected no findings for valid rules:needs:; got %v", findings)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckRulesNeeds_Optional verifies that optional: true downgrades to Warning.
|
||||
func TestCheckRulesNeeds_Optional(t *testing.T) {
|
||||
p := &model.Pipeline{
|
||||
Jobs: map[string]model.Job{
|
||||
"test-job": {
|
||||
Name: "test-job",
|
||||
Script: []any{"make test"},
|
||||
Rules: []model.Rule{
|
||||
{Needs: []any{map[string]any{"job": "ghost", "optional": true}}},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
findings := checkRulesNeeds(p, nil)
|
||||
if len(findings) != 1 || findings[0].Severity != Warning {
|
||||
t.Errorf("optional unknown rules:needs: should produce Warning; got %v", findings)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckRulesNeeds_CrossPipeline verifies that cross-pipeline needs are ignored.
|
||||
func TestCheckRulesNeeds_CrossPipeline(t *testing.T) {
|
||||
p := &model.Pipeline{
|
||||
Jobs: map[string]model.Job{
|
||||
"test-job": {
|
||||
Name: "test-job",
|
||||
Script: []any{"make test"},
|
||||
Rules: []model.Rule{
|
||||
{Needs: []any{map[string]any{"pipeline": "other", "job": "j"}}},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
if findings := checkRulesNeeds(p, nil); len(findings) != 0 {
|
||||
t.Errorf("cross-pipeline rules:needs: should be ignored; got %v", findings)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckRulesNeeds_SkippedJob verifies that a skipped job's rules:needs: violations are suppressed.
|
||||
func TestCheckRulesNeeds_SkippedJob(t *testing.T) {
|
||||
p := &model.Pipeline{
|
||||
Jobs: map[string]model.Job{
|
||||
"test-job": {
|
||||
Name: "test-job",
|
||||
Script: []any{"make test"},
|
||||
Rules: []model.Rule{{Needs: []any{"nonexistent"}}},
|
||||
},
|
||||
},
|
||||
}
|
||||
if findings := checkRulesNeeds(p, map[string]bool{"test-job": true}); len(findings) != 0 {
|
||||
t.Errorf("skipped job: expected no findings; got %v", findings)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckRulesNeeds_NoNeeds verifies no findings when rules have no needs: override.
|
||||
func TestCheckRulesNeeds_NoNeeds(t *testing.T) {
|
||||
p := &model.Pipeline{
|
||||
Jobs: map[string]model.Job{
|
||||
"test-job": {
|
||||
Name: "test-job",
|
||||
Script: []any{"make test"},
|
||||
Rules: []model.Rule{{When: "on_success"}},
|
||||
},
|
||||
},
|
||||
}
|
||||
if findings := checkRulesNeeds(p, nil); len(findings) != 0 {
|
||||
t.Errorf("rules without needs: should produce no findings; got %v", findings)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckNeeds_StageOrder verifies that a job needing a job in a later stage
|
||||
// produces RuleNeedsStageOrder (line 64-76 in needs.go).
|
||||
func TestCheckNeeds_StageOrder(t *testing.T) {
|
||||
|
||||
@@ -154,4 +154,8 @@ const (
|
||||
// pipeline has no 'default:' block, making the declaration a no-op. Also fires
|
||||
// when 'inherit: default: [list]' names fields not set in the default: block.
|
||||
RuleInheritNoDefault = "GL043"
|
||||
|
||||
// GL044: a rules:needs: entry references a job that does not exist in the pipeline.
|
||||
// rules:needs: overrides the top-level needs: when a specific rule matches (GitLab CI 16.4+).
|
||||
RuleRulesNeedsUnknown = "GL044"
|
||||
)
|
||||
|
||||
@@ -89,6 +89,7 @@ type Rule struct {
|
||||
Changes any `yaml:"changes"` // []string or {paths,compare_to} map
|
||||
Exists any `yaml:"exists"` // []string or map form
|
||||
Variables map[string]any `yaml:"variables"` // set/override variables when rule matches (GitLab CI 15.0+)
|
||||
Needs []any `yaml:"needs"` // override needs: when this rule matches (GitLab CI 16.4+)
|
||||
}
|
||||
|
||||
// ReservedKeys are top-level GitLab CI keys that are NOT job definitions.
|
||||
|
||||
Reference in New Issue
Block a user