From 3a3280d1e8aaac34101b2e8d93389a1beead6067 Mon Sep 17 00:00:00 2001
From: caretak3r <50377477+caretak3r@users.noreply.github.com>
Date: Wed, 18 Feb 2026 22:42:38 -0500
Subject: [PATCH] feat(spec): Task 10 - lint rules for HIP-0025 sequencing

Add Sequencing lint rule to pkg/chart/v2/lint/rules/sequencing.go:
- Circular subchart dependency detection (ErrorSev)
- Partial readiness annotation check - only one of
  readiness-success/readiness-failure present (ErrorSev)
- Resource depends-on referencing non-existent resource-group (WarningSev)

Register Sequencing in RunAll (lint.go).
---
 pkg/chart/v2/lint/lint.go                     |   1 +
 pkg/chart/v2/lint/rules/sequencing.go         | 173 ++++++++++++++++++
 pkg/chart/v2/lint/rules/sequencing_test.go    | 134 ++++++++++++++
 .../sequencing-orphan-group/Chart.yaml        |   4 +
 .../templates/configmap.yaml                  |   9 +
 .../sequencing-partial-readiness/Chart.yaml   |   4 +
 .../templates/configmap.yaml                  |   8 +
 7 files changed, 333 insertions(+)
 create mode 100644 pkg/chart/v2/lint/rules/sequencing.go
 create mode 100644 pkg/chart/v2/lint/rules/sequencing_test.go
 create mode 100644 pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/Chart.yaml
 create mode 100644 pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/templates/configmap.yaml
 create mode 100644 pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/Chart.yaml
 create mode 100644 pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/templates/configmap.yaml

diff --git a/pkg/chart/v2/lint/lint.go b/pkg/chart/v2/lint/lint.go
index 1c871d936..affb7882d 100644
--- a/pkg/chart/v2/lint/lint.go
+++ b/pkg/chart/v2/lint/lint.go
@@ -66,6 +66,7 @@ func RunAll(baseDir string, values map[string]interface{}, namespace string, opt
 		rules.TemplateLinterSkipSchemaValidation(lo.SkipSchemaValidation))
 	rules.Dependencies(&result)
 	rules.Crds(&result)
+	rules.Sequencing(&result, namespace, values)
 
 	return result
 }
