fix: correct issue where skaffold setTemplateValues env vars were in some cases empty

docs-v2/content/en/docs/pipeline-stages/deployers/helm.md

@@ -91,10 +91,10 @@ deploy:
     releases:
     - name: my-release
       setValueTemplates:
-        image.repository: "{{.myFirstImage.IMAGE_REPO}}"
-        image.tag: "{{.myFirstImage.IMAGE_TAG}}"
-        image2.repository: "{{.mySecondImage.IMAGE_REPO}}"
-        image2.tag: "{{.mySecondImage.IMAGE_TAG}}"
+        image.repository: "{{.IMAGE_REPO_myFirstImage}}"
+        image.tag: "{{.IMAGE_TAG_myFirstImage}}"
+        image2.repository: "{{.IMAGE_REPO_mySecondImage}}"
+        image2.tag: "{{.IMAGE_TAG_mySecondImage}}"
       setValues:
         image.pullPolicy: "IfNotPresent"
         image2.pullPolicy: "IfNotPresent"
@@ -112,12 +112,12 @@ The `setValues` configuration binds a Helm key to the specified value. The `setV
 | `{{.IMAGE_REPO_NO_DOMAIN_<artifact-name>}}` | `example-repo` |
 
 ### Sanitizing the artifact name from invalid go template characters
-The `<artifact-name>` (eg: `{{.IMAGE_FULLY_QUALIFIED_<artifact-name>}}`) when used with `setValueTemplates` cannot have `/` or `-` characters.  If you have an artifact name with these characters (eg: `localhost/nginx` or `gcr.io/foo-image/foo`), change them to use `_` in place of these characters in the `setValueTemplates` field
+The `<artifact-name>` (eg: `{{.IMAGE_FULLY_QUALIFIED_<artifact-name>}}`) when used with `setValueTemplates` cannot have `/`, `-`, or `.` characters.  If you have an artifact name with these characters (eg: `localhost/nginx` or `gcr.io/foo-image/foo`), change them to use `_` in place of these characters in the `setValueTemplates` field
 
 | Artifact name | Sanitized Name |
 | --------------- | --------------- |
 | `localhost/nginx` | `localhost_nginx`|
-| `gcr.io/example-repo/myImage` | `gcr.io_example_repo_myImage` |
+| `gcr.io/example-repo/myImage` | `gcr_io_example_repo_myImage` |
 
 Example
 ```yaml

docs-v2/content/en/docs/pipeline-stages/renderers/helm.md

@@ -112,12 +112,12 @@ The `setValues` configuration binds a Helm key to the specified value. The `setV
 | `{{.IMAGE_REPO_NO_DOMAIN_<artifact-name>}}` | `example-repo` |
 
 ### Sanitizing the artifact name from invalid go template characters
-The `<artifact-name>` (eg: `{{.IMAGE_FULLY_QUALIFIED_<artifact-name>}}`) when used with `setValueTemplates` cannot have `/` or `-` characters.  If you have an artifact name with these characters (eg: `localhost/nginx` or `gcr.io/foo-image/foo`), change them to use `_` in place of these characters in the `setValueTemplates` field
+The `<artifact-name>` (eg: `{{.IMAGE_FULLY_QUALIFIED_<artifact-name>}}`) when used with `setValueTemplates` cannot have `/`, `-`, or `.` characters.  If you have an artifact name with these characters (eg: `localhost/nginx` or `gcr.io/foo-image/foo`), change them to use `_` in place of these characters in the `setValueTemplates` field
 
 | Artifact name | Sanitized Name |
 | --------------- | --------------- |
 | `localhost/nginx` | `localhost_nginx`|
-| `gcr.io/example-repo/myImage` | `gcr.io_example_repo_myImage` |
+| `gcr.io/example-repo/myImage` | `gcr_io_example_repo_myImage` |
 
 Example
 ```yaml

integration/helm_test.go

@@ -77,6 +77,8 @@ func TestDevHelmMultiConfig(t *testing.T) {
 
 			ns, _ := SetupNamespace(t)
 
+			skaffold.Run(test.args...).InDir(test.dir).InNs(ns.Name).WithEnv(test.env).RunOrFailOutput(t)
+
 			out := skaffold.Run(test.args...).InDir(test.dir).InNs(ns.Name).WithEnv(test.env).RunLive(t)
 			defer skaffold.Delete().InDir(test.dir).InNs(ns.Name).WithEnv(test.env).Run(t)
 
@@ -85,3 +87,43 @@ func TestDevHelmMultiConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestRunHelmStatefulSet(t *testing.T) {
+	var tests = []struct {
+		description string
+		dir         string
+		args        []string
+		pods        []string
+		env         []string
+		targetLog   string
+	}{
+		{
+			description: "helm-statefulset-v1-schema",
+			dir:         "testdata/helm-statefulset-v1-schema",
+			targetLog:   "statefulset/skaffold-helm is ready",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			MarkIntegrationTest(t, CanRunWithoutGcp)
+			if test.targetLog == "" {
+				t.SkipNow()
+			}
+			if test.dir == emptydir {
+				err := os.MkdirAll(filepath.Join(test.dir, "emptydir"), 0755)
+				t.Log("Creating empty directory")
+				if err != nil {
+					t.Errorf("Error creating empty dir: %s", err)
+				}
+			}
+
+			ns, _ := SetupNamespace(t)
+
+			out := skaffold.Run(test.args...).InDir(test.dir).InNs(ns.Name).WithEnv(test.env).RunOrFailOutput(t)
+			defer skaffold.Delete().InDir(test.dir).InNs(ns.Name).WithEnv(test.env).Run(t)
+
+			testutil.CheckContains(t, test.targetLog, string(out))
+		})
+	}
+}

