From 0cfbac0944c1fe7625b20160ca0db34fc5b26443 Mon Sep 17 00:00:00 2001
From: James Rasell <jrasell@hashicorp.com>
Date: Wed, 1 Feb 2023 12:37:24 +0000
Subject: [PATCH 1/2] acl: return 400 not 404 code when creating an invalid
 policy.

---
 command/agent/acl_endpoint_test.go | 39 +++++++++++++++++++++---------
 nomad/acl_endpoint.go              |  2 +-
 nomad/acl_endpoint_test.go         |  7 ++----
 3 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/command/agent/acl_endpoint_test.go b/command/agent/acl_endpoint_test.go
index 848ccdca6cfd..391751150288 100644
--- a/command/agent/acl_endpoint_test.go
+++ b/command/agent/acl_endpoint_test.go
@@ -125,31 +125,46 @@ func TestHTTP_ACLPolicyCreate(t *testing.T) {
 		p1 := mock.ACLPolicy()
 		buf := encodeReq(p1)
 		req, err := http.NewRequest("PUT", "/v1/acl/policy/"+p1.Name, buf)
-		if err != nil {
-			t.Fatalf("err: %v", err)
-		}
+		must.NoError(t, err)
+
 		respW := httptest.NewRecorder()
 		setToken(req, s.RootToken)
 
 		// Make the request
 		obj, err := s.Server.ACLPolicySpecificRequest(respW, req)
-		assert.Nil(t, err)
-		assert.Nil(t, obj)
+		must.NoError(t, err)
+		must.Nil(t, obj)
 
 		// Check for the index
-		if respW.Result().Header.Get("X-Nomad-Index") == "" {
-			t.Fatalf("missing index")
-		}
+		must.StrNotEqFold(t, "", respW.Result().Header.Get("X-Nomad-Index"))
 
 		// Check policy was created
 		state := s.Agent.server.State()
 		out, err := state.ACLPolicyByName(nil, p1.Name)
-		assert.Nil(t, err)
-		assert.NotNil(t, out)
+		must.NoError(t, err)
+		must.NotNil(t, out)
 
 		p1.CreateIndex, p1.ModifyIndex = out.CreateIndex, out.ModifyIndex
-		assert.Equal(t, p1.Name, out.Name)
-		assert.Equal(t, p1, out)
+		must.Eq(t, p1.Name, out.Name)
+		must.Eq(t, p1, out)
+
+		// Create a policy that is invalid. This ensures we call the validation
+		// func in the RPC handler, also that the correct code and error is
+		// returned.
+		aclPolicy2 := mock.ACLPolicy()
+		aclPolicy2.Rules = "invalid"
+
+		aclPolicy2Req, err := http.NewRequest(http.MethodPut, "/v1/acl/policy/"+aclPolicy2.Name, encodeReq(aclPolicy2))
+		must.NoError(t, err)
+
+		respW = httptest.NewRecorder()
+		setToken(aclPolicy2Req, s.RootToken)
+
+		// Make the request
+		aclPolicy2Obj, err := s.Server.ACLPolicySpecificRequest(respW, aclPolicy2Req)
+		must.ErrorContains(t, err, "400")
+		must.ErrorContains(t, err, "failed to parse rules")
+		must.Nil(t, aclPolicy2Obj)
 	})
 }
 
diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go
index 15a9a7c39906..3df43b4d73ea 100644
--- a/nomad/acl_endpoint.go
+++ b/nomad/acl_endpoint.go
@@ -100,7 +100,7 @@ func (a *ACL) UpsertPolicies(args *structs.ACLPolicyUpsertRequest, reply *struct
 	// Validate each policy, compute hash
 	for idx, policy := range args.Policies {
 		if err := policy.Validate(); err != nil {
-			return structs.NewErrRPCCodedf(404, "policy %d invalid: %v", idx, err)
+			return structs.NewErrRPCCodedf(http.StatusBadRequest, "policy %d invalid: %v", idx, err)
 		}
 		policy.SetHash()
 	}
diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go
index fd3baf590905..9233f94173bd 100644
--- a/nomad/acl_endpoint_test.go
+++ b/nomad/acl_endpoint_test.go
@@ -5,7 +5,6 @@ import (
 	"io/ioutil"
 	"net/url"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
@@ -722,10 +721,8 @@ func TestACLEndpoint_UpsertPolicies_Invalid(t *testing.T) {
 	}
 	var resp structs.GenericResponse
 	err := msgpackrpc.CallWithCodec(codec, "ACL.UpsertPolicies", req, &resp)
-	assert.NotNil(t, err)
-	if !strings.Contains(err.Error(), "failed to parse") {
-		t.Fatalf("bad: %s", err)
-	}
+	must.ErrorContains(t, err, "400")
+	must.ErrorContains(t, err, "failed to parse")
 }
 
 func TestACLEndpoint_GetToken(t *testing.T) {

From f56559d6ffab27a6af7694bc82de7e2e3aa8d580 Mon Sep 17 00:00:00 2001
From: James Rasell <jrasell@hashicorp.com>
Date: Wed, 1 Feb 2023 12:48:44 +0000
Subject: [PATCH 2/2] changelog: add entry for #16000

---
 .changelog/16000.txt | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 .changelog/16000.txt

diff --git a/.changelog/16000.txt b/.changelog/16000.txt
new file mode 100644
index 000000000000..1600e4865f37
--- /dev/null
+++ b/.changelog/16000.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+acl: Fixed a bug where creating/updating a policy which was invalid would return a 404 status code, not a 400
+```