diff --git a/internal/release/v2/util/manifest.go b/internal/release/v2/util/manifest.go index 5dbcdaea5..b0b82f907 100644 --- a/internal/release/v2/util/manifest.go +++ b/internal/release/v2/util/manifest.go @@ -34,18 +34,34 @@ type SimpleHead struct { } `json:"metadata,omitempty"` } -var sep = regexp.MustCompile("(?:^|\\s*\n)---\\s*") +// sep matches YAML document separators. A separator is `---` at the start of +// a line (or start of the stream), optionally followed by trailing horizontal +// whitespace, and then a newline or end of stream. +// +// This is intentionally stricter than the Chart v1/v2 regex in +// pkg/release/v1/util/manifest.go. The v1/v2 version tolerates +// `---` (e.g. `---apiVersion: v1`) by treating it as a +// separator glued to content — a silent correction for Go template whitespace +// trimming (`{{-`) eating the newline after `---`. Chart v3 does not carry +// that workaround forward; see the function comment below. +var sep = regexp.MustCompile("(?:^|\\s*\n)---[ \\t]*(?:\\r?\\n|$)") // SplitManifests takes a manifest string and returns a map containing individual manifests. // -// **Note for Chart API v3**: This function (due to the regex above) has allowed _WRONG_ -// Go templates to be defined inside charts across the years. The generated text from Go -// templates may contain `---apiVersion: v1`, and this function magically splits this back -// to `---\napiVersion: v1`. This has caused issues recently after Helm 4 introduced -// kio.ParseAll to inject annotations when post-renderers are used. In Chart API v3, -// we should kill this regex with fire (or change it) and expose charts doing the wrong -// thing Go template-wise. Helm should say a big _NO_ to charts doing the wrong thing, -// with or without post-renderers. +// Chart API v3 note: unlike Chart v1/v2, this implementation does NOT silently +// repair YAML document separators glued to content by Go template whitespace +// trimming. A template such as +// +// --- +// {{- include "mychart.service" . }} +// +// renders `---apiVersion: v1\n...` because `{{-` strips the newline after +// `---`. In Chart v1/v2, SplitManifests detects this and splits the input as +// if the newline were still there; in Chart v3, the glued `---` is left as +// part of the document body and downstream YAML parsing will surface the +// problem. Chart authors should drop the dash (`{{ include ... }}`) or omit +// the explicit `---` separator — Helm inserts one between templates on its +// own. See helm/helm#32036. func SplitManifests(bigFile string) map[string]string { // Basically, we're quickly splitting a stream of YAML documents into an // array of YAML docs. The file name is just a place holder, but should be diff --git a/internal/release/v2/util/manifest_test.go b/internal/release/v2/util/manifest_test.go index 72b095390..85d54048b 100644 --- a/internal/release/v2/util/manifest_test.go +++ b/internal/release/v2/util/manifest_test.go @@ -395,12 +395,13 @@ metadata: }, }, - // **Note for Chart API v3**: The following tests exercise the lenient - // regex that splits `---apiVersion` back into separate documents. - // In Chart API v3, these inputs should return an _ERROR_ instead. - // See the comment on the SplitManifests function for more details. + // Chart API v3 behaviour: separators glued to content (as produced by + // `{{-` trimming the newline after `---`) are NOT split apart. The + // glued `---` stays on the document body so downstream YAML parsing + // can surface the problem instead of Helm silently correcting it. + // See helm/helm#32036 and the SplitManifests doc comment. { - name: "leading glued separator (---apiVersion)", + name: "leading glued separator stays with content", input: ` ---apiVersion: v1 kind: ConfigMap @@ -408,7 +409,7 @@ metadata: name: cm1 `, expected: map[string]string{ - "manifest-0": `apiVersion: v1 + "manifest-0": `---apiVersion: v1 kind: ConfigMap metadata: name: cm1 @@ -416,7 +417,7 @@ metadata: }, }, { - name: "mid-content glued separator (---apiVersion)", + name: "mid-content glued separator stays with first doc", input: ` apiVersion: v1 kind: ConfigMap @@ -431,8 +432,8 @@ metadata: "manifest-0": `apiVersion: v1 kind: ConfigMap metadata: - name: cm1`, - "manifest-1": `apiVersion: v1 + name: cm1 +---apiVersion: v1 kind: ConfigMap metadata: name: cm2 @@ -440,7 +441,7 @@ metadata: }, }, { - name: "multiple glued separators", + name: "multiple glued separators produce a single doc", input: ` ---apiVersion: v1 kind: ConfigMap @@ -456,15 +457,15 @@ metadata: name: cm3 `, expected: map[string]string{ - "manifest-0": `apiVersion: v1 + "manifest-0": `---apiVersion: v1 kind: ConfigMap metadata: - name: cm1`, - "manifest-1": `apiVersion: v1 + name: cm1 +---apiVersion: v1 kind: ConfigMap metadata: - name: cm2`, - "manifest-2": `apiVersion: v1 + name: cm2 +---apiVersion: v1 kind: ConfigMap metadata: name: cm3 @@ -472,7 +473,7 @@ metadata: }, }, { - name: "mixed glued and proper separators", + name: "proper separators split, glued ones do not", input: ` apiVersion: v1 kind: ConfigMap @@ -496,14 +497,35 @@ metadata: "manifest-1": `apiVersion: v1 kind: ConfigMap metadata: - name: cm2`, - "manifest-2": `apiVersion: v1 + name: cm2 +---apiVersion: v1 kind: ConfigMap metadata: name: cm3 `, }, }, + { + name: "trailing separator with no newline is still a separator", + input: "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n---", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1", + }, + }, + { + name: "separator with trailing spaces and tabs is still a separator", + input: "---\t \napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n", + }, + }, + { + name: "CRLF line endings still split", + input: "---\r\napiVersion: v1\r\nkind: ConfigMap\r\nmetadata:\r\n name: cm1\r\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\r\nkind: ConfigMap\r\nmetadata:\r\n name: cm1\r\n", + }, + }, } for _, tt := range tests {