From d5724cf75f9333160732678330a18314aa9cc113 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Fri, 18 Sep 2026 19:20:33 -0700 Subject: [PATCH] fix(postrender): emit block-style YAML for JSON manifests in annotateAndMerge Motivation: annotateAndMerge (used by helm template's post-render pipeline) parses each rendered manifest with kyaml, adds a postrenderer.helm.sh/postrender-filename annotation, and re-serializes all manifests into one merged YAML stream that is piped to the post-renderer. When a chart resource is written as pure JSON, kyaml preserves the original flow style ({...}) when re-emitting the document, but the newly added annotation is inserted using plain YAML syntax (unquoted key, single-quoted value). The result starts with '{' but is not valid JSON. k8s.io/apimachinery/pkg/runtime/serializer/yaml's decoder assumes anything starting with '{' is pure JSON, so a Go post-renderer using that decoder fails to parse the resource, even though the same manifest parses fine without a post-renderer. This worked in Helm 3. Approach: Clear the flow-style flag recursively on every manifest node before merging, so annotateAndMerge always emits unambiguous block-style YAML regardless of whether the source file was JSON or YAML. This also normalizes manifests that intentionally use flow-style YAML (e.g. metadata: {name: foo}) to block style when piped through a post-renderer; the resulting YAML is valid and semantically identical, only its formatting changes. This is a byproduct of how SetAnnotation can insert block-style content under any ancestor in the tree, so any flow-style ancestor (not just the document root) could reproduce the same class of bug. Validation: - Added TestAnnotateAndMerge_JSONManifest_DecodableByApimachinery, which reproduces the issue's exact repro: builds a JSON chart resource, runs it through annotateAndMerge, and decodes the result with k8s.io/apimachinery/pkg/runtime/serializer/yaml's NewDecodingSerializer(unstructured.UnstructuredJSONScheme), the exact decoder from the report. Verified this test fails on the pre-fix code with "invalid character 'a' looking for beginning of object key string" and passes after the fix. - Added a JSON-manifest case to the existing table-driven TestAnnotateAndMerge. - go test ./pkg/action/... (all pass) - go build ./pkg/action/... - go vet ./pkg/action/... - golangci-lint run ./pkg/action/... (0 issues) - make build - go mod tidy -diff (no diff) - make test-source-headers Report: https://github.com/helm/helm/issues/32668 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5 (via Claude Code) --- pkg/action/action.go | 22 +++++++++++++++++ pkg/action/action_test.go | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/pkg/action/action.go b/pkg/action/action.go index e93d6181f..bc2dc3049 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -173,6 +173,19 @@ const ( filenameAnnotation = "postrenderer.helm.sh/postrender-filename" ) +// clearFlowStyle recursively clears the flow-style formatting flag from a YAML +// node and its descendants, forcing block-style output when the node is later +// serialized. +func clearFlowStyle(node *kyaml.Node) { + if node == nil { + return + } + node.Style &^= kyaml.FlowStyle + for _, child := range node.Content { + clearFlowStyle(child) + } +} + // annotateAndMerge combines multiple YAML files into a single stream of documents, // adding filename annotations to each document for later reconstruction. func annotateAndMerge(files map[string]string) (string, error) { @@ -212,6 +225,15 @@ func annotateAndMerge(files map[string]string) (string, error) { if err := manifest.PipeE(kyaml.SetAnnotation(filenameAnnotation, fname)); err != nil { return "", fmt.Errorf("annotating %s: %w", fname, err) } + // kyaml preserves the flow style of documents that were + // originally written as JSON. Left as-is, the annotated + // document can be re-emitted starting with '{' while + // containing YAML-only syntax (unquoted keys, single-quoted + // strings), which k8s.io/apimachinery's YAML decoder + // misidentifies as pure JSON and fails to parse. Clearing + // the flow style forces block-style output, which is always + // unambiguous YAML. + clearFlowStyle(manifest.YNode()) combinedManifests = append(combinedManifests, manifest) } } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 056c539a5..82f1d6cf3 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -28,6 +28,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + kyamlserializer "k8s.io/apimachinery/pkg/runtime/serializer/yaml" fakeclientset "k8s.io/client-go/kubernetes/fake" "helm.sh/helm/v4/internal/logging" @@ -425,6 +427,26 @@ metadata: postrenderer.helm.sh/postrender-filename: 'templates/configmap.yaml' data: key: value +`, + }, + { + name: "single file with JSON manifest", + files: map[string]string{ + "templates/cm.json": `{"apiVersion": "v1", "kind": "ConfigMap", "metadata": {"name": "test"}, "data": {"key": "hello"}}`, + }, + // The merged output must not be re-emitted in flow style: a + // document annotated in flow style can start with '{' while + // containing YAML-only syntax (unquoted keys, single-quoted + // values), which k8s.io/apimachinery's decoder misidentifies as + // pure JSON and fails to parse. + expected: `"apiVersion": "v1" +"kind": "ConfigMap" +"metadata": + "name": "test" + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/cm.json' +"data": + "key": "hello" `, }, { @@ -1799,6 +1821,34 @@ data: } } +// TestAnnotateAndMerge_JSONManifest_DecodableByApimachinery reproduces +// https://github.com/helm/helm/issues/32668: a chart resource written as pure +// JSON must still be decodable by k8s.io/apimachinery's YAML serializer after +// annotateAndMerge adds the post-render filename annotation. Before the fix, +// kyaml preserved the document's flow style, so the annotated document was +// re-emitted starting with '{' while containing YAML-only syntax (an +// unquoted annotation key and single-quoted value); apimachinery's decoder +// assumes anything starting with '{' is pure JSON and failed to parse it. +func TestAnnotateAndMerge_JSONManifest_DecodableByApimachinery(t *testing.T) { + files := map[string]string{ + "templates/cm.json": `{"apiVersion": "v1", "kind": "ConfigMap", "metadata": {"name": "test"}, "data": {"key": "hello"}}`, + } + + merged, err := annotateAndMerge(files) + require.NoError(t, err) + require.False(t, strings.HasPrefix(strings.TrimSpace(merged), "{"), + "merged output must not start with '{', or apimachinery's decoder will misidentify it as pure JSON: %s", merged) + + deserializer := kyamlserializer.NewDecodingSerializer(unstructured.UnstructuredJSONScheme) + for doc := range strings.SplitSeq(merged, "---") { + if strings.TrimSpace(doc) == "" { + continue + } + _, _, err := deserializer.Decode([]byte(doc), nil, new(unstructured.Unstructured)) + require.NoError(t, err) + } +} + func TestRenderResources_PostRenderer_Success(t *testing.T) { cfg := actionConfigFixture(t)