feat: Support post-renderer for helm deployer. (#9100)

* chore: add tests

* chore: nit

* chore: integration test

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)
 		}

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))
 		})

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

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)))

integration/testdata/helm/skaffold.yaml

@@ -23,4 +23,14 @@ profiles:
       - name: skaffold-helm-{{.TEST_NS}}
         chartPath: skaffold-helm
         setValues:
-          pullPolicy: always
+          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"

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

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)
+		})
+	}
+}

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,15 +81,15 @@ 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
 }
 
 // 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
 }
 

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)
 		})
 	}

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)
 		}