diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 000000000..4f9fcf20b --- /dev/null +++ b/PLAN.md @@ -0,0 +1,84 @@ +# PLAN.md — worktree/1: HIP-0025 Foundation (Chart Metadata + DAG) + +## Context + +HIP-0025 introduces native resource and subchart sequencing to Helm v4. +This worktree implements the **foundation layer**: chart metadata extensions +and DAG construction needed before action/CLI integration. + +**Spec source:** `hip-0025/hip-0025.md`, `hip-0025/subchart-sequencing.md` + +## Scope (this branch) + +### 1. Chart Metadata Extension — `DependsOn` field +- Add `DependsOn []string` to `Dependency` struct in both v2 and v3 +- Supports `depends-on` YAML key in Chart.yaml dependencies list +- Add validation: each DependsOn entry must reference a known dependency name/alias +- Sanitize DependsOn strings in `Validate()` + +**Files:** +- `pkg/chart/v2/dependency.go` — add field + validation +- `internal/chart/v3/dependency.go` — add field + validation + +### 2. Annotation Parsing — `helm.sh/depends-on/subcharts` +- Parse `helm.sh/depends-on/subcharts` from `Metadata.Annotations` +- Already available: `Annotations map[string]string` exists on both v2/v3 Metadata +- Utility function to extract and parse the annotation value (JSON array of strings) + +**Files:** +- New: `pkg/chart/v2/util/sequencing.go` — annotation constants, parse helpers +- New: `internal/chart/v3/util/sequencing.go` — v3 variant + +### 3. DAG Construction & Topological Sort +- Build subchart dependency graph from: + - `Dependency.DependsOn` field entries + - `Metadata.Annotations["helm.sh/depends-on/subcharts"]` entries +- Topological sort → produce ordered batches (layers) +- Circular dependency detection with clear error messages +- Orphaned subchart handling (no deps → deployed with parent in final batch) + +**Files:** +- New: `pkg/chart/v2/util/dag.go` — DAG struct, AddNode, AddEdge, TopologicalSort, DetectCycles +- New: `internal/chart/v3/util/dag.go` — v3 variant (or shared via common) + +### 4. Integration with ProcessDependencies +- After existing enable/disable logic, build subchart DAG +- Validate DAG (no cycles, all DependsOn refs resolve) +- Attach DAG to chart processing result for downstream use + +**Files:** +- `pkg/chart/v2/util/dependencies.go` — add DAG building after ProcessDependencies +- `internal/chart/v3/util/dependencies.go` — same + +### 5. Tests +- Unit tests for DependsOn parsing and validation +- Unit tests for DAG construction (linear, diamond, parallel, orphan patterns) +- Unit tests for circular dependency detection +- Unit tests for annotation parsing +- Integration test with chart fixtures + +**Files:** +- `pkg/chart/v2/util/dag_test.go` +- `pkg/chart/v2/util/sequencing_test.go` +- `pkg/chart/v2/dependency_test.go` (extend existing) + +## Verification + +```bash +make test-unit # Full unit test suite +go test ./pkg/chart/v2/... # Chart v2 tests +go test ./pkg/chart/v2/util/... # DAG + sequencing tests +go test ./internal/chart/v3/... # Chart v3 tests +make test-style # Linting +``` + +## bd Issues + +| ID | Phase | Status | +|----|-------|--------| +| helm-btq | Phase 2: Chart Metadata | in-progress | +| helm-ed3 | Phase 3: DAG Construction | blocked-by helm-btq | +| helm-40n | Phase 4: WaitStrategy | future worktree | +| helm-fby | Phase 5: Action System | future worktree | +| helm-0d5 | Phase 6: CLI | future worktree | +| helm-7an | Phase 7: Release Storage | future worktree | diff --git a/internal/chart/v3/dependency.go b/internal/chart/v3/dependency.go index 50ee5552e..2b1df0b8a 100644 --- a/internal/chart/v3/dependency.go +++ b/internal/chart/v3/dependency.go @@ -47,6 +47,10 @@ type Dependency struct { ImportValues []any `json:"import-values,omitempty" yaml:"import-values,omitempty"` // Alias usable alias to be used for the chart Alias string `json:"alias,omitempty" yaml:"alias,omitempty"` + // DependsOn is a list of subchart names or aliases that must be + // fully deployed and ready before this dependency is installed. + // Used for subchart sequencing as defined in HIP-0025. + DependsOn []string `json:"depends-on,omitempty" yaml:"depends-on,omitempty"` } // Validate checks for common problems with the dependency datastructure in @@ -66,6 +70,9 @@ func (d *Dependency) Validate() error { if d.Alias != "" && !aliasNameFormat.MatchString(d.Alias) { return ValidationErrorf("dependency %q has disallowed characters in the alias", d.Name) } + for i := range d.DependsOn { + d.DependsOn[i] = sanitizeString(d.DependsOn[i]) + } return nil } diff --git a/internal/chart/v3/util/dag.go b/internal/chart/v3/util/dag.go new file mode 100644 index 000000000..da3d21578 --- /dev/null +++ b/internal/chart/v3/util/dag.go @@ -0,0 +1,243 @@ +/* +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 util + +import ( + "fmt" + "sort" + "strings" +) + +// DAG is a directed acyclic graph for dependency resolution. +// Nodes represent subcharts or resource-groups. Edges represent +// "must be ready before" relationships. +type DAG struct { + nodes map[string]bool + edges map[string]map[string]bool +} + +// NewDAG creates an empty DAG. +func NewDAG() *DAG { + return &DAG{ + nodes: make(map[string]bool), + edges: make(map[string]map[string]bool), + } +} + +// AddNode adds a node to the graph. Idempotent. +func (d *DAG) AddNode(name string) { + d.nodes[name] = true +} + +// AddEdge adds a directed edge: "from" must be ready before "to" can start. +func (d *DAG) AddEdge(from, to string) error { + if from == to { + return fmt.Errorf("self-dependency detected: %q depends on itself", from) + } + d.AddNode(from) + d.AddNode(to) + if d.edges[to] == nil { + d.edges[to] = make(map[string]bool) + } + d.edges[to][from] = true + return nil +} + +// DetectCycles checks for circular dependencies using DFS. +func (d *DAG) DetectCycles() error { + const ( + unvisited = 0 + visiting = 1 + visited = 2 + ) + + state := make(map[string]int, len(d.nodes)) + path := make([]string, 0) + + var visit func(node string) error + visit = func(node string) error { + state[node] = visiting + path = append(path, node) + + for dep := range d.edges[node] { + switch state[dep] { + case visiting: + cycleStart := -1 + for i, n := range path { + if n == dep { + cycleStart = i + break + } + } + cycle := append(path[cycleStart:], dep) + return fmt.Errorf("circular dependency detected: %s", strings.Join(cycle, " -> ")) + case unvisited: + if err := visit(dep); err != nil { + return err + } + } + } + + path = path[:len(path)-1] + state[node] = visited + return nil + } + + sorted := d.sortedNodes() + for _, node := range sorted { + if state[node] == unvisited { + if err := visit(node); err != nil { + return err + } + } + } + return nil +} + +// TopologicalSort returns nodes in dependency order. +func (d *DAG) TopologicalSort() ([]string, error) { + if err := d.DetectCycles(); err != nil { + return nil, err + } + + inDegree := make(map[string]int, len(d.nodes)) + for node := range d.nodes { + inDegree[node] = len(d.edges[node]) + } + + var queue []string + for node, deg := range inDegree { + if deg == 0 { + queue = append(queue, node) + } + } + sort.Strings(queue) + + var result []string + for len(queue) > 0 { + node := queue[0] + queue = queue[1:] + result = append(result, node) + + for downstream := range d.nodes { + if d.edges[downstream][node] { + inDegree[downstream]-- + if inDegree[downstream] == 0 { + queue = append(queue, downstream) + sort.Strings(queue) + } + } + } + } + + return result, nil +} + +// Batches returns nodes grouped by dependency level (topological layers). +func (d *DAG) Batches() ([][]string, error) { + if err := d.DetectCycles(); err != nil { + return nil, err + } + + if len(d.nodes) == 0 { + return nil, nil + } + + inDegree := make(map[string]int, len(d.nodes)) + for node := range d.nodes { + inDegree[node] = len(d.edges[node]) + } + + remaining := make(map[string]bool) + for node := range d.nodes { + remaining[node] = true + } + + var batches [][]string + for len(remaining) > 0 { + var batch []string + for node := range remaining { + if inDegree[node] == 0 { + batch = append(batch, node) + } + } + + if len(batch) == 0 { + return nil, fmt.Errorf("internal error: no nodes with in-degree 0 among remaining nodes") + } + + sort.Strings(batch) + batches = append(batches, batch) + + for _, node := range batch { + delete(remaining, node) + for downstream := range remaining { + if d.edges[downstream][node] { + inDegree[downstream]-- + } + } + } + } + + return batches, nil +} + +// Nodes returns all node names. +func (d *DAG) Nodes() []string { + return d.sortedNodes() +} + +// DependsOn returns the set of nodes that the given node depends on. +func (d *DAG) DependsOn(node string) []string { + deps := d.edges[node] + result := make([]string, 0, len(deps)) + for dep := range deps { + result = append(result, dep) + } + sort.Strings(result) + return result +} + +// Dependents returns the set of nodes that depend on the given node. +func (d *DAG) Dependents(node string) []string { + var result []string + for n, deps := range d.edges { + if deps[node] { + result = append(result, n) + } + } + sort.Strings(result) + return result +} + +// HasNode returns true if the node exists. +func (d *DAG) HasNode(name string) bool { + return d.nodes[name] +} + +// Len returns the number of nodes. +func (d *DAG) Len() int { + return len(d.nodes) +} + +func (d *DAG) sortedNodes() []string { + result := make([]string, 0, len(d.nodes)) + for n := range d.nodes { + result = append(result, n) + } + sort.Strings(result) + return result +} diff --git a/internal/chart/v3/util/sequencing.go b/internal/chart/v3/util/sequencing.go new file mode 100644 index 000000000..98a14fe67 --- /dev/null +++ b/internal/chart/v3/util/sequencing.go @@ -0,0 +1,126 @@ +/* +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 util + +import ( + "encoding/json" + "fmt" + + chart "helm.sh/helm/v4/internal/chart/v3" +) + +const ( + // AnnotationDependsOnSubcharts is the Chart.yaml annotation key for + // declaring subchart dependencies that must be deployed and ready + // before the current chart's resources are installed. + AnnotationDependsOnSubcharts = "helm.sh/depends-on/subcharts" + + // AnnotationResourceGroup is the template annotation key for declaring + // the resource-group a resource belongs to. + AnnotationResourceGroup = "helm.sh/resource-group" + + // AnnotationDependsOnResourceGroups is the template annotation key for + // declaring resource-group dependencies. + AnnotationDependsOnResourceGroups = "helm.sh/depends-on/resource-groups" +) + +// ParseDependsOnSubcharts extracts the list of subchart names from the +// helm.sh/depends-on/subcharts annotation in Chart.yaml metadata. +// Returns nil if the annotation is not present. +func ParseDependsOnSubcharts(md *chart.Metadata) ([]string, error) { + if md == nil || md.Annotations == nil { + return nil, nil + } + raw, ok := md.Annotations[AnnotationDependsOnSubcharts] + if !ok || raw == "" { + return nil, nil + } + var names []string + if err := json.Unmarshal([]byte(raw), &names); err != nil { + return nil, fmt.Errorf("invalid %s annotation: %w", AnnotationDependsOnSubcharts, err) + } + return names, nil +} + +// BuildSubchartDAG constructs a dependency DAG for subcharts of the given chart. +// It combines dependencies declared via: +// - Dependency.DependsOn field entries in Chart.yaml dependencies +// - Metadata.Annotations["helm.sh/depends-on/subcharts"] for the parent chart +// +// Returns the DAG and any validation error (cycles, unknown references). +func BuildSubchartDAG(c *chart.Chart) (*DAG, error) { + dag := NewDAG() + + if c.Metadata == nil { + return dag, nil + } + + // Build a lookup of known dependency names/aliases + depNames := make(map[string]bool) + for _, dep := range c.Metadata.Dependencies { + key := dep.Name + if dep.Alias != "" { + key = dep.Alias + } + depNames[key] = true + dag.AddNode(key) + } + + // Add the parent chart node + dag.AddNode(c.Metadata.Name) + + // Process Dependency.DependsOn field entries + for _, dep := range c.Metadata.Dependencies { + key := dep.Name + if dep.Alias != "" { + key = dep.Alias + } + for _, upstream := range dep.DependsOn { + if !depNames[upstream] { + return nil, fmt.Errorf( + "dependency %q declares depends-on %q, but %q is not a known dependency", + key, upstream, upstream, + ) + } + if err := dag.AddEdge(upstream, key); err != nil { + return nil, err + } + } + } + + // Process helm.sh/depends-on/subcharts annotation on parent chart + annotationDeps, err := ParseDependsOnSubcharts(c.Metadata) + if err != nil { + return nil, err + } + for _, upstream := range annotationDeps { + if !depNames[upstream] { + return nil, fmt.Errorf( + "chart %q annotation %s references %q, but %q is not a known dependency", + c.Metadata.Name, AnnotationDependsOnSubcharts, upstream, upstream, + ) + } + if err := dag.AddEdge(upstream, c.Metadata.Name); err != nil { + return nil, err + } + } + + if err := dag.DetectCycles(); err != nil { + return nil, err + } + + return dag, nil +} diff --git a/pkg/chart/v2/dependency.go b/pkg/chart/v2/dependency.go index 8a590a036..ada149fbf 100644 --- a/pkg/chart/v2/dependency.go +++ b/pkg/chart/v2/dependency.go @@ -47,6 +47,10 @@ type Dependency struct { ImportValues []interface{} `json:"import-values,omitempty" yaml:"import-values,omitempty"` // Alias usable alias to be used for the chart Alias string `json:"alias,omitempty" yaml:"alias,omitempty"` + // DependsOn is a list of subchart names or aliases that must be + // fully deployed and ready before this dependency is installed. + // Used for subchart sequencing as defined in HIP-0025. + DependsOn []string `json:"depends-on,omitempty" yaml:"depends-on,omitempty"` } // Validate checks for common problems with the dependency datastructure in @@ -66,6 +70,9 @@ func (d *Dependency) Validate() error { if d.Alias != "" && !aliasNameFormat.MatchString(d.Alias) { return ValidationErrorf("dependency %q has disallowed characters in the alias", d.Name) } + for i := range d.DependsOn { + d.DependsOn[i] = sanitizeString(d.DependsOn[i]) + } return nil } diff --git a/pkg/chart/v2/util/dag.go b/pkg/chart/v2/util/dag.go new file mode 100644 index 000000000..713b419eb --- /dev/null +++ b/pkg/chart/v2/util/dag.go @@ -0,0 +1,260 @@ +/* +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 util + +import ( + "fmt" + "sort" + "strings" +) + +// DAG is a directed acyclic graph for dependency resolution. +// Nodes represent subcharts or resource-groups. Edges represent +// "must be ready before" relationships. +type DAG struct { + // nodes is the set of all node names in the graph. + nodes map[string]bool + // edges maps a node to the set of nodes it depends on (upstream). + // If edges["app"] = {"db": true}, then "db" must be ready before "app". + edges map[string]map[string]bool +} + +// NewDAG creates an empty DAG. +func NewDAG() *DAG { + return &DAG{ + nodes: make(map[string]bool), + edges: make(map[string]map[string]bool), + } +} + +// AddNode adds a node to the graph. Idempotent. +func (d *DAG) AddNode(name string) { + d.nodes[name] = true +} + +// AddEdge adds a directed edge: "from" must be ready before "to" can start. +// Both nodes are implicitly added if not present. +// Returns an error if from == to (self-cycle). +func (d *DAG) AddEdge(from, to string) error { + if from == to { + return fmt.Errorf("self-dependency detected: %q depends on itself", from) + } + d.AddNode(from) + d.AddNode(to) + if d.edges[to] == nil { + d.edges[to] = make(map[string]bool) + } + d.edges[to][from] = true + return nil +} + +// DetectCycles checks for circular dependencies using DFS. +// Returns a descriptive error if a cycle is found, nil otherwise. +func (d *DAG) DetectCycles() error { + const ( + unvisited = 0 + visiting = 1 + visited = 2 + ) + + state := make(map[string]int, len(d.nodes)) + path := make([]string, 0) + + var visit func(node string) error + visit = func(node string) error { + state[node] = visiting + path = append(path, node) + + for dep := range d.edges[node] { + switch state[dep] { + case visiting: + // Found cycle — build cycle description + cycleStart := -1 + for i, n := range path { + if n == dep { + cycleStart = i + break + } + } + cycle := append(path[cycleStart:], dep) + return fmt.Errorf("circular dependency detected: %s", strings.Join(cycle, " -> ")) + case unvisited: + if err := visit(dep); err != nil { + return err + } + } + } + + path = path[:len(path)-1] + state[node] = visited + return nil + } + + // Visit in sorted order for deterministic error messages + sorted := d.sortedNodes() + for _, node := range sorted { + if state[node] == unvisited { + if err := visit(node); err != nil { + return err + } + } + } + return nil +} + +// TopologicalSort returns nodes in dependency order: nodes with no +// dependencies come first. Nodes at the same level are sorted +// alphabetically for determinism. +func (d *DAG) TopologicalSort() ([]string, error) { + if err := d.DetectCycles(); err != nil { + return nil, err + } + + inDegree := make(map[string]int, len(d.nodes)) + for node := range d.nodes { + inDegree[node] = len(d.edges[node]) + } + + // Collect nodes with no incoming edges + var queue []string + for node, deg := range inDegree { + if deg == 0 { + queue = append(queue, node) + } + } + sort.Strings(queue) + + var result []string + for len(queue) > 0 { + node := queue[0] + queue = queue[1:] + result = append(result, node) + + // For each node that depends on this one, decrement in-degree + for downstream := range d.nodes { + if d.edges[downstream][node] { + inDegree[downstream]-- + if inDegree[downstream] == 0 { + queue = append(queue, downstream) + sort.Strings(queue) + } + } + } + } + + return result, nil +} + +// Batches returns nodes grouped by dependency level (topological layers). +// Each batch can be deployed in parallel; batches must be deployed sequentially. +// Batch 0 has no dependencies, batch 1 depends only on batch 0, etc. +func (d *DAG) Batches() ([][]string, error) { + if err := d.DetectCycles(); err != nil { + return nil, err + } + + if len(d.nodes) == 0 { + return nil, nil + } + + inDegree := make(map[string]int, len(d.nodes)) + for node := range d.nodes { + inDegree[node] = len(d.edges[node]) + } + + remaining := make(map[string]bool) + for node := range d.nodes { + remaining[node] = true + } + + var batches [][]string + for len(remaining) > 0 { + // Find all nodes with in-degree 0 among remaining + var batch []string + for node := range remaining { + if inDegree[node] == 0 { + batch = append(batch, node) + } + } + + if len(batch) == 0 { + // Should not happen after cycle check, but guard against it + return nil, fmt.Errorf("internal error: no nodes with in-degree 0 among remaining nodes") + } + + sort.Strings(batch) + batches = append(batches, batch) + + // Remove batch nodes and update in-degrees + for _, node := range batch { + delete(remaining, node) + for downstream := range remaining { + if d.edges[downstream][node] { + inDegree[downstream]-- + } + } + } + } + + return batches, nil +} + +// Nodes returns all node names in the DAG. +func (d *DAG) Nodes() []string { + return d.sortedNodes() +} + +// DependsOn returns the set of nodes that the given node depends on. +func (d *DAG) DependsOn(node string) []string { + deps := d.edges[node] + result := make([]string, 0, len(deps)) + for dep := range deps { + result = append(result, dep) + } + sort.Strings(result) + return result +} + +// Dependents returns the set of nodes that depend on the given node. +func (d *DAG) Dependents(node string) []string { + var result []string + for n, deps := range d.edges { + if deps[node] { + result = append(result, n) + } + } + sort.Strings(result) + return result +} + +// HasNode returns true if the node exists in the graph. +func (d *DAG) HasNode(name string) bool { + return d.nodes[name] +} + +// Len returns the number of nodes. +func (d *DAG) Len() int { + return len(d.nodes) +} + +func (d *DAG) sortedNodes() []string { + result := make([]string, 0, len(d.nodes)) + for n := range d.nodes { + result = append(result, n) + } + sort.Strings(result) + return result +} diff --git a/pkg/chart/v2/util/dag_test.go b/pkg/chart/v2/util/dag_test.go new file mode 100644 index 000000000..c66cac357 --- /dev/null +++ b/pkg/chart/v2/util/dag_test.go @@ -0,0 +1,174 @@ +/* +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 util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDAGEmpty(t *testing.T) { + dag := NewDAG() + assert.Equal(t, 0, dag.Len()) + + sorted, err := dag.TopologicalSort() + require.NoError(t, err) + assert.Empty(t, sorted) + + batches, err := dag.Batches() + require.NoError(t, err) + assert.Nil(t, batches) +} + +func TestDAGSingleNode(t *testing.T) { + dag := NewDAG() + dag.AddNode("nginx") + + assert.Equal(t, 1, dag.Len()) + assert.True(t, dag.HasNode("nginx")) + + sorted, err := dag.TopologicalSort() + require.NoError(t, err) + assert.Equal(t, []string{"nginx"}, sorted) + + batches, err := dag.Batches() + require.NoError(t, err) + assert.Equal(t, [][]string{{"nginx"}}, batches) +} + +func TestDAGLinearChain(t *testing.T) { + // A -> B -> C (C depends on B, B depends on A) + dag := NewDAG() + require.NoError(t, dag.AddEdge("A", "B")) + require.NoError(t, dag.AddEdge("B", "C")) + + sorted, err := dag.TopologicalSort() + require.NoError(t, err) + assert.Equal(t, []string{"A", "B", "C"}, sorted) + + batches, err := dag.Batches() + require.NoError(t, err) + assert.Equal(t, [][]string{{"A"}, {"B"}, {"C"}}, batches) +} + +func TestDAGDiamondDependency(t *testing.T) { + // HIP-0025 example: nginx + rabbitmq -> bar -> foo + dag := NewDAG() + require.NoError(t, dag.AddEdge("nginx", "bar")) + require.NoError(t, dag.AddEdge("rabbitmq", "bar")) + require.NoError(t, dag.AddEdge("bar", "foo")) + require.NoError(t, dag.AddEdge("rabbitmq", "foo")) + + sorted, err := dag.TopologicalSort() + require.NoError(t, err) + // nginx and rabbitmq have no deps, come first (alphabetical) + assert.Equal(t, "nginx", sorted[0]) + assert.Equal(t, "rabbitmq", sorted[1]) + // bar depends on both, comes next + assert.Equal(t, "bar", sorted[2]) + // foo depends on bar and rabbitmq + assert.Equal(t, "foo", sorted[3]) + + batches, err := dag.Batches() + require.NoError(t, err) + require.Len(t, batches, 3) + assert.Equal(t, []string{"nginx", "rabbitmq"}, batches[0]) + assert.Equal(t, []string{"bar"}, batches[1]) + assert.Equal(t, []string{"foo"}, batches[2]) +} + +func TestDAGParallelNoDeps(t *testing.T) { + dag := NewDAG() + dag.AddNode("a") + dag.AddNode("b") + dag.AddNode("c") + + batches, err := dag.Batches() + require.NoError(t, err) + require.Len(t, batches, 1) + assert.Equal(t, []string{"a", "b", "c"}, batches[0]) +} + +func TestDAGCircularDependency(t *testing.T) { + dag := NewDAG() + require.NoError(t, dag.AddEdge("A", "B")) + require.NoError(t, dag.AddEdge("B", "C")) + require.NoError(t, dag.AddEdge("C", "A")) + + err := dag.DetectCycles() + require.Error(t, err) + assert.Contains(t, err.Error(), "circular dependency detected") + + _, err = dag.TopologicalSort() + require.Error(t, err) + assert.Contains(t, err.Error(), "circular dependency detected") +} + +func TestDAGSelfDependency(t *testing.T) { + dag := NewDAG() + err := dag.AddEdge("A", "A") + require.Error(t, err) + assert.Contains(t, err.Error(), "self-dependency detected") +} + +func TestDAGDependsOnAndDependents(t *testing.T) { + dag := NewDAG() + require.NoError(t, dag.AddEdge("db", "app")) + require.NoError(t, dag.AddEdge("cache", "app")) + + deps := dag.DependsOn("app") + assert.Equal(t, []string{"cache", "db"}, deps) + + dependents := dag.Dependents("db") + assert.Equal(t, []string{"app"}, dependents) +} + +func TestDAGOrphanedNodes(t *testing.T) { + // Nodes with no edges should appear in first batch + dag := NewDAG() + dag.AddNode("orphan") + require.NoError(t, dag.AddEdge("A", "B")) + + batches, err := dag.Batches() + require.NoError(t, err) + require.Len(t, batches, 2) + // First batch: A and orphan (no deps) + assert.Equal(t, []string{"A", "orphan"}, batches[0]) + assert.Equal(t, []string{"B"}, batches[1]) +} + +func TestDAGComplexGraph(t *testing.T) { + // Complex HIP-0025 scenario: + // database and queue have no deps + // app depends on database and queue + // queue depends on another-group (missing, but we add it) + dag := NewDAG() + dag.AddNode("database") + dag.AddNode("queue") + dag.AddNode("another-group") + require.NoError(t, dag.AddEdge("another-group", "queue")) + require.NoError(t, dag.AddEdge("database", "app")) + require.NoError(t, dag.AddEdge("queue", "app")) + + batches, err := dag.Batches() + require.NoError(t, err) + require.Len(t, batches, 3) + assert.Equal(t, []string{"another-group", "database"}, batches[0]) + assert.Equal(t, []string{"queue"}, batches[1]) + assert.Equal(t, []string{"app"}, batches[2]) +} diff --git a/pkg/chart/v2/util/sequencing.go b/pkg/chart/v2/util/sequencing.go new file mode 100644 index 000000000..2878fd27c --- /dev/null +++ b/pkg/chart/v2/util/sequencing.go @@ -0,0 +1,129 @@ +/* +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 util + +import ( + "encoding/json" + "fmt" + + chart "helm.sh/helm/v4/pkg/chart/v2" +) + +const ( + // AnnotationDependsOnSubcharts is the Chart.yaml annotation key for + // declaring subchart dependencies that must be deployed and ready + // before the current chart's resources are installed. + AnnotationDependsOnSubcharts = "helm.sh/depends-on/subcharts" + + // AnnotationResourceGroup is the template annotation key for declaring + // the resource-group a resource belongs to. + AnnotationResourceGroup = "helm.sh/resource-group" + + // AnnotationDependsOnResourceGroups is the template annotation key for + // declaring resource-group dependencies. + AnnotationDependsOnResourceGroups = "helm.sh/depends-on/resource-groups" +) + +// ParseDependsOnSubcharts extracts the list of subchart names from the +// helm.sh/depends-on/subcharts annotation in Chart.yaml metadata. +// Returns nil if the annotation is not present. +func ParseDependsOnSubcharts(md *chart.Metadata) ([]string, error) { + if md == nil || md.Annotations == nil { + return nil, nil + } + raw, ok := md.Annotations[AnnotationDependsOnSubcharts] + if !ok || raw == "" { + return nil, nil + } + var names []string + if err := json.Unmarshal([]byte(raw), &names); err != nil { + return nil, fmt.Errorf("invalid %s annotation: %w", AnnotationDependsOnSubcharts, err) + } + return names, nil +} + +// BuildSubchartDAG constructs a dependency DAG for subcharts of the given chart. +// It combines dependencies declared via: +// - Dependency.DependsOn field entries in Chart.yaml dependencies +// - Metadata.Annotations["helm.sh/depends-on/subcharts"] for the parent chart +// +// Returns the DAG and any validation error (cycles, unknown references). +func BuildSubchartDAG(c *chart.Chart) (*DAG, error) { + dag := NewDAG() + + if c.Metadata == nil { + return dag, nil + } + + // Build a lookup of known dependency names/aliases + depNames := make(map[string]bool) + for _, dep := range c.Metadata.Dependencies { + key := dep.Name + if dep.Alias != "" { + key = dep.Alias + } + depNames[key] = true + dag.AddNode(key) + } + + // Add the parent chart node + dag.AddNode(c.Metadata.Name) + + // Process Dependency.DependsOn field entries + for _, dep := range c.Metadata.Dependencies { + key := dep.Name + if dep.Alias != "" { + key = dep.Alias + } + for _, upstream := range dep.DependsOn { + if !depNames[upstream] { + return nil, fmt.Errorf( + "dependency %q declares depends-on %q, but %q is not a known dependency", + key, upstream, upstream, + ) + } + // upstream must be ready before key can be installed + if err := dag.AddEdge(upstream, key); err != nil { + return nil, err + } + } + } + + // Process helm.sh/depends-on/subcharts annotation on parent chart + annotationDeps, err := ParseDependsOnSubcharts(c.Metadata) + if err != nil { + return nil, err + } + for _, upstream := range annotationDeps { + if !depNames[upstream] { + return nil, fmt.Errorf( + "chart %q annotation %s references %q, but %q is not a known dependency", + c.Metadata.Name, AnnotationDependsOnSubcharts, upstream, upstream, + ) + } + // upstream must be ready before parent chart resources + if err := dag.AddEdge(upstream, c.Metadata.Name); err != nil { + return nil, err + } + } + + // Validate no cycles + if err := dag.DetectCycles(); err != nil { + return nil, err + } + + return dag, nil +} diff --git a/pkg/chart/v2/util/sequencing_test.go b/pkg/chart/v2/util/sequencing_test.go new file mode 100644 index 000000000..3074d58bc --- /dev/null +++ b/pkg/chart/v2/util/sequencing_test.go @@ -0,0 +1,225 @@ +/* +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 util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + chart "helm.sh/helm/v4/pkg/chart/v2" +) + +func TestParseDependsOnSubcharts(t *testing.T) { + tests := []struct { + name string + metadata *chart.Metadata + expected []string + expectError bool + }{ + { + name: "nil metadata", + metadata: nil, + expected: nil, + }, + { + name: "no annotations", + metadata: &chart.Metadata{}, + expected: nil, + }, + { + name: "no depends-on annotation", + metadata: &chart.Metadata{ + Annotations: map[string]string{ + "other": "value", + }, + }, + expected: nil, + }, + { + name: "valid annotation", + metadata: &chart.Metadata{ + Annotations: map[string]string{ + AnnotationDependsOnSubcharts: `["bar", "rabbitmq"]`, + }, + }, + expected: []string{"bar", "rabbitmq"}, + }, + { + name: "empty array", + metadata: &chart.Metadata{ + Annotations: map[string]string{ + AnnotationDependsOnSubcharts: `[]`, + }, + }, + expected: []string{}, + }, + { + name: "invalid JSON", + metadata: &chart.Metadata{ + Annotations: map[string]string{ + AnnotationDependsOnSubcharts: `not-json`, + }, + }, + expectError: true, + }, + { + name: "empty string value", + metadata: &chart.Metadata{ + Annotations: map[string]string{ + AnnotationDependsOnSubcharts: "", + }, + }, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseDependsOnSubcharts(tt.metadata) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestBuildSubchartDAG(t *testing.T) { + tests := []struct { + name string + chart *chart.Chart + expectedBatch [][]string + expectError bool + errorContains string + }{ + { + name: "no dependencies", + chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "myapp", + }, + }, + expectedBatch: [][]string{{"myapp"}}, + }, + { + name: "HIP-0025 example: nginx+rabbitmq -> bar -> foo", + chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "foo", + Annotations: map[string]string{ + AnnotationDependsOnSubcharts: `["bar", "rabbitmq"]`, + }, + Dependencies: []*chart.Dependency{ + {Name: "nginx"}, + {Name: "rabbitmq"}, + { + Name: "bar", + DependsOn: []string{"nginx", "rabbitmq"}, + }, + }, + }, + }, + expectedBatch: [][]string{ + {"nginx", "rabbitmq"}, + {"bar"}, + {"foo"}, + }, + }, + { + name: "depends-on with alias", + chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "postgresql", Alias: "db"}, + { + Name: "app", + DependsOn: []string{"db"}, + }, + }, + }, + }, + expectedBatch: [][]string{ + {"db", "parent"}, + {"app"}, + }, + }, + { + name: "circular dependency via DependsOn", + chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "A", DependsOn: []string{"B"}}, + {Name: "B", DependsOn: []string{"A"}}, + }, + }, + }, + expectError: true, + errorContains: "circular dependency detected", + }, + { + name: "unknown dependency reference", + chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "app", DependsOn: []string{"nonexistent"}}, + }, + }, + }, + expectError: true, + errorContains: "not a known dependency", + }, + { + name: "unknown annotation reference", + chart: &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Annotations: map[string]string{ + AnnotationDependsOnSubcharts: `["ghost"]`, + }, + Dependencies: []*chart.Dependency{ + {Name: "app"}, + }, + }, + }, + expectError: true, + errorContains: "not a known dependency", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dag, err := BuildSubchartDAG(tt.chart) + if tt.expectError { + require.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + } else { + require.NoError(t, err) + batches, err := dag.Batches() + require.NoError(t, err) + assert.Equal(t, tt.expectedBatch, batches) + } + }) + } +}