Add some integrations tests for Helm Render

integration/render_test.go

@@ -180,3 +180,188 @@ spec:
 		})
 	}
 }
+
+func TestHelmRender(t *testing.T) {
+	if testing.Short() || RunOnGCP() {
+		t.Skip("skipping kind integration test")
+	}
+
+	tests := []struct {
+		description  string
+		builds       []build.Artifact
+		labels       []deploy.Labeller
+		helmReleases []latest.HelmRelease
+		expectedOut  string
+	}{
+		{
+			description: "Bare bones render",
+			builds: []build.Artifact{
+				{
+					ImageName: "gke-loadbalancer",
+					Tag:       "gke-loadbalancer:test",
+				},
+			},
+			labels: []deploy.Labeller{},
+			helmReleases: []latest.HelmRelease{{
+				Name:      "gke_loadbalancer",
+				ChartPath: "testdata/gke_loadbalancer/loadbalancer-helm",
+				Values: map[string]string{
+					"image": "gke-loadbalancer",
+				},
+			}},
+			expectedOut: `---
+# Source: loadbalancer-helm/templates/k8s.yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: gke-loadbalancer
+  labels:
+    app: gke-loadbalancer
+spec:
+  type: LoadBalancer
+  ports:
+    - port: 80
+      targetPort: 3000
+      protocol: TCP
+      name: http
+  selector:
+    app: "gke-loadbalancer"
+---
+# Source: loadbalancer-helm/templates/k8s.yaml
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: gke-loadbalancer
+  labels:
+    app: gke-loadbalancer
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: gke-loadbalancer
+  template:
+    metadata:
+      labels:
+        app: gke-loadbalancer
+    spec:
+      containers:
+        - name: gke-container
+          image: gke-loadbalancer:test
+          ports:
+            - containerPort: 3000
+
+`,
+		},
+		{
+			description: "A more complex template",
+			builds: []build.Artifact{
+				{
+					ImageName: "gcr.io/k8s-skaffold/skaffold-helm",
+					Tag:       "gcr.io/k8s-skaffold/skaffold-helm:sha256-nonsenslettersandnumbers",
+				},
+			},
+			labels: []deploy.Labeller{},
+			helmReleases: []latest.HelmRelease{{
+				Name:      "skaffold-helm",
+				ChartPath: "testdata/helm/skaffold-helm",
+				Values: map[string]string{
+					"image": "gcr.io/k8s-skaffold/skaffold-helm",
+				},
+			}},
+			expectedOut: `---
+# Source: skaffold-helm/templates/service.yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: skaffold-helm-skaffold-helm
+  labels:
+    app: skaffold-helm
+    chart: skaffold-helm-0.1.0
+    release: skaffold-helm
+    heritage: Helm
+spec:
+  type: ClusterIP
+  ports:
+    - port: 80
+      targetPort: 80
+      protocol: TCP
+      name: nginx
+  selector:
+    app: skaffold-helm
+    release: skaffold-helm
+---
+# Source: skaffold-helm/templates/deployment.yaml
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: skaffold-helm
+  labels:
+    app: skaffold-helm
+    chart: skaffold-helm-0.1.0
+    release: skaffold-helm
+    heritage: Helm
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: skaffold-helm
+      release: skaffold-helm
+  template:
+    metadata:
+      labels:
+        app: skaffold-helm
+        release: skaffold-helm
+    spec:
+      containers:
+        - name: skaffold-helm
+          image: gcr.io/k8s-skaffold/skaffold-helm:sha256-nonsenslettersandnumbers
+          imagePullPolicy: 
+          ports:
+            - containerPort: 80
+          resources:
+            {}
+---
+# Source: skaffold-helm/templates/ingress.yaml
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+  name: skaffold-helm-skaffold-helm
+  labels:
+    app: skaffold-helm
+    chart: skaffold-helm-0.1.0
+    release: skaffold-helm
+    heritage: Helm
+  annotations:
+spec:
+  rules:
+    - http:
+        paths:
+          - path: /
+            backend:
+              serviceName: skaffold-helm-skaffold-helm
+              servicePort: 80
+
+`,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			deployer := deploy.NewHelmDeployer(&runcontext.RunContext{
+				Cfg: latest.Pipeline{
+					Deploy: latest.DeployConfig{
+						DeployType: latest.DeployType{
+							HelmDeploy: &latest.HelmDeploy{
+								Releases: test.helmReleases,
+							},
+						},
+					},
+				},
+			})
+			var b bytes.Buffer
+			err := deployer.Render(context.Background(), &b, test.builds, test.labels, "")
+
+			t.CheckNoError(err)
+			t.CheckDeepEqual(test.expectedOut, b.String())
+		})
+	}
+}

