From dae163808e91196210df202edf09bb85055f4b6e Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Thu, 21 May 2026 15:33:43 +0200 Subject: [PATCH] Normalize YAML flow-style docs in post-renderer Helm v4's kyaml round-trips rendered manifests through its YAML parser, converting JSON output from toJson template functions into YAML flow-style with unquoted keys, e.g. {apiVersion: v1, kind: ConfigMap, ...}. k8s.io/apimachinery's IsJSONBuffer detects the leading '{' and returns such documents unchanged assuming they are already valid JSON. Passing them to json.Unmarshal then fails with 'invalid character' because the keys are unquoted. Add normalizeFlowStyleDocs to the post-renderer: before handing the rendered buffer to yaml.ToObjects, each document starting with '{' is checked with json.Valid; documents that fail are round-tripped through sigs.k8s.io/yaml to produce block-style YAML that the existing decode path handles correctly. Add hasFlowStyleCandidate as a zero-allocation fast-path: on every render call, a raw byte scan checks whether any document boundary starts with '{'. When none does (the common case), normalizeFlowStyleDocs returns the original slice immediately without any allocation or buffer rebuild. --- internal/helmdeployer/install_test.go | 34 +++++++ internal/helmdeployer/postrender.go | 116 +++++++++++++++++++++++ internal/helmdeployer/postrender_test.go | 116 +++++++++++++++++++++++ 3 files changed, 266 insertions(+) diff --git a/internal/helmdeployer/install_test.go b/internal/helmdeployer/install_test.go index cc66465af0..4d1c8bf134 100644 --- a/internal/helmdeployer/install_test.go +++ b/internal/helmdeployer/install_test.go @@ -16,6 +16,7 @@ import ( "os" "github.com/rancher/fleet/internal/experimental" + "github.com/rancher/fleet/internal/manifest" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -751,3 +752,36 @@ func TestInstallActionCorrectDriftForce(t *testing.T) { }) } } + +// TestTemplate_ToJsonFlowStyle is a regression test for the Helm v4 + kyaml +// issue where JSON output from a toJson template function is round-tripped +// through kyaml (annotateAndMerge), which converts it into YAML flow-style +// with unquoted keys, e.g. {apiVersion: v1, kind: ConfigMap}. +// k8s.io/apimachinery then mistakes the leading '{' for JSON and passes the +// document through unchanged, causing json.Unmarshal to fail. +// The user-visible error looks like: +// +// error while running post render on files: invalid character 'a' +func TestTemplate_ToJsonFlowStyle(t *testing.T) { + m := &manifest.Manifest{ + Resources: []fleet.BundleResource{ + { + Name: "test-chart/Chart.yaml", + Content: "apiVersion: v2\nname: test-chart\nversion: 0.1.0\ntype: application\n", + }, + { + // Template that outputs a raw JSON document via toJson. + // Helm v4's kyaml converts this to flow-style YAML with unquoted + // keys before Fleet's post-renderer sees it. + Name: "test-chart/templates/configmap.yaml", + Content: "{{- toJson (dict \"apiVersion\" \"v1\" \"kind\" \"ConfigMap\" \"metadata\" (dict \"name\" \"json-output\") \"data\" (dict \"key\" \"value\")) }}\n", + }, + }, + } + opts := fleet.BundleDeploymentOptions{ + DefaultNamespace: "default", + Helm: &fleet.HelmOptions{Chart: "test-chart"}, + } + _, err := Template(context.Background(), "test-bundle", m, opts, "v1.25.0") + require.NoError(t, err) +} diff --git a/internal/helmdeployer/postrender.go b/internal/helmdeployer/postrender.go index 204a2ec219..db33a6cc25 100644 --- a/internal/helmdeployer/postrender.go +++ b/internal/helmdeployer/postrender.go @@ -1,8 +1,12 @@ package helmdeployer import ( + "bufio" "bytes" + "encoding/json" + "errors" "fmt" + "io" "strings" "helm.sh/helm/v4/pkg/kube" @@ -18,10 +22,117 @@ import ( "github.com/rancher/wrangler/v3/pkg/yaml" "k8s.io/apimachinery/pkg/api/meta" + utilyaml "k8s.io/apimachinery/pkg/util/yaml" + sigsyaml "sigs.k8s.io/yaml" ) const CRDKind = "CustomResourceDefinition" +// isYAMLDocMarker reports whether b starts with a valid YAML document-start +// marker: "---" followed by whitespace, CR, LF, or end of input. Per the +// YAML spec, "---" is only a marker when it is the complete token on a line; +// a sequence like "---foo" is NOT a marker and must not be stripped. +func isYAMLDocMarker(b []byte) bool { + if !bytes.HasPrefix(b, []byte("---")) { + return false + } + if len(b) == 3 { + return true + } + c := b[3] + return c == ' ' || c == '\t' || c == '\r' || c == '\n' +} + +// normalizeFlowStyleDocs converts YAML documents that start with '{' but are +// not valid JSON (i.e. YAML flow-style with unquoted keys, as emitted by +// Helm v4's kyaml when templates use the toJson function) into block-style +// YAML. k8s.io/apimachinery's ToJSON treats any '{'-prefixed document as +// already-valid JSON and returns it unchanged, so a flow-style document with +// unquoted keys causes json.Unmarshal to fail with "invalid character". +// +// If no document boundary in data starts with '{', data is returned unchanged +// with no allocations. When flow-style documents are present, all documents +// are re-serialized into a consistent "---\n\n" format; documents +// that are not flow-style YAML are passed through without content changes. +func normalizeFlowStyleDocs(data []byte) ([]byte, error) { + if !hasFlowStyleCandidate(data) { + return data, nil + } + reader := utilyaml.NewYAMLReader(bufio.NewReader(bytes.NewReader(data))) + var out bytes.Buffer + for { + doc, err := reader.Read() + if err != nil { + if errors.Is(err, io.EOF) { + break + } + return nil, err + } + // The YAMLReader may include the document-start marker ("---") as part + // of the document bytes when it appears at the beginning of the input + // stream. Strip it to obtain the raw content of the document. Only + // strip a marker that is valid per YAML (followed by whitespace/EOF). + content := bytes.TrimSpace(doc) + if isYAMLDocMarker(content) { + content = bytes.TrimLeft(content[3:], " \t\r\n") + } + if len(content) == 0 { + continue + } + if content[0] == '{' && !json.Valid(content) { + var obj any + if err := sigsyaml.Unmarshal(content, &obj); err != nil { + return nil, err + } + content, err = sigsyaml.Marshal(obj) + if err != nil { + return nil, err + } + } + out.WriteString("---\n") + out.Write(content) + if content[len(content)-1] != '\n' { + out.WriteByte('\n') + } + } + return out.Bytes(), nil +} + +// hasFlowStyleCandidate reports whether data contains at least one YAML +// document whose first non-whitespace byte is '{'. It is a fast, zero- +// allocation scan used as a pre-check before the more expensive processing +// in normalizeFlowStyleDocs. +func hasFlowStyleCandidate(data []byte) bool { + firstNonSpace := func(b []byte) byte { + for _, c := range b { + if c != ' ' && c != '\t' && c != '\r' && c != '\n' { + return c + } + } + return 0 + } + rest := data + // When data begins with a valid YAML document-start marker ("---" followed + // by whitespace), skip past it so the content of the first document is + // checked rather than the marker itself. CRLF before the marker is handled + // by TrimLeft. + if trimmed := bytes.TrimLeft(data, " \t\r\n"); isYAMLDocMarker(trimmed) { + rest = trimmed[3:] + } + for { + if firstNonSpace(rest) == '{' { + return true + } + // Searching for "\n---" also covers CRLF separators ("\r\n---") because + // "\n---" is a substring of "\r\n---". + i := bytes.Index(rest, []byte("\n---")) + if i < 0 { + return false + } + rest = rest[i+4:] + } +} + type postRender struct { labelPrefix string labelSuffix string @@ -35,6 +146,11 @@ type postRender struct { func (p *postRender) Run(renderedManifests *bytes.Buffer) (modifiedManifests *bytes.Buffer, err error) { data := renderedManifests.Bytes() + data, err = normalizeFlowStyleDocs(data) + if err != nil { + return nil, err + } + objs, err := yaml.ToObjects(bytes.NewBuffer(data)) if err != nil { return nil, err diff --git a/internal/helmdeployer/postrender_test.go b/internal/helmdeployer/postrender_test.go index 565295a46b..ab6af1e9aa 100644 --- a/internal/helmdeployer/postrender_test.go +++ b/internal/helmdeployer/postrender_test.go @@ -177,3 +177,119 @@ func TestPostRenderer_Run_DeleteCRDs(t *testing.T) { }) } + +func TestHasFlowStyleCandidate(t *testing.T) { + tests := map[string]struct { + input string + want bool + }{ + "empty": {input: "", want: false}, + "block-style only": { + input: "apiVersion: v1\nkind: ConfigMap\n", + want: false, + }, + "flow-style, no marker": { + // Document starting directly with '{' (no leading "---"). + input: "{apiVersion: v1, kind: ConfigMap}", + want: true, + }, + "flow-style after LF marker": { + // Helm commonly prefixes with "---\n". + input: "---\n{apiVersion: v1, kind: ConfigMap}", + want: true, + }, + "flow-style after CRLF marker": { + // Windows-style line endings before and after the separator. + input: "---\r\n{apiVersion: v1, kind: ConfigMap}", + want: true, + }, + "flow-style in second doc, LF separator": { + input: "apiVersion: v1\n---\n{apiVersion: v2, kind: ConfigMap}", + want: true, + }, + "flow-style in second doc, CRLF separator": { + // "\n---" is a substring of "\r\n---", so CRLF is handled. + input: "apiVersion: v1\r\n---\r\n{apiVersion: v2, kind: ConfigMap}", + want: true, + }, + "triple-dash not a marker (no trailing whitespace)": { + // "---foo" is NOT a document-start marker per the YAML spec. + input: "---foo: bar\n", + want: false, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if got := hasFlowStyleCandidate([]byte(tc.input)); got != tc.want { + t.Errorf("hasFlowStyleCandidate(%q) = %v, want %v", tc.input, got, tc.want) + } + }) + } +} + +func TestPostRenderer_Run_FlowStyleYAML(t *testing.T) { + // Helm v4's kyaml converts JSON output from toJson template functions into + // YAML flow-style with unquoted keys, e.g. {apiVersion: v1, kind: ConfigMap}. + // k8s.io/apimachinery ToJSON sees the leading '{' and treats it as JSON, + // returning it unchanged; json.Unmarshal then fails on the unquoted keys. + const flowDoc = "{apiVersion: v1, kind: ConfigMap, metadata: {name: test-cm}, data: {key: value}}" + tests := map[string]struct { + input string + }{ + "flow-style with LF document-start marker": { + // Helm render output commonly starts with "---\n". + input: "---\n" + flowDoc, + }, + "flow-style with CRLF document-start marker": { + input: "---\r\n" + flowDoc, + }, + "flow-style with no document-start marker": { + input: flowDoc, + }, + "block-style followed by flow-style": { + input: "apiVersion: v1\nkind: Namespace\nmetadata:\n name: other\n---\n" + flowDoc, + }, + "flow-style followed by block-style": { + input: "---\n" + flowDoc + "\n---\napiVersion: v1\nkind: Namespace\nmetadata:\n name: other\n", + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + pr := postRender{ + manifest: &manifest.Manifest{Resources: []v1alpha1.BundleResource{}}, + chart: &chartv2.Chart{}, + opts: v1alpha1.BundleDeploymentOptions{}, + } + + out, err := pr.Run(bytes.NewBufferString(tc.input)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + objs, err := yaml.ToObjects(bytes.NewBuffer(out.Bytes())) + if err != nil { + t.Fatalf("unexpected error parsing output: %v", err) + } + // Every case should produce at least the ConfigMap from the flow-style doc. + found := false + for _, obj := range objs { + if obj.GetObjectKind().GroupVersionKind().Kind == "ConfigMap" { + found = true + break + } + } + if !found { + t.Errorf("expected a ConfigMap in output, got kinds: %v", objectKinds(objs)) + } + }) + } +} + +func objectKinds(objs []kruntime.Object) []string { + kinds := make([]string, 0, len(objs)) + for _, o := range objs { + kinds = append(kinds, o.GetObjectKind().GroupVersionKind().Kind) + } + return kinds +}