fix(templating): SplitManifests must preserve line endings for downstream YAML parsers by matheuscscp · Pull Request #31952 · helm/helm

matheuscscp · 2026-03-21T11:49:51Z

Fixes: #31948

Builds on: #31941

Please review and merge #31941 first.

Copilot

Pull request overview

This PR updates Helm’s manifest splitting and post-renderer merge logic to preserve trailing line endings so downstream YAML parsing/round-tripping (via kio.ParseAll) behaves consistently with non-post-renderer code paths—especially around YAML block scalars and document separators.

Changes:

Update releaseutil.SplitManifests to trim only leading whitespace (preserving trailing newlines/whitespace).
Rework action.annotateAndMerge to split per-file content via releaseutil.SplitManifests, sort by in-file order, and then parse/annotate each document.
Expand and restructure tests to cover block-scalar chomping and glued-separator behaviors.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
pkg/release/v1/util/manifest.go	Preserve trailing line endings by switching from `TrimSpace` to left-trim only; adds Chart API v3 note.
pkg/release/v1/util/manifest_test.go	Replaces single-case test with a table-driven suite covering separators and trailing newline preservation.
pkg/action/action.go	Uses `SplitManifests` + deterministic ordering before YAML parsing/annotation in post-renderer merge path.
pkg/action/action_test.go	Removes `fixDocSeparators` tests and greatly expands `annotateAndMerge` test coverage (including block scalars and separator edge cases).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

marvin-roesch · 2026-03-22T11:24:03Z

+    postrenderer.helm.sh/postrender-filename: 'templates/cm.yaml'
 data:
-  key: value
+  key: |-


As noted in #31941 (comment), this is an example of where the regex utilized by SplitManifests causes issues again and behaviour differs from Helm 3. Helm 3 would produce this file with clip block scalar mode (i.e. just |, no modifier), not with the chomping like it's returned here.

If it is desired to fully restore Helm 3's behaviour, that's an issue. Playing around some more with Helm 3, there are further differences surrounding trailing newline/whitespace handling related to this. Take the following template as an example:

apiVersion: v1 kind: ConfigMap metadata: name: test-cm name: test data: key: |+ hello

Helm 3 produces the following output:

apiVersion: v1 kind: ConfigMap metadata: name: test-cm name: test data: key: | hello

That is it strips all but the first trailing newline.

This suggests to me that the "more correct" fix (to align with Helm 3) would be to perform both-sided trimming in SplitManifests after all, but always append a newline at the end of each document that is split produced. That would lead kio parsing to leave | and |- alone, but transform |+ into |. Furthermore, templates that originally didn't end in a newline (a test case that is missing here, I think, at least with a block scalar) would also match Helm 3's output.

That fix could be entirely localized to action.annotateAndMerge, leaving SplitManifests alone otherwise.

YAML block scalars are a mess, dealing with them correctly involves a ton of edge cases.

I guess this is a bug in Helm 3 (that was carried over to Helm 4). |+ is specifically to preserve multiple newlines. I vote for fixing this as a long-time bug, without this fix Helm basically does not support |+.

Very much agreed. That's why I emphasized the if on restoring Helm 3 behaviour. Arguably, Helm 4 with a post renderer is most compliant with the YAML or at least best at preserving user intent. Both Helm 3 and 4 without post renderers have deficiencies in that regard.

That notwithstanding, the test case on the line I've put this comment on is an indicator of where this PR would produce incorrect output/change user intent. If I have a document before a separator that ends with a "clip" block scalar, that ought to be preserved and not changed into a chomp modifier.

I should've left the other remark as a separate comment, got a bit carried away there 😅

@scottrigby Arguably this could be backported to Helm 3.20, but at the same time I don't think the impact in Helm 3 users of this bug is wide, it would have surfaced years ago if it was. So while this is a bug from Helm 3, I think we could fix it only in Helm 4.

@marvin-roesch We are planning to backport this PR to our fork and making a Flux patch release. Do you think merging this one as is will make it worse? Do you have a suggestion for fixing this test case?

I don't have an easy way to fix this in mind. If Go's regex engine supported lookaheads, we could just use that for the preceding whitespace from the splitting regex and be fine, I think. Without that, I think a more "manual" approach to splitting like you implemented in the previous fix would be required.

