diff --git a/go.mod b/go.mod index 106c499b2..0c2017de7 100644 --- a/go.mod +++ b/go.mod @@ -32,8 +32,8 @@ require ( github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 golang.org/x/crypto v0.39.0 - golang.org/x/term v0.32.0 - golang.org/x/text v0.26.0 + golang.org/x/term v0.33.0 + golang.org/x/text v0.27.0 gopkg.in/yaml.v3 v3.0.1 k8s.io/api v0.33.2 k8s.io/apiextensions-apiserver v0.33.2 @@ -45,6 +45,7 @@ require ( k8s.io/kubectl v0.33.2 oras.land/oras-go/v2 v2.6.0 sigs.k8s.io/controller-runtime v0.21.0 + sigs.k8s.io/kustomize/kyaml v0.19.0 sigs.k8s.io/yaml v1.5.0 ) @@ -157,12 +158,12 @@ require ( go.yaml.in/yaml/v2 v2.4.2 // indirect go.yaml.in/yaml/v3 v3.0.3 // indirect golang.org/x/mod v0.25.0 // indirect - golang.org/x/net v0.40.0 // indirect + golang.org/x/net v0.41.0 // indirect golang.org/x/oauth2 v0.29.0 // indirect - golang.org/x/sync v0.15.0 // indirect - golang.org/x/sys v0.33.0 // indirect + golang.org/x/sync v0.16.0 // indirect + golang.org/x/sys v0.34.0 // indirect golang.org/x/time v0.11.0 // indirect - golang.org/x/tools v0.33.0 // indirect + golang.org/x/tools v0.34.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20241209162323-e6fa225c2576 // indirect google.golang.org/grpc v1.68.1 // indirect @@ -175,7 +176,6 @@ require ( k8s.io/utils v0.0.0-20250321185631-1f6e0b77f77e // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect sigs.k8s.io/kustomize/api v0.19.0 // indirect - sigs.k8s.io/kustomize/kyaml v0.19.0 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.6.0 // indirect ) diff --git a/go.sum b/go.sum index 8d8fe710e..250a34ee0 100644 --- a/go.sum +++ b/go.sum @@ -411,8 +411,8 @@ golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ= -golang.org/x/net v0.40.0 h1:79Xs7wF06Gbdcg4kdCCIQArK11Z1hr5POQ6+fIYHNuY= -golang.org/x/net v0.40.0/go.mod h1:y0hY0exeL2Pku80/zKK7tpntoX23cqL3Oa6njdgRtds= +golang.org/x/net v0.41.0 h1:vBTly1HeNPEn3wtREYfy4GZ/NECgw2Cnl+nK6Nz3uvw= +golang.org/x/net v0.41.0/go.mod h1:B/K4NNqkfmg07DQYrbwvSluqCJOOXwUjeb/5lOisjbA= golang.org/x/oauth2 v0.29.0 h1:WdYw2tdTK1S8olAzWHdgeqfy+Mtm9XNhv/xJsY65d98= golang.org/x/oauth2 v0.29.0/go.mod h1:onh5ek6nERTohokkhCD/y2cV4Do3fxFHFuAejCkRWT8= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -425,8 +425,8 @@ golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sync v0.15.0 h1:KWH3jNZsfyT6xfAfKiz6MRNmd46ByHDYaZ7KSkCtdW8= -golang.org/x/sync v0.15.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw= +golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -448,8 +448,8 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= -golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.34.0 h1:H5Y5sJ2L2JRdyv7ROF1he/lPdvFsd0mJHFw2ThKHxLA= +golang.org/x/sys v0.34.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= @@ -457,8 +457,8 @@ golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/term v0.14.0/go.mod h1:TySc+nGkYR6qt8km8wUhuFRTVSMIX3XPR58y2lC8vww= -golang.org/x/term v0.32.0 h1:DR4lr0TjUs3epypdhTOkMmuF5CDFJ/8pOnbzMZPQ7bg= -golang.org/x/term v0.32.0/go.mod h1:uZG1FhGx848Sqfsq4/DlJr3xGGsYMu/L5GW4abiaEPQ= +golang.org/x/term v0.33.0 h1:NuFncQrRcaRvVmgRkvM3j/F00gWIAlcmlB8ACEKmGIg= +golang.org/x/term v0.33.0/go.mod h1:s18+ql9tYWp1IfpV9DmCtQDDSRBUjKaw9M1eAv5UeF0= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= @@ -466,8 +466,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.26.0 h1:P42AVeLghgTYr4+xUnTRKDMqpar+PtX7KWuNQL21L8M= -golang.org/x/text v0.26.0/go.mod h1:QK15LZJUUQVJxhz7wXgxSy/CJaTFjd0G+YLonydOVQA= +golang.org/x/text v0.27.0 h1:4fGWRpyh641NLlecmyl4LOe6yDdfaYNrGb2zdfo4JV4= +golang.org/x/text v0.27.0/go.mod h1:1D28KMCvyooCX9hBiosv5Tz/+YLxj0j7XhWjpSUF7CU= golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0= golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -478,8 +478,8 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/tools v0.15.0/go.mod h1:hpksKq4dtpQWS1uQ61JkdqWM3LscIS6Slf+VVkm+wQk= -golang.org/x/tools v0.33.0 h1:4qz2S3zmRxbGIhDIAgjxvFutSvH5EfnsYrRBj0UI0bc= -golang.org/x/tools v0.33.0/go.mod h1:CIJMaWEY88juyUfo7UbgPqbC8rU2OqfAV1h2Qp0oMYI= +golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo= +golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/action/action.go b/pkg/action/action.go index 40194dfd7..69bcf4da2 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -22,9 +22,11 @@ import ( "fmt" "io" "log/slog" + "maps" "os" "path" "path/filepath" + "slices" "strings" "sync" "text/template" @@ -34,6 +36,8 @@ import ( "k8s.io/client-go/discovery" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "sigs.k8s.io/kustomize/kyaml/kio" + kyaml "sigs.k8s.io/kustomize/kyaml/yaml" chart "helm.sh/helm/v4/pkg/chart/v2" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" @@ -91,6 +95,81 @@ type Configuration struct { mutex sync.Mutex } +const ( + // filenameAnnotation is the annotation key used to store the original filename + // information in manifest annotations for post-rendering reconstruction. + filenameAnnotation = "postrenderer.helm.sh/postrender-filename" +) + +// 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) { + var combinedManifests []*kyaml.RNode + + // Get sorted filenames to ensure result is deterministic + fnames := slices.Sorted(maps.Keys(files)) + + for _, fname := range fnames { + content := files[fname] + // Skip partials and empty files. + if strings.HasPrefix(path.Base(fname), "_") || strings.TrimSpace(content) == "" { + continue + } + + manifests, err := kio.ParseAll(content) + 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) + } + } + + merged, err := kio.StringAll(combinedManifests) + if err != nil { + return "", fmt.Errorf("writing merged docs: %w", err) + } + return merged, nil +} + +// splitAndDeannotate reconstructs individual files from a merged YAML stream, +// removing filename annotations and grouping documents by their original filenames. +func splitAndDeannotate(postrendered string) (map[string]string, error) { + manifests, err := kio.ParseAll(postrendered) + if err != nil { + return nil, fmt.Errorf("error parsing YAML: %w", err) + } + + manifestsByFilename := make(map[string][]*kyaml.RNode) + for i, manifest := range manifests { + meta, err := manifest.GetMeta() + if err != nil { + return nil, fmt.Errorf("getting metadata: %w", err) + } + fname := meta.Annotations[filenameAnnotation] + if fname == "" { + fname = fmt.Sprintf("generated-by-postrender-%d.yaml", i) + } + if err := manifest.PipeE(kyaml.ClearAnnotation(filenameAnnotation)); err != nil { + return nil, fmt.Errorf("clearing filename annotation: %w", err) + } + manifestsByFilename[fname] = append(manifestsByFilename[fname], manifest) + } + + reconstructed := make(map[string]string, len(manifestsByFilename)) + for fname, docs := range manifestsByFilename { + fileContents, err := kio.StringAll(docs) + if err != nil { + return nil, fmt.Errorf("re-writing %s: %w", fname, err) + } + reconstructed[fname] = fileContents + } + return reconstructed, nil +} + // renderResources renders the templates in a chart // // TODO: This function is badly in need of a refactor. @@ -160,6 +239,33 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu } notes := notesBuffer.String() + if pr != nil { + // We need to send files to the post-renderer before sorting and splitting + // hooks from manifests. The post-renderer interface expects a stream of + // manifests (similar to what tools like Kustomize and kubectl expect), whereas + // the sorter uses filenames. + // Here, we merge the documents into a stream, post-render them, and then split + // them back into a map of filename -> content. + + // Merge files as stream of documents for sending to post renderer + merged, err := annotateAndMerge(files) + if err != nil { + return hs, b, notes, fmt.Errorf("error merging manifests: %w", err) + } + + // Run the post renderer + postRendered, err := pr.Run(bytes.NewBufferString(merged)) + if err != nil { + return hs, b, notes, fmt.Errorf("error while running post render on files: %w", err) + } + + // Use the file list and contents received from the post renderer + files, err = splitAndDeannotate(postRendered.String()) + if err != nil { + return hs, b, notes, fmt.Errorf("error while parsing post rendered output: %w", err) + } + } + // Sort hooks, manifests, and partials. Only hooks and manifests are returned, // as partials are not used after renderer.Render. Empty manifests are also // removed here. @@ -220,13 +326,6 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu } } - if pr != nil { - b, err = pr.Run(b) - if err != nil { - return hs, b, notes, fmt.Errorf("error while running post render on files: %w", err) - } - } - return hs, b, notes, nil } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 9436abef5..43cf94622 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -16,13 +16,17 @@ limitations under the License. package action import ( + "bytes" + "errors" "flag" "fmt" "io" "log/slog" + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" fakeclientset "k8s.io/client-go/kubernetes/fake" "helm.sh/helm/v4/internal/logging" @@ -368,3 +372,577 @@ func TestGetVersionSet(t *testing.T) { t.Error("Non-existent version is reported found.") } } + +// Mock PostRenderer for testing +type mockPostRenderer struct { + shouldError bool + transform func(string) string +} + +func (m *mockPostRenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, error) { + if m.shouldError { + return nil, errors.New("mock post-renderer error") + } + + content := renderedManifests.String() + if m.transform != nil { + content = m.transform(content) + } + + return bytes.NewBufferString(content), nil +} + +func TestAnnotateAndMerge(t *testing.T) { + tests := []struct { + name string + files map[string]string + expectedError string + expected string + }{ + { + name: "no files", + files: map[string]string{}, + expected: "", + }, + { + name: "single file with single manifest", + files: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + key: value`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/configmap.yaml' +data: + key: value +`, + }, + { + name: "multiple files with multiple manifests", + files: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + key: value`, + "templates/secret.yaml": `apiVersion: v1 +kind: Secret +metadata: + name: test-secret +data: + password: dGVzdA==`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/configmap.yaml' +data: + key: value +--- +apiVersion: v1 +kind: Secret +metadata: + name: test-secret + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/secret.yaml' +data: + password: dGVzdA== +`, + }, + { + name: "file with multiple manifests", + files: map[string]string{ + "templates/multi.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm1 +data: + key: value1 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm2 +data: + key: value2`, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm1 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/multi.yaml' +data: + key: value1 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm2 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/multi.yaml' +data: + key: value2 +`, + }, + { + name: "partials and empty files are removed", + files: map[string]string{ + "templates/cm.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm1 +`, + "templates/_partial.tpl": ` +{{-define name}} + {{- "abracadabra"}} +{{- end -}}`, + "templates/empty.yaml": ``, + }, + expected: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm1 + annotations: + postrenderer.helm.sh/postrender-filename: 'templates/cm.yaml' +`, + }, + { + name: "empty file", + files: map[string]string{ + "templates/empty.yaml": "", + }, + expected: ``, + }, + { + name: "invalid yaml", + files: map[string]string{ + "templates/invalid.yaml": `invalid: yaml: content: + - malformed`, + }, + expectedError: "parsing templates/invalid.yaml", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + merged, err := annotateAndMerge(tt.files) + + if tt.expectedError != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + assert.NoError(t, err) + assert.NotNil(t, merged) + assert.Equal(t, tt.expected, merged) + } + }) + } +} + +func TestSplitAndDeannotate(t *testing.T) { + tests := []struct { + name string + input string + expectedFiles map[string]string + expectedError string + }{ + { + name: "single annotated manifest", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + annotations: + postrenderer.helm.sh/postrender-filename: templates/configmap.yaml +data: + key: value`, + expectedFiles: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + key: value +`, + }, + }, + { + name: "multiple manifests with different filenames", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + annotations: + postrenderer.helm.sh/postrender-filename: templates/configmap.yaml +data: + key: value +--- +apiVersion: v1 +kind: Secret +metadata: + name: test-secret + annotations: + postrenderer.helm.sh/postrender-filename: templates/secret.yaml +data: + password: dGVzdA==`, + expectedFiles: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + key: value +`, + "templates/secret.yaml": `apiVersion: v1 +kind: Secret +metadata: + name: test-secret +data: + password: dGVzdA== +`, + }, + }, + { + name: "multiple manifests with same filename", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm1 + annotations: + postrenderer.helm.sh/postrender-filename: templates/multi.yaml +data: + key: value1 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm2 + annotations: + postrenderer.helm.sh/postrender-filename: templates/multi.yaml +data: + key: value2`, + expectedFiles: map[string]string{ + "templates/multi.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm1 +data: + key: value1 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm2 +data: + key: value2 +`, + }, + }, + { + name: "manifest with other annotations", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + annotations: + postrenderer.helm.sh/postrender-filename: templates/configmap.yaml + other-annotation: should-remain +data: + key: value`, + expectedFiles: map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + annotations: + other-annotation: should-remain +data: + key: value +`, + }, + }, + { + name: "invalid yaml input", + input: "invalid: yaml: content:", + expectedError: "error parsing YAML: MalformedYAMLError", + }, + { + name: "manifest without filename annotation", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + key: value`, + expectedFiles: map[string]string{ + "generated-by-postrender-0.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + key: value +`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + files, err := splitAndDeannotate(tt.input) + + if tt.expectedError != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + assert.NoError(t, err) + assert.Equal(t, len(tt.expectedFiles), len(files)) + + for expectedFile, expectedContent := range tt.expectedFiles { + actualContent, exists := files[expectedFile] + assert.True(t, exists, "Expected file %s not found", expectedFile) + assert.Equal(t, expectedContent, actualContent) + } + } + }) + } +} + +func TestAnnotateAndMerge_SplitAndDeannotate_Roundtrip(t *testing.T) { + // Test that merge/split operations are symmetric + originalFiles := map[string]string{ + "templates/configmap.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm +data: + key: value`, + "templates/secret.yaml": `apiVersion: v1 +kind: Secret +metadata: + name: test-secret +data: + password: dGVzdA==`, + "templates/multi.yaml": `apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm1 +data: + key: value1 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm2 +data: + key: value2`, + } + + // Merge and annotate + merged, err := annotateAndMerge(originalFiles) + require.NoError(t, err) + + // Split and deannotate + reconstructed, err := splitAndDeannotate(merged) + require.NoError(t, err) + + // Compare the results + assert.Equal(t, len(originalFiles), len(reconstructed)) + for filename, originalContent := range originalFiles { + reconstructedContent, exists := reconstructed[filename] + assert.True(t, exists, "File %s should exist in reconstructed files", filename) + + // Normalize whitespace for comparison since YAML processing might affect formatting + normalizeContent := func(content string) string { + return strings.TrimSpace(strings.ReplaceAll(content, "\r\n", "\n")) + } + + assert.Equal(t, normalizeContent(originalContent), normalizeContent(reconstructedContent)) + } +} + +func TestRenderResources_PostRenderer_Success(t *testing.T) { + cfg := actionConfigFixture(t) + + // Create a simple mock post-renderer + mockPR := &mockPostRenderer{ + transform: func(content string) string { + content = strings.ReplaceAll(content, "hello", "yellow") + content = strings.ReplaceAll(content, "goodbye", "foodpie") + return strings.ReplaceAll(content, "test-cm", "test-cm-postrendered") + }, + } + + ch := buildChart(withSampleTemplates()) + values := map[string]interface{}{} + + hooks, buf, notes, err := cfg.renderResources( + ch, values, "test-release", "", false, false, false, + mockPR, false, false, false, + ) + + assert.NoError(t, err) + assert.NotNil(t, hooks) + assert.NotNil(t, buf) + assert.Equal(t, "", notes) + expectedBuf := `--- +# Source: yellow/templates/foodpie +foodpie: world +--- +# Source: yellow/templates/with-partials +yellow: Earth +--- +# Source: yellow/templates/yellow +yellow: world +` + expectedHook := `kind: ConfigMap +metadata: + name: test-cm-postrendered + annotations: + "helm.sh/hook": post-install,pre-delete,post-upgrade +data: + name: value` + + assert.Equal(t, expectedBuf, buf.String()) + assert.Len(t, hooks, 1) + assert.Equal(t, expectedHook, hooks[0].Manifest) +} + +func TestRenderResources_PostRenderer_Error(t *testing.T) { + cfg := actionConfigFixture(t) + + // Create a post-renderer that returns an error + mockPR := &mockPostRenderer{ + shouldError: true, + } + + ch := buildChart(withSampleTemplates()) + values := map[string]interface{}{} + + _, _, _, err := cfg.renderResources( + ch, values, "test-release", "", false, false, false, + mockPR, false, false, false, + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "error while running post render on files") +} + +func TestRenderResources_PostRenderer_MergeError(t *testing.T) { + cfg := actionConfigFixture(t) + + // Create a mock post-renderer + mockPR := &mockPostRenderer{} + + // Create a chart with invalid YAML that would cause AnnotateAndMerge to fail + ch := &chart.Chart{ + Metadata: &chart.Metadata{ + APIVersion: "v1", + Name: "test-chart", + Version: "0.1.0", + }, + Templates: []*chart.File{ + {Name: "templates/invalid", Data: []byte("invalid: yaml: content:")}, + }, + } + values := map[string]interface{}{} + + _, _, _, err := cfg.renderResources( + ch, values, "test-release", "", false, false, false, + mockPR, false, false, false, + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "error merging manifests") +} + +func TestRenderResources_PostRenderer_SplitError(t *testing.T) { + cfg := actionConfigFixture(t) + + // Create a post-renderer that returns invalid YAML + mockPR := &mockPostRenderer{ + transform: func(_ string) string { + return "invalid: yaml: content:" + }, + } + + ch := buildChart(withSampleTemplates()) + values := map[string]interface{}{} + + _, _, _, err := cfg.renderResources( + ch, values, "test-release", "", false, false, false, + mockPR, false, false, false, + ) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "error while parsing post rendered output: error parsing YAML: MalformedYAMLError:") +} + +func TestRenderResources_PostRenderer_Integration(t *testing.T) { + cfg := actionConfigFixture(t) + + mockPR := &mockPostRenderer{ + transform: func(content string) string { + return strings.ReplaceAll(content, "metadata:", "color: blue\nmetadata:") + }, + } + + ch := buildChart(withSampleTemplates()) + values := map[string]interface{}{} + + hooks, buf, notes, err := cfg.renderResources( + ch, values, "test-release", "", false, false, false, + mockPR, false, false, false, + ) + + assert.NoError(t, err) + assert.NotNil(t, hooks) + assert.NotNil(t, buf) + assert.Equal(t, "", notes) // Notes should be empty for this test + + // Verify that the post-renderer modifications are present in the output + output := buf.String() + expected := `--- +# 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")) + assert.Equal(t, expected, output) +} + +func TestRenderResources_NoPostRenderer(t *testing.T) { + cfg := actionConfigFixture(t) + + ch := buildChart(withSampleTemplates()) + values := map[string]interface{}{} + + hooks, buf, notes, err := cfg.renderResources( + ch, values, "test-release", "", false, false, false, + nil, false, false, false, + ) + + assert.NoError(t, err) + assert.NotNil(t, hooks) + assert.NotNil(t, buf) + assert.Equal(t, "", notes) +} diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 348c78edb..b43165975 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -851,6 +851,20 @@ func writeLock(chartpath string, lock *chart.Lock, legacyLockfile bool) error { lockfileName = "requirements.lock" } dest := filepath.Join(chartpath, lockfileName) + + info, err := os.Lstat(dest) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("error getting info for %q: %w", dest, err) + } else if err == nil { + if info.Mode()&os.ModeSymlink != 0 { + link, err := os.Readlink(dest) + if err != nil { + return fmt.Errorf("error reading symlink for %q: %w", dest, err) + } + return fmt.Errorf("the %s file is a symlink to %q", lockfileName, link) + } + } + return os.WriteFile(dest, data, 0644) } diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index 53955c45b..f01a5d7ad 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -23,8 +23,10 @@ import ( "path/filepath" "reflect" "testing" + "time" "github.com/stretchr/testify/assert" + "sigs.k8s.io/yaml" chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/chart/v2/loader" @@ -672,3 +674,94 @@ func TestDedupeRepos(t *testing.T) { }) } } + +func TestWriteLock(t *testing.T) { + fixedTime, err := time.Parse(time.RFC3339, "2025-07-04T00:00:00Z") + assert.NoError(t, err) + lock := &chart.Lock{ + Generated: fixedTime, + Digest: "sha256:12345", + Dependencies: []*chart.Dependency{ + { + Name: "fantastic-chart", + Version: "1.2.3", + Repository: "https://example.com/charts", + }, + }, + } + expectedContent, err := yaml.Marshal(lock) + assert.NoError(t, err) + + t.Run("v2 lock file", func(t *testing.T) { + dir := t.TempDir() + err := writeLock(dir, lock, false) + assert.NoError(t, err) + + lockfilePath := filepath.Join(dir, "Chart.lock") + _, err = os.Stat(lockfilePath) + assert.NoError(t, err, "Chart.lock should exist") + + content, err := os.ReadFile(lockfilePath) + assert.NoError(t, err) + assert.Equal(t, expectedContent, content) + + // Check that requirements.lock does not exist + _, err = os.Stat(filepath.Join(dir, "requirements.lock")) + assert.Error(t, err) + assert.True(t, os.IsNotExist(err)) + }) + + t.Run("v1 lock file", func(t *testing.T) { + dir := t.TempDir() + err := writeLock(dir, lock, true) + assert.NoError(t, err) + + lockfilePath := filepath.Join(dir, "requirements.lock") + _, err = os.Stat(lockfilePath) + assert.NoError(t, err, "requirements.lock should exist") + + content, err := os.ReadFile(lockfilePath) + assert.NoError(t, err) + assert.Equal(t, expectedContent, content) + + // Check that Chart.lock does not exist + _, err = os.Stat(filepath.Join(dir, "Chart.lock")) + assert.Error(t, err) + assert.True(t, os.IsNotExist(err)) + }) + + t.Run("overwrite existing lock file", func(t *testing.T) { + dir := t.TempDir() + lockfilePath := filepath.Join(dir, "Chart.lock") + assert.NoError(t, os.WriteFile(lockfilePath, []byte("old content"), 0644)) + + err = writeLock(dir, lock, false) + assert.NoError(t, err) + + content, err := os.ReadFile(lockfilePath) + assert.NoError(t, err) + assert.Equal(t, expectedContent, content) + }) + + t.Run("lock file is a symlink", func(t *testing.T) { + dir := t.TempDir() + dummyFile := filepath.Join(dir, "dummy.txt") + assert.NoError(t, os.WriteFile(dummyFile, []byte("dummy"), 0644)) + + lockfilePath := filepath.Join(dir, "Chart.lock") + assert.NoError(t, os.Symlink(dummyFile, lockfilePath)) + + err = writeLock(dir, lock, false) + assert.Error(t, err) + assert.Contains(t, err.Error(), "the Chart.lock file is a symlink to") + }) + + t.Run("chart path is not a directory", func(t *testing.T) { + dir := t.TempDir() + filePath := filepath.Join(dir, "not-a-dir") + assert.NoError(t, os.WriteFile(filePath, []byte("file"), 0644)) + + err = writeLock(filePath, lock, false) + assert.Error(t, err) + }) +} diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go index e8fcba4e3..b270e51cc 100644 --- a/pkg/registry/utils_test.go +++ b/pkg/registry/utils_test.go @@ -29,6 +29,7 @@ import ( "os" "path/filepath" "strings" + "sync" "time" "github.com/distribution/distribution/v3/configuration" @@ -172,6 +173,9 @@ func setup(suite *TestSuite, tlsEnabled, insecure bool) *registry.Registry { } func teardown(suite *TestSuite) { + var lock sync.Mutex + lock.Lock() + defer lock.Unlock() if suite.srv != nil { mockdns.UnpatchNet(net.DefaultResolver) suite.srv.Close() diff --git a/pkg/repo/chartrepo.go b/pkg/repo/chartrepo.go index e41226fa4..c54197d60 100644 --- a/pkg/repo/chartrepo.go +++ b/pkg/repo/chartrepo.go @@ -160,7 +160,7 @@ func WithClientTLS(certFile, keyFile, caFile string) FindChartInRepoURLOption { } } -// WithInsecureSkipTLSverify skips TLS verification for repostory communication +// WithInsecureSkipTLSverify skips TLS verification for repository communication func WithInsecureSkipTLSverify(insecureSkipTLSverify bool) FindChartInRepoURLOption { return func(options *findChartInRepoURLOptions) { options.InsecureSkipTLSverify = insecureSkipTLSverify