fix(action): preserve trailing newlines when writing manifests

Manifest splitting preserves trailing newlines so post-renderers can
parse block scalars correctly. The output writers still append another
newline, changing |+ and >+ values in installed resources as well as
adding blank lines to rendered YAML.

Trim one newline before the format string adds it back. This preserves
all existing newlines and adds one only if missing. Document separators
need a preceding line break; adding a missing newline also matches how
Kubernetes' YAMLOrJSONDecoder handles an unterminated final line.

Apply this rule to renderResources and its output-dir writer, leaving
manifest splitting and post-renderer parsing unchanged.

Test all six block scalar indicators with zero, one and two authored
newlines, in final and non-final positions. Check exact output bytes and
decoded values for both buffer and file output, including Kubernetes'
EOF normalization.

Signed-off-by: 胡玮文 <huweiwen.hww@alibaba-inc.com>
pull/32683/head
胡玮文 1 week ago
parent 0a4b962d4e
commit bcc3acd9fe

@ -270,6 +270,14 @@ func splitAndDeannotate(postrendered, fallbackPrefix string) (map[string]string,
return reconstructed, nil
}
func appendSourceManifest(b []byte, source, body string) []byte {
// Preserve existing trailing newlines: block scalars may retain them.
// Adding a missing newline can change a scalar's value, but document
// separators require a line break and Kubernetes' YAMLOrJSONDecoder
// also adds one to an unterminated final line.
return fmt.Appendf(b, "---\n# Source: %s\n%s\n", source, strings.TrimSuffix(body, "\n"))
}
// renderResources renders the templates in a chart
//
// TODO: This function is badly in need of a refactor.
@ -355,7 +363,7 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
if strings.TrimSpace(content) == "" {
continue
}
b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", name, content)
b = appendSourceManifest(b, name, content)
}
return hs, b, "", err
}
@ -473,7 +481,7 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
if strings.TrimSpace(content) == "" {
continue
}
b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", name, content)
b = appendSourceManifest(b, name, content)
}
return hs, b, "", err
}
@ -484,7 +492,7 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
if includeCrds {
for _, crd := range ch.CRDObjects() {
if outputDir == "" {
b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", crd.Filename, string(crd.File.Data))
b = appendSourceManifest(b, crd.Filename, string(crd.File.Data))
} else {
err = writeToFile(outputDir, crd.Filename, string(crd.File.Data), fileWritten[crd.Filename])
if err != nil {
@ -498,9 +506,9 @@ func (cfg *Configuration) renderResources(ctx context.Context, ch *chart.Chart,
for _, m := range manifests {
if outputDir == "" {
if hideSecret && m.Head.Kind == "Secret" && m.Head.Version == "v1" {
b = fmt.Appendf(b, "---\n# Source: %s\n# HIDDEN: The Secret output has been suppressed\n", m.Name)
b = appendSourceManifest(b, m.Name, "# HIDDEN: The Secret output has been suppressed\n")
} else {
b = fmt.Appendf(b, "---\n# Source: %s\n%s\n", m.Name, m.Content)
b = appendSourceManifest(b, m.Name, m.Content)
}
} else {
newDir := outputDir

@ -22,12 +22,16 @@ import (
"fmt"
"io"
"log/slog"
"os"
"path/filepath"
"strings"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.yaml.in/yaml/v3"
k8syaml "k8s.io/apimachinery/pkg/util/yaml"
fakeclientset "k8s.io/client-go/kubernetes/fake"
"helm.sh/helm/v4/internal/logging"
@ -1826,15 +1830,12 @@ func TestRenderResources_PostRenderer_Success(t *testing.T) {
expectedBuf := `---
# Source: yellow/templates/foodpie
foodpie: world
---
# Source: yellow/templates/with-partials
yellow: Earth
---
# Source: yellow/templates/yellow
yellow: world
`
expectedHook := `kind: ConfigMap
metadata:
@ -1946,17 +1947,14 @@ func TestRenderResources_PostRenderer_Integration(t *testing.T) {
# Source: hello/templates/goodbye
goodbye: world
color: blue
---
# Source: hello/templates/hello
hello: world
color: blue
---
# Source: hello/templates/with-partials
hello: Earth
color: blue
`
assert.Contains(t, output, "color: blue")
assert.Equal(t, 3, strings.Count(output, "color: blue"))
@ -2276,6 +2274,74 @@ metadata:
assert.ErrorContains(t, err, "bogus")
}
func TestRenderResources_BlockScalarChomping(t *testing.T) {
tests := []struct {
name string
indicator string
value string
wantNewlines [3]int // indexed by the number of authored trailing newlines
}{
{"literal_clip", "|", "line1\nline2", [3]int{1, 1, 1}},
{"literal_strip", "|-", "line1\nline2", [3]int{0, 0, 0}},
{"literal_keep", "|+", "line1\nline2", [3]int{1, 1, 2}},
{"folded_clip", ">", "line1 line2", [3]int{1, 1, 1}},
{"folded_strip", ">-", "line1 line2", [3]int{0, 0, 0}},
{"folded_keep", ">+", "line1 line2", [3]int{1, 1, 2}},
}
for _, tc := range tests {
for trailing := range 3 {
for _, output := range []string{"buffer", "output-dir"} {
t.Run(fmt.Sprintf("%s/%d_newlines/%s", tc.name, trailing, output), func(t *testing.T) {
var files []*common.File
var wantOutput strings.Builder
for _, name := range []string{"first", "last"} {
body := fmt.Sprintf("apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: %s\ndata:\n cfg: %s\n line1\n line2", name, tc.indicator)
path := "templates/" + name + ".yaml"
files = append(files, &common.File{Name: path, Data: []byte(body + strings.Repeat("\n", trailing))})
fmt.Fprintf(&wantOutput, "---\n# Source: hello/%s\n%s%s", path, body, strings.Repeat("\n", max(1, trailing)))
}
outputDir := ""
if output == "output-dir" {
outputDir = t.TempDir()
}
cfg := actionConfigFixture(t)
_, rendered, _, err := cfg.renderResources(
t.Context(), buildChartWithTemplates(files), nil, "test-release", outputDir, false, false, false,
nil, false, false, false, PostRenderStrategyCombined,
)
require.NoError(t, err)
if outputDir != "" {
assert.Empty(t, rendered)
for _, file := range files {
data, err := os.ReadFile(filepath.Join(outputDir, "hello", file.Name))
require.NoError(t, err)
rendered = append(rendered, data...)
}
}
assert.Equal(t, wantOutput.String(), string(rendered))
var cm struct {
Data map[string]string `json:"data" yaml:"data"`
}
wantValue := tc.value + strings.Repeat("\n", tc.wantNewlines[trailing])
// Kubernetes also normalizes a missing newline in a standalone document.
require.NoError(t, k8syaml.NewYAMLOrJSONDecoder(bytes.NewReader(files[0].Data), 4096).Decode(&cm))
assert.Equal(t, wantValue, cm.Data["cfg"])
// Decode the whole stream without Kubernetes' implicit EOF newline.
decoder := yaml.NewDecoder(bytes.NewReader(rendered))
for range 2 {
require.NoError(t, decoder.Decode(&cm))
assert.Equal(t, wantValue, cm.Data["cfg"])
}
require.ErrorIs(t, decoder.Decode(&cm), io.EOF)
})
}
}
}
}
func TestDetermineReleaseSSAApplyMethod(t *testing.T) {
assert.Equal(t, release.ApplyMethodClientSideApply, determineReleaseSSApplyMethod(false))
assert.Equal(t, release.ApplyMethodServerSideApply, determineReleaseSSApplyMethod(true))

@ -730,7 +730,7 @@ func writeToFile(outputDir, name, data string, appendData bool) error {
defer f.Close()
_, err = fmt.Fprintf(f, "---\n# Source: %s\n%s\n", name, data)
_, err = fmt.Fprintf(f, "---\n# Source: %s\n%s\n", name, strings.TrimSuffix(data, "\n"))
if err != nil {
return err
}

@ -9,7 +9,6 @@ rules:
resources: ["pods", "pods/exec", "pods/log"]
verbs: ["*"]
---
# Source: hello/templates/rbac
apiVersion: rbac.authorization.k8s.io/v1
@ -25,4 +24,3 @@ subjects:
- kind: ServiceAccount
name: schedule-agents
namespace: spaced

@ -15,7 +15,6 @@ metadata:
name: test-secret
stringData:
foo: bar
---
# Source: chart-with-secret/templates/configmap.yaml
apiVersion: v1
@ -25,4 +24,3 @@ metadata:
data:
foo: bar

@ -15,7 +15,6 @@ hash:
key4: 4
key5: 5
key6: 6
---
# Source: issue-9027/templates/values.yaml
global:

@ -11,7 +11,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/01-a.yml
# 2
@ -25,7 +24,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/01-a.yml
# 3
@ -39,7 +37,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 5
@ -53,7 +50,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 7
@ -67,7 +63,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 8
@ -81,7 +76,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 9
@ -95,7 +89,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 10
@ -109,7 +102,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 11
@ -123,7 +115,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 12
@ -137,7 +128,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 13
@ -151,7 +141,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 14
@ -165,7 +154,6 @@ spec:
- Egress
- Ingress
---
# Source: object-order/templates/02-b.yml
# 15 (11th object within 02-b.yml, in order to test `SplitManifests` which assigns `manifest-10`
@ -179,7 +167,6 @@ spec:
policyTypes:
- Egress
- Ingress
---
# Source: object-order/templates/01-a.yml
# 4 (Deployment should come after all NetworkPolicy manifests, since 'helm template' outputs in install order)

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -15,7 +14,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -30,7 +28,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -48,7 +45,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -66,7 +62,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -15,7 +14,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -30,7 +28,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -48,7 +45,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -66,7 +62,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -9,7 +9,6 @@ rules:
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -25,4 +24,3 @@ subjects:
name: subchart-sa
namespace: default

@ -38,4 +38,3 @@ spec:
selector:
app.kubernetes.io/name: subcharta

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -15,7 +14,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -30,7 +28,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -48,7 +45,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -66,7 +62,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/configmap.yaml
apiVersion: v1
@ -23,7 +22,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -38,7 +36,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -56,7 +53,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -74,7 +70,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/configmap.yaml
apiVersion: v1
@ -23,7 +22,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -38,7 +36,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -56,7 +53,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -74,7 +70,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/configmap.yaml
apiVersion: v1
@ -23,7 +22,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -38,7 +36,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -56,7 +53,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -74,7 +70,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -15,7 +14,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -30,7 +28,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -48,7 +45,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -66,7 +62,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -15,7 +14,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -30,7 +28,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -48,7 +45,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -66,7 +62,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -14,14 +14,12 @@ spec:
shortNames:
- tc
singular: authconfig
---
# Source: subchart/templates/subdir/serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -32,7 +30,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -47,7 +44,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -65,7 +61,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -83,7 +78,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -15,7 +14,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -30,7 +28,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -48,7 +45,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -66,7 +62,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

@ -4,7 +4,6 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: subchart-sa
---
# Source: subchart/templates/subdir/role.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -15,7 +14,6 @@ rules:
- apiGroups: [""]
resources: ["pods"]
verbs: ["get","list","watch"]
---
# Source: subchart/templates/subdir/rolebinding.yaml
apiVersion: rbac.authorization.k8s.io/v1
@ -30,7 +28,6 @@ subjects:
- kind: ServiceAccount
name: subchart-sa
namespace: default
---
# Source: subchart/charts/subcharta/templates/service.yaml
apiVersion: v1
@ -48,7 +45,6 @@ spec:
name: apache
selector:
app.kubernetes.io/name: subcharta
---
# Source: subchart/charts/subchartb/templates/service.yaml
apiVersion: v1
@ -66,7 +62,6 @@ spec:
name: nginx
selector:
app.kubernetes.io/name: subchartb
---
# Source: subchart/templates/service.yaml
apiVersion: v1

Loading…
Cancel
Save