feat: support tempalte parameterization for helm (#8911)

GoogleContainerTools · Jun 26, 2023 · e4e28ba · e4e28ba
1 parent 29ae963
commit e4e28ba
Show file tree

Hide file tree

Showing 7 changed files with 325 additions and 20 deletions.
diff --git a/cmd/skaffold/app/cmd/filter.go b/cmd/skaffold/app/cmd/filter.go
@@ -31,6 +31,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/graph"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/debugging"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/manifest"
+	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/applysetters"
 	rUtil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/render/renderer/util"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/runner"
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
@@ -69,7 +70,15 @@ func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildA
 		if err != nil {
 			return fmt.Errorf("loading manifests: %w", err)
 		}
-
+		var ass applysetters.ApplySetters
+		manifestOverrides := pkgutil.EnvSliceToMap(opts.ManifestsOverrides, "=")
+		for k, v := range manifestOverrides {
+			ass.Setters = append(ass.Setters, applysetters.Setter{Name: k, Value: v})
+		}
+		manifestList, err = ass.Apply(ctx, manifestList)
+		if err != nil {
+			return err
+		}
 		allow, deny := getTransformList(configs)
 
 		manifestList, err = manifestList.SetLabels(pkgutil.EnvSliceToMap(opts.CustomLabels, "="),

diff --git a/cmd/skaffold/app/cmd/flags.go b/cmd/skaffold/app/cmd/flags.go
@@ -761,7 +761,7 @@ The build result from a previous 'skaffold build --file-output' run can be used
 		Value:         &opts.ManifestsOverrides,
 		DefValue:      []string{},
 		FlagAddMethod: "StringSliceVar",
-		DefinedOn:     []string{"render"},
+		DefinedOn:     []string{"render", "filter"},
 	},
 	{
 		Name:          "set-value-file",

diff --git a/integration/render_test.go b/integration/render_test.go
@@ -1107,13 +1107,14 @@ spec:
 	}
 }
 