integration/testdata/helm/skaffold-helm/values.yaml

@@ -18,7 +18,7 @@ ingress:
   # Used to create an Ingress record.
   hosts:
     # - chart-example.local
-  annotations:
+  annotations: {}
     # kubernetes.io/ingress.class: nginx
     # kubernetes.io/tls-acme: "true"
   tls:

pkg/skaffold/deploy/helm.go

@@ -238,7 +238,10 @@ func (h *HelmDeployer) Render(ctx context.Context, out io.Writer, builds []build
 			args = append(args, "--values", vf)
 		}
 
-		params := pairParamsToArtifacts(builds, r.Values)
+		params, err := pairParamsToArtifacts(builds, r.Values)
+		if err != nil {
+			return fmt.Errorf("matching build results to chart values: %w", err)
+		}
 
 		for k, v := range params {
 			var value string
@@ -465,16 +468,21 @@ func installArgs(r latest.HelmRelease, builds []build.Artifact, valuesSet map[st
 		args = append(args, "--namespace", o.namespace)
 	}
 
-	params := pairParamsToArtifacts(builds, r.Values)
+	params, err := pairParamsToArtifacts(builds, r.Values)
+	if err != nil {
+		return nil, fmt.Errorf("matching build results to chart values: %w", err)
+	}
 
 	if len(r.Overrides.Values) != 0 {
 		args = append(args, "-f", constants.HelmOverridesFilename)
 	}
 
 	for k, v := range params {
+		var value string
+
 		cfg := r.ImageStrategy.HelmImageConfig.HelmConventionConfig
 
-		value, err := imageSetFromConfig(cfg, k, v.Tag)
+		value, err = imageSetFromConfig(cfg, k, v.Tag)
 		if err != nil {
 			return nil, err
 		}
@@ -657,7 +665,7 @@ func imageSetFromConfig(cfg *latest.HelmConventionConfig, valueName string, tag
 }
 
 // pairParamsToArtifacts associates parameters to the build artifact it creates
-func pairParamsToArtifacts(builds []build.Artifact, params map[string]string) map[string]build.Artifact {
+func pairParamsToArtifacts(builds []build.Artifact, params map[string]string) (map[string]build.Artifact, error) {
 	imageToBuildResult := map[string]build.Artifact{}
 	for _, b := range builds {
 		imageToBuildResult[b.ImageName] = b
@@ -667,10 +675,12 @@ func pairParamsToArtifacts(builds []build.Artifact, params map[string]string) ma
 
 	for param, imageName := range params {
 		b, ok := imageToBuildResult[imageName]
-		if ok {
-			paramToBuildResult[param] = b
+		if !ok {
+			return nil, fmt.Errorf("no build present for %s", imageName)
 		}
+
+		paramToBuildResult[param] = b
 	}
 
-	return paramToBuildResult
+	return paramToBuildResult, nil
 }

pkg/skaffold/deploy/helm_test.go

@@ -486,21 +486,16 @@ func TestHelmDeploy(t *testing.T) {
 			builds:     testBuilds,
 		},
 		{
-			description: "deploy should warn for unmatched parameter",
+			description: "deploy should error for unmatched parameter",
 			commands: testutil.
 				CmdRunWithOutput("helm version", version21).
 				AndRun("helm --kube-context kubecontext get skaffold-helm --kubeconfig kubeconfig").
 				AndRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig").
-				AndRun("helm --kube-context kubecontext upgrade skaffold-helm examples/test --kubeconfig kubeconfig").
+				AndRun("helm --kube-context kubecontext upgrade skaffold-helm examples/test -f skaffold-overrides.yaml --set-string image=docker.io:5000/skaffold-helm:3605e7bc17cf46e53f4d81c4cbc24e5b4c495184 --set some.key=somevalue --kubeconfig kubeconfig").
 				AndRun("helm --kube-context kubecontext get skaffold-helm --kubeconfig kubeconfig"),
 			runContext: makeRunContext(testDeployConfigParameterUnmatched, false),
 			builds:     testBuilds,
-			shouldErr:  false,
-			expectedWarnings: []string{
-				"See helm sample for how to replace image names with their actual tags: https://github.com/GoogleContainerTools/skaffold/blob/master/examples/helm-deployment/skaffold.yaml",
-				"image [docker.io:5000/skaffold-helm:3605e7bc17cf46e53f4d81c4cbc24e5b4c495184] is not used.",
-				"image [skaffold-helm] is used instead.",
-			},
+			shouldErr:  true,
 		},
 		{
 			description: "deploy success remote chart with skipBuildDependencies",