feat: respect CLI flag in skaffold render (GoogleContainerTools#7664)

* feat: respect CLI flag in skaffold render

* fix: fix integration test

* fix: integration test namespace

* wip

* fix: integration test when metadata name is not present

* don't traverse recursively for setting namespace since it  only appears at the parent level

* feat: only set namespaces if a manifest is in resource selector

* address code review comments

* fix integration tests to make them compatible with v1 behavior

* fix integration tests

* offline set to true, then don't set namespace

* fix unit tests

* add namesapce back

* fix typos

cmd/skaffold/app/cmd/flags.go

@@ -109,7 +109,7 @@ var flagRegistry = []Flag{
 	{
 		Name:          "namespace",
 		Shorthand:     "n",
-		Usage:         "Run deployments in the specified namespace",
+		Usage:         "Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace",
 		Value:         &opts.Namespace,
 		DefValue:      "",
 		FlagAddMethod: "StringVar",

docs-v2/content/en/docs/references/api-v2/grpc.md

@@ -1208,6 +1208,8 @@ For Cancelled Error code, use range 800 to 850.<br>
 | RENDER_KPTFILE_INIT_ERR | 1501 | Render errors The Kptfile cannot be created via `kpt pkg init`. |
 | RENDER_KPTFILE_INVALID_YAML_ERR | 1401 | The Kptfile is not a valid yaml file |
 | RENDER_KPTFILE_INVALID_SCHEMA_ERR | 1402 | The Kptfile is not a valid API schema |
+| RENDER_SET_NAMESPACE_ERR | 1403 | Error setting namespace. |
+| RENDER_NAMESPACE_ALREADY_SET_ERR | 1404 | Namespace is already set. |
 | CONFIG_FILE_PARSING_ERR | 1201 | Catch-all configuration file parsing error |
 | CONFIG_FILE_NOT_FOUND_ERR | 1202 | Main configuration file not found |
 | CONFIG_DEPENDENCY_NOT_FOUND_ERR | 1203 | Dependency configuration file not found |
@@ -1288,6 +1290,7 @@ Enum for Suggestion codes
 | SET_RENDER_FLAG_OFFLINE_FALSE | 600 | Rerun with correct offline flag value. |
 | KPTFILE_MANUAL_INIT | 601 | Manually run `kpt pkg init` or `kpt live init` |
 | KPTFILE_CHECK_YAML | 602 | Check if the Kptfile is correct. |
+| REMOVE_NAMESPACE_FROM_MANIFESTS | 603 | Remove namespace from manifests |
 | CONFIG_CHECK_FILE_PATH | 700 | Check configuration file path |
 | CONFIG_CHECK_DEPENDENCY_DEFINITION | 701 | Check dependency config definition |
 | CONFIG_CHANGE_NAMES | 702 | Change config name to avoid duplicates |

docs-v2/content/en/docs/references/api/grpc.md

@@ -1063,6 +1063,8 @@ For Cancelled Error code, use range 800 to 850.<br>
 | RENDER_KPTFILE_INIT_ERR | 1501 | Render errors The Kptfile cannot be created via `kpt pkg init`. |
 | RENDER_KPTFILE_INVALID_YAML_ERR | 1401 | The Kptfile is not a valid yaml file |
 | RENDER_KPTFILE_INVALID_SCHEMA_ERR | 1402 | The Kptfile is not a valid API schema |
+| RENDER_SET_NAMESPACE_ERR | 1403 | Error setting namespace. |
+| RENDER_NAMESPACE_ALREADY_SET_ERR | 1404 | Namespace is already set. |
 | CONFIG_FILE_PARSING_ERR | 1201 | Catch-all configuration file parsing error |
 | CONFIG_FILE_NOT_FOUND_ERR | 1202 | Main configuration file not found |
 | CONFIG_DEPENDENCY_NOT_FOUND_ERR | 1203 | Dependency configuration file not found |
@@ -1143,6 +1145,7 @@ Enum for Suggestion codes
 | SET_RENDER_FLAG_OFFLINE_FALSE | 600 | Rerun with correct offline flag value. |
 | KPTFILE_MANUAL_INIT | 601 | Manually run `kpt pkg init` or `kpt live init` |
 | KPTFILE_CHECK_YAML | 602 | Check if the Kptfile is correct. |
+| REMOVE_NAMESPACE_FROM_MANIFESTS | 603 | Remove namespace from manifests |
 | CONFIG_CHECK_FILE_PATH | 700 | Check configuration file path |
 | CONFIG_CHECK_DEPENDENCY_DEFINITION | 701 | Check dependency config definition |
 | CONFIG_CHANGE_NAMES | 702 | Change config name to avoid duplicates |

docs/content/en/docs/references/api-v2/grpc.md

@@ -1208,6 +1208,8 @@ For Cancelled Error code, use range 800 to 850.<br>
 | RENDER_KPTFILE_INIT_ERR | 1501 | Render errors The Kptfile cannot be created via `kpt pkg init`. |
 | RENDER_KPTFILE_INVALID_YAML_ERR | 1401 | The Kptfile is not a valid yaml file |
 | RENDER_KPTFILE_INVALID_SCHEMA_ERR | 1402 | The Kptfile is not a valid API schema |
+| RENDER_SET_NAMESPACE_ERR | 1403 | Error setting namespace. |
+| RENDER_NAMESPACE_ALREADY_SET_ERR | 1404 | Namespace is already set. |
 | CONFIG_FILE_PARSING_ERR | 1201 | Catch-all configuration file parsing error |
 | CONFIG_FILE_NOT_FOUND_ERR | 1202 | Main configuration file not found |
 | CONFIG_DEPENDENCY_NOT_FOUND_ERR | 1203 | Dependency configuration file not found |
@@ -1288,6 +1290,7 @@ Enum for Suggestion codes
 | SET_RENDER_FLAG_OFFLINE_FALSE | 600 | Rerun with correct offline flag value. |
 | KPTFILE_MANUAL_INIT | 601 | Manually run `kpt pkg init` or `kpt live init` |
 | KPTFILE_CHECK_YAML | 602 | Check if the Kptfile is correct. |
+| REMOVE_NAMESPACE_FROM_MANIFESTS | 603 | Remove namespace from manifests |
 | CONFIG_CHECK_FILE_PATH | 700 | Check configuration file path |
 | CONFIG_CHECK_DEPENDENCY_DEFINITION | 701 | Check dependency config definition |
 | CONFIG_CHANGE_NAMES | 702 | Change config name to avoid duplicates |

docs/content/en/docs/references/api/grpc.md

@@ -1063,6 +1063,8 @@ For Cancelled Error code, use range 800 to 850.<br>
 | RENDER_KPTFILE_INIT_ERR | 1501 | Render errors The Kptfile cannot be created via `kpt pkg init`. |
 | RENDER_KPTFILE_INVALID_YAML_ERR | 1401 | The Kptfile is not a valid yaml file |
 | RENDER_KPTFILE_INVALID_SCHEMA_ERR | 1402 | The Kptfile is not a valid API schema |
+| RENDER_SET_NAMESPACE_ERR | 1403 | Error setting namespace. |
+| RENDER_NAMESPACE_ALREADY_SET_ERR | 1404 | Namespace is already set. |
 | CONFIG_FILE_PARSING_ERR | 1201 | Catch-all configuration file parsing error |
 | CONFIG_FILE_NOT_FOUND_ERR | 1202 | Main configuration file not found |
 | CONFIG_DEPENDENCY_NOT_FOUND_ERR | 1203 | Dependency configuration file not found |
@@ -1143,6 +1145,7 @@ Enum for Suggestion codes
 | SET_RENDER_FLAG_OFFLINE_FALSE | 600 | Rerun with correct offline flag value. |
 | KPTFILE_MANUAL_INIT | 601 | Manually run `kpt pkg init` or `kpt live init` |
 | KPTFILE_CHECK_YAML | 602 | Check if the Kptfile is correct. |
+| REMOVE_NAMESPACE_FROM_MANIFESTS | 603 | Remove namespace from manifests |
 | CONFIG_CHECK_FILE_PATH | 700 | Check configuration file path |
 | CONFIG_CHECK_DEPENDENCY_DEFINITION | 701 | Check dependency config definition |
 | CONFIG_CHANGE_NAMES | 702 | Change config name to avoid duplicates |

docs/content/en/docs/references/cli/_index.md

@@ -131,7 +131,7 @@ Options:
       --kube-context='': Deploy to this Kubernetes context
       --kubeconfig='': Path to the kubeconfig file to use for CLI requests.
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
   -p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
       --remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
       --status-check=: Wait for deployed resources to stabilize
@@ -209,7 +209,7 @@ Options:
       --kubeconfig='': Path to the kubeconfig file to use for CLI requests.
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
       --mute-logs=[]: mute logs for specified stages in pipeline (build, deploy, status-check, none, all)
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
   -o, --output={{json .}}: Used in conjunction with --quiet flag. Format output with go-template. For full struct documentation, see https://godoc.org/github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags#BuildOutput
       --platform=[]: The platform to target for the build artifacts
   -p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
@@ -425,7 +425,7 @@ Options:
   -l, --label=[]: Add custom labels to deployed objects. Set multiple times for multiple labels
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
       --mute-logs=[]: mute logs for specified stages in pipeline (build, deploy, status-check, none, all)
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
       --no-prune=false: Skip removing images and containers built by Skaffold
       --no-prune-children=false: Skip removing layers reused by Skaffold
       --platform=[]: The platform to target for the build artifacts
@@ -536,7 +536,7 @@ Options:
       --kube-context='': Deploy to this Kubernetes context
       --kubeconfig='': Path to the kubeconfig file to use for CLI requests.
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
   -p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
       --profile-auto-activation=true: Set to false to disable profile auto activation
       --propagate-profiles=true: Setting '--propagate-profiles=false' disables propagating profiles set by the '--profile' flag across config dependencies. This mean that only profiles defined directly in the target 'skaffold.yaml' file are activated.
@@ -608,7 +608,7 @@ Options:
       --load-images=false: If true, skaffold will force load the container images into the local cluster.
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
       --mute-logs=[]: mute logs for specified stages in pipeline (build, deploy, status-check, none, all)
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
       --platform=[]: The platform to target for the build artifacts
       --port-forward=off: Port-forward exposes service ports and container ports within pods and other resources (off, user, services, debug, pods)
   -p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
@@ -711,7 +711,7 @@ Options:
   -l, --label=[]: Add custom labels to deployed objects. Set multiple times for multiple labels
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
       --mute-logs=[]: mute logs for specified stages in pipeline (build, deploy, status-check, none, all)
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
       --no-prune=false: Skip removing images and containers built by Skaffold
       --no-prune-children=false: Skip removing layers reused by Skaffold
       --platform=[]: The platform to target for the build artifacts
@@ -978,7 +978,7 @@ Options:
   -l, --label=[]: Add custom labels to deployed objects. Set multiple times for multiple labels
       --loud=false: Show the build logs and output
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
       --offline=false: Do not connect to Kubernetes API server for manifest creation and validation. This is helpful when no Kubernetes cluster is available (e.g. GitOps model). No metadata.namespace attribute is injected in this case - the manifest content does not get changed.
   -o, --output='': File to write rendered manifests to
   -p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
@@ -1060,7 +1060,7 @@ Options:
   -l, --label=[]: Add custom labels to deployed objects. Set multiple times for multiple labels
   -m, --module=[]: Filter Skaffold configs to only the provided named modules
       --mute-logs=[]: mute logs for specified stages in pipeline (build, deploy, status-check, none, all)
-  -n, --namespace='': Run deployments in the specified namespace
+  -n, --namespace='': Runs deployments in the specified namespace. When used with 'render' command, renders manifests contain the namespace
       --no-prune=false: Skip removing images and containers built by Skaffold
       --no-prune-children=false: Skip removing layers reused by Skaffold
       --platform=[]: The platform to target for the build artifacts

examples/getting-started/k8s-pod.yaml

@@ -2,6 +2,7 @@ apiVersion: v1
 kind: Pod
 metadata:
   name: getting-started
+  namespace: ""
 spec:
   containers:
   - name: getting-started

integration/render_test.go

@@ -19,6 +19,7 @@ package integration
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path"
 	"regexp"
@@ -36,7 +37,7 @@ import (
 
 func TestKubectlRenderOutput(t *testing.T) {
 	MarkIntegrationTest(t, CanRunWithoutGcp)
-
+	ns, _ := SetupNamespace(t)
 	test := struct {
 		description string
 		builds      []graph.Artifact
@@ -57,12 +58,14 @@ spec:
   - image: gcr.io/k8s-skaffold/skaffold
     name: skaffold
 `,
-		expectedOut: `apiVersion: v1
+		expectedOut: fmt.Sprintf(`apiVersion: v1
 kind: Pod
+metadata:
+  namespace: %s
 spec:
   containers:
   - image: gcr.io/k8s-skaffold/skaffold:test
-    name: skaffold`}
+    name: skaffold`, ns.Name)}
 
 	testutil.Run(t, test.description, func(t *testutil.T) {
 		tmpDir := t.NewTempDir()
@@ -73,7 +76,7 @@ spec:
 				RawK8s: []string{"deployment.yaml"}},
 		}
 		mockCfg := render.MockConfig{WorkingDir: tmpDir.Root()}
-		r, err := kubectl.New(mockCfg, rc, map[string]string{}, "default")
+		r, err := kubectl.New(mockCfg, rc, map[string]string{}, "default", ns.Name)
 		t.RequireNoError(err)
 		var b bytes.Buffer
 		l, err := r.Render(context.Background(), &b, test.builds, false)
@@ -86,7 +89,7 @@ spec:
 
 func TestKubectlRender(t *testing.T) {
 	MarkIntegrationTest(t, CanRunWithoutGcp)
-
+	ns, _ := SetupNamespace(t)
 	tests := []struct {
 		description string
 		builds      []graph.Artifact
@@ -110,14 +113,15 @@ spec:
   - image: gcr.io/k8s-skaffold/skaffold
     name: skaffold
 `,
-			expectedOut: `apiVersion: v1
+			expectedOut: fmt.Sprintf(`apiVersion: v1
 kind: Pod
 metadata:
   name: my-pod-123
+  namespace: %s
 spec:
   containers:
   - image: gcr.io/k8s-skaffold/skaffold:test
-    name: skaffold`,
+    name: skaffold`, ns.Name),
 		},
 		{
 			description: "two artifacts",
@@ -142,16 +146,17 @@ spec:
   - image: gcr.io/project/image2
     name: image2
 `,
-			expectedOut: `apiVersion: v1
+			expectedOut: fmt.Sprintf(`apiVersion: v1
 kind: Pod
 metadata:
   name: my-pod-123
+  namespace: %s
 spec:
   containers:
   - image: gcr.io/project/image1:tag1
     name: image1
   - image: gcr.io/project/image2:tag2
-    name: image2`,
+    name: image2`, ns.Name),
 		},
 		{
 			description: "two artifacts, combined manifests",
@@ -183,10 +188,11 @@ spec:
   - image: gcr.io/project/image2
     name: image2
 `,
-			expectedOut: `apiVersion: v1
+			expectedOut: fmt.Sprintf(`apiVersion: v1
 kind: Pod
 metadata:
   name: my-pod-123
+  namespace: %s
 spec:
   containers:
   - image: gcr.io/project/image1:tag1
@@ -196,10 +202,11 @@ apiVersion: v1
 kind: Pod
 metadata:
   name: my-pod-456
+  namespace: %s
 spec:
   containers:
   - image: gcr.io/project/image2:tag2
-    name: image2`,
+    name: image2`, ns.Name, ns.Name),
 		},
 	}
 	for _, test := range tests {
@@ -212,7 +219,7 @@ spec:
 					RawK8s: []string{"deployment.yaml"}},
 			}
 			mockCfg := render.MockConfig{WorkingDir: tmpDir.Root()}
-			r, err := kubectl.New(mockCfg, rc, map[string]string{}, "default")
+			r, err := kubectl.New(mockCfg, rc, map[string]string{}, "default", ns.Name)
 			t.RequireNoError(err)
 			var b bytes.Buffer
 			l, err := r.Render(context.Background(), &b, test.builds, false)
@@ -411,6 +418,7 @@ spec:
 kind: Pod
 metadata:
   name: my-pod-123
+  namespace: default
 spec:
   containers:
   - image: 12345.dkr.ecr.eu-central-1.amazonaws.com/my/project-a:4da6a56988057d23f68a4e988f4905dd930ea438-dirty@sha256:d8a33c260c50385ea54077bc7032dba0a860dc8870464f6795fd0aa548d117bf
@@ -419,7 +427,6 @@ spec:
     name: b
 `,
 		},
-
 		{
 			description: "kubectl render from build output, offline, no labels",
 			config: `
@@ -639,6 +646,7 @@ spec:
 kind: Pod
 metadata:
   name: my-pod-123
+  namespace: default
 spec:
   containers:
   - image: 12345.dkr.ecr.eu-central-1.amazonaws.com/my/project-a:4da6a56988057d23f68a4e988f4905dd930ea438-dirty@sha256:d8a33c260c50385ea54077bc7032dba0a860dc8870464f6795fd0aa548d117bf
@@ -675,7 +683,7 @@ spec:
 
 			if test.offline {
 				env := []string{"KUBECONFIG=not-supposed-to-be-used-in-offline-mode"}
-				args = append(args, "--offline")
+				args = append(args, "--offline=true")
 				skaffold.Render(args...).WithEnv(env).RunOrFail(t.T)
 			} else {
 				skaffold.Render(args...).RunOrFail(t.T)

pkg/skaffold/deploy/helm/helm.go

@@ -126,6 +126,7 @@ type Config interface {
 	kstatus.Config
 	kloader.Config
 	portforward.Config
+	GetNamespace() string
 	IsMultiConfig() bool
 	JSONParseConfig() latest.JSONParseConfig
 }

pkg/skaffold/deploy/kubectl/cli.go

@@ -57,6 +57,7 @@ type Config interface {
 	WaitForDeletions() config.WaitForDeletions
 	Mode() config.RunMode
 	HydratedManifests() []string
+	GetNamespace() string
 	DefaultPipeline() latest.Pipeline
 	Tail() bool
 	PipelineForImage(imageName string) (latest.Pipeline, bool)

pkg/skaffold/deploy/kubectl/kubectl_test.go

@@ -235,7 +235,7 @@ func TestKubectlV1RenderDeploy(t *testing.T) {
 				},
 			}
 
-			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName)
+			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName, "")
 			t.CheckNoError(err)
 			var b bytes.Buffer
 			m, errR := r.Render(context.Background(), &b, []graph.Artifact{{ImageName: "leeroy-web", Tag: "leeroy-web:v1"}},
@@ -330,7 +330,7 @@ func TestKubectlCleanup(t *testing.T) {
 				},
 			}
 
-			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName)
+			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName, "")
 			t.CheckNoError(err)
 			var b bytes.Buffer
 			m, errR := r.Render(context.Background(), &b, []graph.Artifact{{ImageName: "leeroy-web", Tag: "leeroy-web:v1"}},
@@ -534,7 +534,7 @@ func TestGCSManifests(t *testing.T) {
 					}),
 				},
 			}
