From 00162cab973d13461916b2e8faed060eb1ff3915 Mon Sep 17 00:00:00 2001 From: Artem Kamenev Date: Fri, 31 Jan 2025 14:18:15 +0100 Subject: [PATCH 1/4] feat(helm): add depBuild and template flags to HelmDeployFlags schema --- docs-v2/content/en/schemas/v4beta12.json | 22 ++- pkg/skaffold/render/renderer/helm/args.go | 58 +++++++ .../render/renderer/helm/args_test.go | 151 ++++++++++++++++++ pkg/skaffold/render/renderer/helm/helm.go | 35 ++-- pkg/skaffold/schema/latest/config.go | 8 +- 5 files changed, 245 insertions(+), 29 deletions(-) create mode 100644 pkg/skaffold/render/renderer/helm/args.go create mode 100644 pkg/skaffold/render/renderer/helm/args_test.go diff --git a/docs-v2/content/en/schemas/v4beta12.json b/docs-v2/content/en/schemas/v4beta12.json index 9f7232ff241..7e1cc33dbf7 100755 --- a/docs-v2/content/en/schemas/v4beta12.json +++ b/docs-v2/content/en/schemas/v4beta12.json @@ -2300,6 +2300,15 @@ }, "HelmDeployFlags": { "properties": { + "depBuild": { + "items": { + "type": "string" + }, + "type": "array", + "description": "additional flags passed to (`helm dep build`).", + "x-intellij-html-description": "additional flags passed to (helm dep build).", + "default": "[]" + }, "global": { "items": { "type": "string" @@ -2318,6 +2327,15 @@ "x-intellij-html-description": "additional flags passed to (helm install).", "default": "[]" }, + "template": { + "items": { + "type": "string" + }, + "type": "array", + "description": "additional flags passed to (`helm template`).", + "x-intellij-html-description": "additional flags passed to (helm template).", + "default": "[]" + }, "upgrade": { "items": { "type": "string" @@ -2331,7 +2349,9 @@ "preferredOrder": [ "global", "install", - "upgrade" + "upgrade", + "depBuild", + "template" ], "additionalProperties": false, "type": "object", diff --git a/pkg/skaffold/render/renderer/helm/args.go b/pkg/skaffold/render/renderer/helm/args.go new file mode 100644 index 00000000000..36ba7198f9e --- /dev/null +++ b/pkg/skaffold/render/renderer/helm/args.go @@ -0,0 +1,58 @@ +/* +Copyright 2022 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helm + +import ( + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/graph" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/helm" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" +) + +func (h Helm) depBuildArgs(chartPath string) []string { + args := []string{"dep", "build", chartPath} + args = append(args, h.config.Flags.DepBuild...) + return args +} + +func (h Helm) templateArgs(releaseName string, release latest.HelmRelease, builds []graph.Artifact, namespace string, additionalArgs []string) ([]string, error) { + var err error + args := []string{"template", releaseName, helm.ChartSource(release)} + args = append(args, h.config.Flags.Template...) + 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) + } + + if namespace != "" { + args = append(args, "--namespace", namespace) + } + if release.Repo != "" { + args = append(args, "--repo") + args = append(args, release.Repo) + } + if release.SkipTests { + args = append(args, "--skip-tests") + } + + return args, nil +} diff --git a/pkg/skaffold/render/renderer/helm/args_test.go b/pkg/skaffold/render/renderer/helm/args_test.go new file mode 100644 index 00000000000..a9f7517dc35 --- /dev/null +++ b/pkg/skaffold/render/renderer/helm/args_test.go @@ -0,0 +1,151 @@ +/* +Copyright 2022 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package helm + +import ( + "testing" + + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/graph" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" + "github.com/GoogleContainerTools/skaffold/v2/testutil" +) + +func TestDepBuildArgs(t *testing.T) { + tests := []struct { + description string + chartPath string + flags latest.HelmDeployFlags + expected []string + }{ + { + description: "basic", + chartPath: "chart/path", + expected: []string{"dep", "build", "chart/path"}, + }, + { + description: "with flags", + chartPath: "chart/path", + flags: latest.HelmDeployFlags{ + DepBuild: []string{"--skip-refresh"}, + }, + expected: []string{"dep", "build", "chart/path", "--skip-refresh"}, + }, + { + description: "with multiple flags", + chartPath: "chart/path", + flags: latest.HelmDeployFlags{ + DepBuild: []string{"--skip-refresh", "--debug"}, + }, + expected: []string{"dep", "build", "chart/path", "--skip-refresh", "--debug"}, + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + h := Helm{ + config: &latest.Helm{ + Flags: test.flags, + }, + } + args := h.depBuildArgs(test.chartPath) + t.CheckDeepEqual(test.expected, args) + }) + } +} + +func TestTemplateArgs(t *testing.T) { + tests := []struct { + description string + releaseName string + release latest.HelmRelease + builds []graph.Artifact + namespace string + additionalArgs []string + flags latest.HelmDeployFlags + expected []string + shouldErr bool + }{ + { + description: "basic template", + releaseName: "release", + release: latest.HelmRelease{ + ChartPath: "chart/path", + }, + expected: []string{"template", "release", "chart/path"}, + }, + { + description: "with version", + releaseName: "release", + release: latest.HelmRelease{ + ChartPath: "chart/path", + Version: "1.2.3", + }, + expected: []string{"template", "release", "chart/path", "--version", "1.2.3"}, + }, + { + description: "with namespace", + releaseName: "release", + release: latest.HelmRelease{ + ChartPath: "chart/path", + }, + namespace: "namespace", + expected: []string{"template", "release", "chart/path", "--namespace", "namespace"}, + }, + { + description: "with repo", + releaseName: "release", + release: latest.HelmRelease{ + ChartPath: "chart/path", + Repo: "repo-url", + }, + expected: []string{"template", "release", "chart/path", "--repo", "repo-url"}, + }, + { + description: "with skipTests", + releaseName: "release", + release: latest.HelmRelease{ + ChartPath: "chart/path", + SkipTests: true, + }, + expected: []string{"template", "release", "chart/path", "--skip-tests"}, + }, + { + description: "with additional args", + releaseName: "release", + release: latest.HelmRelease{ChartPath: "chart/path"}, + additionalArgs: []string{"--foo", "bar"}, + expected: []string{"template", "release", "chart/path", "--foo", "bar"}, + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + h := Helm{ + config: &latest.Helm{ + Flags: test.flags, + }, + } + args, err := h.templateArgs(test.releaseName, test.release, test.builds, test.namespace, test.additionalArgs) + if test.shouldErr { + t.CheckError(true, err) + } else { + t.CheckError(false, err) + t.CheckDeepEqual(test.expected, args) + } + }) + } +} diff --git a/pkg/skaffold/render/renderer/helm/helm.go b/pkg/skaffold/render/renderer/helm/helm.go index b442214e5a6..64f7b1a6eb0 100644 --- a/pkg/skaffold/render/renderer/helm/helm.go +++ b/pkg/skaffold/render/renderer/helm/helm.go @@ -154,24 +154,13 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, 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) - } - if len(release.Overrides.Values) > 0 { overrides, err := yaml.Marshal(release.Overrides) if err != nil { return nil, helm.UserErr("cannot marshal overrides to create overrides values.yaml", err) } - if err := os.WriteFile(constants.HelmOverridesFilename, overrides, 0666); err != nil { + if err := os.WriteFile(constants.HelmOverridesFilename, overrides, 0o666); err != nil { return nil, helm.UserErr(fmt.Sprintf("cannot create file %q", constants.HelmOverridesFilename), err) } @@ -179,11 +168,7 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, os.Remove(constants.HelmOverridesFilename) }() - args = append(args, "-f", constants.HelmOverridesFilename) - } - - if release.SkipTests { - args = append(args, "--skip-tests") + additionalArgs = append(additionalArgs, "-f", constants.HelmOverridesFilename) } namespace, err := helm.ReleaseNamespace(h.namespace, release) @@ -193,14 +178,6 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, if h.namespace != "" { namespace = h.namespace } - if namespace != "" { - args = append(args, "--namespace", namespace) - } - - if release.Repo != "" { - args = append(args, "--repo") - args = append(args, release.Repo) - } outBuffer := new(bytes.Buffer) errBuffer := new(bytes.Buffer) @@ -208,12 +185,18 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, // 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 { + args := h.depBuildArgs(release.ChartPath) + if err := helm.ExecWithStdoutAndStderr(ctx, h, io.Discard, errBuffer, false, env, args...); err != nil { log.Entry(ctx).Info(errBuffer.String()) return nil, helm.UserErr("building helm dependencies", err) } } + args, err := h.templateArgs(releaseName, release, builds, namespace, additionalArgs) + if err != nil { + return nil, helm.UserErr("cannot construct helm template args", err) + } + err = helm.ExecWithStdoutAndStderr(ctx, h, outBuffer, errBuffer, release.UseHelmSecrets, env, args...) errorMsg := errBuffer.String() diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go index 88955bcb8c0..d63ddb729ad 100644 --- a/pkg/skaffold/schema/latest/config.go +++ b/pkg/skaffold/schema/latest/config.go @@ -995,6 +995,12 @@ type HelmDeployFlags struct { // Upgrade are additional flags passed to (`helm upgrade`). Upgrade []string `yaml:"upgrade,omitempty"` + + // DepBuild are additional flags passed to (`helm dep build`). + DepBuild []string `yaml:"depBuild,omitempty"` + + // Template are additional flags passed to (`helm template`). + Template []string `yaml:"template,omitempty"` } // HelmRelease describes a helm release to be deployed. @@ -1856,7 +1862,6 @@ func (clusterDetails *ClusterDetails) UnmarshalYAML(value *yaml.Node) error { // Unmarshal the remaining values aux := (*ClusterDetailsForUnmarshaling)(clusterDetails) err = yaml.Unmarshal(remaining, aux) - if err != nil { return err } @@ -1883,7 +1888,6 @@ func (ka *KanikoArtifact) UnmarshalYAML(value *yaml.Node) error { // Unmarshal the remaining values aux := (*KanikoArtifactForUnmarshaling)(ka) err = yaml.Unmarshal(remaining, aux) - if err != nil { return err } From d2d671b85aabb3eb66d69436f55cdabd6253cf72 Mon Sep 17 00:00:00 2001 From: Artem Kamenev Date: Fri, 31 Jan 2025 14:30:04 +0100 Subject: [PATCH 2/4] fix(renderer): revert file permission mode for HelmOverridesFilename --- pkg/skaffold/render/renderer/helm/helm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skaffold/render/renderer/helm/helm.go b/pkg/skaffold/render/renderer/helm/helm.go index 64f7b1a6eb0..0f33b642b11 100644 --- a/pkg/skaffold/render/renderer/helm/helm.go +++ b/pkg/skaffold/render/renderer/helm/helm.go @@ -160,7 +160,7 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, return nil, helm.UserErr("cannot marshal overrides to create overrides values.yaml", err) } - if err := os.WriteFile(constants.HelmOverridesFilename, overrides, 0o666); err != nil { + if err := os.WriteFile(constants.HelmOverridesFilename, overrides, 0666); err != nil { return nil, helm.UserErr(fmt.Sprintf("cannot create file %q", constants.HelmOverridesFilename), err) } From 798d4f079e7306084f0ff1db0c95c24fbea274e1 Mon Sep 17 00:00:00 2001 From: Artem <19182088+art-shutter@users.noreply.github.com> Date: Fri, 31 Jan 2025 15:20:10 +0100 Subject: [PATCH 3/4] fix: update copyright year to 2025 in helm args files --- pkg/skaffold/render/renderer/helm/args.go | 2 +- pkg/skaffold/render/renderer/helm/args_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/render/renderer/helm/args.go b/pkg/skaffold/render/renderer/helm/args.go index 36ba7198f9e..a7255978873 100644 --- a/pkg/skaffold/render/renderer/helm/args.go +++ b/pkg/skaffold/render/renderer/helm/args.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Skaffold Authors +Copyright 2025 The Skaffold Authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/pkg/skaffold/render/renderer/helm/args_test.go b/pkg/skaffold/render/renderer/helm/args_test.go index a9f7517dc35..c4198776b6b 100644 --- a/pkg/skaffold/render/renderer/helm/args_test.go +++ b/pkg/skaffold/render/renderer/helm/args_test.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Skaffold Authors +Copyright 2025 The Skaffold Authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From 7495584c7d30d82e9adb065ca2302c699072c3c4 Mon Sep 17 00:00:00 2001 From: Artem <19182088+art-shutter@users.noreply.github.com> Date: Fri, 31 Jan 2025 15:24:28 +0100 Subject: [PATCH 4/4] fix(helm): move templateArgs call before deps build in generateHelmManifest --- pkg/skaffold/render/renderer/helm/helm.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/skaffold/render/renderer/helm/helm.go b/pkg/skaffold/render/renderer/helm/helm.go index 0f33b642b11..007c1f0b6e1 100644 --- a/pkg/skaffold/render/renderer/helm/helm.go +++ b/pkg/skaffold/render/renderer/helm/helm.go @@ -182,6 +182,11 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, outBuffer := new(bytes.Buffer) errBuffer := new(bytes.Buffer) + args, err := h.templateArgs(releaseName, release, builds, namespace, additionalArgs) + if err != nil { + return nil, helm.UserErr("cannot construct helm template args", err) + } + // Build Chart dependencies, but allow a user to skip it. if !release.SkipBuildDependencies && release.ChartPath != "" { log.Entry(ctx).Info("Building helm dependencies...") @@ -192,11 +197,6 @@ func (h Helm) generateHelmManifest(ctx context.Context, builds []graph.Artifact, } } - args, err := h.templateArgs(releaseName, release, builds, namespace, additionalArgs) - if err != nil { - return nil, helm.UserErr("cannot construct helm template args", err) - } - err = helm.ExecWithStdoutAndStderr(ctx, h, outBuffer, errBuffer, release.UseHelmSecrets, env, args...) errorMsg := errBuffer.String()