Allow wildcard for Evaluations API (#13530)

* Failing test and TODO for wildcard

* Alias the namespace query parameter for Evals

* eval: fix list when using ACLs and * namespace

Apply the same verification process as in job, allocs and scaling
policy list endpoints to handle the eval list when using an ACL token
with limited namespace support but querying using the `*` wildcard
namespace.

* changelog: add entry for #13530

* ui: set namespace when querying eval

Evals have a unique UUID as ID, but when querying them the Nomad API
still expects a namespace query param, otherwise it assumes `default`.

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>

.changelog/13530.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+api: Fix listing evaluations with the wildcard namespace and an ACL token
+```
+
+```release-note:bug
+ui: Fix a bug that prevented viewing the details of an evaluation in a non-default namespace
+```

nomad/eval_endpoint.go

@@ -571,11 +571,14 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon
 	namespace := args.RequestNamespace()
 
 	// Check for read-job permissions
-	if aclObj, err := e.srv.ResolveToken(args.AuthToken); err != nil {
+	aclObj, err := e.srv.ResolveToken(args.AuthToken)
+	if err != nil {
 		return err
-	} else if aclObj != nil && !aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadJob) {
+	}
+	if !aclObj.AllowNsOp(namespace, acl.NamespaceCapabilityReadJob) {
 		return structs.ErrPermissionDenied
 	}
+	allow := aclObj.AllowNsOpFunc(acl.NamespaceCapabilityReadJob)
 
 	if args.Filter != "" {
 		// Check for incompatible filtering.
@@ -596,57 +599,72 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon
 			var iter memdb.ResultIterator
 			var opts paginator.StructsTokenizerOptions
 
-			if prefix := args.QueryOptions.Prefix; prefix != "" {
+			// Get the namespaces the user is allowed to access.
-				iter, err = store.EvalsByIDPrefix(ws, namespace, prefix, sort)
+			allowableNamespaces, err := allowedNSes(aclObj, store, allow)
-				opts = paginator.StructsTokenizerOptions{
+			if err == structs.ErrPermissionDenied {
-					WithID: true,
+				// return empty evals if token isn't authorized for any
-				}
+				// namespace, matching other endpoints
-			} else if namespace != structs.AllNamespacesSentinel {
+				reply.Evaluations = make([]*structs.Evaluation, 0)
-				iter, err = store.EvalsByNamespaceOrdered(ws, namespace, sort)
+			} else if err != nil {
-				opts = paginator.StructsTokenizerOptions{
-					WithCreateIndex: true,
-					WithID:          true,
-				}
-			} else {
-				iter, err = store.Evals(ws, sort)
-				opts = paginator.StructsTokenizerOptions{
-					WithCreateIndex: true,
-					WithID:          true,
-				}
-			}
-			if err != nil {
 				return err
-			}
+			} else {
-
+				if prefix := args.QueryOptions.Prefix; prefix != "" {
-			iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool {
+					iter, err = store.EvalsByIDPrefix(ws, namespace, prefix, sort)
-				if eval := raw.(*structs.Evaluation); eval != nil {
+					opts = paginator.StructsTokenizerOptions{
-					return args.ShouldBeFiltered(eval)
+						WithID: true,
+					}
+				} else if namespace != structs.AllNamespacesSentinel {
+					iter, err = store.EvalsByNamespaceOrdered(ws, namespace, sort)
+					opts = paginator.StructsTokenizerOptions{
+						WithCreateIndex: true,
+						WithID:          true,
+					}
+				} else {
+					iter, err = store.Evals(ws, sort)
+					opts = paginator.StructsTokenizerOptions{
+						WithCreateIndex: true,
+						WithID:          true,
+					}
+				}
+				if err != nil {
+					return err
 				}
-				return false
-			})
 
-			tokenizer := paginator.NewStructsTokenizer(iter, opts)
+				iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool {
-
+					if eval := raw.(*structs.Evaluation); eval != nil {
-			var evals []*structs.Evaluation
+						return args.ShouldBeFiltered(eval)
-			paginator, err := paginator.NewPaginator(iter, tokenizer, nil, args.QueryOptions,
+					}
-				func(raw interface{}) error {
+					return false
-					eval := raw.(*structs.Evaluation)
-					evals = append(evals, eval)
-					return nil
 				})
-			if err != nil {
-				return structs.NewErrRPCCodedf(
-					http.StatusBadRequest, "failed to create result paginator: %v", err)
-			}
 
-			nextToken, err := paginator.Page()
+				tokenizer := paginator.NewStructsTokenizer(iter, opts)
-			if err != nil {
+				filters := []paginator.Filter{
-				return structs.NewErrRPCCodedf(
+					paginator.NamespaceFilter{
-					http.StatusBadRequest, "failed to read result page: %v", err)
+						AllowableNamespaces: allowableNamespaces,
-			}
+					},
+				}
 
-			reply.QueryMeta.NextToken = nextToken
+				var evals []*structs.Evaluation
-			reply.Evaluations = evals
+				paginator, err := paginator.NewPaginator(iter, tokenizer, filters, args.QueryOptions,
+					func(raw interface{}) error {
+						eval := raw.(*structs.Evaluation)
+						evals = append(evals, eval)
+						return nil
+					})
+				if err != nil {
+					return structs.NewErrRPCCodedf(
+						http.StatusBadRequest, "failed to create result paginator: %v", err)
+				}
+
+				nextToken, err := paginator.Page()
+				if err != nil {
+					return structs.NewErrRPCCodedf(
+						http.StatusBadRequest, "failed to read result page: %v", err)
+				}
+
+				reply.QueryMeta.NextToken = nextToken
+				reply.Evaluations = evals
+			}
 
 			// Use the last index that affected the jobs table
 			index, err := store.Index("evals")

nomad/eval_endpoint_test.go

@@ -1244,62 +1244,104 @@ func TestEvalEndpoint_List_ACL(t *testing.T) {
 	defer cleanupS1()
 	codec := rpcClient(t, s1)
 	testutil.WaitForLeader(t, s1.RPC)
-	assert := assert.New(t)
+
+	// Create dev namespace
+	devNS := mock.Namespace()
+	devNS.Name = "dev"
+	err := s1.fsm.State().UpsertNamespaces(999, []*structs.Namespace{devNS})
+	require.NoError(t, err)
 
 	// Create the register request
 	eval1 := mock.Eval()
 	eval1.ID = "aaaaaaaa-3350-4b4b-d185-0e1992ed43e9"
 	eval2 := mock.Eval()
 	eval2.ID = "aaaabbbb-3350-4b4b-d185-0e1992ed43e9"
+	eval3 := mock.Eval()
+	eval3.ID = "aaaacccc-3350-4b4b-d185-0e1992ed43e9"
+	eval3.Namespace = devNS.Name
 	state := s1.fsm.State()
-	assert.Nil(state.UpsertEvals(structs.MsgTypeTestSetup, 1000, []*structs.Evaluation{eval1, eval2}))
+	err = state.UpsertEvals(structs.MsgTypeTestSetup, 1000, []*structs.Evaluation{eval1, eval2, eval3})
+	require.NoError(t, err)
 
 	// Create ACL tokens
 	validToken := mock.CreatePolicyAndToken(t, state, 1003, "test-valid",
 		mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob}))
 	invalidToken := mock.CreatePolicyAndToken(t, state, 1001, "test-invalid",
 		mock.NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityListJobs}))
+	devToken := mock.CreatePolicyAndToken(t, state, 1005, "test-dev",
+		mock.NamespacePolicy("dev", "", []string{acl.NamespaceCapabilityReadJob}))
 
-	get := &structs.EvalListRequest{
+	testCases := []struct {
-		QueryOptions: structs.QueryOptions{
+		name          string
-			Region:    "global",
+		namespace     string
-			Namespace: structs.DefaultNamespace,
+		token         string
+		expectedEvals []string
+		expectedError string
+	}{
+		{
+			name:          "no token",
+			token:         "",
+			namespace:     structs.DefaultNamespace,
+			expectedError: structs.ErrPermissionDenied.Error(),
+		},
+		{
+			name:          "invalid token",
+			token:         invalidToken.SecretID,
+			namespace:     structs.DefaultNamespace,
+			expectedError: structs.ErrPermissionDenied.Error(),
+		},
+		{
+			name:          "valid token",
+			token:         validToken.SecretID,
+			namespace:     structs.DefaultNamespace,
+			expectedEvals: []string{eval1.ID, eval2.ID},
+		},
+		{
+			name:          "root token default namespace",
+			token:         root.SecretID,
+			namespace:     structs.DefaultNamespace,
+			expectedEvals: []string{eval1.ID, eval2.ID},
+		},
+		{
+			name:          "root token all namespaces",
+			token:         root.SecretID,
+			namespace:     structs.AllNamespacesSentinel,
+			expectedEvals: []string{eval1.ID, eval2.ID, eval3.ID},
+		},
+		{
+			name:          "dev token all namespaces",
+			token:         devToken.SecretID,
+			namespace:     structs.AllNamespacesSentinel,
+			expectedEvals: []string{eval3.ID},
 		},
 	}
 
-	// Try without a token and expect permission denied
+	for _, tc := range testCases {
-	{
+		t.Run(tc.name, func(t *testing.T) {
-		var resp structs.EvalListResponse
+			get := &structs.EvalListRequest{
-		err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)
+				QueryOptions: structs.QueryOptions{
-		assert.NotNil(err)
+					AuthToken: tc.token,
-		assert.Contains(err.Error(), structs.ErrPermissionDenied.Error())
+					Region:    "global",
-	}
+					Namespace: tc.namespace,
+				},
+			}
 
-	// Try with an invalid token and expect permission denied
+			var resp structs.EvalListResponse
-	{
+			err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)
-		get.AuthToken = invalidToken.SecretID
-		var resp structs.EvalListResponse
-		err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)
-		assert.NotNil(err)
-		assert.Contains(err.Error(), structs.ErrPermissionDenied.Error())
-	}
 
-	// List evals with a valid token
+			if tc.expectedError != "" {
-	{
+				require.Contains(t, err.Error(), tc.expectedError)
-		get.AuthToken = validToken.SecretID
+			} else {
-		var resp structs.EvalListResponse
+				require.NoError(t, err)
-		assert.Nil(msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp))
+				require.Equal(t, uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000)
-		assert.Equal(uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000)
-		assert.Lenf(resp.Evaluations, 2, "bad: %#v", resp.Evaluations)
-	}
 
-	// List evals with a root token
+				got := make([]string, len(resp.Evaluations))
-	{
+				for i, eval := range resp.Evaluations {
-		get.AuthToken = root.SecretID
+					got[i] = eval.ID
-		var resp structs.EvalListResponse
+				}
-		assert.Nil(msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp))
+				require.ElementsMatch(t, got, tc.expectedEvals)
-		assert.Equal(uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000)
+			}
-		assert.Lenf(resp.Evaluations, 2, "bad: %#v", resp.Evaluations)
+		})
 	}
 }
 
@@ -1377,6 +1419,12 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) {
 	codec := rpcClient(t, s1)
 	testutil.WaitForLeader(t, s1.RPC)
 
+	// Create non-default namespace
+	nondefaultNS := mock.Namespace()
+	nondefaultNS.Name = "non-default"
+	err := s1.fsm.State().UpsertNamespaces(999, []*structs.Namespace{nondefaultNS})
+	require.NoError(t, err)
+
 	// create a set of evals and field values to filter on. these are
 	// in the order that the state store will return them from the
 	// iterator (sorted by create index), for ease of writing tests
@@ -1388,7 +1436,7 @@ func TestEvalEndpoint_List_PaginationFiltering(t *testing.T) {
 	}{
 		{ids: []string{"aaaa1111-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"},                    // 0
 		{ids: []string{"aaaaaa22-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example"},                    // 1
-		{ids: []string{"aaaaaa33-3350-4b4b-d185-0e1992ed43e9"}, namespace: "non-default"},            // 2
+		{ids: []string{"aaaaaa33-3350-4b4b-d185-0e1992ed43e9"}, namespace: nondefaultNS.Name},        // 2
 		{ids: []string{"aaaaaaaa-3350-4b4b-d185-0e1992ed43e9"}, jobID: "example", status: "blocked"}, // 3
 		{ids: []string{"aaaaaabb-3350-4b4b-d185-0e1992ed43e9"}},                                      // 4
 		{ids: []string{"aaaaaacc-3350-4b4b-d185-0e1992ed43e9"}},                                      // 5

nomad/structs/structs.go

@@ -10956,6 +10956,14 @@ func (e *Evaluation) GetID() string {
 	return e.ID
 }
 
+// GetNamespace implements the NamespaceGetter interface, required for pagination.
+func (e *Evaluation) GetNamespace() string {
+	if e == nil {
+		return ""
+	}
+	return e.Namespace
+}
+
 // GetCreateIndex implements the CreateIndexGetter interface, required for
 // pagination.
 func (e *Evaluation) GetCreateIndex() uint64 {

ui/app/adapters/evaluation.js

@@ -10,10 +10,12 @@ export default class EvaluationAdapter extends ApplicationAdapter {
   }
 
   urlForFindRecord(_id, _modelName, snapshot) {
-    const url = super.urlForFindRecord(...arguments);
+    const namespace = snapshot.attr('namespace') || 'default';
+    const baseURL = super.urlForFindRecord(...arguments);
+    const url = `${baseURL}?namespace=${namespace}`;
 
     if (snapshot.adapterOptions?.related) {
-      return `${url}?related=true`;
+      return `${url}&related=true`;
     }
     return url;
   }

ui/app/controllers/evaluations/index.js

@@ -35,7 +35,7 @@ export default class EvaluationsController extends Controller {
     'currentEval',
     'pageSize',
     'status',
-    'qpNamespace',
+    { qpNamespace: 'namespace' },
     'type',
     'searchTerm',
   ];

ui/tests/acceptance/evaluations-test.js

@@ -676,7 +676,14 @@ module('Acceptance | evaluations list', function (hooks) {
   module('evaluation detail', function () {
     test('clicking an evaluation opens the detail view', async function (assert) {
       server.get('/evaluations', getStandardRes);
-      server.get('/evaluation/:id', function (_, { params }) {
+      server.get('/evaluation/:id', function (_, { queryParams, params }) {
+        const expectedNamespaces = ['default', 'ted-lasso'];
+        assert.notEqual(
+          expectedNamespaces.indexOf(queryParams.namespace),
+          -1,
+          'Eval details request has namespace query param'
+        );
+
         return { ...generateAcceptanceTestEvalMock(params.id), ID: params.id };
       });