As for the impact of merging this iteration: I guess multi document templates are rare enough as is, and trimmed whitespace at the end only causes problems in certain situations, too. Since we're templating Kubernetes resources, it's also always an option to move e.g. the kind: ... line to the end of the document to preserve any whitespace there. Documenting this edge case with a suggested workaround in the release notes might be wise.

…ream YAML parsers Signed-off-by: Matheus Pimenta <[email protected]>

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

banjoh · 2026-04-10T18:33:19Z

Code review

Found 1 issue:

internal/release/v2/util/manifest.go is not updated to match the v1 fix — SplitManifests in the v2 package still uses strings.TrimSpace on both the global input (line 47) and per-document (line 55), while the v1 version was corrected to use strings.TrimLeftFunc. This creates diverging trailing-newline behavior between the Chart API v1 and v2 (Chart API v3) processing paths. Any post-renderer using the v2 code path will still hit the same kio.ParseAll block-scalar chomping issue this PR is fixing.

helm/internal/release/v2/util/manifest.go

Lines 45 to 58 in 2202747

    
           res := map[string]string{} 
        
           // Making sure that any extra whitespace in YAML stream doesn't interfere in splitting documents correctly. 
        
           bigFileTmp := strings.TrimSpace(bigFile) 
        
           docs := sep.Split(bigFileTmp, -1) 
        
           var count int 
        
           for _, d := range docs { 
        
           	if d == "" { 
        
           		continue 
        
           	} 
        
           	d = strings.TrimSpace(d) 
        
           	res[fmt.Sprintf(tpl, count)] = d 
        
           	count = count + 1 
        
           }

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

banjoh · 2026-04-10T18:38:24Z

Code review

Found 1 issue:

internal/release/v2/util/manifest.go is not updated to match the v1 fix — SplitManifests in the v2 package still uses strings.TrimSpace on both the global input (line 47) and per-document (line 55), while the v1 version was corrected to use strings.TrimLeftFunc. This creates diverging trailing-newline behavior between the Chart API v1 and v2 (Chart API v3) processing paths. Any post-renderer using the v2 code path will still hit the same kio.ParseAll block-scalar chomping issue this PR is fixing.

helm/internal/release/v2/util/manifest.go

Lines 45 to 58 in 2202747

res := map[string]string{}

// Making sure that any extra whitespace in YAML stream doesn't interfere in splitting documents correctly.

bigFileTmp := strings.TrimSpace(bigFile)

docs := sep.Split(bigFileTmp, -1)

var count int

for _, d := range docs {

if d == "" {

continue

}

d = strings.TrimSpace(d)

res[fmt.Sprintf(tpl, count)] = d

count = count + 1

}

🤖 Generated with Claude Code

If this code review was useful, please react with 👍. Otherwise, react with 👎.

Since at this point rendering Charts v2 and v3 is pretty much the same, I suggest making sure this fix is there as well so as not to diverge

Signed-off-by: Matheus Pimenta <[email protected]>

Copilot

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

pkg/release/v1/util/manifest.go:47

The doc comment for SplitManifests contains informal/unprofessional phrasing (e.g., “kill this regex with fire”, “Helm should say a big NO”). Since this is effectively public documentation, please reword it in a neutral/professional tone while keeping the same technical intent.

// **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,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Matheus Pimenta <[email protected]>

banjoh

LGTM

TerryHowe

/lgtm

Are we concerned about the change in yaml output format with the extra line?

TerryHowe · 2026-04-14T15:20:29Z

+// 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


I don't like regex for the performance hit. I'd like to see it gone.

matheuscscp · 2026-04-14T15:31:22Z

Are we concerned about the change in yaml output format with the extra line?

I think it's ok, it's a direct consequence of the change, which is a bugfix for | and |+. @marvin-roesch What do you think?

matheuscscp · 2026-04-15T09:24:25Z

@marvin-roesch The PR already has two approvals, do you have any major concerns? Otherwise I'll ping the maintainers to merge it and we'll ship it in our Flux fork

marvin-roesch · 2026-04-15T13:05:04Z

I think all my concerns have been addressed, thanks! I agree that this should be dealt with ultimately in Chart V3.

Looking forward to have this shipped in Flux! We have a few charts with workarounds right now 😅

