From d1868405b4daa28b82445467ae3040888b798968 Mon Sep 17 00:00:00 2001
From: Chelsea Holland Komlo <me@chelseakomlo.com>
Date: Mon, 16 Oct 2017 12:03:00 -0400
Subject: [PATCH 1/2] return error if tokens cannot be deleted because they do
 not exist

---
 nomad/acl_endpoint.go      |  9 ++++++++-
 nomad/acl_endpoint_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go
index e77cd1048..414968afe 100644
--- a/nomad/acl_endpoint.go
+++ b/nomad/acl_endpoint.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	metrics "github.com/armon/go-metrics"
@@ -578,12 +579,14 @@ func (a *ACL) DeleteTokens(args *structs.ACLTokenDeleteRequest, reply *structs.G
 	// Determine if we are deleting local or global tokens
 	hasGlobal := false
 	allGlobal := true
+	nonexistentTokens := make([]string, 0)
 	for _, accessor := range args.AccessorIDs {
 		token, err := state.ACLTokenByAccessorID(nil, accessor)
 		if err != nil {
 			return fmt.Errorf("token lookup failed: %v", err)
 		}
-		if token == nil {
+		if token == nil { // why continue if the token is nil???
+			nonexistentTokens = append(nonexistentTokens, accessor)
 			continue
 		}
 		if token.Global {
@@ -593,6 +596,10 @@ func (a *ACL) DeleteTokens(args *structs.ACLTokenDeleteRequest, reply *structs.G
 		}
 	}
 
+	if len(nonexistentTokens) != 0 {
+		return fmt.Errorf("Cannot delete nonexistant tokens: %v", strings.Join(nonexistentTokens, ", "))
+	}
+
 	// Disallow mixed requests with global and non-global tokens since we forward
 	// the entire request as a single batch.
 	if hasGlobal {
diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go
index 8c668aa75..7e03cc178 100644
--- a/nomad/acl_endpoint_test.go
+++ b/nomad/acl_endpoint_test.go
@@ -907,6 +907,33 @@ func TestACLEndpoint_DeleteTokens(t *testing.T) {
 	assert.NotEqual(t, uint64(0), resp.Index)
 }
 
+func TestACLEndpoint_DeleteTokens_WithNonexistantToken(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	s1, root := testACLServer(t, nil)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	testutil.WaitForLeader(t, s1.RPC)
+
+	nonExistentToken := mock.ACLToken()
+
+	// Lookup the policies
+	req := &structs.ACLTokenDeleteRequest{
+		AccessorIDs: []string{nonExistentToken.AccessorID},
+		WriteRequest: structs.WriteRequest{
+			Region:    "global",
+			AuthToken: root.SecretID,
+		},
+	}
+	var resp structs.GenericResponse
+	err := msgpackrpc.CallWithCodec(codec, "ACL.DeleteTokens", req, &resp)
+
+	assert.NotNil(err)
+	expectedError := fmt.Sprintf("Cannot delete nonexistant tokens: %s", nonExistentToken.AccessorID)
+	assert.Contains(expectedError, err.Error())
+}
+
 func TestACLEndpoint_Bootstrap(t *testing.T) {
 	t.Parallel()
 	s1 := testServer(t, func(c *Config) {

From 27cef91227983073bd66bb30ff4b3a170af98010 Mon Sep 17 00:00:00 2001
From: Chelsea Holland Komlo <me@chelseakomlo.com>
Date: Mon, 16 Oct 2017 15:47:13 -0400
Subject: [PATCH 2/2] review feedback

---
 nomad/acl_endpoint.go      | 4 ++--
 nomad/acl_endpoint_test.go | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go
index 414968afe..6c0d0d7fe 100644
--- a/nomad/acl_endpoint.go
+++ b/nomad/acl_endpoint.go
@@ -585,7 +585,7 @@ func (a *ACL) DeleteTokens(args *structs.ACLTokenDeleteRequest, reply *structs.G
 		if err != nil {
 			return fmt.Errorf("token lookup failed: %v", err)
 		}
-		if token == nil { // why continue if the token is nil???
+		if token == nil {
 			nonexistentTokens = append(nonexistentTokens, accessor)
 			continue
 		}
@@ -597,7 +597,7 @@ func (a *ACL) DeleteTokens(args *structs.ACLTokenDeleteRequest, reply *structs.G
 	}
 
 	if len(nonexistentTokens) != 0 {
-		return fmt.Errorf("Cannot delete nonexistant tokens: %v", strings.Join(nonexistentTokens, ", "))
+		return fmt.Errorf("Cannot delete nonexistent tokens: %v", strings.Join(nonexistentTokens, ", "))
 	}
 
 	// Disallow mixed requests with global and non-global tokens since we forward
diff --git a/nomad/acl_endpoint_test.go b/nomad/acl_endpoint_test.go
index 7e03cc178..944f7c5a8 100644
--- a/nomad/acl_endpoint_test.go
+++ b/nomad/acl_endpoint_test.go
@@ -930,7 +930,7 @@ func TestACLEndpoint_DeleteTokens_WithNonexistantToken(t *testing.T) {
 	err := msgpackrpc.CallWithCodec(codec, "ACL.DeleteTokens", req, &resp)
 
 	assert.NotNil(err)
-	expectedError := fmt.Sprintf("Cannot delete nonexistant tokens: %s", nonExistentToken.AccessorID)
+	expectedError := fmt.Sprintf("Cannot delete nonexistent tokens: %s", nonExistentToken.AccessorID)
 	assert.Contains(expectedError, err.Error())
 }