diff --git a/pkg/action/sequencing.go b/pkg/action/sequencing.go index d290d0fd2..3c5a64d7d 100644 --- a/pkg/action/sequencing.go +++ b/pkg/action/sequencing.go @@ -299,14 +299,6 @@ func (s *sequencedDeployment) deployResourceGroupBatches(ctx context.Context, ma return nil } -// helmSequencingAnnotations lists annotation keys used internally by Helm for -// resource sequencing. These are stripped from resources before applying to -// Kubernetes because some (e.g. helm.sh/depends-on/resource-groups) contain -// multiple slashes which is invalid per the K8s annotation key format. -var helmSequencingAnnotations = []string{ - releaseutil.AnnotationDependsOnResourceGroups, -} - // stripSequencingAnnotations removes Helm-internal sequencing annotations from // resources before they are applied to Kubernetes. This prevents K8s API // validation errors for annotation keys that are not valid K8s label keys. @@ -324,7 +316,7 @@ func stripSequencingAnnotations(resources kube.ResourceList) error { return nil } changed := false - for _, key := range helmSequencingAnnotations { + for _, key := range releaseutil.HelmInternalSequencingAnnotations { if _, exists := annotations[key]; exists { delete(annotations, key) changed = true diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index 5e7086b4e..f3bcd4cf1 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -138,14 +138,14 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if skipTests && isTestHook(m) { continue } - fmt.Fprintf(out, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) + fmt.Fprintf(out, "---\n# Source: %s\n%s\n", m.Path, releaseutil.StripHelmInternalAnnotations(m.Manifest)) } } } } if !orderedRendered { var manifests bytes.Buffer - fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest)) + fmt.Fprintln(&manifests, strings.TrimSpace(releaseutil.StripHelmInternalAnnotations(rel.Manifest))) if !client.DisableHooks { fileWritten := make(map[string]bool) for _, m := range rel.Hooks { @@ -153,7 +153,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { continue } if client.OutputDir == "" { - fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) + fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, releaseutil.StripHelmInternalAnnotations(m.Manifest)) } else { newDir := client.OutputDir if client.UseReleaseName { @@ -401,7 +401,7 @@ func renderOrderedResourceGroups(manifests []releaseutil.Manifest, chartPath str for _, groupName := range batch { fmt.Fprintf(out, "## START resource-group: %s %s\n", chartPath, groupName) for _, manifest := range result.Groups[groupName] { - fmt.Fprintf(out, "---\n%s\n", manifest.Content) + fmt.Fprintf(out, "---\n%s\n", releaseutil.StripHelmInternalAnnotations(manifest.Content)) } fmt.Fprintf(out, "## END resource-group: %s %s\n", chartPath, groupName) } @@ -409,7 +409,7 @@ func renderOrderedResourceGroups(manifests []releaseutil.Manifest, chartPath str } for _, manifest := range result.Unsequenced { - fmt.Fprintf(out, "---\n%s\n", manifest.Content) + fmt.Fprintf(out, "---\n%s\n", releaseutil.StripHelmInternalAnnotations(manifest.Content)) } return nil @@ -529,3 +529,4 @@ func ensureDirectoryForFile(file string) error { return os.MkdirAll(baseDir, 0755) } + diff --git a/pkg/cmd/template_test.go b/pkg/cmd/template_test.go index 6e08ac6a3..6f613a77a 100644 --- a/pkg/cmd/template_test.go +++ b/pkg/cmd/template_test.go @@ -185,6 +185,31 @@ func TestTemplateWithoutOrderedWaitHasNoDelimiters(t *testing.T) { require.Contains(t, out, "# Source: sequenced-chart/charts/worker/templates/aa-worker-configmap.yaml") } +// TestTemplateStripsHelmInternalAnnotations asserts that `helm template` output +// never contains the multi-slash internal annotation key +// `helm.sh/depends-on/resource-groups` — its presence in the K8s API would +// fail annotation-key validation and break `helm template | kubectl apply -f -`. +// Valid sibling keys (single-slash) like `helm.sh/resource-group` must survive. +func TestTemplateStripsHelmInternalAnnotations(t *testing.T) { + const internalKey = "helm.sh/depends-on/resource-groups" + const siblingKey = "helm.sh/resource-group" + + t.Run("flat path", func(t *testing.T) { + _, out, err := executeActionCommand("template 'testdata/testcharts/sequenced-chart'") + require.NoError(t, err) + require.NotContains(t, out, internalKey, "internal annotation must be stripped from flat template output") + require.Contains(t, out, siblingKey, "valid-key sibling annotation must be preserved") + }) + + t.Run("ordered path", func(t *testing.T) { + _, out, err := executeActionCommand("template --wait=ordered 'testdata/testcharts/sequenced-chart'") + require.NoError(t, err) + require.NotContains(t, out, internalKey, "internal annotation must be stripped from ordered template output") + require.Contains(t, out, siblingKey, "valid-key sibling annotation must be preserved") + require.Contains(t, out, "## START resource-group:", "ordered output must still emit group delimiters") + }) +} + func TestTemplateVersionCompletion(t *testing.T) { repoFile := "testdata/helmhome/helm/repositories.yaml" repoCache := "testdata/helmhome/helm/repository" diff --git a/pkg/cmd/testdata/output/template-ordered-delimiters.txt b/pkg/cmd/testdata/output/template-ordered-delimiters.txt index fad44551d..bbdeaad08 100644 --- a/pkg/cmd/testdata/output/template-ordered-delimiters.txt +++ b/pkg/cmd/testdata/output/template-ordered-delimiters.txt @@ -31,7 +31,6 @@ metadata: name: app-config annotations: helm.sh/resource-group: app - helm.sh/depends-on/resource-groups: '["databases"]' data: db_host: localhost ## END resource-group: sequenced-chart app diff --git a/pkg/release/v1/util/manifest.go b/pkg/release/v1/util/manifest.go index 9596f5f51..31babdbb5 100644 --- a/pkg/release/v1/util/manifest.go +++ b/pkg/release/v1/util/manifest.go @@ -81,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 516ac42d7..adfbed1a6 100644 --- a/pkg/release/v1/util/manifest_test.go +++ b/pkg/release/v1/util/manifest_test.go @@ -18,6 +18,7 @@ package util // import "helm.sh/helm/v4/pkg/release/v1/util" import ( "reflect" + "strings" "testing" ) @@ -515,3 +516,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 index 03665b7b5..d441bb9c7 100644 --- a/pkg/release/v1/util/resource_group.go +++ b/pkg/release/v1/util/resource_group.go @@ -32,6 +32,16 @@ const ( AnnotationDependsOnResourceGroups = "helm.sh/depends-on/resource-groups" ) +// 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.