From 0bb6ae52dc8c959a7099da2368aca1d8d8e5d266 Mon Sep 17 00:00:00 2001 From: Aaron Prindle Date: Sun, 18 Dec 2022 07:57:39 +0000 Subject: [PATCH] fix: correct issue where skaffold setTemplateValues env vars were in some cases empty --- .../en/docs/pipeline-stages/deployers/helm.md | 12 +++++----- .../en/docs/pipeline-stages/renderers/helm.md | 4 ++-- .../multi-config-microservices/skaffold.yaml | 2 +- integration/helm_test.go | 2 ++ .../testdata/helm-multi-config/Makefile | 22 ------------------- .../skaffold/app1/skaffold.yml | 2 +- .../skaffold/app2/skaffold.yml | 2 +- pkg/skaffold/util/util.go | 4 ++-- 8 files changed, 15 insertions(+), 35 deletions(-) delete mode 100644 integration/testdata/helm-multi-config/Makefile diff --git a/docs-v2/content/en/docs/pipeline-stages/deployers/helm.md b/docs-v2/content/en/docs/pipeline-stages/deployers/helm.md index 2713cdbc2b0..aeb31d70cf2 100644 --- a/docs-v2/content/en/docs/pipeline-stages/deployers/helm.md +++ b/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_}}` | `example-repo` | ### Sanitizing the artifact name from invalid go template characters -The `` (eg: `{{.IMAGE_FULLY_QUALIFIED_}}`) 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 `` (eg: `{{.IMAGE_FULLY_QUALIFIED_}}`) 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 diff --git a/docs-v2/content/en/docs/pipeline-stages/renderers/helm.md b/docs-v2/content/en/docs/pipeline-stages/renderers/helm.md index b93a98a70f5..9ddeb88a391 100644 --- a/docs-v2/content/en/docs/pipeline-stages/renderers/helm.md +++ b/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_}}` | `example-repo` | ### Sanitizing the artifact name from invalid go template characters -The `` (eg: `{{.IMAGE_FULLY_QUALIFIED_}}`) 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 `` (eg: `{{.IMAGE_FULLY_QUALIFIED_}}`) 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 diff --git a/examples/multi-config-microservices/skaffold.yaml b/examples/multi-config-microservices/skaffold.yaml index db4776baf3b..3ce4e66aa3b 100644 --- a/examples/multi-config-microservices/skaffold.yaml +++ b/examples/multi-config-microservices/skaffold.yaml @@ -1,4 +1,4 @@ -apiVersion: skaffold/v4beta1 +apiVersion: skaffold/v4beta2 kind: Config requires: - path: ./leeroy-app diff --git a/integration/helm_test.go b/integration/helm_test.go index 54879fdb359..80019c114e6 100644 --- a/integration/helm_test.go +++ b/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) diff --git a/integration/testdata/helm-multi-config/Makefile b/integration/testdata/helm-multi-config/Makefile deleted file mode 100644 index 5c5203028fc..00000000000 --- a/integration/testdata/helm-multi-config/Makefile +++ /dev/null @@ -1,22 +0,0 @@ -build: - cd skaffold; \ - DOCKER_HOST="unix:///Users/$$(whoami)/.docker/run/docker.sock" \ - skaffold build \ - --verbosity info \ - --default-repo docker.io/bringes - -dev: - cd skaffold; \ - DOCKER_HOST="unix:///Users/$$(whoami)/.docker/run/docker.sock" \ - skaffold dev \ - --verbosity info \ - --default-repo docker.io/bringes \ - --kubeconfig ../kubeconfig.yml - -render: - cd skaffold; \ - DOCKER_HOST="unix:///Users/$$(whoami)/.docker/run/docker.sock" \ - skaffold render \ - --verbosity info \ - --default-repo docker.io/bringes \ - -vdebug diff --git a/integration/testdata/helm-multi-config/skaffold/app1/skaffold.yml b/integration/testdata/helm-multi-config/skaffold/app1/skaffold.yml index e773626028f..76cfb6e8f02 100644 --- a/integration/testdata/helm-multi-config/skaffold/app1/skaffold.yml +++ b/integration/testdata/helm-multi-config/skaffold/app1/skaffold.yml @@ -1,4 +1,4 @@ -apiVersion: skaffold/v3 +apiVersion: skaffold/v4beta2 kind: Config build: artifacts: diff --git a/integration/testdata/helm-multi-config/skaffold/app2/skaffold.yml b/integration/testdata/helm-multi-config/skaffold/app2/skaffold.yml index 5c97273d10d..67e1c24a601 100644 --- a/integration/testdata/helm-multi-config/skaffold/app2/skaffold.yml +++ b/integration/testdata/helm-multi-config/skaffold/app2/skaffold.yml @@ -1,4 +1,4 @@ -apiVersion: skaffold/v3 +apiVersion: skaffold/v4beta2 kind: Config build: artifacts: diff --git a/pkg/skaffold/util/util.go b/pkg/skaffold/util/util.go index 18aec54a395..39c8f9f9ea1 100644 --- a/pkg/skaffold/util/util.go +++ b/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, ".", "_"), "-", "_"), "/", "_") }