chore(templating): drop glued-separator workaround for chart v3

For chart apiVersion v3, tighten SplitManifests so it no longer silently
splits `---` glued to content (as happens when `{{-` trims the newline
after `---`). The Chart v1/v2 path in pkg/release/v1/util keeps the
lenient behaviour for backwards compatibility.

The regex now requires `---` to be alone on a line (optional trailing
horizontal whitespace, then newline or end of stream) before it counts as
a document separator. Glued input like `---apiVersion: v1` stays on the
document body so downstream YAML parsing surfaces the problem, matching
standard Go template semantics for `{{-`.

Refs helm/helm#32036

Signed-off-by: texasich <texasich@users.noreply.github.com>
pull/32185/head
texasich 5 months ago committed by texasich
parent 827a960ec1
commit fc0450e6b3

@ -34,18 +34,34 @@ type SimpleHead struct {
} `json:"metadata,omitempty"` } `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. // 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_ // Chart API v3 note: unlike Chart v1/v2, this implementation does NOT silently
// Go templates to be defined inside charts across the years. The generated text from Go // repair YAML document separators glued to content by Go template whitespace
// templates may contain `---apiVersion: v1`, and this function magically splits this back // trimming. A template such as
// 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 // {{- include "mychart.service" . }}
// thing Go template-wise. Helm should say a big _NO_ to charts doing the wrong thing, //
// with or without post-renderers. // 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 and the v2 → v3 migration guide.
func SplitManifests(bigFile string) map[string]string { func SplitManifests(bigFile string) map[string]string {
// Basically, we're quickly splitting a stream of YAML documents into an // 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 // array of YAML docs. The file name is just a place holder, but should be

@ -395,12 +395,13 @@ metadata:
}, },
}, },
// **Note for Chart API v3**: The following tests exercise the lenient // Chart API v3 behaviour: separators glued to content (as produced by
// regex that splits `---apiVersion` back into separate documents. // `{{-` trimming the newline after `---`) are NOT split apart. The
// In Chart API v3, these inputs should return an _ERROR_ instead. // glued `---` stays on the document body so downstream YAML parsing
// See the comment on the SplitManifests function for more details. // 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: ` input: `
---apiVersion: v1 ---apiVersion: v1
kind: ConfigMap kind: ConfigMap
@ -408,7 +409,7 @@ metadata:
name: cm1 name: cm1
`, `,
expected: map[string]string{ expected: map[string]string{
"manifest-0": `apiVersion: v1 "manifest-0": `---apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm1 name: cm1
@ -416,7 +417,7 @@ metadata:
}, },
}, },
{ {
name: "mid-content glued separator (---apiVersion)", name: "mid-content glued separator stays with first doc",
input: ` input: `
apiVersion: v1 apiVersion: v1
kind: ConfigMap kind: ConfigMap
@ -431,8 +432,8 @@ metadata:
"manifest-0": `apiVersion: v1 "manifest-0": `apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm1`, name: cm1
"manifest-1": `apiVersion: v1 ---apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm2 name: cm2
@ -440,7 +441,7 @@ metadata:
}, },
}, },
{ {
name: "multiple glued separators", name: "multiple glued separators produce a single doc",
input: ` input: `
---apiVersion: v1 ---apiVersion: v1
kind: ConfigMap kind: ConfigMap
@ -456,15 +457,15 @@ metadata:
name: cm3 name: cm3
`, `,
expected: map[string]string{ expected: map[string]string{
"manifest-0": `apiVersion: v1 "manifest-0": `---apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm1`, name: cm1
"manifest-1": `apiVersion: v1 ---apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm2`, name: cm2
"manifest-2": `apiVersion: v1 ---apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm3 name: cm3
@ -472,7 +473,7 @@ metadata:
}, },
}, },
{ {
name: "mixed glued and proper separators", name: "proper separators split, glued ones do not",
input: ` input: `
apiVersion: v1 apiVersion: v1
kind: ConfigMap kind: ConfigMap
@ -496,14 +497,35 @@ metadata:
"manifest-1": `apiVersion: v1 "manifest-1": `apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm2`, name: cm2
"manifest-2": `apiVersion: v1 ---apiVersion: v1
kind: ConfigMap kind: ConfigMap
metadata: metadata:
name: cm3 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 { for _, tt := range tests {

Loading…
Cancel
Save