From c18503939e5ebf29f79b8d4341e6568cd0fdfe5d Mon Sep 17 00:00:00 2001 From: Rohit Gudi <50377477+caretak3r@users.noreply.github.com> Date: Thu, 11 Jun 2026 22:39:17 -0400 Subject: [PATCH] feat(hip-0025): add resource-group parsing and release sequencing metadata Resource-group annotation parsing into a DAG, the SequencingInfo field recorded on releases for sequenced uninstall/rollback, and the helm-internal annotation stripping helper. Backward-compatible JSON round-trip for releases stored without SequencingInfo. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com> --- pkg/release/v1/release.go | 24 + pkg/release/v1/release_test.go | 95 +++ pkg/release/v1/sequence/builder.go | 320 +++++++++ pkg/release/v1/sequence/builder_test.go | 732 +++++++++++++++++++++ pkg/release/v1/sequence/manifest_parse.go | 75 +++ pkg/release/v1/sequence/plan.go | 136 ++++ pkg/release/v1/sequence/plan_test.go | 171 +++++ pkg/release/v1/util/manifest.go | 32 + pkg/release/v1/util/manifest_test.go | 98 +++ pkg/release/v1/util/resource_group.go | 247 +++++++ pkg/release/v1/util/resource_group_test.go | 381 +++++++++++ 11 files changed, 2311 insertions(+) create mode 100644 pkg/release/v1/release_test.go create mode 100644 pkg/release/v1/sequence/builder.go create mode 100644 pkg/release/v1/sequence/builder_test.go create mode 100644 pkg/release/v1/sequence/manifest_parse.go create mode 100644 pkg/release/v1/sequence/plan.go create mode 100644 pkg/release/v1/sequence/plan_test.go create mode 100644 pkg/release/v1/util/resource_group.go create mode 100644 pkg/release/v1/util/resource_group_test.go diff --git a/pkg/release/v1/release.go b/pkg/release/v1/release.go index 3bbc0e4ce..d38391900 100644 --- a/pkg/release/v1/release.go +++ b/pkg/release/v1/release.go @@ -25,6 +25,15 @@ type ApplyMethod string const ApplyMethodClientSideApply ApplyMethod = "csa" const ApplyMethodServerSideApply ApplyMethod = "ssa" +// SequencingInfo is the legacy ordered-sequencing record stored by earlier +// builds of this fork. It is retained so those releases keep decoding; new +// code reads Release.IsSequenced and writers are migrating to the Sequenced +// field. +type SequencingInfo struct { + Enabled bool `json:"enabled,omitempty"` + Strategy string `json:"strategy,omitempty"` +} + // Release describes a deployment of a chart, together with the chart // and the variables used to deploy that chart. type Release struct { @@ -51,6 +60,13 @@ type Release struct { // ApplyMethod stores whether server-side or client-side apply was used for the release // Unset (empty string) should be treated as the default of client-side apply ApplyMethod string `json:"apply_method,omitempty"` // "ssa" | "csa" + // Sequenced records that this revision was deployed with ordered sequencing + // (--wait=ordered / WaitStrategy=ordered). It is set before the release record + // is first persisted so that any failure-cleanup path sees it. + Sequenced bool `json:"sequenced,omitempty"` + // SequencingInfo is read for releases stored by earlier builds of this fork. + // Legacy, decode-only: writers are migrating to Sequenced; read via IsSequenced. + SequencingInfo *SequencingInfo `json:"sequencing_info,omitempty"` } // SetStatus is a helper for setting the status on a release. @@ -58,3 +74,11 @@ func (r *Release) SetStatus(status common.Status, msg string) { r.Info.Status = status r.Info.Description = msg } + +// IsSequenced reports whether this release revision was deployed with ordered +// sequencing. It reads both the current Sequenced field and the legacy +// SequencingInfo shape stored by earlier builds, so pre-existing sequenced +// releases keep sequencing through uninstall and rollback. +func (r *Release) IsSequenced() bool { + return r.Sequenced || (r.SequencingInfo != nil && r.SequencingInfo.Enabled) +} diff --git a/pkg/release/v1/release_test.go b/pkg/release/v1/release_test.go new file mode 100644 index 000000000..2d2b3340e --- /dev/null +++ b/pkg/release/v1/release_test.go @@ -0,0 +1,95 @@ +/* +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 v1 + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReleaseSequencingInfoBackwardCompatibility(t *testing.T) { + release := Release{Name: "demo"} + + data, err := json.Marshal(&release) + require.NoError(t, err) + assert.NotContains(t, string(data), "sequencing_info") + + var decoded Release + err = json.Unmarshal([]byte(`{"name":"demo"}`), &decoded) + require.NoError(t, err) + assert.Nil(t, decoded.SequencingInfo) +} + +func TestReleaseSequencingInfoRoundTrip(t *testing.T) { + release := Release{ + Name: "demo", + SequencingInfo: &SequencingInfo{ + Enabled: true, + Strategy: "ordered", + }, + } + + data, err := json.Marshal(&release) + require.NoError(t, err) + assert.Contains(t, string(data), `"sequencing_info"`) + + var decoded Release + err = json.Unmarshal(data, &decoded) + require.NoError(t, err) + require.NotNil(t, decoded.SequencingInfo) + assert.True(t, decoded.SequencingInfo.Enabled) + assert.Equal(t, "ordered", decoded.SequencingInfo.Strategy) +} + +func TestReleaseIsSequencedLegacyDecode(t *testing.T) { + var decoded Release + err := json.Unmarshal([]byte(`{"name":"demo","sequencing_info":{"enabled":true}}`), &decoded) + require.NoError(t, err) + assert.False(t, decoded.Sequenced) + assert.True(t, decoded.IsSequenced()) +} + +func TestReleaseSequencedRoundTrip(t *testing.T) { + release := Release{Name: "demo", Sequenced: true} + data, err := json.Marshal(&release) + require.NoError(t, err) + assert.Contains(t, string(data), `"sequenced":true`) + + var decoded Release + require.NoError(t, json.Unmarshal(data, &decoded)) + assert.True(t, decoded.Sequenced) + assert.True(t, decoded.IsSequenced()) + assert.Nil(t, decoded.SequencingInfo) +} + +func TestReleaseNotSequencedOmitted(t *testing.T) { + release := Release{Name: "demo"} + data, err := json.Marshal(&release) + require.NoError(t, err) + // Non-sequenced release JSON is byte-identical to pre-schema-change output. + assert.NotContains(t, string(data), `"sequenced"`) + assert.NotContains(t, string(data), `"sequencing_info"`) + assert.False(t, release.IsSequenced()) +} + +func TestReleaseIsSequencedDisabledLegacyInfo(t *testing.T) { + release := Release{Name: "demo", SequencingInfo: &SequencingInfo{Enabled: false}} + assert.False(t, release.IsSequenced()) +} diff --git a/pkg/release/v1/sequence/builder.go b/pkg/release/v1/sequence/builder.go new file mode 100644 index 000000000..e7e13f3d9 --- /dev/null +++ b/pkg/release/v1/sequence/builder.go @@ -0,0 +1,320 @@ +/* +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 sequence + +import ( + "fmt" + "maps" + "slices" + "strings" + + chart "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +type builder struct { + plan *Plan +} + +// GroupManifestsByDirectSubchart groups manifests by the direct subchart they belong to. +// chartPath is the full path-prefix for the current chart level — at the top level +// it is the chart name (e.g. "parent"); at deeper recursion levels it is the joined +// path through "/charts/" segments (e.g. "parent/charts/sub"). +// The current chart level's own manifests are returned under the empty string key "". +// Direct subcharts are keyed by their immediate directory name under +// "/charts//". Nested grandchildren are grouped under their +// direct subchart parent ("sub"), since nested sequencing is handled recursively. +func GroupManifestsByDirectSubchart(manifests []releaseutil.Manifest, chartPath string) map[string][]releaseutil.Manifest { + result := make(map[string][]releaseutil.Manifest) + if chartPath == "" { + result[""] = append(result[""], manifests...) + return result + } + + chartsPrefix := chartPath + "/charts/" + for _, m := range manifests { + if !strings.HasPrefix(m.Name, chartsPrefix) { + result[""] = append(result[""], m) + continue + } + rest := m.Name[len(chartsPrefix):] + subchartName, _, ok := strings.Cut(rest, "/") + if !ok { + result[""] = append(result[""], m) + continue + } + result[subchartName] = append(result[subchartName], m) + } + return result +} + +// FindSubchart resolves a direct dependency by effective name (alias if set, +// else name) — the single home for the chart-walker subchart lookup. It uses +// the parent chart's Metadata.Dependencies to resolve aliases, since the alias +// is stored on the Dependency struct in Chart.yaml, not on the subchart's own +// Metadata. +func FindSubchart(chrt *chart.Chart, nameOrAlias string) *chart.Chart { + if chrt == nil { + return nil + } + aliasMap := make(map[string]string) // chart name → effective name (alias or name) + if chrt.Metadata != nil { + for _, dep := range chrt.Metadata.Dependencies { + if dep == nil { + continue + } + effective := dep.Name + if dep.Alias != "" { + effective = dep.Alias + } + aliasMap[dep.Name] = effective + } + } + for _, dep := range chrt.Dependencies() { + effective := dep.Name() + if alias, ok := aliasMap[dep.Name()]; ok { + effective = alias + } + if effective == nameOrAlias || dep.Name() == nameOrAlias { + return dep + } + } + return nil +} + +// Build constructs the deployment plan for chrt's rendered manifests. +// +// Preconditions: manifests are hook-free (SortManifests output or a stored +// rel.Manifest — hooks are stored separately) and chrt has been through +// ProcessDependencies (true for both freshly-loaded and storage-decoded +// charts, per BuildSubchartDAG's contract). Build does not re-filter hooks. +// chrt == nil yields a flat single-level plan at ChartPath "". +// +// Errors (fatal — the caller must not proceed): +// - subchart DAG construction failure: depends-on referencing unknown/disabled +// subcharts, malformed helm.sh/depends-on/subcharts JSON (BuildSubchartDAG) +// - cycle in a subchart DAG or a resource-group DAG (any level) +// - a resource assigned to two different groups (ParseResourceGroups) +// +// Warnings (non-fatal, returned in Plan.Warnings; the manifest is still placed): +// - invalid depends-on/resource-groups JSON on a resource → demoted to unsequenced +// - group referencing a missing group → whole group demoted (transitively) +// - isolated group among ≥2 groups → demoted to unsequenced +// - resource with only one of the two readiness annotations (falls back to kstatus) +// - rendered subchart not declared in Chart.yaml (deployed after declared subcharts) +// - rendered subchart not resolvable to a chart object (flat fallback at its path) +// +// Postcondition: every input manifest appears in exactly one batch; +// len(manifests) == Σ len(batch.Manifests()). Unit-enforced. +// +// Build is deterministic: identical input yields an identical plan. +func Build(chrt *chart.Chart, manifests []releaseutil.Manifest) (*Plan, error) { + b := &builder{plan: &Plan{}} + if chrt == nil { + b.plan.Levels = append(b.plan.Levels, ChartLevel{Path: "", Depth: 0}) + if err := b.appendResourceGroupBatches("", 0, manifests); err != nil { + return nil, err + } + return b.plan, nil + } + + if err := b.buildLevel(chrt, manifests, chrt.Name(), 0); err != nil { + return nil, err + } + return b.plan, nil +} + +func (b *builder) warnf(chartPath, format string, args ...any) { + b.plan.Warnings = append(b.plan.Warnings, Warning{ + ChartPath: chartPath, + Message: fmt.Sprintf(format, args...), + }) +} + +func (b *builder) buildLevel(c *chart.Chart, manifests []releaseutil.Manifest, chartPath string, depth int) error { + levelIdx := len(b.plan.Levels) + b.plan.Levels = append(b.plan.Levels, ChartLevel{Path: chartPath, Depth: depth}) + + grouped := GroupManifestsByDirectSubchart(manifests, chartPath) + + dag, err := chartutil.BuildSubchartDAG(c) + if err != nil { + return fmt.Errorf("building subchart DAG for %s: %w", chartPath, err) + } + + batches, err := dag.GetBatches() + if err != nil { + return fmt.Errorf("getting subchart batches for %s: %w", chartPath, err) + } + b.plan.Levels[levelIdx].SubchartBatches = batches + + declared := make(map[string]bool, len(batches)) + for _, batch := range batches { + for _, name := range batch { + declared[name] = true + if err := b.buildSubchart(c, chartPath, name, grouped[name], depth, levelIdx); err != nil { + return err + } + } + } + + for _, name := range slices.Sorted(maps.Keys(grouped)) { + if name == "" || declared[name] { + continue + } + b.plan.Levels[levelIdx].Undeclared = append(b.plan.Levels[levelIdx].Undeclared, name) + b.warnf(chartPath, "rendered subchart %q is not declared in Chart.yaml dependencies; sequencing it after declared subcharts", name) + if err := b.buildSubchart(c, chartPath, name, grouped[name], depth, levelIdx); err != nil { + return err + } + } + slices.Sort(b.plan.Levels[levelIdx].Unresolved) + + return b.appendResourceGroupBatches(chartPath, depth, grouped[""]) +} + +func (b *builder) buildSubchart(parent *chart.Chart, chartPath, name string, manifests []releaseutil.Manifest, depth, parentLevelIdx int) error { + if len(manifests) == 0 { + return nil + } + + subPath := chartPath + "/charts/" + name + sub := FindSubchart(parent, name) + if sub == nil { + b.plan.Levels[parentLevelIdx].Unresolved = append(b.plan.Levels[parentLevelIdx].Unresolved, name) + b.warnf(chartPath, "subchart %q not found in chart dependencies; deploying its manifests without subchart sequencing", name) + return b.appendResourceGroupBatches(subPath, depth+1, manifests) + } + + return b.buildLevel(sub, manifests, subPath, depth+1) +} + +func (b *builder) appendResourceGroupBatches(chartPath string, depth int, manifests []releaseutil.Manifest) error { + if len(manifests) == 0 { + return nil + } + + for _, manifest := range manifests { + if manifest.Head == nil || manifest.Head.Metadata == nil { + continue + } + annotations := manifest.Head.Metadata.Annotations + _, hasSuccess := annotations[releaseutil.AnnotationReadinessSuccess] + _, hasFailure := annotations[releaseutil.AnnotationReadinessFailure] + if hasSuccess != hasFailure { + b.warnf(chartPath, "resource %q has only one of %s and %s; falling back to kstatus readiness", manifest.Head.Metadata.Name, releaseutil.AnnotationReadinessSuccess, releaseutil.AnnotationReadinessFailure) + } + } + + result, warnings, err := releaseutil.ParseResourceGroups(manifests) + if err != nil { + return fmt.Errorf("parsing resource-group annotations for %s: %w", chartPath, err) + } + for _, warning := range warnings { + b.plan.Warnings = append(b.plan.Warnings, Warning{ + ChartPath: chartPath, + Message: warning, + }) + } + + unsequenced := result.Unsequenced + if len(result.Groups) >= 2 { + dependents := resourceGroupDependents(result.GroupDeps) + for _, groupName := range slices.Sorted(maps.Keys(result.Groups)) { + if len(result.GroupDeps[groupName]) != 0 || dependents[groupName] { + continue + } + b.warnf(chartPath, "resource-group %q is isolated (no depends-on edges and no dependents); deploying it in the unsequenced batch after sequenced groups", groupName) + unsequenced = append(unsequenced, result.Groups[groupName]...) + delete(result.Groups, groupName) + delete(result.GroupDeps, groupName) + } + } + + if len(result.Groups) > 0 { + dag, err := releaseutil.BuildResourceGroupDAG(result) + if err != nil { + return fmt.Errorf("building resource-group DAG for %s: %w", chartPath, err) + } + + groupBatches, err := dag.GetBatches() + if err != nil { + return fmt.Errorf("getting resource-group batches for %s: %w", chartPath, err) + } + + dependents := resourceGroupDependents(result.GroupDeps) + for _, groupBatch := range groupBatches { + batch := Batch{ + ChartPath: chartPath, + Depth: depth, + Kind: BatchKindGroups, + Wait: true, + } + for _, groupName := range groupBatch { + batch.Groups = append(batch.Groups, Group{ + Name: groupName, + Manifests: result.Groups[groupName], + }) + if !dependents[groupName] { + batch.LeafGroups = append(batch.LeafGroups, groupName) + } + } + batch.HasCustomReadiness = hasCustomReadiness(batch.Manifests()) + b.plan.Batches = append(b.plan.Batches, batch) + } + } + + if len(unsequenced) > 0 { + b.plan.Batches = append(b.plan.Batches, Batch{ + ChartPath: chartPath, + Depth: depth, + Kind: BatchKindUnsequenced, + Groups: []Group{{Name: "", Manifests: unsequenced}}, + Wait: true, + HasCustomReadiness: hasCustomReadiness(unsequenced), + }) + } + + return nil +} + +func resourceGroupDependents(groupDeps map[string][]string) map[string]bool { + dependents := make(map[string]bool) + for _, deps := range groupDeps { + for _, dep := range deps { + dependents[dep] = true + } + } + return dependents +} + +func hasCustomReadiness(manifests []releaseutil.Manifest) bool { + for _, manifest := range manifests { + if manifest.Head == nil || manifest.Head.Metadata == nil { + continue + } + annotations := manifest.Head.Metadata.Annotations + if annotations == nil { + continue + } + if annotations[releaseutil.AnnotationReadinessSuccess] != "" && annotations[releaseutil.AnnotationReadinessFailure] != "" { + return true + } + } + return false +} diff --git a/pkg/release/v1/sequence/builder_test.go b/pkg/release/v1/sequence/builder_test.go new file mode 100644 index 000000000..86f9ab047 --- /dev/null +++ b/pkg/release/v1/sequence/builder_test.go @@ -0,0 +1,732 @@ +/* +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 sequence + +import ( + "encoding/json" + "fmt" + "maps" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + chart "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func TestBuild_NilChart_FlatPlan(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("db", "parent/templates/db.yaml", groupAnnotations("db")), + makeManifest("app", "parent/templates/app.yaml", groupAnnotations("app", "db")), + makeManifest("plain", "parent/templates/plain.yaml", nil), + } + + plan, err := Build(nil, manifests) + require.NoError(t, err) + assert.Equal(t, []ChartLevel{{Path: "", Depth: 0}}, plan.Levels) + assert.Equal(t, []batchSummary{ + {ChartPath: "", Depth: 0, Kind: BatchKindGroups, Groups: []string{"db"}}, + {ChartPath: "", Depth: 0, Kind: BatchKindGroups, Groups: []string{"app"}}, + {ChartPath: "", Depth: 0, Kind: BatchKindUnsequenced, Groups: []string{""}}, + }, batchSummaries(plan)) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_EmptyChart(t *testing.T) { + t.Parallel() + + plan, err := Build(newChart("parent"), nil) + require.NoError(t, err) + assert.Equal(t, []ChartLevel{{Path: "parent", Depth: 0}}, plan.Levels) + assert.Empty(t, plan.Batches) + assert.Empty(t, plan.Warnings) +} + +func TestBuild_NoAnnotations_SingleFlatBatch(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("a", "parent/templates/a.yaml", nil), + makeManifest("b", "parent/templates/b.yaml", nil), + makeManifest("c", "parent/templates/c.yaml", nil), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + require.Len(t, plan.Batches, 1) + assert.Equal(t, BatchKindUnsequenced, plan.Batches[0].Kind) + assert.Equal(t, manifestNames(manifests), manifestNames(plan.Batches[0].Manifests())) + assert.True(t, plan.Batches[0].Wait) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_ResourceGroupOrdering(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("db", "parent/templates/db.yaml", groupAnnotations("db")), + makeManifest("app", "parent/templates/app.yaml", groupAnnotations("app", "db")), + makeManifest("plain", "parent/templates/plain.yaml", nil), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + assert.Equal(t, []batchSummary{ + {ChartPath: "parent", Depth: 0, Kind: BatchKindGroups, Groups: []string{"db"}}, + {ChartPath: "parent", Depth: 0, Kind: BatchKindGroups, Groups: []string{"app"}}, + {ChartPath: "parent", Depth: 0, Kind: BatchKindUnsequenced, Groups: []string{""}}, + }, batchSummaries(plan)) + for _, batch := range plan.Batches { + assert.True(t, batch.Wait) + } + assert.Empty(t, plan.Batches[0].LeafGroups) + assert.Equal(t, []string{"app"}, plan.Batches[1].LeafGroups) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_LeafGroups_Diamond(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("base", "parent/templates/base.yaml", groupAnnotations("base")), + makeManifest("left", "parent/templates/left.yaml", groupAnnotations("left", "base")), + makeManifest("right", "parent/templates/right.yaml", groupAnnotations("right", "base")), + makeManifest("top", "parent/templates/top.yaml", groupAnnotations("top", "left", "right")), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + assert.Equal(t, [][]string{{"base"}, {"left", "right"}, {"top"}}, batchGroupNames(plan)) + assert.Empty(t, plan.Batches[0].LeafGroups) + assert.Empty(t, plan.Batches[1].LeafGroups) + assert.Equal(t, []string{"top"}, plan.Batches[2].LeafGroups) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_NestedSubcharts_ThreeLevels(t *testing.T) { + t.Parallel() + + grand := newChart("grand") + child := newChart("child", enabledDependency("grand")) + child.SetDependencies(grand) + parent := newChart("parent", enabledDependency("child")) + parent.SetDependencies(child) + manifests := []releaseutil.Manifest{ + makeManifest("parent", "parent/templates/parent.yaml", nil), + makeManifest("child", "parent/charts/child/templates/child.yaml", nil), + makeManifest("grand", "parent/charts/child/charts/grand/templates/grand.yaml", nil), + } + + plan, err := Build(parent, manifests) + require.NoError(t, err) + assert.Equal(t, []string{ + "parent/charts/child/charts/grand", + "parent/charts/child", + "parent", + }, batchChartPaths(plan)) + assert.Equal(t, []int{2, 1, 0}, batchDepths(plan)) + assert.Equal(t, []ChartLevel{ + {Path: "parent", Depth: 0, SubchartBatches: [][]string{{"child"}}}, + {Path: "parent/charts/child", Depth: 1, SubchartBatches: [][]string{{"grand"}}}, + {Path: "parent/charts/child/charts/grand", Depth: 2}, + }, plan.Levels) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_SubchartDependencyOrder(t *testing.T) { + t.Parallel() + + parent := newChart( + "parent", + enabledDependency("postgres"), + enabledDependency("rabbitmq", "postgres"), + enabledDependency("app", "rabbitmq"), + ) + manifests := []releaseutil.Manifest{ + makeManifest("app", "parent/charts/app/templates/app.yaml", nil), + makeManifest("rabbitmq", "parent/charts/rabbitmq/templates/rabbitmq.yaml", nil), + makeManifest("postgres", "parent/charts/postgres/templates/postgres.yaml", nil), + } + + plan, err := Build(parent, manifests) + require.NoError(t, err) + assert.Equal(t, []string{"parent/charts/postgres", "parent/charts/rabbitmq", "parent/charts/app"}, batchChartPaths(plan)) + require.Len(t, plan.Levels, 4) + assert.Equal(t, [][]string{{"postgres"}, {"rabbitmq"}, {"app"}}, plan.Levels[0].SubchartBatches) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_Aliases_RealPipeline(t *testing.T) { + t.Parallel() + + parent := pipelineChart( + pipelineDependency("postgres", "primary-db"), + pipelineDependency("app", "", "postgres"), + ) + require.NoError(t, chartutil.ProcessDependencies(parent, map[string]any{})) + manifests := []releaseutil.Manifest{ + makeManifest("app", "parent/charts/app/templates/app.yaml", nil), + makeManifest("primary", "parent/charts/primary-db/templates/primary.yaml", nil), + } + + plan, err := Build(parent, manifests) + require.NoError(t, err) + assert.Equal(t, []string{"parent/charts/primary-db", "parent/charts/app"}, batchChartPaths(plan)) + require.NotEmpty(t, plan.Levels) + assert.Equal(t, [][]string{{"primary-db"}, {"app"}}, plan.Levels[0].SubchartBatches) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_SubchartCycle_Fatal(t *testing.T) { + t.Parallel() + + parent := newChart("parent", enabledDependency("a", "b"), enabledDependency("b", "a")) + plan, err := Build(parent, []releaseutil.Manifest{makeManifest("a", "parent/charts/a/templates/a.yaml", nil)}) + require.Error(t, err) + assert.Nil(t, plan) + assert.Contains(t, err.Error(), "cycle") +} + +func TestBuild_UnknownDependsOnRef_Fatal(t *testing.T) { + t.Parallel() + + parent := newChart("parent", enabledDependency("app", "missing")) + plan, err := Build(parent, nil) + require.Error(t, err) + assert.Nil(t, plan) + assert.Contains(t, err.Error(), `depends-on unknown or disabled subchart "missing"`) +} + +func TestBuild_ResourceGroupCycle_Fatal_NestedLevel(t *testing.T) { + t.Parallel() + + parent := newChart("parent", enabledDependency("child")) + manifests := []releaseutil.Manifest{ + makeManifest("a", "parent/charts/child/templates/a.yaml", groupAnnotations("a", "b")), + makeManifest("b", "parent/charts/child/templates/b.yaml", groupAnnotations("b", "a")), + } + + plan, err := Build(parent, manifests) + require.Error(t, err) + assert.Nil(t, plan) + assert.Contains(t, err.Error(), "cycle") + assert.Contains(t, err.Error(), "parent/charts/child") +} + +func TestBuild_MultiGroupAssignment_Fatal(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeTypedManifest("same", "parent/templates/one.yaml", "v1", "ConfigMap", "default", groupAnnotations("one")), + makeTypedManifest("same", "parent/templates/two.yaml", "v1", "ConfigMap", "default", groupAnnotations("two")), + } + + plan, err := Build(newChart("parent"), manifests) + require.Error(t, err) + assert.Nil(t, plan) + assert.Contains(t, err.Error(), "assigned to multiple resource groups") +} + +func TestBuild_IsolatedGroupDemoted(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("db", "parent/templates/db.yaml", groupAnnotations("db")), + makeManifest("app", "parent/templates/app.yaml", groupAnnotations("app", "db")), + makeManifest("plain", "parent/templates/plain.yaml", nil), + makeManifest("metrics", "parent/templates/metrics.yaml", groupAnnotations("metrics")), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + assert.Equal(t, [][]string{{"db"}, {"app"}, {""}}, batchGroupNames(plan)) + assert.NotContains(t, slices.Concat(batchGroupNames(plan)[:2]...), "metrics") + require.Len(t, plan.Batches, 3) + assert.Equal(t, BatchKindUnsequenced, plan.Batches[2].Kind) + assert.ElementsMatch(t, []string{"parent/templates/plain.yaml", "parent/templates/metrics.yaml"}, manifestNames(plan.Batches[2].Manifests())) + assertWarningContains(t, plan, "parent", "isolated", "metrics") + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_TwoGroupChain_NotDemoted(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("db", "parent/templates/db.yaml", groupAnnotations("db")), + makeManifest("app", "parent/templates/app.yaml", groupAnnotations("app", "db")), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + assert.Equal(t, [][]string{{"db"}, {"app"}}, batchGroupNames(plan)) + assertNoWarningContains(t, plan, "isolated") + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_SingleGroup_NotDemoted(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("solo", "parent/templates/solo.yaml", groupAnnotations("solo")), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + assert.Equal(t, [][]string{{"solo"}}, batchGroupNames(plan)) + assert.Equal(t, BatchKindGroups, plan.Batches[0].Kind) + assertNoWarningContains(t, plan, "isolated") + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_AllGroupsIsolated_AllDemoted(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("beta", "parent/templates/beta.yaml", groupAnnotations("beta")), + makeManifest("alpha", "parent/templates/alpha.yaml", groupAnnotations("alpha")), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + require.Len(t, plan.Batches, 1) + assert.Equal(t, BatchKindUnsequenced, plan.Batches[0].Kind) + assert.Equal(t, []string{"parent/templates/alpha.yaml", "parent/templates/beta.yaml"}, manifestNames(plan.Batches[0].Manifests())) + require.Len(t, plan.Warnings, 2) + assert.Contains(t, plan.Warnings[0].Message, "alpha") + assert.Contains(t, plan.Warnings[1].Message, "beta") + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_MissingGroupDep_TransitiveDemotion(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("app", "parent/templates/app.yaml", groupAnnotations("app", "nope")), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + require.Len(t, plan.Batches, 1) + assert.Equal(t, BatchKindUnsequenced, plan.Batches[0].Kind) + assert.Equal(t, []string{"parent/templates/app.yaml"}, manifestNames(plan.Batches[0].Manifests())) + assertWarningContains(t, plan, "parent", "depends-on non-existent group") + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_InvalidDependsOnJSON_Demoted(t *testing.T) { + t.Parallel() + + annotations := groupAnnotations("app") + annotations[releaseutil.AnnotationDependsOnResourceGroups] = "not-json" + manifests := []releaseutil.Manifest{ + makeManifest("app", "parent/templates/app.yaml", annotations), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + require.Len(t, plan.Batches, 1) + assert.Equal(t, BatchKindUnsequenced, plan.Batches[0].Kind) + assert.Equal(t, []string{"parent/templates/app.yaml"}, manifestNames(plan.Batches[0].Manifests())) + assertWarningContains(t, plan, "parent", "invalid JSON") + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_UndeclaredSubchartIncluded(t *testing.T) { + t.Parallel() + + parent := newChart("parent", enabledDependency("declared")) + parent.AddDependency(newChart("vendored")) + manifests := []releaseutil.Manifest{ + makeManifest("declared", "parent/charts/declared/templates/declared.yaml", nil), + makeManifest("vendored", "parent/charts/vendored/templates/vendored.yaml", nil), + makeManifest("parent", "parent/templates/parent.yaml", nil), + } + + plan, err := Build(parent, manifests) + require.NoError(t, err) + assert.Equal(t, []string{"parent/charts/declared", "parent/charts/vendored", "parent"}, batchChartPaths(plan)) + require.NotEmpty(t, plan.Levels) + assert.Equal(t, []string{"vendored"}, plan.Levels[0].Undeclared) + assertWarningContains(t, plan, "parent", "not declared") + assertPlanComplete(t, plan, manifests) + + reversedPaths := batchChartPaths(plan.Reverse()) + assert.Contains(t, reversedPaths, "parent/charts/vendored") +} + +func TestBuild_UnresolvableSubchart_FlatFallback(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("db", "parent/charts/ghost/templates/db.yaml", groupAnnotations("db")), + makeManifest("app", "parent/charts/ghost/templates/app.yaml", groupAnnotations("app", "db")), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + assertWarningContains(t, plan, "parent", "not found in chart dependencies") + require.Len(t, plan.Levels, 1) + assert.Equal(t, []string{"ghost"}, plan.Levels[0].Unresolved) + assert.Equal(t, []string{"parent/charts/ghost", "parent/charts/ghost"}, batchChartPaths(plan)) + assert.Equal(t, []int{1, 1}, batchDepths(plan)) + assert.Equal(t, [][]string{{"db"}, {"app"}}, batchGroupNames(plan)) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_HookManifestNotFiltered(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("hook", "parent/templates/hook.yaml", map[string]string{"helm.sh/hook": "pre-install"}), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + require.Len(t, plan.Batches, 1) + assert.Equal(t, []string{"parent/templates/hook.yaml"}, manifestNames(plan.Batches[0].Manifests())) + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_HasCustomReadiness(t *testing.T) { + t.Parallel() + + groupA := groupAnnotations("a") + groupA[releaseutil.AnnotationReadinessSuccess] = `{.status.ready} == true` + groupA[releaseutil.AnnotationReadinessFailure] = `{.status.failed} == true` + groupB := groupAnnotations("b", "a") + groupB[releaseutil.AnnotationReadinessSuccess] = `{.status.ready} == true` + manifests := []releaseutil.Manifest{ + makeManifest("a", "parent/templates/a.yaml", groupA), + makeManifest("b", "parent/templates/b.yaml", groupB), + makeManifest("plain", "parent/templates/plain.yaml", nil), + } + + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + require.Len(t, plan.Batches, 3) + assert.True(t, plan.Batches[0].HasCustomReadiness) + assert.False(t, plan.Batches[1].HasCustomReadiness) + assert.False(t, plan.Batches[2].HasCustomReadiness) + assertWarningContains(t, plan, "parent", "only one of") + assertPlanComplete(t, plan, manifests) +} + +func TestBuild_Deterministic(t *testing.T) { + t.Parallel() + + chartOne, manifestsOne := kitchenSinkFixture() + chartTwo, manifestsTwo := kitchenSinkFixture() + + planOne, err := Build(chartOne, manifestsOne) + require.NoError(t, err) + planTwo, err := Build(chartTwo, manifestsTwo) + require.NoError(t, err) + assert.Equal(t, planOne, planTwo) +} + +func TestBuild_Completeness_KitchenSink(t *testing.T) { + t.Parallel() + + chrt, manifests := kitchenSinkFixture() + plan, err := Build(chrt, manifests) + require.NoError(t, err) + assertPlanComplete(t, plan, manifests) + for _, batch := range plan.Batches { + assert.NotEmpty(t, batch.Manifests()) + if batch.Kind == BatchKindUnsequenced { + require.Len(t, batch.Groups, 1) + assert.Empty(t, batch.Groups[0].Name) + } + } +} + +type batchSummary struct { + ChartPath string + Depth int + Kind BatchKind + Groups []string +} + +func summarizeBatch(batch Batch) batchSummary { + return batchSummary{ + ChartPath: batch.ChartPath, + Depth: batch.Depth, + Kind: batch.Kind, + Groups: groupNames(batch), + } +} + +func batchSummaries(plan *Plan) []batchSummary { + summaries := make([]batchSummary, 0, len(plan.Batches)) + for _, batch := range plan.Batches { + summaries = append(summaries, summarizeBatch(batch)) + } + return summaries +} + +func batchGroupNames(plan *Plan) [][]string { + names := make([][]string, 0, len(plan.Batches)) + for _, batch := range plan.Batches { + names = append(names, groupNames(batch)) + } + return names +} + +func batchChartPaths(plan *Plan) []string { + paths := make([]string, 0, len(plan.Batches)) + for _, batch := range plan.Batches { + paths = append(paths, batch.ChartPath) + } + return paths +} + +func batchDepths(plan *Plan) []int { + depths := make([]int, 0, len(plan.Batches)) + for _, batch := range plan.Batches { + depths = append(depths, batch.Depth) + } + return depths +} + +func groupNames(batch Batch) []string { + names := make([]string, 0, len(batch.Groups)) + for _, group := range batch.Groups { + names = append(names, group.Name) + } + return names +} + +func manifestNames(manifests []releaseutil.Manifest) []string { + names := make([]string, 0, len(manifests)) + for _, manifest := range manifests { + names = append(names, manifest.Name) + } + return names +} + +func assertPlanComplete(t *testing.T, plan *Plan, manifests []releaseutil.Manifest) { + t.Helper() + + require.NotNil(t, plan) + expected := make(map[string]int, len(manifests)) + for _, manifest := range manifests { + expected[manifest.Name]++ + } + + actual := make(map[string]int, len(manifests)) + var total int + for _, batch := range plan.Batches { + for _, manifest := range batch.Manifests() { + actual[manifest.Name]++ + total++ + } + } + + assert.Equal(t, len(manifests), total) + assert.Equal(t, expected, actual) +} + +func assertWarningContains(t *testing.T, plan *Plan, chartPath string, parts ...string) { + t.Helper() + + for _, warning := range plan.Warnings { + if warning.ChartPath != chartPath { + continue + } + matches := true + for _, part := range parts { + if !strings.Contains(warning.Message, part) { + matches = false + break + } + } + if matches { + return + } + } + assert.Failf(t, "missing warning", "chartPath=%q parts=%v warnings=%v", chartPath, parts, plan.Warnings) +} + +func assertNoWarningContains(t *testing.T, plan *Plan, part string) { + t.Helper() + + for _, warning := range plan.Warnings { + assert.NotContains(t, warning.Message, part) + } +} + +func makeManifest(name, sourcePath string, annotations map[string]string) releaseutil.Manifest { + return makeTypedManifest(name, sourcePath, "v1", "ConfigMap", "", annotations) +} + +func makeTypedManifest(name, sourcePath, version, kind, namespace string, annotations map[string]string) releaseutil.Manifest { + annotations = maps.Clone(annotations) + var content strings.Builder + fmt.Fprintf(&content, "apiVersion: %s\nkind: %s\nmetadata:\n name: %s\n", version, kind, name) + if namespace != "" { + fmt.Fprintf(&content, " namespace: %s\n", namespace) + } + if len(annotations) > 0 { + content.WriteString(" annotations:\n") + for _, key := range slices.Sorted(maps.Keys(annotations)) { + fmt.Fprintf(&content, " %s: %q\n", key, annotations[key]) + } + } + + head := &releaseutil.SimpleHead{ + Version: version, + Kind: kind, + Metadata: &struct { + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + Annotations map[string]string `json:"annotations"` + }{ + Name: name, + Namespace: namespace, + Annotations: annotations, + }, + } + + return releaseutil.Manifest{ + Name: sourcePath, + Content: content.String(), + Head: head, + } +} + +func groupAnnotations(group string, deps ...string) map[string]string { + annotations := map[string]string{ + releaseutil.AnnotationResourceGroup: group, + } + if len(deps) > 0 { + encoded, err := json.Marshal(deps) + if err != nil { + panic(err) + } + annotations[releaseutil.AnnotationDependsOnResourceGroups] = string(encoded) + } + return annotations +} + +func newChart(name string, deps ...*chart.Dependency) *chart.Chart { + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: name, + Version: "0.1.0", + APIVersion: chart.APIVersionV2, + Dependencies: deps, + }, + } + for _, dep := range deps { + if dep == nil || !dep.Enabled { + continue + } + subName := dep.Alias + if subName == "" { + subName = dep.Name + } + c.AddDependency(newChart(subName)) + } + return c +} + +func enabledDependency(name string, dependsOn ...string) *chart.Dependency { + return &chart.Dependency{ + Name: name, + Version: "0.1.0", + Enabled: true, + DependsOn: dependsOn, + } +} + +func aliasedDependency(name, alias string, dependsOn ...string) *chart.Dependency { + return &chart.Dependency{ + Name: name, + Version: "0.1.0", + Alias: alias, + Enabled: true, + DependsOn: dependsOn, + } +} + +func pipelineChart(deps ...*chart.Dependency) *chart.Chart { + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Version: "0.1.0", + APIVersion: chart.APIVersionV2, + Dependencies: deps, + }, + } + added := make(map[string]bool) + for _, dep := range deps { + if added[dep.Name] { + continue + } + added[dep.Name] = true + c.AddDependency(&chart.Chart{ + Metadata: &chart.Metadata{ + Name: dep.Name, + Version: "0.1.0", + APIVersion: chart.APIVersionV2, + }, + }) + } + return c +} + +func pipelineDependency(name, alias string, dependsOn ...string) *chart.Dependency { + return &chart.Dependency{ + Name: name, + Version: "0.1.0", + Alias: alias, + DependsOn: dependsOn, + } +} + +func kitchenSinkFixture() (*chart.Chart, []releaseutil.Manifest) { + primary := newChart("primary-db") + worker := newChart("worker") + app := newChart("app", enabledDependency("worker")) + app.SetDependencies(worker) + vendored := newChart("vendored") + + parent := newChart( + "parent", + aliasedDependency("postgres", "primary-db"), + enabledDependency("app", "primary-db"), + ) + parent.SetDependencies(primary, app, vendored) + + manifests := []releaseutil.Manifest{ + makeManifest("primary", "parent/charts/primary-db/templates/primary.yaml", nil), + makeManifest("worker", "parent/charts/app/charts/worker/templates/worker.yaml", nil), + makeManifest("app-sub", "parent/charts/app/templates/app.yaml", groupAnnotations("app-sub")), + makeManifest("vendored", "parent/charts/vendored/templates/vendored.yaml", nil), + makeManifest("db", "parent/templates/db.yaml", groupAnnotations("db")), + makeManifest("app", "parent/templates/app.yaml", groupAnnotations("app", "db")), + makeManifest("metrics", "parent/templates/metrics.yaml", groupAnnotations("metrics")), + makeManifest("plain", "parent/templates/plain.yaml", nil), + } + return parent, manifests +} diff --git a/pkg/release/v1/sequence/manifest_parse.go b/pkg/release/v1/sequence/manifest_parse.go new file mode 100644 index 000000000..2d68c0ebf --- /dev/null +++ b/pkg/release/v1/sequence/manifest_parse.go @@ -0,0 +1,75 @@ +/* +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 sequence + +import ( + "fmt" + "sort" + "strings" + + "sigs.k8s.io/yaml" + + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +// ParseStoredManifests parses a stored release manifest stream (rel.Manifest) +// or `helm template` output into named manifests, recovering source-template +// paths from "# Source:" comments. Documents without a Source comment keep +// their positional "manifest-%d" split key, and therefore route to the +// top-level/flat portion of a plan built from them. Returns an error only on +// YAML head-parse failure. +func ParseStoredManifests(manifest string) ([]releaseutil.Manifest, error) { + rawManifests := releaseutil.SplitManifests(manifest) + keys := make([]string, 0, len(rawManifests)) + for key := range rawManifests { + keys = append(keys, key) + } + sort.Sort(releaseutil.BySplitManifestsOrder(keys)) + + manifests := make([]releaseutil.Manifest, 0, len(keys)) + for _, key := range keys { + content := rawManifests[key] + name := manifestSourcePath(content) + if name == "" { + name = key + } + + var head releaseutil.SimpleHead + if err := yaml.Unmarshal([]byte(content), &head); err != nil { + return nil, fmt.Errorf("YAML parse error on %s: %w", name, err) + } + + manifests = append(manifests, releaseutil.Manifest{ + Name: name, + Content: content, + Head: &head, + }) + } + + return manifests, nil +} + +func manifestSourcePath(content string) string { + for line := range strings.SplitSeq(content, "\n") { + line = strings.TrimSpace(line) + if source, ok := strings.CutPrefix(line, "# Source: "); ok { + return source + } + } + + return "" +} diff --git a/pkg/release/v1/sequence/plan.go b/pkg/release/v1/sequence/plan.go new file mode 100644 index 000000000..533a2c387 --- /dev/null +++ b/pkg/release/v1/sequence/plan.go @@ -0,0 +1,136 @@ +/* +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 sequence computes the ordered deployment plan for a release's +// rendered manifests per HIP-0025: a two-level walk over the chart's subchart +// DAG and each chart level's resource-group DAG, flattened into an ordered +// list of apply-and-wait batches. The package is pure — it never talks to a +// cluster and must not import pkg/kube, pkg/action, or pkg/cmd — so the same +// plan is usable by install/upgrade drivers, uninstall/rollback (reversed), +// `helm template`/`helm dag` rendering, and lint. +package sequence + +import ( + "strings" + + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +// BatchKind distinguishes the two batch flavors a chart level produces. +type BatchKind uint8 + +const ( + // BatchKindGroups: one topological level of the resource-group DAG — + // one or more groups deployable in parallel. + BatchKindGroups BatchKind = iota + // BatchKindUnsequenced: a chart level's trailing batch — resources with no + // group annotation, resources demoted for invalid depends-on JSON, + // groups demoted for missing references, and isolated groups. + BatchKindUnsequenced +) + +// Group is a named set of manifests deployed as one unit within a batch. +type Group struct { + Name string // "" only inside a BatchKindUnsequenced batch + Manifests []releaseutil.Manifest // input order preserved +} + +// Batch is one apply-and-wait unit. Drivers process Batches strictly in order. +type Batch struct { + // ChartPath is the manifest path-prefix of the owning chart level: + // "parent", "parent/charts/db", "parent/charts/db/charts/redis". + // "" when the plan was built without a chart (flat fallback). + ChartPath string + // Depth is the chart-nesting depth (0 = top level). Display-only. + Depth int + Kind BatchKind + // Groups: for BatchKindGroups, ≥1 groups sorted by Name (one DAG level); + // for BatchKindUnsequenced, exactly one Group{Name: ""}. + Groups []Group + // Wait: gate on readiness (install/upgrade) or deletion (reversed plans) + // before starting the next batch. The builder sets true for every batch; + // the field exists so a strict-HIP mode can flip leaf-only batches later. + Wait bool + // HasCustomReadiness: at least one manifest in the batch carries BOTH + // helm.sh/readiness-success and helm.sh/readiness-failure (drivers add + // kube.WithCustomReadinessStatusReader for such batches). + HasCustomReadiness bool + // LeafGroups: names of groups in this batch with no dependents in this + // chart level's DAG. Informational — display and future policy only. + LeafGroups []string +} + +// Manifests flattens the batch's groups in order. Convenience for drivers. +func (b Batch) Manifests() []releaseutil.Manifest { + var manifests []releaseutil.Manifest + for _, group := range b.Groups { + manifests = append(manifests, group.Manifests...) + } + return manifests +} + +// Warning is a non-fatal misconfiguration surfaced during Build. +// Drivers log them; lint promotes them to errors. +type Warning struct { + ChartPath string // "" for chart-independent warnings + Message string +} + +// ChartLevel summarizes one chart's slot in the walk, in pre-order. +// Consumed by `helm dag` (subchart-batch display) and diagnostics. +type ChartLevel struct { + Path string // manifest path-prefix (as Batch.ChartPath) + Depth int + SubchartBatches [][]string // declared subcharts per parallel DAG batch (effective names) + Undeclared []string // rendered subcharts absent from Chart.yaml deps (sorted) + Unresolved []string // rendered subchart dirs with no resolvable chart object (sorted) +} + +// Plan is the complete deployment sequence for one release's manifests. +type Plan struct { + Batches []Batch // forward (install) order + Levels []ChartLevel // pre-order chart traversal + Warnings []Warning +} + +// Reverse returns a new Plan whose Batches are in exact reverse order, for +// uninstall and removed-resource deletion (HIP: uninstall is the exact reverse +// of install). Levels and Warnings are shared unchanged; Wait retains its +// meaning ("gate before next batch", i.e. wait-for-delete). +func (p *Plan) Reverse() *Plan { + if p == nil { + return nil + } + + reversed := make([]Batch, len(p.Batches)) + for i, batch := range p.Batches { + reversed[len(p.Batches)-1-i] = batch + } + + return &Plan{ + Batches: reversed, + Levels: p.Levels, + Warnings: p.Warnings, + } +} + +// DisplayPath converts a manifest path-prefix to the HIP display form used by +// template markers and `helm dag`: "parent/charts/db/charts/redis" → "parent/db/redis". +// (Chart names cannot contain '/', so replacing "/charts/" is unambiguous; +// a subchart literally named "charts" is pathological and unsupported.) +func DisplayPath(chartPath string) string { + return strings.ReplaceAll(chartPath, "/charts/", "/") +} diff --git a/pkg/release/v1/sequence/plan_test.go b/pkg/release/v1/sequence/plan_test.go new file mode 100644 index 000000000..c84dacbce --- /dev/null +++ b/pkg/release/v1/sequence/plan_test.go @@ -0,0 +1,171 @@ +/* +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 sequence + +import ( + "fmt" + "go/parser" + "go/token" + "os" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func TestPlanReverse(t *testing.T) { + t.Parallel() + + manifests := []releaseutil.Manifest{ + makeManifest("base", "parent/templates/base.yaml", groupAnnotations("base")), + makeManifest("left", "parent/templates/left.yaml", groupAnnotations("left", "base")), + makeManifest("right", "parent/templates/right.yaml", groupAnnotations("right", "base")), + makeManifest("top", "parent/templates/top.yaml", groupAnnotations("top", "left", "right")), + makeManifest("plain", "parent/templates/plain.yaml", nil), + } + plan, err := Build(newChart("parent"), manifests) + require.NoError(t, err) + require.GreaterOrEqual(t, len(plan.Batches), 4) + + original := batchSummaries(plan) + reversed := plan.Reverse() + require.NotNil(t, reversed) + require.Len(t, reversed.Batches, len(plan.Batches)) + + for i := range reversed.Batches { + assert.Equal(t, original[len(original)-1-i], summarizeBatch(reversed.Batches[i])) + } + assert.Equal(t, original, batchSummaries(plan), "Reverse must not mutate the original plan") + assert.Equal(t, plan.Levels, reversed.Levels) + assert.Equal(t, plan.Warnings, reversed.Warnings) + assert.Equal(t, original, batchSummaries(reversed.Reverse())) +} + +func TestDisplayPath(t *testing.T) { + t.Parallel() + + tests := map[string]string{ + "parent": "parent", + "parent/charts/db": "parent/db", + "parent/charts/db/charts/redis": "parent/db/redis", + "": "", + } + + for input, expected := range tests { + assert.Equal(t, expected, DisplayPath(input)) + } +} + +func TestBatchManifests(t *testing.T) { + t.Parallel() + + first := makeManifest("first", "parent/templates/first.yaml", nil) + second := makeManifest("second", "parent/templates/second.yaml", nil) + third := makeManifest("third", "parent/templates/third.yaml", nil) + batch := Batch{ + Groups: []Group{ + {Name: "a", Manifests: []releaseutil.Manifest{first, second}}, + {Name: "b", Manifests: []releaseutil.Manifest{third}}, + }, + } + + assert.Equal(t, []releaseutil.Manifest{first, second, third}, batch.Manifests()) +} + +func TestReadinessAnnotationValues(t *testing.T) { + t.Parallel() + + assert.Equal(t, "helm.sh/readiness-success", releaseutil.AnnotationReadinessSuccess) + assert.Equal(t, "helm.sh/readiness-failure", releaseutil.AnnotationReadinessFailure) +} + +func TestPackageImportPurity(t *testing.T) { + t.Parallel() + + entries, err := os.ReadDir(".") + require.NoError(t, err) + + allowedHelmImports := map[string]bool{ + "helm.sh/helm/v4/pkg/chart/v2": true, + "helm.sh/helm/v4/pkg/chart/v2/util": true, + "helm.sh/helm/v4/pkg/release/v1/util": true, + } + + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".go") || strings.HasSuffix(entry.Name(), "_test.go") { + continue + } + + file, err := parser.ParseFile(token.NewFileSet(), entry.Name(), nil, parser.ImportsOnly) + require.NoError(t, err) + for _, imported := range file.Imports { + path, err := strconv.Unquote(imported.Path.Value) + require.NoError(t, err) + if strings.HasPrefix(path, "helm.sh/helm/v4/") { + assert.Truef(t, allowedHelmImports[path], "forbidden helm import %q in %s", path, entry.Name()) + } + assert.Falsef(t, strings.HasPrefix(path, "k8s.io/"), "forbidden Kubernetes import %q in %s", path, entry.Name()) + } + } +} + +func TestParseStoredManifests(t *testing.T) { + t.Parallel() + + stream := strings.Join([]string{ + "---\n# Source: parent/templates/a.yaml\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: a\n annotations:\n example.com/key: value\n", + "---\n# Source: parent/charts/db/templates/b.yaml\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: b\n", + "---\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: c\n", + }, "") + + parsed, err := ParseStoredManifests(stream) + require.NoError(t, err) + require.Len(t, parsed, 3) + assert.Equal(t, []string{"parent/templates/a.yaml", "parent/charts/db/templates/b.yaml", "manifest-2"}, manifestNames(parsed)) + assert.Equal(t, "ConfigMap", parsed[0].Head.Kind) + assert.Equal(t, "a", parsed[0].Head.Metadata.Name) + assert.Equal(t, "value", parsed[0].Head.Metadata.Annotations["example.com/key"]) + assert.Equal(t, "b", parsed[1].Head.Metadata.Name) + assert.Equal(t, "c", parsed[2].Head.Metadata.Name) + + _, err = ParseStoredManifests("---\n# Source: bad.yaml\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: [\n") + require.Error(t, err) + assert.Contains(t, err.Error(), "YAML parse error") + + flat, err := Build(nil, parsed) + require.NoError(t, err) + assertPlanComplete(t, flat, parsed) + require.Len(t, flat.Batches, 1) + assert.Empty(t, flat.Batches[0].ChartPath) + + db := makeManifest("db", "parent/templates/db.yaml", groupAnnotations("db")) + app := makeManifest("app", "parent/templates/app.yaml", groupAnnotations("app", "db")) + stored := fmt.Sprintf("---\n# Source: %s\n%s---\n# Source: %s\n%s", db.Name, db.Content, app.Name, app.Content) + parsedStored, err := ParseStoredManifests(stored) + require.NoError(t, err) + + fromStored, err := Build(newChart("parent"), parsedStored) + require.NoError(t, err) + fromDirect, err := Build(newChart("parent"), []releaseutil.Manifest{db, app}) + require.NoError(t, err) + assert.Equal(t, batchSummaries(fromDirect), batchSummaries(fromStored)) + assertPlanComplete(t, fromStored, parsedStored) +} diff --git a/pkg/release/v1/util/manifest.go b/pkg/release/v1/util/manifest.go index fa26f6256..31babdbb5 100644 --- a/pkg/release/v1/util/manifest.go +++ b/pkg/release/v1/util/manifest.go @@ -30,6 +30,7 @@ type SimpleHead struct { Kind string `json:"kind,omitempty"` Metadata *struct { Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` Annotations map[string]string `json:"annotations"` } `json:"metadata,omitempty"` } @@ -80,3 +81,34 @@ func (a BySplitManifestsOrder) Less(i, j int) bool { return anum < bnum } func (a BySplitManifestsOrder) Swap(i, j int) { a[i], a[j] = a[j], a[i] } + +// helmInternalAnnotationLineRE matches a single YAML line whose key is one of +// HelmInternalSequencingAnnotations. The pattern is intentionally line-based: +// Helm always emits these annotations as single-line JSON-encoded values, so a +// surgical line strip preserves the surrounding manifest byte-for-byte and +// keeps `helm template | diff` workflows stable. The regex is compiled lazily +// from HelmInternalSequencingAnnotations so that adding a new helm-internal +// key only requires updating the slice. +var helmInternalAnnotationLineRE = func() *regexp.Regexp { + keys := make([]string, len(HelmInternalSequencingAnnotations)) + for i, k := range HelmInternalSequencingAnnotations { + keys[i] = regexp.QuoteMeta(k) + } + return regexp.MustCompile(`(?m)^[ \t]+(?:` + strings.Join(keys, "|") + `):[^\n]*\r?\n?`) +}() + +// StripHelmInternalAnnotations returns the manifest content with any +// HelmInternalSequencingAnnotations removed. The strip is line-based and +// preserves the surrounding byte order of the input document. Empty content +// passes through unchanged. +// +// This exists so that `helm template` output remains directly apply-able via +// `kubectl apply -f -`, even when charts use HIP-0025 sequencing annotations +// whose keys contain multiple `/` separators (which fail Kubernetes +// annotation-key validation). +func StripHelmInternalAnnotations(content string) string { + if strings.TrimSpace(content) == "" { + return content + } + return helmInternalAnnotationLineRE.ReplaceAllString(content, "") +} diff --git a/pkg/release/v1/util/manifest_test.go b/pkg/release/v1/util/manifest_test.go index d1efa21e1..6f4c5b636 100644 --- a/pkg/release/v1/util/manifest_test.go +++ b/pkg/release/v1/util/manifest_test.go @@ -17,6 +17,7 @@ limitations under the License. package util // import "helm.sh/helm/v4/pkg/release/v1/util" import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -514,3 +515,100 @@ metadata: }) } } + +func TestStripHelmInternalAnnotations(t *testing.T) { + tests := []struct { + name string + input string + mustNotContain []string + mustContain []string + mustEqualVerbatim bool + }{ + { + name: "strips multi-slash key, preserves siblings", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-a + annotations: + helm.sh/depends-on/resource-groups: '["foo"]' + helm.sh/resource-group: app + other.example.com/keep: "yes" +data: + k: v +`, + mustNotContain: []string{"helm.sh/depends-on/resource-groups"}, + mustContain: []string{"helm.sh/resource-group", "other.example.com/keep", "name: cm-a"}, + }, + { + name: "leaves dangling empty annotations key when sole annotation stripped", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-b + annotations: + helm.sh/depends-on/resource-groups: '["foo"]' +data: + k: v +`, + mustNotContain: []string{"helm.sh/depends-on/resource-groups"}, + mustContain: []string{"name: cm-b", "data:"}, + }, + { + name: "no annotations leaves doc unchanged", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-c +data: + k: v +`, + mustEqualVerbatim: true, + }, + { + name: "non-internal annotations preserved", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-d + annotations: + helm.sh/resource-group: app +data: + k: v +`, + mustEqualVerbatim: true, + }, + { + name: "non-kubernetes yaml passes through", + input: "foo: bar\nbaz: 1\n", + mustEqualVerbatim: true, + }, + { + name: "invalid yaml passes through", + input: ":\n\tnot valid yaml at all: [", + mustEqualVerbatim: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := StripHelmInternalAnnotations(tt.input) + if tt.mustEqualVerbatim { + if got != tt.input { + t.Errorf("expected verbatim passthrough, got:\n%s", got) + } + return + } + for _, s := range tt.mustNotContain { + if strings.Contains(got, s) { + t.Errorf("output unexpectedly contains %q:\n%s", s, got) + } + } + for _, s := range tt.mustContain { + if !strings.Contains(got, s) { + t.Errorf("output missing expected %q:\n%s", s, got) + } + } + }) + } +} diff --git a/pkg/release/v1/util/resource_group.go b/pkg/release/v1/util/resource_group.go new file mode 100644 index 000000000..e5d96ccbf --- /dev/null +++ b/pkg/release/v1/util/resource_group.go @@ -0,0 +1,247 @@ +/* +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" + + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" +) + +const ( + // AnnotationResourceGroup declares the resource-group a resource belongs to. + AnnotationResourceGroup = "helm.sh/resource-group" + + // AnnotationDependsOnResourceGroups declares prerequisite resource-groups for a + // resource's group as a JSON string array. + AnnotationDependsOnResourceGroups = "helm.sh/depends-on/resource-groups" + + // AnnotationReadinessSuccess declares custom readiness success conditions. + // Its value is a JSON array of expressions in the form: + // "{.fieldPath} " + // If any success condition evaluates to true, the resource is considered ready. + // This is the canonical home of the key (moved from pkg/kube, which keeps + // value-identical aliases) so the pure sequencing plan builder can read it + // without importing cluster machinery. + AnnotationReadinessSuccess = "helm.sh/readiness-success" + + // AnnotationReadinessFailure declares custom readiness failure conditions. + // Its value is a JSON array of expressions in the form: + // "{.fieldPath} " + // If any failure condition evaluates to true, the resource is considered failed. + // Failure conditions take precedence over success conditions. + AnnotationReadinessFailure = "helm.sh/readiness-failure" +) + +// HelmInternalSequencingAnnotations lists annotation keys used by Helm for +// resource sequencing that are NOT valid Kubernetes annotation keys (their +// names contain multiple `/` separators). Helm strips these before applying +// resources to the API server, and before printing manifests via +// `helm template` so downstream tooling like `kubectl apply` accepts the +// output unmodified. +var HelmInternalSequencingAnnotations = []string{ + AnnotationDependsOnResourceGroups, +} + +// ResourceGroupResult holds the output of ParseResourceGroups. +type ResourceGroupResult struct { + // Groups maps group name to manifests assigned to that group. + Groups map[string][]Manifest + + // GroupDeps maps group name to the group names it depends on. + GroupDeps map[string][]string + + // Unsequenced contains manifests that should be deployed outside the DAG. + Unsequenced []Manifest +} + +// ParseResourceGroups extracts resource-group annotations from rendered +// manifests and partitions them into sequenced groups and unsequenced manifests. +// +// Manifests without a resource-group annotation are treated as unsequenced. +// Invalid depends-on annotations emit a warning and demote that manifest to the +// unsequenced batch. References to unknown groups emit a warning and demote the +// entire referencing group to the unsequenced batch. If the same resource is +// assigned to different groups, ParseResourceGroups returns an error. +func ParseResourceGroups(manifests []Manifest) (ResourceGroupResult, []string, error) { + result := ResourceGroupResult{ + Groups: make(map[string][]Manifest), + GroupDeps: make(map[string][]string), + } + var warnings []string + + resourceAssignments := make(map[string]string) + groupDeps := make(map[string][]string) + groupOrder := make([]string, 0) + + for _, manifest := range manifests { + groupName, hasGroup := resourceGroupName(manifest) + if !hasGroup { + result.Unsequenced = append(result.Unsequenced, manifest) + continue + } + + resourceID := resourceGroupResourceID(manifest) + if existingGroup, ok := resourceAssignments[resourceID]; ok && existingGroup != groupName { + return ResourceGroupResult{}, warnings, fmt.Errorf( + "resource %q assigned to multiple resource groups %q and %q", + resourceID, + existingGroup, + groupName, + ) + } + resourceAssignments[resourceID] = groupName + + deps, warning, err := resourceGroupDependencies(manifest) + if err != nil { + warnings = append(warnings, warning) + result.Unsequenced = append(result.Unsequenced, manifest) + continue + } + + if _, ok := result.Groups[groupName]; !ok { + groupOrder = append(groupOrder, groupName) + } + + result.Groups[groupName] = append(result.Groups[groupName], manifest) + groupDeps[groupName] = appendUniqueStrings(groupDeps[groupName], deps...) + } + + for changed := true; changed; { + changed = false + + for _, groupName := range groupOrder { + groupManifests, ok := result.Groups[groupName] + if !ok { + continue + } + + missingDep := firstMissingDependency(result.Groups, groupDeps[groupName]) + if missingDep == "" { + continue + } + + warnings = append(warnings, fmt.Sprintf( + "group %q depends-on non-existent group %q; moving group to unsequenced batch", + groupName, + missingDep, + )) + result.Unsequenced = append(result.Unsequenced, groupManifests...) + delete(result.Groups, groupName) + changed = true + } + } + + for _, groupName := range groupOrder { + if _, ok := result.Groups[groupName]; !ok { + continue + } + + result.GroupDeps[groupName] = groupDeps[groupName] + } + + return result, warnings, nil +} + +// BuildResourceGroupDAG constructs a DAG from the resource-group parse result. +// Each group becomes a DAG node and every dependency becomes an edge. +func BuildResourceGroupDAG(result ResourceGroupResult) (*chartutil.DAG, error) { + dag := chartutil.NewDAG() + + for groupName := range result.Groups { + dag.AddNode(groupName) + } + + for groupName, deps := range result.GroupDeps { + for _, dep := range deps { + if err := dag.AddEdge(dep, groupName); err != nil { + return nil, fmt.Errorf("resource-group sequencing edge %s→%s: %w", dep, groupName, err) + } + } + } + + return dag, nil +} + +func resourceGroupName(manifest Manifest) (string, bool) { + if manifest.Head == nil || manifest.Head.Metadata == nil || len(manifest.Head.Metadata.Annotations) == 0 { + return "", false + } + + groupName, ok := manifest.Head.Metadata.Annotations[AnnotationResourceGroup] + if !ok || groupName == "" { + return "", false + } + + return groupName, true +} + +func resourceGroupDependencies(manifest Manifest) ([]string, string, error) { + annotations := manifest.Head.Metadata.Annotations + rawDeps, ok := annotations[AnnotationDependsOnResourceGroups] + if !ok { + return nil, "", nil + } + + var deps []string + if err := json.Unmarshal([]byte(rawDeps), &deps); err != nil { + return nil, fmt.Sprintf( + "manifest %q: invalid JSON in %s annotation: %v; moving to unsequenced batch", + manifest.Name, + AnnotationDependsOnResourceGroups, + err, + ), err + } + + return deps, "", nil +} + +func firstMissingDependency(groups map[string][]Manifest, deps []string) string { + for _, dep := range deps { + if _, ok := groups[dep]; !ok { + return dep + } + } + + return "" +} + +func appendUniqueStrings(existing []string, values ...string) []string { + seen := make(map[string]struct{}, len(existing)) + for _, value := range existing { + seen[value] = struct{}{} + } + + for _, value := range values { + if _, ok := seen[value]; ok { + continue + } + existing = append(existing, value) + seen[value] = struct{}{} + } + + return existing +} + +func resourceGroupResourceID(manifest Manifest) string { + if manifest.Head == nil || manifest.Head.Metadata == nil || manifest.Head.Metadata.Name == "" { + return manifest.Name + } + + return fmt.Sprintf("%s/%s/%s/%s", manifest.Head.Version, manifest.Head.Kind, manifest.Head.Metadata.Namespace, manifest.Head.Metadata.Name) +} diff --git a/pkg/release/v1/util/resource_group_test.go b/pkg/release/v1/util/resource_group_test.go new file mode 100644 index 000000000..14f6c95dc --- /dev/null +++ b/pkg/release/v1/util/resource_group_test.go @@ -0,0 +1,381 @@ +/* +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" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseResourceGroups_NoAnnotations(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("plain-a", "chart/templates/plain-a.yaml", nil), + makeManifest("plain-b", "chart/templates/plain-b.yaml", nil), + ) + + assert.Empty(t, result.Groups) + assert.Empty(t, result.GroupDeps) + require.Len(t, result.Unsequenced, 2) + assert.Equal(t, []string{"chart/templates/plain-a.yaml", "chart/templates/plain-b.yaml"}, manifestPaths(result.Unsequenced)) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_GroupWithDependency(t *testing.T) { + t.Parallel() + + result, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + require.Len(t, result.Groups, 2) + assert.Equal(t, [][]string{{"database"}, {"app"}}, batches) + assert.Empty(t, result.Unsequenced) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_MultipleResourcesPerGroup(t *testing.T) { + t.Parallel() + + result, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database-config", "chart/templates/database-config.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("database-secret", "chart/templates/database-secret.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + require.Len(t, result.Groups["database"], 2) + assert.Equal(t, []string{ + "chart/templates/database-config.yaml", + "chart/templates/database-secret.yaml", + }, manifestPaths(result.Groups["database"])) + assert.Equal(t, [][]string{{"database"}, {"app"}}, batches) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_ResourceAssignedToMultipleGroups(t *testing.T) { + t.Parallel() + + _, _, err := ParseResourceGroups([]Manifest{ + makeManifest("shared", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("shared", "chart/templates/cache.yaml", map[string]string{ + AnnotationResourceGroup: "cache", + }), + }) + + require.Error(t, err) + assert.ErrorContains(t, err, "assigned to multiple resource groups") + assert.ErrorContains(t, err, "database") + assert.ErrorContains(t, err, "cache") +} + +// TestParseResourceGroups_SameNameDifferentNamespaces locks the contract +// that two resources sharing apiVersion+kind+name but living in distinct +// namespaces are NOT collapsed into a single resource ID. Without the +// namespace dimension in resourceGroupResourceID, this scenario would +// raise a false "assigned to multiple resource groups" error. +func TestParseResourceGroups_SameNameDifferentNamespaces(t *testing.T) { + t.Parallel() + + m1 := makeManifest("shared", "chart/templates/a.yaml", map[string]string{ + AnnotationResourceGroup: "groupA", + }) + m1.Head.Metadata.Namespace = "ns-a" + m2 := makeManifest("shared", "chart/templates/b.yaml", map[string]string{ + AnnotationResourceGroup: "groupB", + }) + m2.Head.Metadata.Namespace = "ns-b" + + result, warnings, err := ParseResourceGroups([]Manifest{m1, m2}) + + require.NoError(t, err, "same-name resources in different namespaces must not collide") + assert.Empty(t, warnings) + assert.Len(t, result.Groups["groupA"], 1) + assert.Len(t, result.Groups["groupB"], 1) +} + +func TestParseResourceGroups_NonExistentGroupReferenceWarning(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["missing"]`, + }), + ) + + assert.Empty(t, result.Groups) + require.Len(t, result.Unsequenced, 1) + assert.Equal(t, []string{"chart/templates/app.yaml"}, manifestPaths(result.Unsequenced)) + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "non-existent group") + assert.Contains(t, warnings[0], "missing") +} + +func TestParseResourceGroups_CascadingMissingDeps(t *testing.T) { + t.Parallel() + + manifests := []Manifest{ + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + AnnotationDependsOnResourceGroups: `["missing"]`, + }), + } + + for range 64 { + result, warnings := parseResourceGroups(t, manifests...) + + assert.Empty(t, result.Groups) + assert.Empty(t, result.GroupDeps) + require.Len(t, result.Unsequenced, 2) + assert.ElementsMatch(t, []string{ + "chart/templates/app.yaml", + "chart/templates/database.yaml", + }, manifestPaths(result.Unsequenced)) + require.Len(t, warnings, 2) + assert.Contains(t, warnings[0], `group "database" depends-on non-existent group "missing"`) + assert.Contains(t, warnings[1], `group "app" depends-on non-existent group "database"`) + + dag, err := BuildResourceGroupDAG(result) + require.NoError(t, err) + batches, err := dag.GetBatches() + require.NoError(t, err) + assert.Empty(t, batches) + } +} + +func TestParseResourceGroups_InvalidDependsOnJSON(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `not-valid-json`, + }), + ) + + assert.Empty(t, result.Groups) + require.Len(t, result.Unsequenced, 1) + assert.Equal(t, []string{"chart/templates/app.yaml"}, manifestPaths(result.Unsequenced)) + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "invalid JSON") +} + +func TestParseResourceGroups_MixedSequencedAndUnsequenced(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("plain", "chart/templates/plain.yaml", nil), + ) + + require.Len(t, result.Groups, 1) + assert.Contains(t, result.Groups, "database") + require.Len(t, result.Unsequenced, 1) + assert.Equal(t, []string{"chart/templates/plain.yaml"}, manifestPaths(result.Unsequenced)) + assert.Empty(t, warnings) +} + +func TestResourceGroupDAG_CycleDetection(t *testing.T) { + t.Parallel() + + result, _ := parseResourceGroups(t, + makeManifest("a", "chart/templates/a.yaml", map[string]string{ + AnnotationResourceGroup: "a", + AnnotationDependsOnResourceGroups: `["c"]`, + }), + makeManifest("b", "chart/templates/b.yaml", map[string]string{ + AnnotationResourceGroup: "b", + AnnotationDependsOnResourceGroups: `["a"]`, + }), + makeManifest("c", "chart/templates/c.yaml", map[string]string{ + AnnotationResourceGroup: "c", + AnnotationDependsOnResourceGroups: `["b"]`, + }), + ) + + dag, err := BuildResourceGroupDAG(result) + require.NoError(t, err) + + batches, err := dag.GetBatches() + require.Error(t, err) + assert.Nil(t, batches) + assert.ErrorContains(t, err, "cycle") +} + +func TestParseResourceGroups_DeduplicatesDependencies(t *testing.T) { + t.Parallel() + + result, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("app-config", "chart/templates/app-config.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeManifest("app-secret", "chart/templates/app-secret.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + assert.Equal(t, []string{"database"}, result.GroupDeps["app"]) + assert.Equal(t, [][]string{{"database"}, {"app"}}, batches) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_IsolatedGroupsRemainInBatch0(t *testing.T) { + t.Parallel() + + _, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("metrics", "chart/templates/metrics.yaml", map[string]string{ + AnnotationResourceGroup: "metrics", + }), + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + assert.Equal(t, [][]string{{"database", "metrics"}, {"app"}}, batches) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_ComplexDAG(t *testing.T) { + t.Parallel() + + _, warnings, batches := parseResourceGroupBatches(t, + makeManifest("cache", "chart/templates/cache.yaml", map[string]string{ + AnnotationResourceGroup: "cache", + }), + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("queue", "chart/templates/queue.yaml", map[string]string{ + AnnotationResourceGroup: "queue", + }), + makeManifest("api", "chart/templates/api.yaml", map[string]string{ + AnnotationResourceGroup: "api", + AnnotationDependsOnResourceGroups: `["database","queue"]`, + }), + makeManifest("worker", "chart/templates/worker.yaml", map[string]string{ + AnnotationResourceGroup: "worker", + AnnotationDependsOnResourceGroups: `["cache","queue"]`, + }), + makeManifest("frontend", "chart/templates/frontend.yaml", map[string]string{ + AnnotationResourceGroup: "frontend", + AnnotationDependsOnResourceGroups: `["api"]`, + }), + ) + + assert.Equal(t, [][]string{ + {"cache", "database", "queue"}, + {"api", "worker"}, + {"frontend"}, + }, batches) + assert.Empty(t, warnings) +} + +func parseResourceGroups(t *testing.T, manifests ...Manifest) (ResourceGroupResult, []string) { + t.Helper() + + result, warnings, err := ParseResourceGroups(manifests) + require.NoError(t, err) + + return result, warnings +} + +func parseResourceGroupBatches(t *testing.T, manifests ...Manifest) (ResourceGroupResult, []string, [][]string) { + t.Helper() + + result, warnings := parseResourceGroups(t, manifests...) + dag, err := BuildResourceGroupDAG(result) + require.NoError(t, err) + + batches, err := dag.GetBatches() + require.NoError(t, err) + + return result, warnings, batches +} + +func makeManifest(name, sourcePath string, annotations map[string]string) Manifest { + var content strings.Builder + fmt.Fprintf(&content, "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: %s\n", name) + if len(annotations) > 0 { + content.WriteString(" annotations:\n") + for key, value := range annotations { + fmt.Fprintf(&content, " %s: %q\n", key, value) + } + } + + head := &SimpleHead{ + Version: "v1", + Kind: "ConfigMap", + Metadata: &struct { + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + Annotations map[string]string `json:"annotations"` + }{ + Name: name, + Annotations: annotations, + }, + } + + return Manifest{ + Name: sourcePath, + Content: content.String(), + Head: head, + } +} + +func manifestPaths(manifests []Manifest) []string { + paths := make([]string, 0, len(manifests)) + for _, manifest := range manifests { + paths = append(paths, manifest.Name) + } + + return paths +}