Normalize YAML flow-style docs in post-renderer [SURE-11642] (#5180) (#5223)

* 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.

Author: thardeck

internal/helmdeployer/install_test.go

@@ -20,6 +20,8 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kubernetesfake "k8s.io/client-go/kubernetes/fake"
+
+	"github.com/rancher/fleet/internal/manifest"
 )
 
 func TestValuesFrom(t *testing.T) {
@@ -754,3 +756,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)
+}

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,122 @@ 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"
 
+var (
+	yamlDocMarker    = []byte("---")
+	newlineDocMarker = []byte("\n---")
+)
+
+// 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, yamlDocMarker) {
+		return false
+	}
+	if len(b) == len(yamlDocMarker) {
+		return true
+	}
+	c := b[len(yamlDocMarker)]
+	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<content>\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[len(yamlDocMarker):], " \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[len(yamlDocMarker):]
+	}
+	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, newlineDocMarker)
+		if i < 0 {
+			return false
+		}
+		rest = rest[i+len(newlineDocMarker):]
+	}
+}
+
 type postRender struct {
 	labelPrefix string
 	labelSuffix string
@@ -35,6 +151,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

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