From 7dcdadc792c5468a283cd4115d8895ab306af4cc Mon Sep 17 00:00:00 2001 From: Bryce Lampe Date: Thu, 16 May 2024 10:57:44 -0700 Subject: [PATCH 1/2] Use upstream's MergeValues with assets --- provider/pkg/helm/tool.go | 13 +-- provider/pkg/provider/helm/v4/chart.go | 15 ++-- .../pkg/{helm => provider/helm/v4}/values.go | 84 ++++++++++++------- .../{helm => provider/helm/v4}/values_test.go | 27 +++--- 4 files changed, 77 insertions(+), 62 deletions(-) rename provider/pkg/{helm => provider/helm/v4}/values.go (58%) rename provider/pkg/{helm => provider/helm/v4}/values_test.go (91%) diff --git a/provider/pkg/helm/tool.go b/provider/pkg/helm/tool.go index b1f6852010..2532088cb8 100644 --- a/provider/pkg/helm/tool.go +++ b/provider/pkg/helm/tool.go @@ -35,6 +35,7 @@ import ( "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/cli" + "helm.sh/helm/v3/pkg/cli/values" "helm.sh/helm/v3/pkg/downloader" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/registry" @@ -98,7 +99,7 @@ type TemplateOrInstallCommand struct { Chart string // Values to be applied to the chart. - Values ValueOpts + Values values.Options tool *Tool actionConfig *action.Configuration @@ -130,17 +131,9 @@ func (cmd *TemplateOrInstallCommand) addInstallFlags() { client.SubNotes = false client.Labels = nil client.EnableDNS = false - cmd.addValueOptionsFlags() cmd.addChartPathOptionsFlags() } -func (cmd *TemplateOrInstallCommand) addValueOptionsFlags() { - // https://github.com/helm/helm/blob/14d0c13e9eefff5b4a1b511cf50643529692ec94/cmd/helm/flags.go#L45-L51 - v := cmd.Values - v.Values = map[string]any{} - v.ValuesFiles = []pulumi.Asset{} -} - func (cmd *TemplateOrInstallCommand) addChartPathOptionsFlags() { // https://github.com/helm/helm/blob/14d0c13e9eefff5b4a1b511cf50643529692ec94/cmd/helm/flags.go#L54-L66 c := &cmd.Install.ChartPathOptions @@ -180,7 +173,6 @@ func (t *Tool) Template() *TemplateCommand { tool: t, actionConfig: actionConfig, Install: action.NewInstall(actionConfig), - Values: ValueOpts{}, }, } @@ -472,7 +464,6 @@ type cleanupF func() error // downloadAsset downloads an asset to the local filesystem. func downloadAsset(p getter.Providers, asset pulumi.AssetOrArchive) (string, cleanupF, error) { - a, isAsset := asset.(pulumi.Asset) if !isAsset { return "", nil, errors.New("expected an asset") diff --git a/provider/pkg/provider/helm/v4/chart.go b/provider/pkg/provider/helm/v4/chart.go index c3aa3747c1..5309b95721 100644 --- a/provider/pkg/provider/helm/v4/chart.go +++ b/provider/pkg/provider/helm/v4/chart.go @@ -194,8 +194,12 @@ func (r *ChartProvider) Construct(ctx *pulumi.Context, typ, name string, inputs cmd.Keyring = keyring } - cmd.Values.Values = chartArgs.Values - cmd.Values.ValuesFiles = chartArgs.ValuesFiles + valueOpts, cleanup, err := readValues(p, chartArgs.Values, chartArgs.ValuesFiles) + defer cleanup() + if err != nil { + return nil, err + } + cmd.Values = valueOpts cmd.IncludeCRDs = !chartArgs.SkipCrds cmd.DisableHooks = true cmd.ReleaseName = chartArgs.Name @@ -247,7 +251,8 @@ func (r *ChartProvider) Construct(ctx *pulumi.Context, typ, name string, inputs SkipAwait: chartArgs.SkipAwait, ResourceOptions: []pulumi.ResourceOption{pulumi.Parent(comp)}, PreRegisterF: func(ctx *pulumi.Context, apiVersion, kind, resourceName string, obj *unstructured.Unstructured, - resourceOpts []pulumi.ResourceOption) (*unstructured.Unstructured, []pulumi.ResourceOption) { + resourceOpts []pulumi.ResourceOption, + ) (*unstructured.Unstructured, []pulumi.ResourceOption) { return preregister(ctx, comp, obj, resourceOpts) }, } @@ -261,8 +266,8 @@ func (r *ChartProvider) Construct(ctx *pulumi.Context, typ, name string, inputs } func preregister(ctx *pulumi.Context, comp *ChartState, obj *unstructured.Unstructured, - resourceOpts []pulumi.ResourceOption) (*unstructured.Unstructured, []pulumi.ResourceOption) { - + resourceOpts []pulumi.ResourceOption, +) (*unstructured.Unstructured, []pulumi.ResourceOption) { // Implement support for Helm resource policies. // https://helm.sh/docs/howto/charts_tips_and_tricks/#tell-helm-not-to-uninstall-a-resource policy, hasPolicy, err := unstructured.NestedString(obj.Object, "metadata", "annotations", helmkube.ResourcePolicyAnno) diff --git a/provider/pkg/helm/values.go b/provider/pkg/provider/helm/v4/values.go similarity index 58% rename from provider/pkg/helm/values.go rename to provider/pkg/provider/helm/v4/values.go index 8e11702b65..e3c4190ee9 100644 --- a/provider/pkg/helm/values.go +++ b/provider/pkg/provider/helm/v4/values.go @@ -1,4 +1,4 @@ -// Copyright 2016-2022, Pulumi Corporation. +// Copyright 2024, Pulumi Corporation. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -12,55 +12,75 @@ // See the License for the specific language governing permissions and // limitations under the License. -package helm +package v4 import ( "fmt" "net/url" "os" + "path/filepath" "github.com/pkg/errors" "github.com/pulumi/pulumi/sdk/v3/go/pulumi" + "gopkg.in/yaml.v3" + "helm.sh/helm/v3/pkg/cli/values" "helm.sh/helm/v3/pkg/getter" - "sigs.k8s.io/yaml" ) -// ValueOpts handles merging of chart values from various sources. -type ValueOpts struct { - // ValuesFiles is a list of Helm values files encapsulated as Pulumi assets. - ValuesFiles []pulumi.Asset - // Values is a map of Pulumi values. - Values map[string]any -} - -// MergeValues merges the values in Helm's priority order. -func (opts *ValueOpts) MergeValues(p getter.Providers) (map[string]interface{}, error) { - base := map[string]interface{}{} +// readValues hydrates Assets and persists values on-disk in order to provide +// them to upstream's MergeValues logic. +// +// The returned function cleans up the on-disk values and should always be +// called, even on error. +func readValues(p getter.Providers, v map[string]any, files []pulumi.Asset) (values.Options, func(), error) { + opts := values.Options{} + tmp, err := os.MkdirTemp(os.TempDir(), "pulumi-kubernetes") + if err != nil { + return opts, func() {}, err + } + cleanup := func() { + _ = os.RemoveAll(tmp) + } - // User specified a values files via -f/--values - for _, asset := range opts.ValuesFiles { - currentMap := map[string]interface{}{} + valuesFiles := make([]string, 0, len(files)+1) - bytes, err := readAsset(p, asset) + persist := func(out []byte) error { + fname := filepath.Join(tmp, fmt.Sprintf("values-%d.yaml", len(valuesFiles))) + err := os.WriteFile(fname, out, 0o600) if err != nil { - return nil, err + return err } + valuesFiles = append(valuesFiles, fname) + return nil + } - if err := yaml.Unmarshal(bytes, ¤tMap); err != nil { - return nil, err + for _, f := range files { + out, err := readAsset(p, f) + if err != nil { + return opts, cleanup, err + } + err = persist(out) + if err != nil { + return opts, cleanup, err } - // Merge with the previous map - base = MergeMaps(base, currentMap) } - // User specified a literal value map (possibly containing assets) - values, err := marshalValues(p, opts.Values) + values, err := readAssets(p, v) + if err != nil { + return opts, cleanup, err + } + out, err := yaml.Marshal(values) + if err != nil { + return opts, cleanup, err + } + err = persist(out) if err != nil { - return nil, err + return opts, cleanup, err } - base = MergeMaps(base, values) - return base, nil + opts.ValueFiles = valuesFiles + + return opts, cleanup, nil } // readAsset reads the content of a Pulumi asset. @@ -93,14 +113,14 @@ func readAsset(p getter.Providers, asset pulumi.Asset) ([]byte, error) { } } -// marshalValues converts Pulumi values to Helm values. -// - Expands assets to their content (to support --set-file). -func marshalValues(p getter.Providers, a map[string]interface{}) (map[string]interface{}, error) { +// readAssets converts Pulumi values to Helm values, hydrating Asset values +// along the way. +func readAssets(p getter.Providers, a map[string]interface{}) (map[string]interface{}, error) { var err error out := make(map[string]interface{}, len(a)) for k, v := range a { if v, ok := v.(map[string]interface{}); ok { - out[k], err = marshalValues(p, v) + out[k], err = readAssets(p, v) if err != nil { return nil, err } diff --git a/provider/pkg/helm/values_test.go b/provider/pkg/provider/helm/v4/values_test.go similarity index 91% rename from provider/pkg/helm/values_test.go rename to provider/pkg/provider/helm/v4/values_test.go index 0206afaaad..b65962b13e 100644 --- a/provider/pkg/helm/values_test.go +++ b/provider/pkg/provider/helm/v4/values_test.go @@ -12,16 +12,18 @@ // See the License for the specific language governing permissions and // limitations under the License. -package helm +package v4 import ( "bytes" "os" + "path/filepath" "testing" "github.com/pulumi/pulumi/sdk/v3/go/pulumi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "helm.sh/helm/v3/pkg/cli" "helm.sh/helm/v3/pkg/getter" ) @@ -37,7 +39,6 @@ func (m *mockGetter) Get(url string, options ...getter.Option) (*bytes.Buffer, e } func TestMergeValues(t *testing.T) { - bitnamiImage := ` image: repository: bitnami/nginx @@ -121,7 +122,7 @@ image: }, want: map[string]interface{}{ "configuration": map[string]any{ - "backupStorageLocation": []map[string]any{}, + "backupStorageLocation": []any{}, }, }, }, @@ -143,12 +144,11 @@ image: for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - merger := &ValueOpts{ - ValuesFiles: tt.valuesFiles, - Values: tt.values, - } + p := getter.All(cli.New()) + opts, cleanup, err := readValues(p, tt.values, tt.valuesFiles) + defer cleanup() - actual, err := merger.MergeValues(getter.Providers{}) + actual, err := opts.MergeValues(p) require.NoError(t, err) assert.Equal(t, tt.want, actual) }) @@ -156,17 +156,16 @@ image: } func TestReadAsset(t *testing.T) { - bitnamiImage := ` image: repository: bitnami/nginx tag: latest ` - bitnamiImageFile, err := os.CreateTemp("", "pulumi-TestReadAsset-*.yaml") + + tmp := t.TempDir() + fname := filepath.Join(tmp, "bitnami.yaml") + err := os.WriteFile(fname, []byte(bitnamiImage), 0o600) require.NoError(t, err) - _, _ = bitnamiImageFile.WriteString(bitnamiImage) - _ = bitnamiImageFile.Close() - defer os.Remove(bitnamiImageFile.Name()) tests := []struct { name string @@ -182,7 +181,7 @@ image: }, { name: "file asset", - asset: pulumi.NewFileAsset(bitnamiImageFile.Name()), + asset: pulumi.NewFileAsset(fname), want: []byte(bitnamiImage), }, { From cadae3a0d296f2c17b60f02148e5a7b781e04978 Mon Sep 17 00:00:00 2001 From: Bryce Lampe Date: Thu, 16 May 2024 12:35:52 -0700 Subject: [PATCH 2/2] lint --- provider/pkg/provider/helm/v4/values_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/provider/pkg/provider/helm/v4/values_test.go b/provider/pkg/provider/helm/v4/values_test.go index b65962b13e..8ae28e683d 100644 --- a/provider/pkg/provider/helm/v4/values_test.go +++ b/provider/pkg/provider/helm/v4/values_test.go @@ -147,6 +147,7 @@ image: p := getter.All(cli.New()) opts, cleanup, err := readValues(p, tt.values, tt.valuesFiles) defer cleanup() + require.NoError(t, err) actual, err := opts.MergeValues(p) require.NoError(t, err)