diff --git a/cmd/skaffold/app/cmd/filter.go b/cmd/skaffold/app/cmd/filter.go index eca21feffcc..bc292f9bf4b 100644 --- a/cmd/skaffold/app/cmd/filter.go +++ b/cmd/skaffold/app/cmd/filter.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "os" + "os/exec" "github.com/spf13/cobra" apim "k8s.io/apimachinery/pkg/runtime/schema" @@ -46,7 +47,7 @@ var doFilter = runFilter func NewCmdFilter() *cobra.Command { var debuggingFilters bool var renderFromBuildOutputFile flags.BuildOutputFileFlag - + var postRenderer string return NewCmd("filter"). Hidden(). // internal command WithDescription("Filter and transform a set of Kubernetes manifests from stdin"). @@ -56,17 +57,39 @@ func NewCmdFilter() *cobra.Command { {Value: &renderFromBuildOutputFile, Name: "build-artifacts", Shorthand: "a", Usage: "File containing build result from a previous 'skaffold build --file-output'"}, {Value: &debuggingFilters, Name: "debugging", DefValue: false, Usage: `Apply debug transforms similar to "skaffold debug"`, IsEnum: true}, {Value: &debug.Protocols, Name: "protocols", DefValue: []string{}, Usage: "Priority sorted order of debugger protocols to support."}, + {Value: &postRenderer, Name: "post-renderer", DefValue: "", FlagAddMethod: "StringVar", Usage: "Any executable that accepts rendered Kubernetes manifests on STDIN and returns valid Kubernetes manifests on STDOUT"}, }). NoArgs(func(ctx context.Context, out io.Writer) error { - return doFilter(ctx, out, debuggingFilters, renderFromBuildOutputFile.BuildArtifacts()) + return doFilter(ctx, out, debuggingFilters, postRenderer, renderFromBuildOutputFile.BuildArtifacts()) }) } // runFilter loads the Kubernetes manifests from stdin and applies the debug transformations. // Unlike `skaffold debug`, this filtering affects all images and not just the built artifacts. -func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildArtifacts []graph.Artifact) error { +func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, postRenderer string, buildArtifacts []graph.Artifact) error { return withRunner(ctx, out, func(r runner.Runner, configs []util.VersionedConfig) error { - manifestList, err := manifest.Load(os.Stdin) + var manifestList manifest.ManifestList + var err error + if postRenderer != "" { + cmd := exec.CommandContext(ctx, postRenderer) + cmd.Stdin = os.Stdin + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return fmt.Errorf("running post-renderer: %w", err) + } + err = cmd.Start() + if err != nil { + return fmt.Errorf("running post-renderer: %w", err) + } + + manifestList, err = manifest.Load(stdoutPipe) + if err != nil { + return fmt.Errorf("loading post-renderer result: %w", err) + } + stdoutPipe.Close() + } else { + manifestList, err = manifest.Load(os.Stdin) + } if err != nil { return fmt.Errorf("loading manifests: %w", err) } diff --git a/cmd/skaffold/app/cmd/filter_test.go b/cmd/skaffold/app/cmd/filter_test.go index 0c4ba6d1f31..5a7045dbb1d 100644 --- a/cmd/skaffold/app/cmd/filter_test.go +++ b/cmd/skaffold/app/cmd/filter_test.go @@ -97,7 +97,7 @@ spec: }) t.SetStdin([]byte(test.manifestsStr)) var b bytes.Buffer - err := runFilter(context.TODO(), &b, false, test.buildArtifacts) + err := runFilter(context.TODO(), &b, false, "", test.buildArtifacts) t.CheckNoError(err) t.CheckDeepEqual(test.expected, b.String(), testutil.YamlObj(t.T)) }) diff --git a/integration/helm_test.go b/integration/helm_test.go index e02180927d5..e91146de2ca 100644 --- a/integration/helm_test.go +++ b/integration/helm_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/GoogleContainerTools/skaffold/v2/integration/skaffold" + "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/v2/testutil" ) @@ -41,6 +42,22 @@ func TestHelmDeploy(t *testing.T) { skaffold.Delete().InDir("testdata/helm").InNs(ns.Name).WithEnv(env).RunOrFail(t) } +func TestHelmDeployWithHook(t *testing.T) { + MarkIntegrationTest(t, CanRunWithoutGcp) + + ns, client := SetupNamespace(t) + + // To fix #1823, we make use of env variable templating for release name + replicas := 5 + env := []string{fmt.Sprintf("REPLICAS=%d", replicas), fmt.Sprintf("TEST_NS=%s", ns.Name)} + skaffold.Deploy("--images", "us-central1-docker.pkg.dev/k8s-skaffold/testing/skaffold-helm", "-p", "helm-hook").InDir("testdata/helm").InNs(ns.Name).WithEnv(env).RunOrFail(t) + + dep := client.GetDeployment("skaffold-helm-" + ns.Name) + testutil.CheckDeepEqual(t, dep.Spec.Replicas, util.Ptr(int32(replicas))) + + skaffold.Delete().InDir("testdata/helm").InNs(ns.Name).WithEnv(env).RunOrFail(t) +} + func TestRunHelmMultiConfig(t *testing.T) { var tests = []struct { description string diff --git a/integration/testdata/helm/change_replicas.py b/integration/testdata/helm/change_replicas.py new file mode 100755 index 00000000000..86f0dee2fc7 --- /dev/null +++ b/integration/testdata/helm/change_replicas.py @@ -0,0 +1,7 @@ +#!/usr/bin/env python3 +import sys +import os +stdin_data = sys.stdin.read() +count = os.getenv("REPLICAS") + +print(stdin_data.replace("replicas: 1", "replicas: {}".format(count))) \ No newline at end of file diff --git a/integration/testdata/helm/skaffold.yaml b/integration/testdata/helm/skaffold.yaml index acfe2d3bdb9..f1e75e47c4c 100644 --- a/integration/testdata/helm/skaffold.yaml +++ b/integration/testdata/helm/skaffold.yaml @@ -23,4 +23,14 @@ profiles: - name: skaffold-helm-{{.TEST_NS}} chartPath: skaffold-helm setValues: - pullPolicy: always \ No newline at end of file + pullPolicy: always +- name: helm-hook + deploy: + helm: + releases: + # seed test namespace in the release name. + - name: skaffold-helm-{{.TEST_NS}} + chartPath: skaffold-helm + flags: + install: + - "--post-renderer=./change_replicas.py" \ No newline at end of file diff --git a/pkg/skaffold/deploy/helm/helm.go b/pkg/skaffold/deploy/helm/helm.go index b0e74e6725b..b1b853beac8 100644 --- a/pkg/skaffold/deploy/helm/helm.go +++ b/pkg/skaffold/deploy/helm/helm.go @@ -447,20 +447,6 @@ func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, releaseName version: chartVersion, } - installEnv := util.OSEnviron() - - skaffoldBinary, filterEnv, cleanup, err := helm.PrepareSkaffoldFilter(h, builds) - if err != nil { - return nil, nil, fmt.Errorf("could not prepare `skaffold filter`: %w", err) - } - - if cleanup != nil { - defer cleanup() - } - // need to include current environment, specifically for HOME to lookup ~/.kube/config - installEnv = append(installEnv, filterEnv...) - opts.postRenderer = skaffoldBinary - opts.namespace, err = helm.ReleaseNamespace(h.namespace, r) if err != nil { return nil, nil, err @@ -481,6 +467,23 @@ func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, releaseName } } + installEnv := util.OSEnviron() + // skaffold use the post-renderer feature to do skaffold specific rendering such as image replacement, adding debugging annotation in helm rendered result, + // as Helm doesn't support to run multiple post-renderers, this is used to run user-defined render inside skaffold filter which happens before skaffold + // post-rendering process for helm releases. + postRendererFlag := getPostRendererFlag(opts.flags) + skaffoldBinary, filterEnv, cleanup, err := helm.PrepareSkaffoldFilter(h, builds, postRendererFlag) + if err != nil { + return nil, nil, fmt.Errorf("could not prepare `skaffold filter`: %w", err) + } + + if cleanup != nil { + defer cleanup() + } + // need to include current environment, specifically for HOME to lookup ~/.kube/config + installEnv = append(installEnv, filterEnv...) + opts.postRenderer = skaffoldBinary + // Only build local dependencies, but allow a user to skip them. if !r.SkipBuildDependencies && r.ChartPath != "" { olog.Entry(ctx).Info("Building helm dependencies...") @@ -534,6 +537,20 @@ func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, releaseName return b, artifacts, nil } +func getPostRendererFlag(flags []string) []string { + for i, ele := range flags { + if strings.HasPrefix(ele, "--post-renderer") { + // "--post-renderer", "executable" + if ele == "--post-renderer" { + return []string{ele, flags[i+1]} + } + // "--post-renderer=executable" + return []string{ele} + } + } + return []string{} +} + // getReleaseManifest confirms that a release is visible to helm and returns the release manifest func (h *Deployer) getReleaseManifest(ctx context.Context, releaseName string, namespace string) ([]byte, error) { // Retry, because sometimes a release may not be immediately visible diff --git a/pkg/skaffold/deploy/helm/helm_test.go b/pkg/skaffold/deploy/helm/helm_test.go index d396e4d69e6..18602f30623 100644 --- a/pkg/skaffold/deploy/helm/helm_test.go +++ b/pkg/skaffold/deploy/helm/helm_test.go @@ -1457,3 +1457,30 @@ func TestHasRunnableHooks(t *testing.T) { }) } } + +func Test_getPostRendererFlag(t *testing.T) { + tests := []struct { + description string + flags []string + expected []string + }{ + {description: "--post-render xxx found", + flags: []string{"-f", "file.yaml", "--post-renderer", "xxx"}, + expected: []string{"--post-renderer", "xxx"}, + }, + {description: "--post-render=xxx found", + flags: []string{"-f", "file.yaml", "--post-renderer=xxx"}, + expected: []string{"--post-renderer=xxx"}, + }, + {description: "post renderer flags not found", + flags: []string{"-f", "file.yaml", "--renderer-post=xxx"}, + expected: []string{}, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + actual := getPostRendererFlag(test.flags) + t.CheckDeepEqual(test.expected, actual) + }) + } +} diff --git a/pkg/skaffold/helm/bin.go b/pkg/skaffold/helm/bin.go index 5b0f342f085..c95d359624c 100644 --- a/pkg/skaffold/helm/bin.go +++ b/pkg/skaffold/helm/bin.go @@ -68,7 +68,7 @@ func BinVer(ctx context.Context) (semver.Version, error) { return semver.ParseTolerant(matches[1]) } -func PrepareSkaffoldFilter(h Client, builds []graph.Artifact) (skaffoldBinary string, env []string, cleanup func(), err error) { +func PrepareSkaffoldFilter(h Client, builds []graph.Artifact, flags []string) (skaffoldBinary string, env []string, cleanup func(), err error) { skaffoldBinary, err = OSExecutable() if err != nil { return "", nil, nil, fmt.Errorf("cannot locate this Skaffold binary: %w", err) @@ -81,7 +81,7 @@ func PrepareSkaffoldFilter(h Client, builds []graph.Artifact) (skaffoldBinary st return "", nil, nil, fmt.Errorf("could not write build-artifacts: %w", err) } } - cmdLine := generateSkaffoldFilter(h, buildsFile) + cmdLine := generateSkaffoldFilter(h, buildsFile, flags) env = append(env, fmt.Sprintf("SKAFFOLD_CMDLINE=%s", shell.Join(cmdLine...))) env = append(env, fmt.Sprintf("SKAFFOLD_FILENAME=%s", h.ConfigFile())) return @@ -89,7 +89,7 @@ func PrepareSkaffoldFilter(h Client, builds []graph.Artifact) (skaffoldBinary st // generateSkaffoldFilter creates a "skaffold filter" command-line for applying the various // Skaffold manifest filters, such a debugging, image replacement, and applying labels. -func generateSkaffoldFilter(h Client, buildsFile string) []string { +func generateSkaffoldFilter(h Client, buildsFile string, flags []string) []string { args := []string{"filter", "--kube-context", h.KubeContext()} if h.EnableDebug() { args = append(args, "--debugging") @@ -111,6 +111,8 @@ func generateSkaffoldFilter(h Client, buildsFile string) []string { if h.KubeConfig() != "" { args = append(args, "--kubeconfig", h.KubeConfig()) } + + args = append(args, flags...) return args } diff --git a/pkg/skaffold/helm/bin_test.go b/pkg/skaffold/helm/bin_test.go index 1ef74d9167b..8ae1307997a 100644 --- a/pkg/skaffold/helm/bin_test.go +++ b/pkg/skaffold/helm/bin_test.go @@ -117,7 +117,7 @@ func TestGenerateSkaffoldFilter(t *testing.T) { manifestsOverrides: map[string]string{}, } - result := generateSkaffoldFilter(h, test.buildFile) + result := generateSkaffoldFilter(h, test.buildFile, []string{}) t.CheckDeepEqual(test.result, result) }) } diff --git a/pkg/skaffold/render/renderer/helm/helm.go b/pkg/skaffold/render/renderer/helm/helm.go index d0e14ca0800..ae2d479e87c 100644 --- a/pkg/skaffold/render/renderer/helm/helm.go +++ b/pkg/skaffold/render/renderer/helm/helm.go @@ -116,7 +116,7 @@ func (h Helm) generateHelmManifests(ctx context.Context, builds []graph.Artifact var postRendererArgs []string if len(builds) > 0 { - skaffoldBinary, filterEnv, cleanup, err := helm.PrepareSkaffoldFilter(h, builds) + skaffoldBinary, filterEnv, cleanup, err := helm.PrepareSkaffoldFilter(h, builds, []string{}) if err != nil { return nil, fmt.Errorf("could not prepare `skaffold filter`: %w", err) }