-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: skaffold render namespace regression in v2 #8482
fix: skaffold render namespace regression in v2 #8482
Conversation
Codecov Report
@@ Coverage Diff @@
## main #8482 +/- ##
==========================================
- Coverage 70.48% 65.20% -5.28%
==========================================
Files 515 602 +87
Lines 23150 29912 +6762
==========================================
+ Hits 16317 19505 +3188
- Misses 5776 8937 +3161
- Partials 1057 1470 +413
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM, only thing that might make sense is to add a validation/error for |
2e402e0
to
9a49100
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! We should create an issue to create an integration test for this case. I can create it if you'd like
(cherry picked from commit e5a8ea9)
Fixes: b/265799040
Description
From helm/helm#3553
helm template
command run with the--namespace
flag doesn't set themetadata.namespace
in the rendered manifests but only sets{{.Release.Namespace}}
templates. In Skaffold v1, thedeploy.helm.releases[i].namespace
property was also being passed as the--namespace
flag for askaffold apply
command. To preserve this behavior in v2 when upgrading from an old config, the upgrade logic is changed to duplicate thedeploy.helm.releases
tomanifests.helm.releases
. So we can use the helm release namespace values again in theskaffold apply
command.