-func TestRenderWithTransformer(t *testing.T) {
+func TestRenderParameterization(t *testing.T) {
 	tests := []struct {
-		description string
-		args        []string
-		config      string
-		input       map[string]string // file path => content
-		expectedOut string
+		description        string
+		args               []string
+		WithBuildArtifacts bool
+		config             string
+		input              map[string]string // file path => content
+		expectedOut        string
 	}{
 		{
 			description: "kubectl set manifest label with apply-setters",
@@ -1239,8 +1240,8 @@ manifests:
   kustomize:
     paths:
     - overlays/dev
-`, input: map[string]string{
-				"base/kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
+`, input: map[string]string{"base/kustomization.yaml": `
+apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 
 resources:
@@ -1413,6 +1414,278 @@ spec:
   containers:
     - name: getting-started
       image: skaffold-example
+`},
+		{
+			description: "test set param with helm native template(replicaCount)",
+			args:        []string{"--offline=true", "--set", "replicaCount=5"},
+			config: `
+apiVersion: skaffold/v4beta2
+kind: Config
+manifests:
+  helm:
+    releases:
+      - name: skaffold-helm
+        chartPath: charts
+        namespace: helm-namespace
+`,
+			input: map[string]string{
+				"charts/templates/deployments.yaml": `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Chart.Name }}
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ .Chart.Name }}
+spec:
+  selector:
+    matchLabels:
+      app: {{ .Chart.Name }}
+  replicas: {{ .Values.replicaCount }}
+  template:
+    metadata:
+      labels:
+        app: {{ .Chart.Name }}
+    spec:
+      containers:
+      - name: {{ .Chart.Name }}
+        image: {{ .Values.image }}
+`,
+				"charts/Chart.yaml": `
+apiVersion: v1
+description: Skaffold example with Helm
+name: skaffold-helm
+version: 0.1.0
+`,
+				"charts/values.yaml": `
+replicaCount: 2
+image: skaffold-helm:latest
+`,
+			}, expectedOut: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: skaffold-helm
+  namespace: helm-namespace
+  labels:
+    app: skaffold-helm
+spec:
+  selector:
+    matchLabels:
+      app: skaffold-helm
+  replicas: 5
+  template:
+    metadata:
+      labels:
+        app: skaffold-helm
+    spec:
+      containers:
+      - name: skaffold-helm
+        image: skaffold-helm:latest
+`},
+		{
+			description:        "test set param with helm #from-param overrides native template(replicaCount) is comment templated field provided",
+			args:               []string{"--offline=true", "--set", "replicaCount=5", "--set", "count=6"},
+			WithBuildArtifacts: true,
+			config: `
+apiVersion: skaffold/v4beta2
+kind: Config
+manifests:
+  helm:
+    releases:
+      - name: skaffold-helm
+        chartPath: charts
+        namespace: helm-namespace
+`,
+			input: map[string]string{
+				"charts/templates/deployments.yaml": `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Chart.Name }}
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ .Chart.Name }}
+spec:
+  selector:
+    matchLabels:
+      app: {{ .Chart.Name }}
+  replicas: {{ .Values.replicaCount }} # from-param: ${count}
+  template:
+    metadata:
+      labels:
+        app: {{ .Chart.Name }}
+    spec:
+      containers:
+      - name: {{ .Chart.Name }}
+        image: {{ .Values.image }}
+`,
+				"charts/Chart.yaml": `
+apiVersion: v1
+description: Skaffold example with Helm
+name: skaffold-helm
+version: 0.1.0
+`,
+				"charts/values.yaml": `
+replicaCount: 2
+image: skaffold-helm:latest
+`,
+			}, expectedOut: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: skaffold-helm
+  namespace: helm-namespace
+  labels:
+    app: skaffold-helm
+spec:
+  selector:
+    matchLabels:
+      app: skaffold-helm
+  replicas: 6
+  template:
+    metadata:
+      labels:
+        app: skaffold-helm
+    spec:
+      containers:
+      - name: skaffold-helm
+        image: skaffold-helm:latest
+`},
+		{
+			description:        "test set param with helm #from-param has no effect on native template(replicaCount) when comment templated field value not provided",
+			args:               []string{"--offline=true", "--set", "replicaCount=5"},
+			WithBuildArtifacts: true,
+			config: `
+apiVersion: skaffold/v4beta2
+kind: Config
+manifests:
+  helm:
+    releases:
+      - name: skaffold-helm
+        chartPath: charts
+        namespace: helm-namespace
+`,
+			input: map[string]string{
+				"charts/templates/deployments.yaml": `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Chart.Name }}
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ .Chart.Name }}
+spec:
+  selector:
+    matchLabels:
+      app: {{ .Chart.Name }}
+  replicas: {{ .Values.replicaCount }} # from-param: ${count}
+  template:
+    metadata:
+      labels:
+        app: {{ .Chart.Name }}
+    spec:
+      containers:
+      - name: {{ .Chart.Name }}
+        image: {{ .Values.image }}
+`,
+				"charts/Chart.yaml": `
+apiVersion: v1
+description: Skaffold example with Helm
+name: skaffold-helm
+version: 0.1.0
+`,
+				"charts/values.yaml": `
+replicaCount: 2
+image: skaffold-helm:latest
+`,
+			}, expectedOut: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: skaffold-helm
+  namespace: helm-namespace
+  labels:
+    app: skaffold-helm
+spec:
+  selector:
+    matchLabels:
+      app: skaffold-helm
+  replicas: 5
+  template:
+    metadata:
+      labels:
+        app: skaffold-helm
+    spec:
+      containers:
+      - name: skaffold-helm
+        image: skaffold-helm:latest
+`},
+		{
+			description:        "test set param with helm #from-param, values are provided through file",
+			args:               []string{"--offline=true", "--set-value-file", "values.env"},
+			WithBuildArtifacts: true,
+			config: `
+apiVersion: skaffold/v4beta2
+kind: Config
+manifests:
+  helm:
+    releases:
+      - name: skaffold-helm
+        chartPath: charts
+        namespace: helm-namespace
+`,
+			input: map[string]string{
+				"charts/templates/deployments.yaml": `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Chart.Name }}
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: {{ .Chart.Name }}
+spec:
+  selector:
+    matchLabels:
+      app: {{ .Chart.Name }}
+  replicas: {{ .Values.replicaCount }} # from-param: ${count}
+  template:
+    metadata:
+      labels:
+        app: {{ .Chart.Name }}
+    spec:
+      containers:
+      - name: {{ .Chart.Name }}
+        image: {{ .Values.image }}
+`,
+				"charts/Chart.yaml": `
+apiVersion: v1
+description: Skaffold example with Helm
+name: skaffold-helm
+version: 0.1.0
+`,
+				"charts/values.yaml": `
+replicaCount: 2
+image: skaffold-helm:latest
+`,
+				"values.env": `count=3`,
+			}, expectedOut: `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: skaffold-helm
+  namespace: helm-namespace
+  labels:
+    app: skaffold-helm
+spec:
+  selector:
+    matchLabels:
+      app: skaffold-helm
+  replicas: 3
+  template:
+    metadata:
+      labels:
+        app: skaffold-helm
+    spec:
+      containers:
+      - name: skaffold-helm
+        image: skaffold-helm:latest
 `},
 	}
 
@@ -1426,6 +1699,11 @@ spec:
 				tmpDir.Write(filePath, content)
 			}
 
+			if test.WithBuildArtifacts {
+				dir, _ := os.Getwd()
+				test.args = append(test.args, "--build-artifacts="+filepath.Join(dir, "testdata/render/build-output.json"))
+			}
+
 			tmpDir.Chdir()
 			output := skaffold.Render(test.args...).RunOrFailOutput(t.T)
 

diff --git a/pkg/skaffold/deploy/helm/helm.go b/pkg/skaffold/deploy/helm/helm.go
@@ -115,6 +115,10 @@ type Deployer struct {
 	transformableDenylist  map[apimachinery.GroupKind]latest.ResourceFilter
 }
 
+func (h Deployer) ManifestOverrides() map[string]string {
+	return map[string]string{}
+}
+
 func (h Deployer) EnableDebug() bool           { return h.enableDebug }
 func (h Deployer) OverrideProtocols() []string { return h.overrideProtocols }
 func (h Deployer) ConfigFile() string          { return h.configFile }

diff --git a/pkg/skaffold/helm/bin.go b/pkg/skaffold/helm/bin.go
@@ -50,6 +50,7 @@ type Client interface {
 	KubeContext() string
 	Labels() map[string]string
 	GlobalFlags() []string
+	ManifestOverrides() map[string]string
 }
 
 // BinVer returns the version of the helm binary found in PATH.
@@ -99,6 +100,9 @@ func generateSkaffoldFilter(h Client, buildsFile string) []string {
 	for k, v := range h.Labels() {
 		args = append(args, fmt.Sprintf("--label=%s=%s", k, v))
 	}
+	for k, v := range h.ManifestOverrides() {
+		args = append(args, fmt.Sprintf("--set=%s=%s", k, v))
+	}
 	if len(buildsFile) > 0 {
 		args = append(args, "--build-artifacts", buildsFile)
 	}