-			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName)
+			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName, "")
 			t.CheckNoError(err)
 			var b bytes.Buffer
 			m, errR := r.Render(context.Background(), &b, []graph.Artifact{{ImageName: "leeroy-web", Tag: "leeroy-web:v1"}},

pkg/skaffold/deploy/kubectl/kustomize_test.go

@@ -142,7 +142,7 @@ func TestKustomizeRenderDeploy(t *testing.T) {
 			mockCfg := &kubectlConfig{
 				RunContext: runcontext.RunContext{},
 			}
-			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName)
+			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName, "")
 			t.CheckNoError(err)
 			var b bytes.Buffer
 			m, errR := r.Render(context.Background(), &b, test.builds, true)
@@ -224,7 +224,7 @@ func TestKustomizeCleanup(t *testing.T) {
 				workingDir: tmpDir.Root(),
 				RunContext: runcontext.RunContext{},
 			}
-			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName)
+			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, configName, "")
 			t.CheckNoError(err)
 			var b bytes.Buffer
 			m, errR := r.Render(context.Background(), &b, []graph.Artifact{{ImageName: "leeroy-web", Tag: "leeroy-web:v1"}},
@@ -444,7 +444,7 @@ func TestDependenciesForKustomization(t *testing.T) {
 			mockCfg := &kubectlConfig{
 				RunContext: runcontext.RunContext{},
 			}
-			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, "default")
+			r, err := kubectlR.New(mockCfg, rc, map[string]string{}, "default", "")
 			t.CheckNoError(err)
 
 			deps, err := r.ManifestDeps()