Skip to content

Commit

Permalink
fix: correct issue where skaffold setTemplateValues env vars were in …
Browse files Browse the repository at this point in the history
…some cases empty
  • Loading branch information
aaron-prindle committed Dec 20, 2022
1 parent b51d82c commit f7de7e2
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 34 deletions.
12 changes: 6 additions & 6 deletions docs-v2/content/en/docs/pipeline-stages/deployers/helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs-v2/content/en/docs/pipeline-stages/renderers/helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions integration/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
99 changes: 99 additions & 0 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
}
22 changes: 0 additions & 22 deletions integration/testdata/helm-multi-config/Makefile

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: skaffold/v3
apiVersion: skaffold/v4beta1
kind: Config
build:
artifacts:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: skaffold/v3
apiVersion: skaffold/v4beta1
kind: Config
build:
artifacts:
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ".", "_"), "-", "_"), "/", "_")
}

0 comments on commit f7de7e2

Please sign in to comment.