From c8459cb5cfdfb4f48f14dbab337319af718f771f Mon Sep 17 00:00:00 2001 From: Jake Valenzuela <5650707+jakeva@users.noreply.github.com> Date: Tue, 31 Mar 2026 02:18:35 -0400 Subject: [PATCH] feat: add orphaned-stages rule (warning severity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a fifth lint rule that flags stages completely disconnected from the pipeline graph — no prerequisites and nothing depends on them. Violations are emitted at warning severity, rendering as [warn: orphaned-stages] in text output and "level": "warning" in SARIF. - Violation struct gains a Severity field (empty = error, "warning" = warn) - Reporter renders warn prefix in text; passes level through in SARIF - testdata/complex.json: 20-stage fixture exercising all five rules - CONTRIBUTING.md: contributor guide --- CONTRIBUTING.md | 105 ++++++++++++++++++++++ README.md | 32 ++++++- pkg/reporter/reporter.go | 12 ++- pkg/rules/orphaned_stages.go | 43 +++++++++ pkg/rules/orphaned_stages_test.go | 141 ++++++++++++++++++++++++++++++ pkg/rules/registry.go | 1 + pkg/rules/rule.go | 7 +- testdata/complex.json | 31 +++++++ 8 files changed, 364 insertions(+), 8 deletions(-) create mode 100644 CONTRIBUTING.md create mode 100644 pkg/rules/orphaned_stages.go create mode 100644 pkg/rules/orphaned_stages_test.go create mode 100644 testdata/complex.json diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..1ebdf0d --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,105 @@ +# Contributing to spinlint + +## Adding a Rule + +All lint rules implement the two-method [`Rule` interface](pkg/rules/rule.go): + +```go +type Rule interface { + Name() string + Check(pipeline schema.Pipeline) []Violation +} +``` + +`Violation` carries the rule name, the affected stage's `refId`, a human-readable message, and an optional severity: + +```go +type Violation struct { + Rule string `json:"rule"` + Stage string `json:"stage,omitempty"` + Message string `json:"message"` + Severity string `json:"severity,omitempty"` // "warning" or "" (treated as "error") +} +``` + +Follow these steps to add a new rule. For more context, see the [Adding a Rule](README.md#adding-a-rule) section of the README. + +### 1. Create the rule file + +Create `pkg/rules/.go`: + +```go +package rules + +import ( + "fmt" + "github.com/jakeva/spinlint/pkg/schema" +) + +type YourRule struct{} + +func (r YourRule) Name() string { return "your-rule-name" } + +func (r YourRule) Check(pipeline schema.Pipeline) []Violation { + var violations []Violation + for _, stage := range pipeline.Stages { + if /* condition */ { + violations = append(violations, Violation{ + Rule: r.Name(), + Stage: stage.RefID, + Message: fmt.Sprintf("..."), + // Severity: "warning", // omit for error (the default) + }) + } + } + return violations +} +``` + +Set `Severity: "warning"` for advisory checks; omit it for hard errors. Errors cause a non-zero exit code and map to `"error"` level in SARIF output; warnings map to `"warning"` level. + +### 2. Register the rule + +Add your rule to `pkg/rules/registry.go`: + +```go +var All = []Rule{ + RequiredStageFields{}, + BrokenRequisiteRefs{}, + DuplicateRefIDs{}, + CircularDependencies{}, + OrphanedStages{}, + YourRule{}, // add here +} +``` + +Rules run in the order listed. + +### 3. Add a schema field (if needed) + +If your rule needs data not yet modelled, add it to `pkg/schema/pipeline.go`: + +```go +type Stage struct { + // ...existing fields... + YourNewField bool `json:"yourNewField"` +} +``` + +### 4. Write tests + +Create `pkg/rules/_test.go` using the `rules_test` package. See existing `*_test.go` files for the pattern. At minimum test the happy path (no violations) and one violation case. + +### 5. Update the README + +Add your rule to the **Rules** section of `README.md` following the same format as existing rules. + +--- + +## Development workflow + +```bash +make lint && make test +``` + +Requirements: Go 1.22+, `golangci-lint` (for the `lint` target only). diff --git a/README.md b/README.md index ce004e6..d4911d1 100644 --- a/README.md +++ b/README.md @@ -232,6 +232,26 @@ Self-loops (`requisiteStageRefIds: ["1"]` on stage `"1"`) are also detected. --- +### `orphaned-stages` _(warning)_ + +Flags stages that are completely disconnected from the pipeline graph: they have no prerequisites of their own (`requisiteStageRefIds` is empty) **and** no other stage depends on them. These are isolated islands that Spinnaker will run unconditionally in parallel with the rest of the pipeline, which is usually unintentional. + +This rule does **not** flag valid terminal stages (stages at the end of a dependency chain that nothing else depends on) — those have prerequisites and are a normal part of pipeline design. + +Stages with an empty `refId` are skipped (caught by `required-stage-fields` instead). + +**Checks:** every stage with an empty `requisiteStageRefIds` is depended on by at least one other stage. + +**Severity:** warning — violations appear as `[warn: orphaned-stages]` in text output and as `"level": "warning"` in SARIF. + +**Example violation:** + +``` +pipelines/deploy.json: [warn: orphaned-stages] stage "20" is an orphaned island: nothing depends on it and it has no prerequisites +``` + +--- + ## Exit Codes | Code | Meaning | @@ -251,6 +271,9 @@ spinlint validate 'pipelines/*.json' || exit 1 ``` spinlint/ +├── .github/ +│ └── workflows/ +│ └── ci.yml # CI: lint + test + SARIF upload on push/PR ├── cmd/ │ └── spinlint/ │ └── main.go # CLI entry point (cobra root + validate command) @@ -263,16 +286,19 @@ spinlint/ │ │ ├── required_fields.go │ │ ├── requisite_refs.go │ │ ├── duplicate_ref_ids.go -│ │ └── circular_deps.go +│ │ ├── circular_deps.go +│ │ └── orphaned_stages.go │ ├── loader/ │ │ └── loader.go # Glob expansion and JSON file loading │ └── reporter/ -│ └── reporter.go # Text and JSON output formatting +│ └── reporter.go # Text, JSON, and SARIF output formatting ├── testdata/ │ ├── valid.json # A well-formed 3-stage pipeline -│ └── invalid.json # Pipeline with violations across multiple rules +│ ├── invalid.json # Pipeline with violations across multiple rules +│ └── complex.json # 20-stage pipeline hitting all 5 rules ├── go.mod ├── Makefile +├── CONTRIBUTING.md └── README.md ``` diff --git a/pkg/reporter/reporter.go b/pkg/reporter/reporter.go index 6d59c37..5e13056 100644 --- a/pkg/reporter/reporter.go +++ b/pkg/reporter/reporter.go @@ -36,7 +36,11 @@ func (r *Reporter) Add(file string, violations []rules.Violation) { return } for _, v := range violations { - fmt.Fprintf(r.out, "%s: [%s] %s\n", file, v.Rule, v.Message) + if v.Severity == "warning" { + fmt.Fprintf(r.out, "%s: [warn: %s] %s\n", file, v.Rule, v.Message) + } else { + fmt.Fprintf(r.out, "%s: [%s] %s\n", file, v.Rule, v.Message) + } } return } @@ -130,9 +134,13 @@ func (r *Reporter) flushSARIF() error { ShortDescription: sarifMessage{Text: v.Rule}, }) } + level := v.Severity + if level == "" { + level = "error" + } results = append(results, sarifResult{ RuleID: v.Rule, - Level: "error", + Level: level, Message: sarifMessage{Text: v.Message}, Locations: []sarifLocation{ { diff --git a/pkg/rules/orphaned_stages.go b/pkg/rules/orphaned_stages.go new file mode 100644 index 0000000..2e92406 --- /dev/null +++ b/pkg/rules/orphaned_stages.go @@ -0,0 +1,43 @@ +package rules + +import ( + "fmt" + + "github.com/jakeva/spinlint/pkg/schema" +) + +// OrphanedStages flags stages that are completely disconnected from the pipeline +// graph: no other stage depends on them AND they have no prerequisites of their own. +// These are isolated islands that Spinnaker will execute independently, which is +// usually unintentional. Severity: warning. +type OrphanedStages struct{} + +func (r OrphanedStages) Name() string { return "orphaned-stages" } + +func (r OrphanedStages) Check(pipeline schema.Pipeline) []Violation { + // Build the set of refIds that at least one other stage depends on. + dependedOn := make(map[string]bool) + for _, stage := range pipeline.Stages { + for _, dep := range stage.RequisiteStageRefIds { + dependedOn[dep] = true + } + } + + var violations []Violation + for _, stage := range pipeline.Stages { + if stage.RefID == "" { + continue // empty refId is caught by required-stage-fields + } + // A stage is an orphaned island if nothing depends on it AND it has no + // prerequisites of its own — i.e. it is completely disconnected. + if !dependedOn[stage.RefID] && len(stage.RequisiteStageRefIds) == 0 { + violations = append(violations, Violation{ + Rule: r.Name(), + Stage: stage.RefID, + Severity: "warning", + Message: fmt.Sprintf("stage %q is an orphaned island: nothing depends on it and it has no prerequisites", stage.RefID), + }) + } + } + return violations +} diff --git a/pkg/rules/orphaned_stages_test.go b/pkg/rules/orphaned_stages_test.go new file mode 100644 index 0000000..58a7793 --- /dev/null +++ b/pkg/rules/orphaned_stages_test.go @@ -0,0 +1,141 @@ +package rules_test + +import ( + "testing" + + "github.com/jakeva/spinlint/pkg/rules" + "github.com/jakeva/spinlint/pkg/schema" +) + +func TestOrphanedStages_NoOrphans(t *testing.T) { + // Linear chain: 1 ← 2. Stage 1 is a root (depended on), stage 2 is a leaf (has prereqs). + pipeline := schema.Pipeline{ + Stages: []schema.Stage{ + {Type: "wait", Name: "A", RefID: "1", RequisiteStageRefIds: []string{}}, + {Type: "wait", Name: "B", RefID: "2", RequisiteStageRefIds: []string{"1"}}, + }, + } + v := rules.OrphanedStages{}.Check(pipeline) + if len(v) != 0 { + t.Errorf("expected no violations, got %d: %v", len(v), v) + } +} + +func TestOrphanedStages_OrphanedIsland(t *testing.T) { + // Stage 3 has no prerequisites and nothing depends on it — orphaned island. + pipeline := schema.Pipeline{ + Stages: []schema.Stage{ + {Type: "wait", Name: "A", RefID: "1", RequisiteStageRefIds: []string{}}, + {Type: "wait", Name: "B", RefID: "2", RequisiteStageRefIds: []string{"1"}}, + {Type: "wait", Name: "C", RefID: "3", RequisiteStageRefIds: []string{}}, + }, + } + v := rules.OrphanedStages{}.Check(pipeline) + if len(v) != 1 { + t.Fatalf("expected 1 violation, got %d: %v", len(v), v) + } + if v[0].Stage != "3" { + t.Errorf("expected Stage %q, got %q", "3", v[0].Stage) + } + if v[0].Severity != "warning" { + t.Errorf("expected Severity %q, got %q", "warning", v[0].Severity) + } +} + +func TestOrphanedStages_TerminalStageNotFlagged(t *testing.T) { + // Stage 2 is a leaf node (nothing depends on it) but has prerequisites — valid terminal stage. + pipeline := schema.Pipeline{ + Stages: []schema.Stage{ + {Type: "wait", Name: "A", RefID: "1", RequisiteStageRefIds: []string{}}, + {Type: "wait", Name: "B", RefID: "2", RequisiteStageRefIds: []string{"1"}}, + }, + } + v := rules.OrphanedStages{}.Check(pipeline) + if len(v) != 0 { + t.Errorf("terminal stage should not be flagged, got %d: %v", len(v), v) + } +} + +func TestOrphanedStages_SkipsEmptyRefID(t *testing.T) { + // A stage with no refId is skipped — caught by required-stage-fields, not us. + pipeline := schema.Pipeline{ + Stages: []schema.Stage{ + {Type: "wait", Name: "A", RefID: "", RequisiteStageRefIds: []string{}}, + }, + } + v := rules.OrphanedStages{}.Check(pipeline) + if len(v) != 0 { + t.Errorf("stage with empty refId should be skipped, got %d: %v", len(v), v) + } +} + +func TestOrphanedStages_MultipleOrphans(t *testing.T) { + // Two disconnected islands alongside a valid chain. + pipeline := schema.Pipeline{ + Stages: []schema.Stage{ + {Type: "wait", Name: "Chain start", RefID: "1", RequisiteStageRefIds: []string{}}, + {Type: "wait", Name: "Chain end", RefID: "2", RequisiteStageRefIds: []string{"1"}}, + {Type: "wait", Name: "Orphan A", RefID: "3", RequisiteStageRefIds: []string{}}, + {Type: "wait", Name: "Orphan B", RefID: "4", RequisiteStageRefIds: []string{}}, + }, + } + v := rules.OrphanedStages{}.Check(pipeline) + if len(v) != 2 { + t.Errorf("expected 2 violations, got %d: %v", len(v), v) + } +} + +func TestOrphanedStages_ComplexTopology(t *testing.T) { + // Pipeline layout: + // + // Diamond subgraph: + // "1" ──► "2" ──► "4" ──► "5" (terminal — has prereqs, not orphaned) + // "1" ──► "3" ──► "4" + // + // Separate independent chain: + // "6" ──► "7" (root "6" is depended-on; terminal "7" has prereqs) + // + // Orphaned islands (no prereqs, nothing depends on them): + // "8", "9" + // + // Stage with empty refId — must be skipped. + // + // Expected: exactly 2 warnings, for stages "8" and "9". + pipeline := schema.Pipeline{ + Stages: []schema.Stage{ + {Type: "jenkins", Name: "Build", RefID: "1", RequisiteStageRefIds: []string{}}, + {Type: "deploy", Name: "Deploy canary", RefID: "2", RequisiteStageRefIds: []string{"1"}}, + {Type: "deploy", Name: "Deploy baseline", RefID: "3", RequisiteStageRefIds: []string{"1"}}, + {Type: "manualJudgment", Name: "Approve", RefID: "4", RequisiteStageRefIds: []string{"2", "3"}}, + {Type: "deploy", Name: "Deploy prod", RefID: "5", RequisiteStageRefIds: []string{"4"}}, + {Type: "bake", Name: "Bake image", RefID: "6", RequisiteStageRefIds: []string{}}, + {Type: "deploy", Name: "Deploy staging", RefID: "7", RequisiteStageRefIds: []string{"6"}}, + {Type: "wait", Name: "Stale wait", RefID: "8", RequisiteStageRefIds: []string{}}, + {Type: "webhook", Name: "Dead webhook", RefID: "9", RequisiteStageRefIds: []string{}}, + {Type: "wait", Name: "No refId", RefID: "", RequisiteStageRefIds: []string{}}, + }, + } + + v := rules.OrphanedStages{}.Check(pipeline) + if len(v) != 2 { + t.Fatalf("expected 2 violations, got %d: %v", len(v), v) + } + + orphaned := map[string]bool{} + for _, viol := range v { + orphaned[viol.Stage] = true + if viol.Severity != "warning" { + t.Errorf("stage %q: expected severity %q, got %q", viol.Stage, "warning", viol.Severity) + } + } + for _, id := range []string{"8", "9"} { + if !orphaned[id] { + t.Errorf("expected stage %q to be flagged as orphaned", id) + } + } + for _, id := range []string{"1", "2", "3", "4", "5", "6", "7"} { + if orphaned[id] { + t.Errorf("stage %q should not be flagged as orphaned", id) + } + } +} diff --git a/pkg/rules/registry.go b/pkg/rules/registry.go index 87430fd..963f6af 100644 --- a/pkg/rules/registry.go +++ b/pkg/rules/registry.go @@ -6,4 +6,5 @@ var All = []Rule{ BrokenRequisiteRefs{}, DuplicateRefIDs{}, CircularDependencies{}, + OrphanedStages{}, } diff --git a/pkg/rules/rule.go b/pkg/rules/rule.go index aebf7da..0cf40ac 100644 --- a/pkg/rules/rule.go +++ b/pkg/rules/rule.go @@ -4,9 +4,10 @@ import "github.com/jakeva/spinlint/pkg/schema" // Violation describes a single rule failure found in a pipeline. type Violation struct { - Rule string `json:"rule"` - Stage string `json:"stage,omitempty"` // refId or "index N" if refId is absent - Message string `json:"message"` + Rule string `json:"rule"` + Stage string `json:"stage,omitempty"` // refId or "index N" if refId is absent + Message string `json:"message"` + Severity string `json:"severity,omitempty"` // "warning" or "" (treated as "error") } // Rule is implemented by every lint check. diff --git a/testdata/complex.json b/testdata/complex.json new file mode 100644 index 0000000..60e2a7c --- /dev/null +++ b/testdata/complex.json @@ -0,0 +1,31 @@ +{ + "name": "complex-multi-cloud-deploy", + "application": "payments-service", + "limitConcurrent": true, + "keepWaitingPipelines": false, + "stages": [ + {"type": "jenkins", "name": "Run integration tests", "refId": "1", "requisiteStageRefIds": []}, + {"type": "bake", "name": "Bake AMI", "refId": "2", "requisiteStageRefIds": ["1"]}, + {"type": "deploy", "name": "Deploy to staging", "refId": "3", "requisiteStageRefIds": ["2"]}, + { "refId": "4", "requisiteStageRefIds": ["3"]}, + {"type": "manualJudgment", "name": "Approve prod rollout", "refId": "5", "requisiteStageRefIds": ["4"]}, + {"type": "deploy", "name": "Deploy to prod us-east-1", "refId": "6", "requisiteStageRefIds": ["5"]}, + {"type": "deploy", "name": "Deploy to prod us-west-2", "refId": "6", "requisiteStageRefIds": ["5"]}, + {"type": "webhook", "name": "Notify PagerDuty", "refId": "8", "requisiteStageRefIds": ["6", "7"]}, + {"type": "wait", "name": "Wait for DNS propagation", "refId": "9", "requisiteStageRefIds": ["8"]}, + {"type": "checkPreconditions", "name": "Validate health checks","refId": "10", "requisiteStageRefIds": ["9", "99"]}, + {"type": "runJob", "name": "Run smoke tests", "refId": "11", "requisiteStageRefIds": ["10"]}, + {"type": "scaleDown","name": "Scale down canary", "refId": "12", "requisiteStageRefIds": ["11"]}, + {"type": "enableServerGroup", "name": "Enable new server group", "refId": "13", "requisiteStageRefIds": ["12"]}, + {"type": "disableServerGroup", "name": "Disable old server group", "refId": "14", "requisiteStageRefIds": ["13"]}, + {"type": "destroyServerGroup", "name": "Destroy old server group", "refId": "15", "requisiteStageRefIds": ["16"]}, + {"type": "findImage","name": "Find latest approved image", "refId": "16", "requisiteStageRefIds": ["15"]}, + {"type": "pipeline", "name": "Trigger rollback pipeline", "refId": "17", "requisiteStageRefIds": ["14"]}, + {"type": "jenkins", "refId": "18", "requisiteStageRefIds": ["17"]}, + {"type": "deploy", "name": "", "refId": "19", "requisiteStageRefIds": ["18"]}, + {"type": "wait", "name": "Orphaned stage - no one depends on this", "refId": "20", "requisiteStageRefIds": []} + ], + "triggers": [], + "parameters": [], + "notifications": [] +}