diff --git a/pkg/chart/v2/lint/rules/sequencing.go b/pkg/chart/v2/lint/rules/sequencing.go
new file mode 100644
index 000000000..916d72052
--- /dev/null
+++ b/pkg/chart/v2/lint/rules/sequencing.go
@@ -0,0 +1,173 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rules // import "helm.sh/helm/v4/pkg/chart/v2/lint/rules"
+
+import (
+	"fmt"
+	"path"
+	"strings"
+
+	"sigs.k8s.io/yaml"
+
+	chart "helm.sh/helm/v4/pkg/chart/v2"
+	"helm.sh/helm/v4/pkg/chart/common"
+	commonutil "helm.sh/helm/v4/pkg/chart/common/util"
+	"helm.sh/helm/v4/pkg/chart/v2/lint/support"
+	"helm.sh/helm/v4/pkg/chart/v2/loader"
+	chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
+	"helm.sh/helm/v4/pkg/engine"
+	releaseutil "helm.sh/helm/v4/pkg/release/v1/util"
+)
+
+const (
+	annotationReadinessSuccess = "helm.sh/readiness-success"
+	annotationReadinessFailure = "helm.sh/readiness-failure"
+)
+
+// Sequencing runs lint rules for HIP-0025 sequencing annotations.
+//
+// It checks for:
+//   - Circular dependencies in subchart ordering (error)
+//   - Partial readiness annotations (only one of readiness-success/readiness-failure) (error)
+//   - Resource-group depends-on referencing a non-existent group (warning)
+func Sequencing(linter *support.Linter, namespace string, values map[string]interface{}) {
+	c, err := loader.LoadDir(linter.ChartDir)
+	if err != nil {
+		// Chart load errors are already reported by other lint rules (Chartfile, Dependencies).
+		// Silently skip sequencing checks rather than producing duplicate messages.
+		return
+	}
+
+	// Check subchart circular dependencies.
+	linter.RunLinterRule(support.ErrorSev, linter.ChartDir, validateSubchartSequencing(c))
+
+	// Check resource annotation issues via rendered templates.
+	validateRenderedSequencingAnnotations(linter, c, namespace, values)
+}
+
+// validateSubchartSequencing checks for circular dependencies in subchart ordering.
+//
+// All dependencies are treated as enabled since conditions and tags cannot be
+// evaluated at lint time (they depend on runtime values).
+func validateSubchartSequencing(c *chart.Chart) error {
+	if c.Metadata == nil || len(c.Metadata.Dependencies) == 0 {
+		return nil
+	}
+
+	// In lint context, conditions/tags cannot be evaluated. Treat all deps as
+	// enabled to catch cycles that would manifest with any values configuration.
+	for _, dep := range c.Metadata.Dependencies {
+		dep.Enabled = true
+	}
+
+	dag, err := chartutil.BuildSubchartDAG(c)
+	if err != nil {
+		return err
+	}
+	if _, err := dag.GetBatches(); err != nil {
+		return fmt.Errorf("subchart circular dependency detected: %w", err)
+	}
+	return nil
+}
+
+// validateRenderedSequencingAnnotations renders templates and checks sequencing
+// annotation correctness:
+//   - Only one of readiness-success/readiness-failure present → error
+//   - depends-on/resource-groups referencing non-existent group → warning
+func validateRenderedSequencingAnnotations(linter *support.Linter, c *chart.Chart, namespace string, values map[string]interface{}) {
+	if err := chartutil.ProcessDependencies(c, values); err != nil {
+		return
+	}
+
+	opts := common.ReleaseOptions{
+		Name:      "test-release",
+		Namespace: namespace,
+	}
+	caps := common.DefaultCapabilities.Copy()
+
+	cvals, err := commonutil.CoalesceValues(c, values)
+	if err != nil {
+		return
+	}
+
+	valuesToRender, err := commonutil.ToRenderValues(c, cvals, opts, caps)
+	if err != nil {
+		return
+	}
+
+	var e engine.Engine
+	e.LintMode = true
+	renderedContentMap, err := e.Render(c, valuesToRender)
+	if err != nil {
+		// Template rendering errors are already caught by the Templates lint rule.
+		return
+	}
+
+	// Collect all rendered YAML across templates.
+	var allContent strings.Builder
+	for _, t := range c.Templates {
+		content := renderedContentMap[path.Join(c.Name(), t.Name)]
+		if strings.TrimSpace(content) != "" {
+			allContent.WriteString(content)
+			allContent.WriteString("\n")
+		}
+	}
+
+	// Parse rendered manifests into Manifest structs for annotation checking.
+	rawManifests := releaseutil.SplitManifests(allContent.String())
+	var manifests []releaseutil.Manifest
+	for _, raw := range rawManifests {
+		if strings.TrimSpace(raw) == "" {
+			continue
+		}
+		var head releaseutil.SimpleHead
+		if err := yaml.Unmarshal([]byte(raw), &head); err != nil {
+			continue
+		}
+		name := ""
+		if head.Metadata != nil {
+			name = head.Metadata.Name
+		}
+		manifests = append(manifests, releaseutil.Manifest{
+			Name:    name,
+			Content: raw,
+			Head:    &head,
+		})
+	}
+
+	// Check partial readiness annotations.
+	for _, m := range manifests {
+		if m.Head == nil || m.Head.Metadata == nil {
+			continue
+		}
+		ann := m.Head.Metadata.Annotations
+		_, hasSuccess := ann[annotationReadinessSuccess]
+		_, hasFailure := ann[annotationReadinessFailure]
+		if hasSuccess != hasFailure {
+			linter.RunLinterRule(support.ErrorSev, linter.ChartDir,
+				fmt.Errorf("resource %q has only one of %q / %q annotations; both must be present or absent together",
+					m.Head.Metadata.Name, annotationReadinessSuccess, annotationReadinessFailure))
+		}
+	}
+
+	// Check resource-group depends-on references via ParseResourceGroups.
+	// Warnings indicate references to non-existent groups.
+	_, warnings := releaseutil.ParseResourceGroups(manifests)
+	for _, w := range warnings {
+		linter.RunLinterRule(support.WarningSev, linter.ChartDir, fmt.Errorf("%s", w))
+	}
+}
diff --git a/pkg/chart/v2/lint/rules/sequencing_test.go b/pkg/chart/v2/lint/rules/sequencing_test.go
new file mode 100644
index 000000000..33ff0aa50
--- /dev/null
+++ b/pkg/chart/v2/lint/rules/sequencing_test.go
@@ -0,0 +1,134 @@
+/*
+Copyright The Helm Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package rules
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	chart "helm.sh/helm/v4/pkg/chart/v2"
+	"helm.sh/helm/v4/pkg/chart/v2/lint/support"
+	chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
+)
+
+func TestSequencing_SubchartCircularDep(t *testing.T) {
+	tmp := t.TempDir()
+	c := &chart.Chart{
+		Metadata: &chart.Metadata{
+			Name:       "testchart",
+			Version:    "0.1.0",
+			APIVersion: "v2",
+			Dependencies: []*chart.Dependency{
+				{Name: "subchart-a", DependsOn: []string{"subchart-b"}},
+				{Name: "subchart-b", DependsOn: []string{"subchart-a"}},
+			},
+		},
+	}
+	if err := chartutil.SaveDir(c, tmp); err != nil {
+		t.Fatalf("SaveDir: %v", err)
+	}
+
+	linter := support.Linter{ChartDir: filepath.Join(tmp, "testchart")}
+	Sequencing(&linter, "testns", nil)
+
+	// Expect at least one ErrorSev message about circular dependency
+	found := false
+	for _, msg := range linter.Messages {
+		if msg.Severity == support.ErrorSev && strings.Contains(msg.Err.Error(), "circular") {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("expected circular dependency error, got messages: %v", linter.Messages)
+	}
+}
+
+func TestSequencing_SubchartNoDeps(t *testing.T) {
+	tmp := t.TempDir()
+	c := &chart.Chart{
+		Metadata: &chart.Metadata{
+			Name:       "testchart",
+			Version:    "0.1.0",
+			APIVersion: "v2",
+			Dependencies: []*chart.Dependency{
+				{Name: "subchart-a"},
+				{Name: "subchart-b"},
+			},
+		},
+	}
+	if err := chartutil.SaveDir(c, tmp); err != nil {
+		t.Fatalf("SaveDir: %v", err)
+	}
+
+	linter := support.Linter{ChartDir: filepath.Join(tmp, "testchart")}
+	Sequencing(&linter, "testns", nil)
+
+	// No circular dependency errors expected
+	for _, msg := range linter.Messages {
+		if msg.Severity == support.ErrorSev && strings.Contains(msg.Err.Error(), "circular") {
+			t.Errorf("unexpected circular dependency error: %v", msg)
+		}
+	}
+}
+
+func TestSequencing_PartialReadinessAnnotation(t *testing.T) {
+	linter := support.Linter{ChartDir: "./testdata/sequencing-partial-readiness"}
+	Sequencing(&linter, "testns", nil)
+
+	// Expect an error about partial readiness annotation
+	found := false
+	for _, msg := range linter.Messages {
+		if msg.Severity == support.ErrorSev && strings.Contains(msg.Err.Error(), "readiness") {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("expected partial readiness annotation error, got messages: %v", linter.Messages)
+	}
+}
+
+func TestSequencing_OrphanResourceGroup(t *testing.T) {
+	linter := support.Linter{ChartDir: "./testdata/sequencing-orphan-group"}
+	Sequencing(&linter, "testns", nil)
+
+	// Expect a warning about non-existent group reference
+	found := false
+	for _, msg := range linter.Messages {
+		if msg.Severity == support.WarningSev {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("expected warning about non-existent group reference, got messages: %v", linter.Messages)
+	}
+}
+
+func TestSequencing_ValidChart(t *testing.T) {
+	linter := support.Linter{ChartDir: "./testdata/albatross"}
+	Sequencing(&linter, "testns", map[string]interface{}{"nameOverride": "", "httpPort": 80})
+
+	// Should produce no ErrorSev messages for sequencing issues
+	for _, msg := range linter.Messages {
+		if msg.Severity == support.ErrorSev {
+			t.Errorf("unexpected error on valid chart: %v", msg)
+		}
+	}
+}
diff --git a/pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/Chart.yaml b/pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/Chart.yaml
new file mode 100644
index 000000000..d01da0e26
--- /dev/null
+++ b/pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/Chart.yaml
@@ -0,0 +1,4 @@
+apiVersion: v2
+name: sequencing-orphan-group
+description: Chart with resource referencing non-existent group
+version: 0.1.0
diff --git a/pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/templates/configmap.yaml b/pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/templates/configmap.yaml
new file mode 100644
index 000000000..0363ffc62
--- /dev/null
+++ b/pkg/chart/v2/lint/rules/testdata/sequencing-orphan-group/templates/configmap.yaml
@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: orphan-group-resource
+  annotations:
+    helm.sh/resource-group: mygroup
+    helm.sh/depends-on/resource-groups: '["nonexistent-group"]'
+data:
+  key: value
diff --git a/pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/Chart.yaml b/pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/Chart.yaml
new file mode 100644
index 000000000..2a53e1d50
--- /dev/null
+++ b/pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/Chart.yaml
@@ -0,0 +1,4 @@
+apiVersion: v2
+name: sequencing-partial-readiness
+description: Chart with partial readiness annotation for lint testing
+version: 0.1.0
diff --git a/pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/templates/configmap.yaml b/pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/templates/configmap.yaml
new file mode 100644
index 000000000..24a557b2d
--- /dev/null
+++ b/pkg/chart/v2/lint/rules/testdata/sequencing-partial-readiness/templates/configmap.yaml
@@ -0,0 +1,8 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: partial-readiness
+  annotations:
+    helm.sh/readiness-success: "{.status.ready} == true"
+data:
+  key: value