From d34c39927da613addd30c2068eb594eef02c9709 Mon Sep 17 00:00:00 2001 From: k3nny Date: Thu, 11 Jun 2026 21:27:16 +0200 Subject: [PATCH] fix(linter): downgrade needs optional:true missing-job to warning parseNeedJobNames is replaced by parseNeedEntries which preserves the optional flag from each needs: entry. When a referenced job does not exist and optional:true is set, the finding is now WARNING instead of ERROR, matching GitLab CI runtime behavior (the dependency is silently skipped when the job is absent from a conditional include). Optional missing deps are also excluded from the cycle-detection graph since there is no real dependency edge to trace. Adds a fixture case in testdata/needs.yml to prevent regression. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 2 ++ internal/linter/needs.go | 50 ++++++++++++++++++++++++++-------------- testdata/needs.yml | 9 ++++++++ 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a20ad35..f9ee067 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ This project uses [Semantic Versioning](https://semver.org). ### Fixed +- **`needs: optional: true` downgraded to warning** — a `needs:` entry that carries `optional: true` and references a job not present in the pipeline now emits `[WARNING]` instead of `[ERROR]`. GitLab CI silently skips such dependencies at runtime (the job is absent when its include was not triggered), so the finding was a false positive. Non-optional missing needs remain errors. Optional missing deps are also excluded from the cycle-detection graph. + - **`extends:` jobs with missing script downgraded to warning** — a job that declares `extends:` but has no `script` after resolution now emits `[WARNING]` instead of `[ERROR]`. The script may legitimately come from a base job in a remote include that could not be fetched at lint time (e.g. no token configured). - **Variable map form now parses correctly** — `variables:` entries that use the extended `{value, description, options}` form (GitLab CI 13.7+) no longer cause `yaml: cannot unmarshal !!map into string`. Both `Pipeline.Variables` and per-job `Variables` now accept either plain strings or map-form declarations. diff --git a/internal/linter/needs.go b/internal/linter/needs.go index dbe2ab2..7b5b7b7 100644 --- a/internal/linter/needs.go +++ b/internal/linter/needs.go @@ -6,6 +6,12 @@ import ( "git.k3nny.fr/glint/internal/model" ) +// needEntry is a parsed element from a job's needs: list. +type needEntry struct { + job string + optional bool // true when the needs entry carries optional: true +} + func checkNeeds(p *model.Pipeline) []Finding { var findings []Finding @@ -15,7 +21,7 @@ func checkNeeds(p *model.Pipeline) []Finding { stageIndex[s] = i } - // needsGraph maps each job to the list of jobs it depends on. + // needsGraph maps each job to the jobs it depends on (existing jobs only). // Used for cycle detection after individual checks. needsGraph := make(map[string][]string) @@ -24,24 +30,33 @@ func checkNeeds(p *model.Pipeline) []Finding { continue } - neededNames := parseNeedJobNames(job.Needs) - needsGraph[name] = neededNames - + entries := parseNeedEntries(job.Needs) jobStageIdx, jobHasStage := stageIndex[job.Stage] - for _, needed := range neededNames { - neededJob, exists := p.Jobs[needed] + for _, entry := range entries { + neededJob, exists := p.Jobs[entry.job] if !exists { + // optional: true means GitLab CI will silently skip the + // dependency when the job is absent (e.g. from a conditional + // include). Downgrade to warning so users are informed without + // failing the lint. + sev := Error + if entry.optional { + sev = Warning + } findings = append(findings, Finding{ - Severity: Error, + Severity: sev, Job: name, File: job.File, Line: job.Line, - Message: fmt.Sprintf("needs unknown job %q", needed), + Message: fmt.Sprintf("needs unknown job %q", entry.job), }) continue } + // Add to the cycle-detection graph only when the dep exists. + needsGraph[name] = append(needsGraph[name], entry.job) + // A job cannot need a job in a later stage. if len(p.Stages) > 0 && jobHasStage && neededJob.Stage != "" { neededStageIdx, neededHasStage := stageIndex[neededJob.Stage] @@ -53,7 +68,7 @@ func checkNeeds(p *model.Pipeline) []Finding { Line: job.Line, Message: fmt.Sprintf( "needs %q which is in a later stage (%q after %q)", - needed, neededJob.Stage, job.Stage, + entry.job, neededJob.Stage, job.Stage, ), }) } @@ -65,25 +80,26 @@ func checkNeeds(p *model.Pipeline) []Finding { return findings } -// parseNeedJobNames extracts job names from a needs: list. -// Each element is either a plain string or a map with a "job" key. -// Cross-pipeline needs (maps with a "pipeline" key) are skipped. -func parseNeedJobNames(needs []any) []string { - var names []string +// 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. +func parseNeedEntries(needs []any) []needEntry { + var entries []needEntry for _, n := range needs { switch v := n.(type) { case string: - names = append(names, v) + entries = append(entries, needEntry{job: v}) case map[string]any: if _, crossPipeline := v["pipeline"]; crossPipeline { continue } if job, ok := v["job"].(string); ok { - names = append(names, job) + optional, _ := v["optional"].(bool) + entries = append(entries, needEntry{job: job, optional: optional}) } } } - return names + return entries } func detectNeedsCycles(graph map[string][]string, jobs map[string]model.Job) []Finding { diff --git a/testdata/needs.yml b/testdata/needs.yml index d48c16f..8a73581 100644 --- a/testdata/needs.yml +++ b/testdata/needs.yml @@ -38,3 +38,12 @@ missing-needs-job: - echo "bad" needs: - nonexistent-job # ERROR: job doesn't exist + +# optional: true — missing dep should be WARNING not ERROR +optional-needs-job: + stage: build + script: + - echo "I depend on something that may not exist" + needs: + - job: nonexistent-optional-job + optional: true