Fail Helm deployments early with missing templated values #5158
Conversation
- error if the release name or namespaces have missing template values - align formatting in cmd_helper to make it easier to compare error strings
Codecov Report
@@ Coverage Diff @@
## master #5158 +/- ##
==========================================
- Coverage 72.32% 71.90% -0.42%
==========================================
Files 382 384 +2
Lines 13589 13730 +141
==========================================
+ Hits 9828 9873 +45
- Misses 3040 3131 +91
- Partials 721 726 +5
Continue to review full report at Codecov.
|
pkg/skaffold/deploy/helm/deploy.go
Outdated
| return nil, userErr(fmt.Sprintf("deploying %q", releaseName), err) | ||
| } | ||
|
|
||
| // collect namespaces | ||
| for _, r := range results { | ||
| var namespace string | ||
| namespace, err = util.ExpandEnvTemplate(r.Namespace, nil) | ||
| // `<no value>` is not allowed within a namespace | ||
| namespace, err = util.ExpandEnvTemplateOrFail(r.Namespace, nil) |
There was a problem hiding this comment.
shd we move this check before deploy for config helm.releases[x].Namespace ?
There was a problem hiding this comment.
This namespace is actually expanded and checked in deployRelease() itself. So we're good on that front.
But this should be using h.releaseNamespace(r) since the user could override the namespace on the command-line.
There was a problem hiding this comment.
Ah, we're ignoring the returned namespaces in our test.
There was a problem hiding this comment.
Hmm, this should be the deployed artifact's namespace, and so should be expanded. Looking into this further.
There was a problem hiding this comment.
We should just be using the namespace returned from Helm.
This code path isn't tested, but let me see if I can add some tests for this.
Fixes #5072
Description
Fails Helm deployments early when release or namespaces reference missing templated values.
User facing changes (remove if N/A)
Helm deployments fail if the release name or namespaces are templated with missing values.