From a64de4a84f27d45b3e1d3ebc9a8b34c998d589e2 Mon Sep 17 00:00:00 2001
From: ericzzzzzzz <102683393+ericzzzzzzz@users.noreply.github.com>
Date: Tue, 3 Oct 2023 12:14:49 -0400
Subject: [PATCH] feat: Support post-renderer for helm deployer. (#9100)
* chore: add tests
* chore: nit
* chore: integration test
---
cmd/skaffold/app/cmd/filter.go | 31 ++++++++++++--
cmd/skaffold/app/cmd/filter_test.go | 2 +-
integration/helm_test.go | 17 ++++++++
integration/testdata/helm/change_replicas.py | 7 +++
integration/testdata/helm/skaffold.yaml | 12 +++++-
pkg/skaffold/deploy/helm/helm.go | 45 ++++++++++++++------
pkg/skaffold/deploy/helm/helm_test.go | 27 ++++++++++++
pkg/skaffold/helm/bin.go | 8 ++--
pkg/skaffold/helm/bin_test.go | 2 +-
pkg/skaffold/render/renderer/helm/helm.go | 2 +-
10 files changed, 128 insertions(+), 25 deletions(-)
create mode 100755 integration/testdata/helm/change_replicas.py
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)
}