From d5f0db8a5eb466572c9552bceb9755b7393fa8ca Mon Sep 17 00:00:00 2001
From: Michael Schurter <mschurter@hashicorp.com>
Date: Tue, 21 Feb 2023 10:53:10 -0800
Subject: [PATCH] Task API / Dynamic Node Metadata E2E test fixes (#16219)

* taskapi: return Forbidden on bad credentials

Prior to this change a "Server error" would be returned when ACLs are
enabled which did not match when ACLs are disabled.

* e2e: love love love datacenter wildcard default

* e2e: skip windows nodes on linux only test

The Logfs are a bit weird because they're most useful when converted to
Printfs to make debugging the test much faster, but that makes CI noisy.

In a perfect world Go would expose how many tests are being run and we
could stream output live if there's only 1. For now I left these helpful
lines in as basically glorified comments.
---
 command/agent/http.go                     |  9 +++++++++
 e2e/workload_id/input/api-auth.nomad.hcl  |  3 +--
 e2e/workload_id/input/api-win.nomad.hcl   |  3 +--
 e2e/workload_id/input/node-meta.nomad.hcl |  3 +--
 e2e/workload_id/nodemeta_test.go          | 10 ++++++++--
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/command/agent/http.go b/command/agent/http.go
index ab3f87340..384ccd4dc 100644
--- a/command/agent/http.go
+++ b/command/agent/http.go
@@ -1091,6 +1091,15 @@ func (a *authMiddleware) ServeHTTP(resp http.ResponseWriter, req *http.Request)
 	}
 
 	if err := a.srv.agent.RPC("ACL.WhoAmI", &args, &reply); err != nil {
+		// When ACLs are enabled, WhoAmI returns ErrPermissionDenied on bad
+		// credentials, so convert it to a Forbidden response code.
+		if err.Error() == structs.ErrPermissionDenied.Error() {
+			a.srv.logger.Debug("Failed to authenticated Task API request", "method", req.Method, "url", req.URL)
+			resp.WriteHeader(http.StatusForbidden)
+			resp.Write([]byte(http.StatusText(http.StatusForbidden)))
+			return
+		}
+
 		a.srv.logger.Error("error authenticating built API request", "error", err, "url", req.URL, "method", req.Method)
 		resp.WriteHeader(500)
 		resp.Write([]byte("Server error authenticating request\n"))
diff --git a/e2e/workload_id/input/api-auth.nomad.hcl b/e2e/workload_id/input/api-auth.nomad.hcl
index dae134697..9f414f2ca 100644
--- a/e2e/workload_id/input/api-auth.nomad.hcl
+++ b/e2e/workload_id/input/api-auth.nomad.hcl
@@ -1,6 +1,5 @@
 job "api-auth" {
-  datacenters = ["dc1"]
-  type        = "batch"
+  type = "batch"
 
   constraint {
     attribute = "${attr.kernel.name}"
diff --git a/e2e/workload_id/input/api-win.nomad.hcl b/e2e/workload_id/input/api-win.nomad.hcl
index 7552500f3..8224887f4 100644
--- a/e2e/workload_id/input/api-win.nomad.hcl
+++ b/e2e/workload_id/input/api-win.nomad.hcl
@@ -1,6 +1,5 @@
 job "api-win" {
-  datacenters = ["dc1"]
-  type        = "batch"
+  type = "batch"
 
   constraint {
     attribute = "${attr.kernel.name}"
diff --git a/e2e/workload_id/input/node-meta.nomad.hcl b/e2e/workload_id/input/node-meta.nomad.hcl
index 89ec582e3..0d274d149 100644
--- a/e2e/workload_id/input/node-meta.nomad.hcl
+++ b/e2e/workload_id/input/node-meta.nomad.hcl
@@ -23,8 +23,7 @@ variable "unset_constraint" {
 }
 
 job "node-meta" {
-  datacenters = ["dc1"]
-  type        = "batch"
+  type = "batch"
 
   constraint {
     attribute = "${attr.kernel.name}"
diff --git a/e2e/workload_id/nodemeta_test.go b/e2e/workload_id/nodemeta_test.go
index 31cf099b9..7d80dceda 100644
--- a/e2e/workload_id/nodemeta_test.go
+++ b/e2e/workload_id/nodemeta_test.go
@@ -30,13 +30,14 @@ func TestDynamicNodeMetadata(t *testing.T) {
 func testDynamicNodeMetadata(t *testing.T) {
 	nomad := e2eutil.NomadClient(t)
 
-	nodes, _, err := nomad.Nodes().List(nil)
+	nodes, err := e2eutil.ListLinuxClientNodes(nomad)
 	must.NoError(t, err)
 	if len(nodes) == 0 {
 		t.Skip("requires at least 1 linux node")
 	}
 
-	node := nodes[0]
+	node, _, err := nomad.Nodes().Info(nodes[0], nil)
+	must.NoError(t, err)
 
 	keyFoo := "foo-" + uuid.Short()
 	keyEmpty := "empty-" + uuid.Short()
@@ -48,6 +49,9 @@ func testDynamicNodeMetadata(t *testing.T) {
 	jobIDs := []string{jobID}
 	t.Cleanup(e2eutil.CleanupJobsAndGC(t, &jobIDs))
 
+	t.Logf("test config: job=%s node=%s foo=%s empty=%s unset=%s",
+		jobID, node.ID, keyFoo, keyEmpty, keyUnset)
+
 	path := "./input/node-meta.nomad.hcl"
 	jobBytes, err := os.ReadFile(path)
 	must.NoError(t, err)
@@ -85,6 +89,8 @@ func testDynamicNodeMetadata(t *testing.T) {
 	must.Eq(t, "", resp.Meta[keyEmpty])
 	must.MapNotContainsKey(t, resp.Meta, keyUnset)
 
+	t.Logf("job submitted, node metadata applied, waiting for metadata to be visible...")
+
 	// Wait up to 10 seconds (with 1s buffer) for updates to be visible to the
 	// scheduler.
 	qo := &api.QueryOptions{