From 38af39e1b28ba41ec21f5413912544e9eb59ddb0 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Thu, 8 Jul 2021 13:13:43 -0400
Subject: [PATCH] cli: `-namespace` should override job namespace

When a jobspec doesn't include a namespace, we provide it with the default
namespace, but this ends up overriding the explicit `-namespace` flag. This
changeset uses the same logic as region parsing to create an order of
precedence: the query string parameter (the `-namespace` flag) overrides the
API request body which overrides the jobspec.
---
 .changelog/10875.txt               |  3 ++
 command/agent/job_endpoint.go      | 28 ++++++++++--
 command/agent/job_endpoint_test.go | 71 ++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 3 deletions(-)
 create mode 100644 .changelog/10875.txt

diff --git a/.changelog/10875.txt b/.changelog/10875.txt
new file mode 100644
index 000000000000..67f6fab0dd54
--- /dev/null
+++ b/.changelog/10875.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+cli: Fixed a bug where `-namespace` flag was not respected for `job run` and `job plan` commands.
+```
diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go
index e0a9d0531538..db8277756a31 100644
--- a/command/agent/job_endpoint.go
+++ b/command/agent/job_endpoint.go
@@ -706,10 +706,9 @@ func (s *HTTPServer) apiJobAndRequestToStructs(job *api.Job, req *http.Request,
 		AuthToken: apiReq.SecretID,
 	}
 
-	queryRegion := req.URL.Query().Get("region")
 	s.parseToken(req, &writeReq.AuthToken)
-	parseNamespace(req, &writeReq.Namespace)
 
+	queryRegion := req.URL.Query().Get("region")
 	requestRegion, jobRegion := regionForJob(
 		job, queryRegion, writeReq.Region, s.agent.config.Region,
 	)
@@ -717,7 +716,11 @@ func (s *HTTPServer) apiJobAndRequestToStructs(job *api.Job, req *http.Request,
 	sJob := ApiJobToStructJob(job)
 	sJob.Region = jobRegion
 	writeReq.Region = requestRegion
-	writeReq.Namespace = sJob.Namespace
+
+	queryNamespace := req.URL.Query().Get("namespace")
+	namespace := namespaceForJob(job.Namespace, queryNamespace, writeReq.Namespace)
+	sJob.Namespace = namespace
+	writeReq.Namespace = namespace
 
 	return sJob, writeReq
 }
@@ -774,6 +777,25 @@ func regionForJob(job *api.Job, queryRegion, apiRegion, agentRegion string) (str
 	return requestRegion, jobRegion
 }
 
+func namespaceForJob(jobNamespace *string, queryNamespace, apiNamespace string) string {
+
+	// Namespace in query param (-namespace flag) takes precedence.
+	if queryNamespace != "" {
+		return queryNamespace
+	}
+
+	// Next the request body...
+	if apiNamespace != "" {
+		return apiNamespace
+	}
+
+	if jobNamespace != nil && *jobNamespace != "" {
+		return *jobNamespace
+	}
+
+	return structs.DefaultNamespace
+}
+
 func ApiJobToStructJob(job *api.Job) *structs.Job {
 	job.Canonicalize()
 
diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go
index bd5668a53f4e..a677bd6c48a9 100644
--- a/command/agent/job_endpoint_test.go
+++ b/command/agent/job_endpoint_test.go
@@ -1845,6 +1845,77 @@ func TestJobs_RegionForJob(t *testing.T) {
 	}
 }
 
+func TestJobs_NamespaceForJob(t *testing.T) {
+	t.Parallel()
+
+	// test namespace for pointer inputs
+	ns := "dev"
+
+	cases := []struct {
+		name           string
+		job            *api.Job
+		queryNamespace string
+		apiNamespace   string
+		expected       string
+	}{
+		{
+			name:     "no namespace provided",
+			job:      &api.Job{},
+			expected: structs.DefaultNamespace,
+		},
+
+		{
+			name:     "jobspec has namespace",
+			job:      &api.Job{Namespace: &ns},
+			expected: "dev",
+		},
+
+		{
+			name:           "-namespace flag overrides empty job namespace",
+			job:            &api.Job{},
+			queryNamespace: "prod",
+			expected:       "prod",
+		},
+
+		{
+			name:           "-namespace flag overrides job namespace",
+			job:            &api.Job{Namespace: &ns},
+			queryNamespace: "prod",
+			expected:       "prod",
+		},
+
+		{
+			name:           "-namespace flag overrides job namespace even if default",
+			job:            &api.Job{Namespace: &ns},
+			queryNamespace: structs.DefaultNamespace,
+			expected:       structs.DefaultNamespace,
+		},
+
+		{
+			name:         "API param overrides empty job namespace",
+			job:          &api.Job{},
+			apiNamespace: "prod",
+			expected:     "prod",
+		},
+
+		{
+			name:           "-namespace flag overrides API param",
+			job:            &api.Job{Namespace: &ns},
+			queryNamespace: "prod",
+			apiNamespace:   "whatever",
+			expected:       "prod",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.expected,
+				namespaceForJob(tc.job.Namespace, tc.queryNamespace, tc.apiNamespace),
+			)
+		})
+	}
+}
+
 func TestJobs_ApiJobToStructsJob(t *testing.T) {
 	apiJob := &api.Job{
 		Stop:        helper.BoolToPtr(true),