diff --git a/internal/release/v2/util/manifest.go b/internal/release/v2/util/manifest.go index 37b9dfbd3..87e6377dd 100644 --- a/internal/release/v2/util/manifest.go +++ b/internal/release/v2/util/manifest.go @@ -34,18 +34,37 @@ type SimpleHead struct { } `json:"metadata,omitempty"` } -var sep = regexp.MustCompile(`(?m)^---[ \t]*`) +// sep matches YAML document separators. A separator is `---` at the start of +// a line (or start of the stream), followed by horizontal whitespace or by a +// newline/end of stream. The whitespace branch allows same-line comments or +// content while leaving their suffix in the following manifest. The multiline +// anchor leaves the preceding document's trailing whitespace and newlines +// intact. +// +// 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(`(?m)^---(?:[ \t]+|\r?$)`) // 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 rather than being silently repaired. Downstream +// validation can then reject the malformed manifest. 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 e76905a63..c58d1ff92 100644 --- a/internal/release/v2/util/manifest_test.go +++ b/internal/release/v2/util/manifest_test.go @@ -28,6 +28,37 @@ func TestSplitManifests(t *testing.T) { input string expected map[string]string }{ + { + name: "commented separator after existing document (LF)", + input: "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n--- # next document\napiVersion: v1\nkind: Service\nmetadata:\n name: svc\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n", + "manifest-1": "# next document\napiVersion: v1\nkind: Service\nmetadata:\n name: svc\n", + }, + }, + { + name: "commented separator after existing document (CRLF)", + input: "apiVersion: v1\r\nkind: ConfigMap\r\nmetadata:\r\n name: cm1\r\n--- # next document\r\napiVersion: v1\r\nkind: Service\r\nmetadata:\r\n name: svc\r\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\r\nkind: ConfigMap\r\nmetadata:\r\n name: cm1\r\n", + "manifest-1": "# next document\r\napiVersion: v1\r\nkind: Service\r\nmetadata:\r\n name: svc\r\n", + }, + }, + { + name: "whitespace-separated same-line content is a document boundary", + input: "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n--- apiVersion: v1\nkind: Service\nmetadata:\n name: svc\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n", + "manifest-1": "apiVersion: v1\nkind: Service\nmetadata:\n name: svc\n", + }, + }, + { + name: "no-whitespace marker remains attached", + input: "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n---#comment\napiVersion: v1\nkind: Service\nmetadata:\n name: svc\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: cm1\n---#comment\napiVersion: v1\nkind: Service\nmetadata:\n name: svc\n", + }, + }, { name: "single doc with leading separator and whitespace", input: ` @@ -401,12 +432,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 @@ -414,7 +446,7 @@ metadata: name: cm1 `, expected: map[string]string{ - "manifest-0": `apiVersion: v1 + "manifest-0": `---apiVersion: v1 kind: ConfigMap metadata: name: cm1 @@ -422,7 +454,7 @@ metadata: }, }, { - name: "mid-content glued separator (---apiVersion)", + name: "mid-content glued separator stays with first doc", input: ` apiVersion: v1 kind: ConfigMap @@ -438,8 +470,7 @@ metadata: kind: ConfigMap metadata: name: cm1 -`, - "manifest-1": `apiVersion: v1 +---apiVersion: v1 kind: ConfigMap metadata: name: cm2 @@ -447,7 +478,7 @@ metadata: }, }, { - name: "multiple glued separators", + name: "multiple glued separators produce a single doc", input: ` ---apiVersion: v1 kind: ConfigMap @@ -463,17 +494,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 +---apiVersion: v1 kind: ConfigMap metadata: name: cm2 -`, - "manifest-2": `apiVersion: v1 +---apiVersion: v1 kind: ConfigMap metadata: name: cm3 @@ -481,7 +510,7 @@ metadata: }, }, { - name: "mixed glued and proper separators", + name: "proper separators split, glued ones do not", input: ` apiVersion: v1 kind: ConfigMap @@ -507,14 +536,50 @@ metadata: kind: ConfigMap metadata: name: cm2 -`, - "manifest-2": `apiVersion: v1 +---apiVersion: v1 kind: ConfigMap metadata: name: cm3 `, }, }, + { + name: "block scalar trailing whitespace survives a following separator", + input: "apiVersion: v1\nkind: ConfigMap\ndata:\n value: |+\n hello \n\n---\napiVersion: v1\nkind: Service\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\nkind: ConfigMap\ndata:\n value: |+\n hello \n\n", + "manifest-1": "apiVersion: v1\nkind: Service\n", + }, + }, + { + name: "CRLF document endings survive a following separator", + input: "apiVersion: v1\r\nkind: ConfigMap\r\n\r\n--- \r\napiVersion: v1\r\nkind: Service\r\n", + expected: map[string]string{ + "manifest-0": "apiVersion: v1\r\nkind: ConfigMap\r\n\r\n", + "manifest-1": "apiVersion: v1\r\nkind: Service\r\n", + }, + }, + { + 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\n", + }, + }, + { + 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 {