From 5a77ecb40162dc83a16d3aa05fcf89c2e215c5bc Mon Sep 17 00:00:00 2001 From: caretak3r <50377477+caretak3r@users.noreply.github.com> Date: Wed, 18 Feb 2026 21:32:02 -0500 Subject: [PATCH] =?UTF-8?q?feat(spec):=20Task=202=20=E2=80=94=20DependsOn?= =?UTF-8?q?=20field=20and=20subchart=20DAG=20construction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pkg/chart/v2/dependency.go | 3 + pkg/chart/v2/util/subchart_dag.go | 148 +++++++++++++++++ pkg/chart/v2/util/subchart_dag_test.go | 219 +++++++++++++++++++++++++ 3 files changed, 370 insertions(+) create mode 100644 pkg/chart/v2/util/subchart_dag.go create mode 100644 pkg/chart/v2/util/subchart_dag_test.go diff --git a/pkg/chart/v2/dependency.go b/pkg/chart/v2/dependency.go index 8a590a036..096a85062 100644 --- a/pkg/chart/v2/dependency.go +++ b/pkg/chart/v2/dependency.go @@ -47,6 +47,9 @@ 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 deployed + // before this subchart. Used for HIP-0025 resource sequencing. + DependsOn []string `json:"dependsOn,omitempty" yaml:"depends-on,omitempty"` } // Validate checks for common problems with the dependency datastructure in diff --git a/pkg/chart/v2/util/subchart_dag.go b/pkg/chart/v2/util/subchart_dag.go new file mode 100644 index 000000000..79bc31816 --- /dev/null +++ b/pkg/chart/v2/util/subchart_dag.go @@ -0,0 +1,148 @@ +/* +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" + "log/slog" + + chart "helm.sh/helm/v4/pkg/chart/v2" +) + +const ( + // AnnotationDependsOnSubcharts is the Chart.yaml annotation key for declaring + // subchart deployment ordering. The value is a JSON object mapping subchart + // names (or aliases) to lists of their prerequisites. + // + // Example: + // annotations: + // helm.sh/depends-on/subcharts: '{"nginx": ["postgres", "redis"]}' + AnnotationDependsOnSubcharts = "helm.sh/depends-on/subcharts" +) + +// depInfo holds metadata about a dependency for subchart DAG construction. +type depInfo struct { + dep *chart.Dependency + disabled bool +} + +// BuildSubchartDAG constructs a DAG from a chart's subchart dependency declarations. +// +// Dependency ordering is read from two sources: +// 1. The `depends-on` field on each entry in Chart.yaml dependencies. +// 2. The `helm.sh/depends-on/subcharts` annotation on the Chart.yaml metadata. +// +// Subcharts are identified by their effective name (alias if set, otherwise name). +// When a referenced subchart is disabled (Enabled == false), the dependency edge +// is silently removed and an info-level log is emitted, since a disabled chart +// produces no resources. A reference to a truly non-existent subchart name is +// an error. +// +// Returns the constructed DAG, ready to call GetBatches() on. +func BuildSubchartDAG(c *chart.Chart) (*DAG, error) { + d := NewDAG() + + if c.Metadata == nil || len(c.Metadata.Dependencies) == 0 { + return d, nil + } + + // Build a map of effective-name → Dependency for quick lookup. + // effective name = alias if set, otherwise name. + // Track disabled subcharts separately — they are valid names but produce no resources. + byName := make(map[string]*depInfo) + for _, dep := range c.Metadata.Dependencies { + effective := dep.Name + if dep.Alias != "" { + effective = dep.Alias + } + disabled := !dep.Enabled && dep.Condition == "" && len(dep.Tags) == 0 + // A dependency with Enabled=false explicitly is disabled. + // We also check: if Enabled is true OR there's a condition/tag, treat as + // potentially enabled (conditions are runtime values we can't evaluate here). + // For DAG purposes: if Enabled is explicitly false and no condition/tag, + // treat as disabled. Otherwise treat as enabled. + if !dep.Enabled { + disabled = true + } + byName[effective] = &depInfo{dep: dep, disabled: disabled} + } + + // Register all non-disabled subcharts as DAG nodes. + for name, info := range byName { + if !info.disabled { + d.AddNode(name) + } + } + + // Process DependsOn fields from each dependency. + for _, dep := range c.Metadata.Dependencies { + effective := dep.Name + if dep.Alias != "" { + effective = dep.Alias + } + if info := byName[effective]; info != nil && info.disabled { + continue // skip disabled subcharts entirely + } + for _, prereq := range dep.DependsOn { + if err := addSubchartEdge(d, byName, effective, prereq); err != nil { + return nil, err + } + } + } + + // Process helm.sh/depends-on/subcharts annotation. + if c.Metadata.Annotations != nil { + if raw, ok := c.Metadata.Annotations[AnnotationDependsOnSubcharts]; ok && raw != "" { + var annotationDeps map[string][]string + if err := json.Unmarshal([]byte(raw), &annotationDeps); err != nil { + return nil, fmt.Errorf("parsing %s annotation: %w", AnnotationDependsOnSubcharts, err) + } + for subchart, prereqs := range annotationDeps { + if info := byName[subchart]; info == nil { + return nil, fmt.Errorf("annotation %s references unknown subchart %q", AnnotationDependsOnSubcharts, subchart) + } else if info.disabled { + slog.Info("skipping annotation dependency for disabled subchart", "subchart", subchart) + continue + } + for _, prereq := range prereqs { + if err := addSubchartEdge(d, byName, subchart, prereq); err != nil { + return nil, err + } + } + } + } + } + + return d, nil +} + +// addSubchartEdge adds an edge prereq→subchart to the DAG, handling disabled prereqs. +func addSubchartEdge(d *DAG, byName map[string]*depInfo, subchart, prereq string) error { + info, ok := byName[prereq] + if !ok { + return fmt.Errorf("subchart %q depends-on unknown subchart %q", subchart, prereq) + } + if info.disabled { + slog.Info("ignoring dependency on disabled subchart", "subchart", subchart, "disabledPrereq", prereq) + return nil + } + if err := d.AddEdge(prereq, subchart); err != nil { + return fmt.Errorf("adding sequencing edge %s→%s: %w", prereq, subchart, err) + } + return nil +} diff --git a/pkg/chart/v2/util/subchart_dag_test.go b/pkg/chart/v2/util/subchart_dag_test.go new file mode 100644 index 000000000..a0548a7c9 --- /dev/null +++ b/pkg/chart/v2/util/subchart_dag_test.go @@ -0,0 +1,219 @@ +/* +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" + + chart "helm.sh/helm/v4/pkg/chart/v2" +) + +func makeChart(name string, deps ...*chart.Dependency) *chart.Chart { + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: name, + }, + } + c.Metadata.Dependencies = deps + return c +} + +// dep creates an enabled dependency. In Helm's runtime, processDependencyEnabled +// sets Enabled=true for all deps before condition evaluation; we mirror that here. +func dep(name string, dependsOn ...string) *chart.Dependency { + return &chart.Dependency{ + Name: name, + Enabled: true, + DependsOn: dependsOn, + } +} + +func depAlias(name, alias string, dependsOn ...string) *chart.Dependency { + return &chart.Dependency{ + Name: name, + Alias: alias, + Enabled: true, + DependsOn: dependsOn, + } +} + +func TestBuildSubchartDAG_Empty(t *testing.T) { + c := makeChart("parent") + dag, err := BuildSubchartDAG(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + batches, err := dag.GetBatches() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(batches) != 0 { + t.Errorf("expected 0 batches, got %d", len(batches)) + } +} + +func TestBuildSubchartDAG_NoDependencies(t *testing.T) { + // Three subcharts with no ordering declarations — all in batch 0. + c := makeChart("parent", + dep("nginx"), + dep("rabbitmq"), + dep("postgres"), + ) + dag, err := BuildSubchartDAG(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + batches, err := dag.GetBatches() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(batches) != 1 { + t.Fatalf("expected 1 batch, got %d: %v", len(batches), batches) + } + if !containsAll(batches[0], "nginx", "rabbitmq", "postgres") { + t.Errorf("expected all subcharts in batch 0, got %v", batches[0]) + } +} + +func TestBuildSubchartDAG_LinearOrder(t *testing.T) { + // postgres → rabbitmq → app + c := makeChart("parent", + dep("postgres"), + dep("rabbitmq", "postgres"), + dep("app", "rabbitmq"), + ) + dag, err := BuildSubchartDAG(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + batches, err := dag.GetBatches() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(batches) != 3 { + t.Fatalf("expected 3 batches, got %d: %v", len(batches), batches) + } +} + +func TestBuildSubchartDAG_AliasResolution(t *testing.T) { + // "db" is aliased as "primary-db". "app" depends on "primary-db" (the alias). + c := makeChart("parent", + depAlias("postgres", "primary-db"), + dep("app", "primary-db"), + ) + dag, err := BuildSubchartDAG(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + batches, err := dag.GetBatches() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(batches) != 2 { + t.Fatalf("expected 2 batches, got %d: %v", len(batches), batches) + } + if !containsExactly(batches[0], "primary-db") { + t.Errorf("expected [primary-db] in batch 0, got %v", batches[0]) + } + if !containsExactly(batches[1], "app") { + t.Errorf("expected [app] in batch 1, got %v", batches[1]) + } +} + +func TestBuildSubchartDAG_NonExistentReference(t *testing.T) { + // "app" depends on "nonexistent" which is not in the deps list. + c := makeChart("parent", + dep("app", "nonexistent"), + ) + _, err := BuildSubchartDAG(c) + if err == nil { + t.Fatal("expected error for non-existent subchart reference, got nil") + } +} + +func TestBuildSubchartDAG_DisabledSubchart(t *testing.T) { + // "app" depends on "cache", but "cache" is disabled. + // The dependency edge should be silently removed (app still deploys). + c := makeChart("parent", + &chart.Dependency{ + Name: "cache", + Enabled: false, // disabled + }, + dep("app", "cache"), + ) + dag, err := BuildSubchartDAG(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + batches, err := dag.GetBatches() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // app should be in batch 0 since cache is disabled (edge removed) + if len(batches) != 1 { + t.Fatalf("expected 1 batch (cache disabled), got %d: %v", len(batches), batches) + } + if !containsExactly(batches[0], "app") { + t.Errorf("expected [app] in batch 0, got %v", batches[0]) + } +} + +func TestBuildSubchartDAG_CycleDetection(t *testing.T) { + c := makeChart("parent", + dep("a", "b"), + dep("b", "c"), + dep("c", "a"), + ) + dag, err := BuildSubchartDAG(c) + if err != nil { + t.Fatalf("unexpected error building DAG: %v", err) + } + _, err = dag.GetBatches() + if err == nil { + t.Fatal("expected cycle error, got nil") + } +} + +func TestBuildSubchartDAG_AnnotationBased(t *testing.T) { + // Uses helm.sh/depends-on/subcharts annotation format: "nginx depends-on postgres" + // The annotation format is: subchart-name: depends-on-list (comma-separated) + c := makeChart("parent", + dep("postgres"), + dep("nginx"), + ) + // Set annotation: nginx depends on postgres + c.Metadata.Annotations = map[string]string{ + "helm.sh/depends-on/subcharts": `{"nginx": ["postgres"]}`, + } + dag, err := BuildSubchartDAG(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + batches, err := dag.GetBatches() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(batches) != 2 { + t.Fatalf("expected 2 batches, got %d: %v", len(batches), batches) + } + if !containsExactly(batches[0], "postgres") { + t.Errorf("expected [postgres] in batch 0, got %v", batches[0]) + } + if !containsExactly(batches[1], "nginx") { + t.Errorf("expected [nginx] in batch 1, got %v", batches[1]) + } +}