Merge 4867fc1298 into 0752c1f5b5

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
+// `---<non-whitespace>` (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

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 {