diff --git a/pkg/action/action.go b/pkg/action/action.go index b6573d788..6ceda515e 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -27,6 +27,7 @@ import ( "path" "path/filepath" "slices" + "sort" "strings" "sync" "text/template" @@ -144,39 +145,6 @@ const ( filenameAnnotation = "postrenderer.helm.sh/postrender-filename" ) -// fixDocSeparators ensures YAML document separators ("---") are always -// followed by a newline in rendered template content. Go template whitespace -// trimming ({{-) can remove the newline after "---", producing e.g. -// "---apiVersion: v1" which is not a valid YAML document separator. -// This function inserts a newline after any "---" at the start of a line -// that is immediately followed by non-whitespace content. -func fixDocSeparators(content string) string { - var b strings.Builder - remaining := content - for { - // Find "---" at the start of a line (or start of content). - idx := strings.Index(remaining, "---") - if idx == -1 { - b.WriteString(remaining) - break - } - // "---" must be at the start of a line: either idx==0 or preceded by '\n'. - if idx > 0 && remaining[idx-1] != '\n' { - b.WriteString(remaining[:idx+3]) - remaining = remaining[idx+3:] - continue - } - b.WriteString(remaining[:idx+3]) - remaining = remaining[idx+3:] - // If "---" is followed by non-whitespace (e.g. "---apiVersion"), - // insert a newline to make it a proper document separator. - if len(remaining) > 0 && remaining[0] != '\n' && remaining[0] != '\r' && remaining[0] != ' ' && remaining[0] != '\t' { - b.WriteByte('\n') - } - } - return b.String() -} - // 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) { @@ -192,22 +160,32 @@ func annotateAndMerge(files map[string]string) (string, error) { continue } - // Fix document separators where Go template whitespace trimming - // ({{-) has removed the newline after "---", producing e.g. - // "---apiVersion: v1" which is not a valid YAML document - // separator. Insert the missing newline so kio.ParseAll can - // parse the content correctly. - content = fixDocSeparators(content) - - manifests, err := kio.ParseAll(content) - if err != nil { - return "", fmt.Errorf("parsing %s: %w", fname, err) + // For consistency with the non-post-renderers code path, we need + // to use releaseutil.SplitManifests here to split the file into + // individual documents before feeding them to kio.ParseAll. In + // Chart API before v3 this function had leniency for badly-written + // Go templates, so this must be preserved for older charts. + splitDocs := releaseutil.SplitManifests(content) + keys := make([]string, 0, len(splitDocs)) + for k := range splitDocs { + keys = append(keys, k) } - for _, manifest := range manifests { - if err := manifest.PipeE(kyaml.SetAnnotation(filenameAnnotation, fname)); err != nil { - return "", fmt.Errorf("annotating %s: %w", fname, err) + sort.Sort(releaseutil.BySplitManifestsOrder(keys)) + for _, key := range keys { + doc := splitDocs[key] + if strings.TrimSpace(doc) == "" { + continue + } + manifests, err := kio.ParseAll(doc) + if err != nil { + return "", fmt.Errorf("parsing %s: %w", fname, err) + } + for _, manifest := range manifests { + if err := manifest.PipeE(kyaml.SetAnnotation(filenameAnnotation, fname)); err != nil { + return "", fmt.Errorf("annotating %s: %w", fname, err) + } + combinedManifests = append(combinedManifests, manifest) } - combinedManifests = append(combinedManifests, manifest) } } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 73d1a9ea5..85124b5dd 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -403,96 +403,6 @@ func (m *mockPostRenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, return bytes.NewBufferString(content), nil } -func TestFixDocSeparators(t *testing.T) { - tests := []struct { - name string - input string - expected string - }{ - { - name: "no separator", - input: "apiVersion: v1\nkind: Service\n", - expected: "apiVersion: v1\nkind: Service\n", - }, - { - name: "separator on its own line", - input: "---\napiVersion: v1\nkind: Service\n", - expected: "---\napiVersion: v1\nkind: Service\n", - }, - { - name: "leading separator glued to content", - input: "---apiVersion: v1\nkind: Service\n", - expected: "---\napiVersion: v1\nkind: Service\n", - }, - { - name: "mid-content separator glued to content", - input: "apiVersion: v1\nkind: ConfigMap\n---apiVersion: v1\nkind: Service\n", - expected: "apiVersion: v1\nkind: ConfigMap\n---\napiVersion: v1\nkind: Service\n", - }, - { - name: "multiple separators all proper", - input: "---\napiVersion: v1\n---\napiVersion: v1\n", - expected: "---\napiVersion: v1\n---\napiVersion: v1\n", - }, - { - name: "multiple separators some glued", - input: "---apiVersion: v1\nkind: ConfigMap\n---apiVersion: v1\nkind: Service\n", - expected: "---\napiVersion: v1\nkind: ConfigMap\n---\napiVersion: v1\nkind: Service\n", - }, - { - name: "empty string", - input: "", - expected: "", - }, - { - name: "only separator", - input: "---\n", - expected: "---\n", - }, - { - name: "triple dash in a value is not a separator", - input: "data:\n key: ---value\n", - expected: "data:\n key: ---value\n", - }, - { - name: "realistic multi-doc template output", - input: "apiVersion: v1\nkind: Deployment\n---\napiVersion: v1\nkind: Ingress\n---apiVersion: v1\nkind: Service\n", - expected: "apiVersion: v1\nkind: Deployment\n---\napiVersion: v1\nkind: Ingress\n---\napiVersion: v1\nkind: Service\n", - }, - { - name: "separator followed by carriage return", - input: "---\r\napiVersion: v1\n", - expected: "---\r\napiVersion: v1\n", - }, - { - name: "separator followed by space", - input: "--- \napiVersion: v1\n", - expected: "--- \napiVersion: v1\n", - }, - { - name: "separator followed by tab", - input: "---\t\napiVersion: v1\n", - expected: "---\t\napiVersion: v1\n", - }, - { - name: "four dashes on its own line", - input: "----\napiVersion: v1\n", - expected: "---\n-\napiVersion: v1\n", - }, - { - name: "four dashes followed by text", - input: "----more\napiVersion: v1\n", - expected: "---\n-more\napiVersion: v1\n", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.expected, fixDocSeparators(tt.input)) - }) - } -} - func TestAnnotateAndMerge(t *testing.T) { tests := []struct { name string @@ -690,6 +600,339 @@ metadata: name: test-svc annotations: postrenderer.helm.sh/postrender-filename: 'templates/all.yaml' +`, + }, + { + name: "ConfigMap with embedded CA certificate", + files: map[string]string{ + "templates/configmap.yaml": ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: ca-bundle +data: + ca.crt: | + ------BEGIN CERTIFICATE------ + MIICEzCCAXygAwIBAgIQMIMChMLGrR+QvmQvpwAU6zAKBggqhkjOPQQDAzASMRAw + DgYDVQQKEwdBY21lIENvMCAXDTcwMDEwMTAwMDAwMFoYDzIwODQwMTI5MTYwMDAw + WjASMRAwDgYDVQQKEwdBY21lIENvMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE7Rmm + ------END CERTIFICATE------ + ------BEGIN CERTIFICATE------ + MIICEzCCAXygAwIBAgIQMIMChMLGrR+QvmQvpwAU6zAKBggqhkjOPQQDAzASMRAw + DgYDVQQKEwdBY21lIENvMCAXDTcwMDEwMTAwMDAwMFoYDzIwODQwMTI5MTYwMDAw + WjASMRAwDgYDVQQKEwdBY21lIENvMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE7Rmm + ------END CERTIFICATE------ +`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: ca-bundle + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/configmap.yaml' +data: + ca.crt: |- + ------BEGIN CERTIFICATE------ + MIICEzCCAXygAwIBAgIQMIMChMLGrR+QvmQvpwAU6zAKBggqhkjOPQQDAzASMRAw + DgYDVQQKEwdBY21lIENvMCAXDTcwMDEwMTAwMDAwMFoYDzIwODQwMTI5MTYwMDAw + WjASMRAwDgYDVQQKEwdBY21lIENvMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE7Rmm + ------END CERTIFICATE------ + ------BEGIN CERTIFICATE------ + MIICEzCCAXygAwIBAgIQMIMChMLGrR+QvmQvpwAU6zAKBggqhkjOPQQDAzASMRAw + DgYDVQQKEwdBY21lIENvMCAXDTcwMDEwMTAwMDAwMFoYDzIwODQwMTI5MTYwMDAw + WjASMRAwDgYDVQQKEwdBY21lIENvMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE7Rmm + ------END CERTIFICATE------ +`, + }, + { + name: "consecutive dashes in YAML value are not treated as document separators", + files: map[string]string{ + "templates/configmap.yaml": ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + config: | + # --------------------------------------------------------------------------- + [section] + key = value + # --------------------------------------------------------------------------- +`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/configmap.yaml' +data: + config: |- + # --------------------------------------------------------------------------- + [section] + key = value + # --------------------------------------------------------------------------- +`, + }, + { + name: "JSON with dashes in values is not corrupted", + files: map[string]string{ + "templates/dashboard.yaml": ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: dashboard +data: + dashboard.json: | + {"options":{"---------":{"color":"#292929","text":"N/A"}}} +`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: dashboard + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/dashboard.yaml' +data: + dashboard.json: |- + {"options":{"---------":{"color":"#292929","text":"N/A"}}} +`, + }, + + // **Note for Chart API v3**: This input should return an _ERROR_ in Chart API v3. + // See the comment on the releaseutil.SplitManifests function for more details. + { + name: "multiple glued separators in same file", + files: map[string]string{ + "templates/multi.yaml": ` +---apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 +---apiVersion: v1 +kind: ConfigMap +metadata: + name: cm2 +---apiVersion: v1 +kind: ConfigMap +metadata: + name: cm3 +`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/multi.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm2 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/multi.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm3 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/multi.yaml' +`, + }, + + // **Note for Chart API v3**: This input should return an _ERROR_ in Chart API v3. + // See the comment on the releaseutil.SplitManifests function for more details. + { + name: "mixed glued and proper separators", + files: map[string]string{ + "templates/mixed.yaml": ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm2 +---apiVersion: v1 +kind: ConfigMap +metadata: + name: cm3 +`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/mixed.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm2 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/mixed.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm3 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/mixed.yaml' +`, + }, + { + name: "12 documents preserve in-file order", + files: map[string]string{ + "templates/many.yaml": ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-01 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-02 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-03 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-04 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-05 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-06 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-07 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-08 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-09 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-10 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-11 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-12 +`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-01 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-02 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-03 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-04 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-05 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-06 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-07 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-08 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-09 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-10 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-11 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-12 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/many.yaml' `, }, } diff --git a/pkg/release/v1/util/manifest.go b/pkg/release/v1/util/manifest.go index 9a87949f8..3160599bc 100644 --- a/pkg/release/v1/util/manifest.go +++ b/pkg/release/v1/util/manifest.go @@ -36,6 +36,15 @@ type SimpleHead struct { var sep = regexp.MustCompile("(?:^|\\s*\n)---\\s*") // SplitManifests takes a string of manifest and returns a map contains individual manifests +// +// **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, +// with or without post-renderers. func SplitManifests(bigFile string) map[string]string { // Basically, we're quickly splitting a stream of YAML documents into an // array of YAML docs. The file name is just a place holder, but should be