fix: use release namespace in render when specified

[renzodavid9]

Fixes: #8185

Description
This PR changes the method called to get the namespace that is used during rendering, from GetNamespace to GetKubeNamespace; in skaffold v1 is read from this last one.

Context:

• GetNamespace returns the namespace set in the detected cluster (even if it is the "default" namespace) when no namespace is passed with the --namespace flag and no namespace is set in a Kubectl deployer.
• GetKubeNamespace returns the namespace set with the --namespace flag

Changing the logic to use GetKubeNamespace instead of GetNamespace (as it was in v1) makes the helm renderer to use the value set in the release when no value is detected in the --namespace flag, and don't care about what value is set as default in the active cluster.

Difference between manifests.helm.releases.namespace and deploy.helm.releases.namespace

Both values do not interfere between each other. In the followin scenario:

apiVersion: skaffold/v4beta1
kind: Config
...
manifests:
  helm:
    releases:
    - name: skaffold-helm
      chartPath: charts
      namespace: helm-test-1
      setValues:
        image: skaffold-helm

deploy:
  helm:
    releases:
    - name: skaffold-helm
      chartPath: charts
      namespace: helm-test-2
      setValues:
        image: skaffold-helm

The output from skaffold render will produce two different sets of manifests:

• one for the release defined in the manifests.helm.releases config with namespace: helm-test-1 namespace set
• other for the release defined in deploy.helm.releases.namespace config with namespace: helm-test-2 namespace set

Behind the scene, skaffold is creating two different renderers: one for the manifests and other for the deploy, each with its corresponding configuration

[codecov]

Codecov Report

Merging #8259 (6dd4999) into main (290280e) will decrease coverage by 4.38%.
The diff coverage is 54.68%.

@@            Coverage Diff             @@
##             main    #8259      +/-   ##
==========================================
- Coverage   70.48%   66.09%   -4.39%     
==========================================
  Files         515      601      +86     
  Lines       23150    29507    +6357     
==========================================
+ Hits        16317    19503    +3186     
- Misses       5776     8542    +2766     
- Partials     1057     1462     +405     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/completion.go	13.04% <0.00%> (-1.25%)	⬇️
cmd/skaffold/app/cmd/config/list.go	65.21% <ø> (ø)
cmd/skaffold/app/cmd/config/set.go	88.72% <ø> (ø)
cmd/skaffold/app/cmd/config/util.go	54.28% <ø> (ø)
cmd/skaffold/app/cmd/credits.go	100.00% <ø> (ø)
cmd/skaffold/app/cmd/credits/export.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go	40.90% <0.00%> (-12.94%)	⬇️
cmd/skaffold/app/cmd/generate_pipeline.go	60.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_modules.go	65.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_profiles.go	66.66% <ø> (ø)
... and 405 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[gsquared94]

should you delete this file?

[aaron-prindle]

+1, it seems builds.out.json isn't used in the integration test -> might make sense to delete

[renzodavid9]

Done! I changed the test logic to add the --build-artifacts=builds.out.json flag conditionally and not for all the cases, and removed the builds.out.json file that I added for the new case

[aaron-prindle]

Can you add some more context in the PR description on GetNamespace vs GetKubeNamespace as well as what namespace config values are looked at currently when rendering helm templates (perhaps a tree of the priority?) as we have 2 locations that releases.namespaces can be set:

• in manifests.helm.releases.namespace
• in deploy.helm.releases.namespace

The original issue mentioned that deploy.* config wasn't respected in a render command. If someone sets both values, what is the resultant namespace currently? (which one has priority?)

[aaron-prindle]

LGTM!