integration/render_test.go

@@ -893,3 +893,102 @@ spec:
 		})
 	}
 }
+
+func TestHelmRenderWithImagesFlag(t *testing.T) {
+	tests := []struct {
+		description  string
+		dir          string
+		args         []string
+		builds       []graph.Artifact
+		helmReleases []latest.HelmRelease
+		expectedOut  string
+	}{
+		{
+			description: "verify --images flag work with helm render",
+			dir:         "testdata/helm-render",
+			args:        []string{"--profile=helm-render", "--images=us-central1-docker.pkg.dev/k8s-skaffold/testing/skaffold-helm:latest@sha256:3e8981b13fadcbb5f4d42d00fdf52a9de128feea5280f0a1f7fb542cf31f1a06"},
+			expectedOut: `apiVersion: v1
+kind: Service
+metadata:
+  labels:
+    app: skaffold-helm
+    chart: skaffold-helm-0.1.0
+    heritage: Helm
+    release: skaffold-helm
+    skaffold.dev/run-id: phony-run-id
+  name: skaffold-helm-skaffold-helm
+spec:
+  ports:
+  - name: nginx
+    port: 80
+    protocol: TCP
+    targetPort: 80
+  selector:
+    app: skaffold-helm
+    release: skaffold-helm
+  type: ClusterIP
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app: skaffold-helm
+    chart: skaffold-helm-0.1.0
+    heritage: Helm
+    release: skaffold-helm
+    skaffold.dev/run-id: phony-run-id
+  name: skaffold-helm
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      app: skaffold-helm
+      release: skaffold-helm
+  template:
+    metadata:
+      labels:
+        app: skaffold-helm
+        release: skaffold-helm
+        skaffold.dev/run-id: phony-run-id
+    spec:
+      containers:
+      - image: us-central1-docker.pkg.dev/k8s-skaffold/testing/skaffold-helm:latest@sha256:3e8981b13fadcbb5f4d42d00fdf52a9de128feea5280f0a1f7fb542cf31f1a06
+        imagePullPolicy: always
+        name: skaffold-helm
+        ports:
+        - containerPort: 80
+        resources: {}
+---
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  annotations: null
+  labels:
+    app: skaffold-helm
+    chart: skaffold-helm-0.1.0
+    heritage: Helm
+    release: skaffold-helm
+  name: skaffold-helm-skaffold-helm
+spec:
+  rules:
+  - http:
+      paths:
+      - backend:
+          service:
+            name: skaffold-helm-skaffold-helm
+            port:
+              number: 80
+        path: /
+        pathType: ImplementationSpecific
+`,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			MarkIntegrationTest(t, CanRunWithoutGcp)
+			out := skaffold.Render(append([]string{"--default-repo=", "--label=skaffold.dev/run-id=phony-run-id"}, test.args...)...).InDir(test.dir).RunOrFailOutput(t)
+
+			testutil.CheckDeepEqual(t, test.expectedOut, string(out))
+		})
+	}
+}

integration/testdata/helm-multi-config/skaffold/app1/skaffold.yml

@@ -1,4 +1,4 @@
-apiVersion: skaffold/v3
+apiVersion: skaffold/v4beta1
 kind: Config
 build:
   artifacts:
@@ -16,4 +16,4 @@ manifests:
         name: app1
         setValueTemplates:
           image.repository: '{{.IMAGE_REPO_app1}}'
-          image.tag: "{{.IMAGE_TAG_app1}}@{{.IMAGE_DIGEST_app1}}"
+          image.tag: "{{.IMAGE_TAG_app1}}"

integration/testdata/helm-multi-config/skaffold/app2/skaffold.yml

@@ -1,4 +1,4 @@
-apiVersion: skaffold/v3
+apiVersion: skaffold/v4beta1
 kind: Config
 build:
   artifacts:
@@ -16,4 +16,4 @@ manifests:
         name: app2
         setValueTemplates:
           image.repository: '{{.IMAGE_REPO_app2}}'
-          image.tag: "{{.IMAGE_TAG_app2}}@{{.IMAGE_DIGEST_app2}}"
+          image.tag: "{{.IMAGE_TAG_app2}}"

integration/testdata/helm-statefulset-v1-schema/Dockerfile

