From 6052ea6f1ea85c2bf4e7ca7c5cd71ba137892bcb Mon Sep 17 00:00:00 2001 From: Jack Wilsdon Date: Fri, 21 Jul 2023 16:05:15 +0100 Subject: [PATCH] feat: support overrides in helm renderer --- pkg/skaffold/deploy/helm/helm_test.go | 39 +++++-- pkg/skaffold/render/renderer/helm/helm.go | 128 +++++++++++++--------- 2 files changed, 109 insertions(+), 58 deletions(-) diff --git a/pkg/skaffold/deploy/helm/helm_test.go b/pkg/skaffold/deploy/helm/helm_test.go index 99eab8a3f72..e9754bde494 100644 --- a/pkg/skaffold/deploy/helm/helm_test.go +++ b/pkg/skaffold/deploy/helm/helm_test.go @@ -270,6 +270,16 @@ var testDeployCreateNamespaceConfig = latest.LegacyHelmDeploy{ }}, } +var testDeployNoOverridesConfig = latest.LegacyHelmDeploy{ + Releases: []latest.HelmRelease{{ + Name: "skaffold-helm", + ChartPath: "examples/test", + SetValues: map[string]string{ + "some.key": "somevalue", + }, + }}, +} + var validDeployYaml = ` # Source: skaffold-helm/templates/deployment.yaml apiVersion: apps/v1 @@ -1174,7 +1184,7 @@ func TestHelmRender(t *testing.T) { shouldErr: false, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue -f skaffold-overrides.yaml --kubeconfig kubeconfig"), helm: testDeployConfig, builds: []graph.Artifact{ { @@ -1187,7 +1197,7 @@ func TestHelmRender(t *testing.T) { shouldErr: false, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set image.name=skaffold-helm --set image.tag=skaffold-helm:tag1 --set missing.key= --set other.key=FOOBAR --set some.key=somevalue --set FOOBAR=somevalue --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set image.name=skaffold-helm --set image.tag=skaffold-helm:tag1 --set missing.key= --set other.key=FOOBAR --set some.key=somevalue --set FOOBAR=somevalue -f skaffold-overrides.yaml --kubeconfig kubeconfig"), helm: testDeployConfigTemplated, builds: []graph.Artifact{ { @@ -1200,7 +1210,7 @@ func TestHelmRender(t *testing.T) { shouldErr: false, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY -f /some/file-FOOBAR.yaml --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY -f /some/file-FOOBAR.yaml -f skaffold-overrides.yaml --kubeconfig kubeconfig"), helm: testDeployConfigValuesFilesTemplated, builds: []graph.Artifact{ { @@ -1219,7 +1229,7 @@ func TestHelmRender(t *testing.T) { env: []string{"USER=user"}, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template user-skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template user-skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue -f skaffold-overrides.yaml --kubeconfig kubeconfig"), helm: testDeployWithTemplatedName, builds: testBuilds, }, @@ -1228,7 +1238,7 @@ func TestHelmRender(t *testing.T) { shouldErr: false, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/FOOBAR --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template skaffold-helm examples/FOOBAR --post-renderer SKAFFOLD-BINARY --set some.key=somevalue --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template skaffold-helm examples/FOOBAR --post-renderer SKAFFOLD-BINARY --set some.key=somevalue -f skaffold-overrides.yaml --kubeconfig kubeconfig"), helm: testDeployWithTemplatedChartPath, builds: testBuilds, }, @@ -1237,7 +1247,7 @@ func TestHelmRender(t *testing.T) { shouldErr: false, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue --namespace testReleaseNamespace --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue -f skaffold-overrides.yaml --namespace testReleaseNamespace --kubeconfig kubeconfig"), helm: testDeployNamespacedConfig, builds: []graph.Artifact{ { @@ -1250,7 +1260,7 @@ func TestHelmRender(t *testing.T) { shouldErr: false, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue --namespace testReleaseFOOBARNamespace --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue -f skaffold-overrides.yaml --namespace testReleaseFOOBARNamespace --kubeconfig kubeconfig"), helm: testDeployEnvTemplateNamespacedConfig, builds: []graph.Artifact{ { @@ -1263,7 +1273,7 @@ func TestHelmRender(t *testing.T) { shouldErr: false, commands: testutil. CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). - AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue --repo https://charts.helm.sh/stable --kubeconfig kubeconfig"), + AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue -f skaffold-overrides.yaml --repo https://charts.helm.sh/stable --kubeconfig kubeconfig"), helm: testDeployConfigRemoteRepo, builds: []graph.Artifact{ { @@ -1317,6 +1327,19 @@ func TestHelmRender(t *testing.T) { // Tag: "skaffold-helm:tag1", // }}, // }, + { + description: "render with no overrides", + shouldErr: false, + commands: testutil. + CmdRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). + AndRun("helm --kube-context kubecontext template skaffold-helm examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue --kubeconfig kubeconfig"), + helm: testDeployNoOverridesConfig, + builds: []graph.Artifact{ + { + ImageName: "skaffold-helm", + Tag: "skaffold-helm:tag1", + }}, + }, } labeller := label.DefaultLabeller{} labels := labeller.Labels() diff --git a/pkg/skaffold/render/renderer/helm/helm.go b/pkg/skaffold/render/renderer/helm/helm.go index bc8477c9a6f..d0e14ca0800 100644 --- a/pkg/skaffold/render/renderer/helm/helm.go +++ b/pkg/skaffold/render/renderer/helm/helm.go @@ -21,10 +21,12 @@ import ( "context" "fmt" "io" + "os" apimachinery "k8s.io/apimachinery/pkg/runtime/schema" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/config" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/debug" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/graph" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/helm" @@ -36,6 +38,7 @@ import ( "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/renderer/util" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" sUtil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/yaml" ) type Helm struct { @@ -124,77 +127,102 @@ func (h Helm) generateHelmManifests(ctx context.Context, builds []graph.Artifact } for _, release := range h.config.Releases { - releaseName, err := sUtil.ExpandEnvTemplateOrFail(release.Name, nil) + m, err := h.generateHelmManifest(ctx, builds, release, helmEnv, postRendererArgs) if err != nil { - return nil, helm.UserErr(fmt.Sprintf("cannot expand release name %q", release.Name), err) + return nil, err } + renderedManifests.Append(m) + } - release.ChartPath, err = sUtil.ExpandEnvTemplateOrFail(release.ChartPath, nil) - if err != nil { - return nil, helm.UserErr(fmt.Sprintf("cannot expand chart path %q", release.ChartPath), err) - } + manifests, err := renderedManifests.SetLabels(h.labels, manifest.NewResourceSelectorLabels(h.transformAllowlist, h.transformDenylist)) + if err != nil { + return nil, err + } - args := []string{"template", releaseName, helm.ChartSource(release)} - args = append(args, postRendererArgs...) - if release.Packaged == nil && release.Version != "" { - args = append(args, "--version", release.Version) - } + return manifests, nil +} - args, err = helm.ConstructOverrideArgs(&release, builds, args, h.manifestOverrides) - if err != nil { - return nil, helm.UserErr("construct override args", err) - } +func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, release latest.HelmRelease, env, additionalArgs []string) ([]byte, error) { + releaseName, err := sUtil.ExpandEnvTemplateOrFail(release.Name, nil) + if err != nil { + return nil, helm.UserErr(fmt.Sprintf("cannot expand release name %q", release.Name), err) + } - if release.SkipTests { - args = append(args, "--skip-tests") - } + release.ChartPath, err = sUtil.ExpandEnvTemplateOrFail(release.ChartPath, nil) + if err != nil { + return nil, helm.UserErr(fmt.Sprintf("cannot expand chart path %q", release.ChartPath), err) + } + + args := []string{"template", releaseName, helm.ChartSource(release)} + args = append(args, additionalArgs...) + if release.Packaged == nil && release.Version != "" { + args = append(args, "--version", release.Version) + } + + args, err = helm.ConstructOverrideArgs(&release, builds, args, h.manifestOverrides) + if err != nil { + return nil, helm.UserErr("construct override args", err) + } - namespace, err := helm.ReleaseNamespace(h.namespace, release) + if len(release.Overrides.Values) > 0 { + overrides, err := yaml.Marshal(release.Overrides) if err != nil { - return nil, err - } - if h.namespace != "" { - namespace = h.namespace - } - if namespace != "" { - args = append(args, "--namespace", namespace) + return nil, helm.UserErr("cannot marshal overrides to create overrides values.yaml", err) } - if release.Repo != "" { - args = append(args, "--repo") - args = append(args, release.Repo) + if err := os.WriteFile(constants.HelmOverridesFilename, overrides, 0666); err != nil { + return nil, helm.UserErr(fmt.Sprintf("cannot create file %q", constants.HelmOverridesFilename), err) } - outBuffer := new(bytes.Buffer) - errBuffer := new(bytes.Buffer) + defer func() { + os.Remove(constants.HelmOverridesFilename) + }() - // Build Chart dependencies, but allow a user to skip it. - if !release.SkipBuildDependencies && release.ChartPath != "" { - log.Entry(ctx).Info("Building helm dependencies...") - if err := helm.ExecWithStdoutAndStderr(ctx, h, io.Discard, errBuffer, false, helmEnv, "dep", "build", release.ChartPath); err != nil { - log.Entry(ctx).Infof(errBuffer.String()) - return nil, helm.UserErr("building helm dependencies", err) - } - } + args = append(args, "-f", constants.HelmOverridesFilename) + } - err = helm.ExecWithStdoutAndStderr(ctx, h, outBuffer, errBuffer, false, helmEnv, args...) - errorMsg := errBuffer.String() + if release.SkipTests { + args = append(args, "--skip-tests") + } - if len(errorMsg) > 0 { - log.Entry(ctx).Infof(errorMsg) - } + namespace, err := helm.ReleaseNamespace(h.namespace, release) + if err != nil { + return nil, err + } + if h.namespace != "" { + namespace = h.namespace + } + if namespace != "" { + args = append(args, "--namespace", namespace) + } - if err != nil { - return nil, helm.UserErr("std out err", fmt.Errorf(outBuffer.String(), fmt.Errorf(errorMsg))) + if release.Repo != "" { + args = append(args, "--repo") + args = append(args, release.Repo) + } + + outBuffer := new(bytes.Buffer) + errBuffer := new(bytes.Buffer) + + // Build Chart dependencies, but allow a user to skip it. + if !release.SkipBuildDependencies && release.ChartPath != "" { + log.Entry(ctx).Info("Building helm dependencies...") + if err := helm.ExecWithStdoutAndStderr(ctx, h, io.Discard, errBuffer, false, env, "dep", "build", release.ChartPath); err != nil { + log.Entry(ctx).Infof(errBuffer.String()) + return nil, helm.UserErr("building helm dependencies", err) } + } + + err = helm.ExecWithStdoutAndStderr(ctx, h, outBuffer, errBuffer, false, env, args...) + errorMsg := errBuffer.String() - renderedManifests.Append(outBuffer.Bytes()) + if len(errorMsg) > 0 { + log.Entry(ctx).Infof(errorMsg) } - manifests, err := renderedManifests.SetLabels(h.labels, manifest.NewResourceSelectorLabels(h.transformAllowlist, h.transformDenylist)) if err != nil { - return nil, err + return nil, helm.UserErr("std out err", fmt.Errorf(outBuffer.String(), fmt.Errorf(errorMsg))) } - return manifests, nil + return outBuffer.Bytes(), nil }