diff --git a/client/alloc_endpoint.go b/client/alloc_endpoint.go index f24326b95..6b1e4eec0 100644 --- a/client/alloc_endpoint.go +++ b/client/alloc_endpoint.go @@ -1,7 +1,6 @@ package client import ( - "fmt" "time" metrics "github.com/armon/go-metrics" @@ -19,7 +18,7 @@ type Allocations struct { func (a *Allocations) GarbageCollectAll(args *nstructs.NodeSpecificRequest, reply *nstructs.GenericResponse) error { defer metrics.MeasureSince([]string{"client", "allocations", "garbage_collect_all"}, time.Now()) - // Check node read permissions + // Check node write permissions if aclObj, err := a.c.ResolveToken(args.AuthToken); err != nil { return err } else if aclObj != nil && !aclObj.AllowNodeWrite() { @@ -34,7 +33,7 @@ func (a *Allocations) GarbageCollectAll(args *nstructs.NodeSpecificRequest, repl func (a *Allocations) GarbageCollect(args *nstructs.AllocSpecificRequest, reply *nstructs.GenericResponse) error { defer metrics.MeasureSince([]string{"client", "allocations", "garbage_collect"}, time.Now()) - // Check node read permissions + // Check submit job permissions if aclObj, err := a.c.ResolveToken(args.AuthToken); err != nil { return err } else if aclObj != nil && !aclObj.AllowNsOp(args.Namespace, acl.NamespaceCapabilitySubmitJob) { @@ -43,7 +42,7 @@ func (a *Allocations) GarbageCollect(args *nstructs.AllocSpecificRequest, reply if !a.c.CollectAllocation(args.AllocID) { // Could not find alloc - return fmt.Errorf("unknown allocation %q", args.AllocID) + return nstructs.NewErrUnknownAllocation(args.AllocID) } return nil @@ -53,7 +52,7 @@ func (a *Allocations) GarbageCollect(args *nstructs.AllocSpecificRequest, reply func (a *Allocations) Stats(args *cstructs.AllocStatsRequest, reply *cstructs.AllocStatsResponse) error { defer metrics.MeasureSince([]string{"client", "allocations", "stats"}, time.Now()) - // Check node read permissions + // Check read job permissions if aclObj, err := a.c.ResolveToken(args.AuthToken); err != nil { return err } else if aclObj != nil && !aclObj.AllowNsOp(args.Namespace, acl.NamespaceCapabilityReadJob) { diff --git a/client/alloc_endpoint_test.go b/client/alloc_endpoint_test.go index 6b57da70b..bccff0c77 100644 --- a/client/alloc_endpoint_test.go +++ b/client/alloc_endpoint_test.go @@ -149,7 +149,7 @@ func TestAllocations_GarbageCollect_ACL(t *testing.T) { var resp nstructs.GenericResponse err := client.ClientRPC("Allocations.GarbageCollect", &req, &resp) - require.Contains(err.Error(), "unknown allocation") + require.True(nstructs.IsErrUnknownAllocation(err)) } // Try request with a management token @@ -159,7 +159,7 @@ func TestAllocations_GarbageCollect_ACL(t *testing.T) { var resp nstructs.GenericResponse err := client.ClientRPC("Allocations.GarbageCollect", &req, &resp) - require.Contains(err.Error(), "unknown allocation") + require.True(nstructs.IsErrUnknownAllocation(err)) } } @@ -239,7 +239,7 @@ func TestAllocations_Stats_ACL(t *testing.T) { var resp cstructs.AllocStatsResponse err := client.ClientRPC("Allocations.Stats", &req, &resp) - require.Contains(err.Error(), "unknown allocation") + require.True(nstructs.IsErrUnknownAllocation(err)) } // Try request with a management token @@ -249,6 +249,6 @@ func TestAllocations_Stats_ACL(t *testing.T) { var resp cstructs.AllocStatsResponse err := client.ClientRPC("Allocations.Stats", &req, &resp) - require.Contains(err.Error(), "unknown allocation") + require.True(nstructs.IsErrUnknownAllocation(err)) } } diff --git a/command/agent/alloc_endpoint.go b/command/agent/alloc_endpoint.go index aa07d8910..422f8906c 100644 --- a/command/agent/alloc_endpoint.go +++ b/command/agent/alloc_endpoint.go @@ -164,7 +164,7 @@ func (s *HTTPServer) allocGC(allocID string, resp http.ResponseWriter, req *http } if rpcErr != nil { - if structs.IsErrNoNodeConn(rpcErr) || strings.Contains(rpcErr.Error(), "unknown allocation") { + if structs.IsErrNoNodeConn(rpcErr) || structs.IsErrUnknownAllocation(rpcErr) { rpcErr = CodedError(404, rpcErr.Error()) } } @@ -216,7 +216,7 @@ func (s *HTTPServer) allocStats(allocID string, resp http.ResponseWriter, req *h } if rpcErr != nil { - if structs.IsErrNoNodeConn(rpcErr) || strings.Contains(rpcErr.Error(), "unknown allocation") { + if structs.IsErrNoNodeConn(rpcErr) || structs.IsErrUnknownAllocation(rpcErr) { rpcErr = CodedError(404, rpcErr.Error()) } } diff --git a/command/agent/alloc_endpoint_test.go b/command/agent/alloc_endpoint_test.go index f9db9a872..e005753ba 100644 --- a/command/agent/alloc_endpoint_test.go +++ b/command/agent/alloc_endpoint_test.go @@ -277,7 +277,7 @@ func TestHTTP_AllocStats(t *testing.T) { // Make the request _, err = s.Server.ClientAllocRequest(respW, req) require.NotNil(err) - require.Contains(err.Error(), "unknown allocation") + require.True(structs.IsErrUnknownAllocation(err)) } // Local node, server resp @@ -291,7 +291,7 @@ func TestHTTP_AllocStats(t *testing.T) { respW := httptest.NewRecorder() _, err = s.Server.ClientAllocRequest(respW, req) require.NotNil(err) - require.Contains(err.Error(), "unknown allocation") + require.True(structs.IsErrUnknownAllocation(err)) s.server = srv } @@ -317,7 +317,7 @@ func TestHTTP_AllocStats(t *testing.T) { respW := httptest.NewRecorder() _, err = s.Server.ClientAllocRequest(respW, req) require.NotNil(err) - require.Contains(err.Error(), "unknown allocation") + require.True(structs.IsErrUnknownAllocation(err)) s.client = c } @@ -551,7 +551,7 @@ func TestHTTP_AllocGC(t *testing.T) { respW := httptest.NewRecorder() _, err = s.Server.ClientAllocRequest(respW, req) - if err == nil || !strings.Contains(err.Error(), "unknown allocation") { + if !structs.IsErrUnknownAllocation(err) { t.Fatalf("unexpected err: %v", err) } } @@ -568,7 +568,7 @@ func TestHTTP_AllocGC(t *testing.T) { respW := httptest.NewRecorder() _, err = s.Server.ClientAllocRequest(respW, req) - if err == nil || !strings.Contains(err.Error(), "unknown allocation") { + if !structs.IsErrUnknownAllocation(err) { t.Fatalf("unexpected err: %v", err) } @@ -598,7 +598,7 @@ func TestHTTP_AllocGC(t *testing.T) { respW := httptest.NewRecorder() _, err = s.Server.ClientAllocRequest(respW, req) require.NotNil(err) - if err == nil || !strings.Contains(err.Error(), "unknown allocation") { + if !structs.IsErrUnknownAllocation(err) { t.Fatalf("unexpected err: %v", err) } diff --git a/nomad/client_alloc_endpoint.go b/nomad/client_alloc_endpoint.go index e174811df..bb28d2d39 100644 --- a/nomad/client_alloc_endpoint.go +++ b/nomad/client_alloc_endpoint.go @@ -2,7 +2,6 @@ package nomad import ( "errors" - "fmt" "time" metrics "github.com/armon/go-metrics" @@ -95,7 +94,7 @@ func (a *ClientAllocations) GarbageCollect(args *structs.AllocSpecificRequest, r } if alloc == nil { - return fmt.Errorf("unknown allocation %q", args.AllocID) + return structs.NewErrUnknownAllocation(args.AllocID) } // Get the connection to the client @@ -145,7 +144,7 @@ func (a *ClientAllocations) Stats(args *cstructs.AllocStatsRequest, reply *cstru } if alloc == nil { - return fmt.Errorf("unknown allocation %q", args.AllocID) + return structs.NewErrUnknownAllocation(args.AllocID) } // Get the connection to the client diff --git a/nomad/client_alloc_endpoint_test.go b/nomad/client_alloc_endpoint_test.go index 4f18942c5..1886aab1c 100644 --- a/nomad/client_alloc_endpoint_test.go +++ b/nomad/client_alloc_endpoint_test.go @@ -295,12 +295,12 @@ func TestClientAllocations_GarbageCollect_Local_ACL(t *testing.T) { { Name: "good token", Token: tokenGood.SecretID, - ExpectedError: "unknown allocation", + ExpectedError: structs.ErrUnknownAllocationPrefix, }, { Name: "root token", Token: root.SecretID, - ExpectedError: "unknown allocation", + ExpectedError: structs.ErrUnknownAllocationPrefix, }, } @@ -526,12 +526,12 @@ func TestClientAllocations_Stats_Local_ACL(t *testing.T) { { Name: "good token", Token: tokenGood.SecretID, - ExpectedError: "unknown allocation", + ExpectedError: structs.ErrUnknownAllocationPrefix, }, { Name: "root token", Token: root.SecretID, - ExpectedError: "unknown allocation", + ExpectedError: structs.ErrUnknownAllocationPrefix, }, }