@@ -0,0 +1,12 @@
+FROM golang:1.15 as builder
+COPY main.go .
+# `skaffold debug` sets SKAFFOLD_GO_GCFLAGS to disable compiler optimizations
+ARG SKAFFOLD_GO_GCFLAGS
+RUN go build -gcflags="${SKAFFOLD_GO_GCFLAGS}" -o /app main.go
+
+FROM alpine:3.10
+# Define GOTRACEBACK to mark this container as using the Go language runtime
+# for `skaffold debug` (https://skaffold.dev/docs/workflows/debug/).
+ENV GOTRACEBACK=single
+CMD ["./app"]
+COPY --from=builder /app .

integration/testdata/helm-statefulset-v1-schema/README.md

@@ -0,0 +1,39 @@
+### Example: deploy multiple releases with Helm
+
+[![Open in Cloud Shell](https://gstatic.com/cloudssh/images/open-btn.svg)](https://ssh.cloud.google.com/cloudshell/editor?cloudshell_git_repo=https://github.com/GoogleContainerTools/skaffold&cloudshell_open_in_editor=README.md&cloudshell_workspace=examples/helm-deployment)
+
+You can deploy multiple releases with skaffold, each will need a chartPath, a values file, and namespace.
+Skaffold can inject intermediate build tags in the the values map in the skaffold.yaml.
+
+Let's walk through the skaffold yaml:
+
+We'll be building an image called `skaffold-helm`, and it's a dockerfile, so we'll add it to the artifacts.
+
+```yaml
+build:
+  artifacts:
+  - image: skaffold-helm
+```
+
+Now, we want to deploy this image with helm.
+We add a new release in the helm part of the deploy stanza.
+
+```yaml
+deploy:
+  helm:
+    releases:
+    - name: skaffold-helm
+      chartPath: charts
+      # namespace: skaffold
+      artifactOverrides:
+        image: skaffold-helm
+      valuesFiles:
+      - values.yaml
+```
+
+This part tells Skaffold to set the `image` parameter of the values file to the built `skaffold-helm` image and tag.
+
+```yaml
+      artifactOverrides:
+        image: skaffold-helm
+```

integration/testdata/helm-statefulset-v1-schema/charts/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+description: Skaffold example with Helm
+name: skaffold-helm
+version: 0.1.0

integration/testdata/helm-statefulset-v1-schema/charts/templates/deployment.yaml

@@ -0,0 +1,18 @@
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: {{ .Chart.Name }}
+spec:
+  serviceName:  {{ .Chart.Name }}
+  replicas: 2
+  selector:
+    matchLabels:
+      app:  {{ .Chart.Name }}
+  template:
+    metadata:
+      labels:
+        app:  {{ .Chart.Name }}
+    spec:
+      containers:
+      - name:  {{ .Chart.Name }}
+        image:  "{{.Values.image.repository}}:{{.Values.image.tag}}"

integration/testdata/helm-statefulset-v1-schema/charts/values.yaml

@@ -0,0 +1,7 @@
+# Default values for skaffold-helm.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+image:
+  registry: gcr.io
+  repository: other-project/other-image
+  tag: latest

integration/testdata/helm-statefulset-v1-schema/main.go

@@ -0,0 +1,14 @@
+package main
+
+import (
+	"fmt"
+	"time"
+)
+
+func main() {
+	for counter := 0; ; counter++ {
+		fmt.Println("Hello world!", counter)
+
+		time.Sleep(time.Second * 1)
+	}
+}

integration/testdata/helm-statefulset-v1-schema/skaffold.yaml

@@ -0,0 +1,15 @@
+apiVersion: skaffold/v2beta29
+kind: Config
+build:
+  artifacts:
+  - image: gcr.io/k8s-skaffold/skaffold-helm
+deploy:
+  helm:
+    releases:
+    - name: skaffold-helm
+      chartPath: charts
+      artifactOverrides:
+        image: gcr.io/k8s-skaffold/skaffold-helm
+      imageStrategy:
+        helm: {}
+

pkg/skaffold/schema/v2beta29/upgrade.go

@@ -124,7 +124,7 @@ func upgradeOnePipeline(oldPipeline, newPipeline interface{}) error {
 					if svts == nil {
 						svts = map[string]string{}
 					}
-					svts[k+".tag"] = fmt.Sprintf("{{.IMAGE_TAG_%s}}@{{.IMAGE_DIGEST_%s}}", validV, validV)
+					svts[k+".tag"] = fmt.Sprintf("{{.IMAGE_TAG_%s}}", validV)
 					svts[k+".repository"] = fmt.Sprintf("{{.IMAGE_REPO_%s}}", validV)
 					if oldPL.Deploy.HelmDeploy.Releases[i].ImageStrategy.HelmConventionConfig.ExplicitRegistry {
 						// is 'helm' imageStrategy + explicitRegistry

pkg/skaffold/util/util.go

@@ -317,6 +317,6 @@ func hasHiddenPrefix(s string) bool {
 }
 
 func SanitizeHelmTemplateValue(s string) string {
-	// replace commonly used image name chars that are illegal helm template chars "/" & "-" with "_"
-	return strings.ReplaceAll(strings.ReplaceAll(s, "-", "_"), "/", "_")
+	// replaces commonly used image name chars that are illegal go template chars -> replaces "/", "-" and "." with "_"
+	return strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(s, ".", "_"), "-", "_"), "/", "_")
 }