…ream YAML parsers (helm#31952) * fix(templating): SplitManifests must preserve line endings for downstream YAML parsers Signed-off-by: Matheus Pimenta <[email protected]> * Address copilot comment about skipping empty docs Signed-off-by: Matheus Pimenta <[email protected]> * Port fix to release v2 Signed-off-by: Matheus Pimenta <[email protected]> * Address copilot comments Signed-off-by: Matheus Pimenta <[email protected]> --------- Signed-off-by: Matheus Pimenta <[email protected]>

Copilot AI review requested due to automatic review settings March 21, 2026 11:49

pull-request-size Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 21, 2026

Copilot started reviewing on behalf of matheuscscp March 21, 2026 11:50 View session

Copilot AI reviewed Mar 21, 2026

View reviewed changes

Comment thread pkg/release/v1/util/manifest.go

matheuscscp force-pushed the fix-31948 branch from b89c2fd to 6d805ff Compare March 21, 2026 12:08

matheuscscp mentioned this pull request Mar 21, 2026

fix(templating): fix wrong YAML separator parsing for post-renderers #31941

Merged

Copilot AI review requested due to automatic review settings March 21, 2026 12:21

matheuscscp force-pushed the fix-31948 branch from 6d805ff to ae8f334 Compare March 21, 2026 12:21

Copilot started reviewing on behalf of matheuscscp March 21, 2026 12:21 View session

Copilot AI reviewed Mar 21, 2026

View reviewed changes

Comment thread pkg/release/v1/util/manifest.go Outdated

matheuscscp force-pushed the fix-31948 branch from ae8f334 to 2202747 Compare March 21, 2026 12:28

marvin-roesch reviewed Mar 22, 2026

View reviewed changes

fix(templating): SplitManifests must preserve line endings for downst…

159198f

…ream YAML parsers Signed-off-by: Matheus Pimenta <[email protected]>

Copilot AI review requested due to automatic review settings April 10, 2026 18:16

matheuscscp force-pushed the fix-31948 branch from 2202747 to 159198f Compare April 10, 2026 18:16

Copilot started reviewing on behalf of matheuscscp April 10, 2026 18:16 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Comment thread pkg/release/v1/util/manifest.go

Comment thread pkg/action/action_test.go

Address copilot comment about skipping empty docs

95be360

Signed-off-by: Matheus Pimenta <[email protected]>

banjoh requested changes Apr 10, 2026

View reviewed changes

Port fix to release v2

0aca3ae

Signed-off-by: Matheus Pimenta <[email protected]>

Copilot AI review requested due to automatic review settings April 10, 2026 19:11

Copilot started reviewing on behalf of matheuscscp April 10, 2026 19:11 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Comment thread internal/release/v2/util/manifest.go Outdated

Comment thread pkg/action/action_test.go Outdated

banjoh mentioned this pull request Apr 10, 2026

Remove YAML separator whitespace-trim workaround in Chart v3 #32036

Open

Address copilot comments

c318a6e

Signed-off-by: Matheus Pimenta <[email protected]>

banjoh approved these changes Apr 10, 2026

View reviewed changes

TerryHowe approved these changes Apr 14, 2026

View reviewed changes

banjoh merged commit 265c5eb into helm:main Apr 15, 2026
5 checks passed

matheuscscp deleted the fix-31948 branch April 15, 2026 13:34

matheuscscp mentioned this pull request Apr 17, 2026

fix(templating): SplitManifests must preserve line endings for downstream YAML parsers fluxcd/helm#2

Merged

matheuscscp mentioned this pull request Jul 10, 2026

Regression from Helm 3: End of file newlines getting stripped #32326

Closed

		// 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

Uh oh!

Conversation

matheuscscp commented Mar 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

marvin-roesch Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

matheuscscp Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marvin-roesch Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

matheuscscp Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matheuscscp Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

marvin-roesch Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

banjoh commented Apr 10, 2026

Code review

Uh oh!

banjoh commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

banjoh left a comment

Choose a reason for hiding this comment

Uh oh!

TerryHowe left a comment

Choose a reason for hiding this comment

Uh oh!

TerryHowe Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

matheuscscp commented Apr 14, 2026

Uh oh!

matheuscscp commented Apr 15, 2026

Uh oh!

marvin-roesch commented Apr 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

matheuscscp Mar 22, 2026 •

edited

Loading

matheuscscp Mar 23, 2026 •

edited

Loading

banjoh commented Apr 10, 2026 •

edited

Loading