diff --git a/.changelog/13530.txt b/.changelog/13530.txt new file mode 100644 index 000000000000..d62572144bf5 --- /dev/null +++ b/.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 +``` diff --git a/nomad/eval_endpoint.go b/nomad/eval_endpoint.go index 111ccf81ba6d..575f7edd9393 100644 --- a/nomad/eval_endpoint.go +++ b/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,58 +599,73 @@ func (e *Eval) List(args *structs.EvalListRequest, reply *structs.EvalListRespon var iter memdb.ResultIterator var opts paginator.StructsTokenizerOptions - if prefix := args.QueryOptions.Prefix; prefix != "" { - iter, err = store.EvalsByIDPrefix(ws, namespace, prefix, sort) - opts = paginator.StructsTokenizerOptions{ - WithID: true, - } - } else if namespace != structs.AllNamespacesSentinel { - iter, err = store.EvalsByNamespaceOrdered(ws, namespace, sort) - opts = paginator.StructsTokenizerOptions{ - WithCreateIndex: true, - WithID: true, - } + // Get the namespaces the user is allowed to access. + allowableNamespaces, err := allowedNSes(aclObj, store, allow) + if err == structs.ErrPermissionDenied { + // return empty evals if token isn't authorized for any + // namespace, matching other endpoints + reply.Evaluations = make([]*structs.Evaluation, 0) + } else if err != nil { + return err } else { - iter, err = store.Evals(ws, sort) - opts = paginator.StructsTokenizerOptions{ - WithCreateIndex: true, - WithID: true, + if prefix := args.QueryOptions.Prefix; prefix != "" { + iter, err = store.EvalsByIDPrefix(ws, namespace, prefix, sort) + opts = paginator.StructsTokenizerOptions{ + 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 } - } - if err != nil { - return err - } - iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool { - if eval := raw.(*structs.Evaluation); eval != nil { - return args.ShouldBeFiltered(eval) + iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool { + if eval := raw.(*structs.Evaluation); eval != nil { + return args.ShouldBeFiltered(eval) + } + return false + }) + + tokenizer := paginator.NewStructsTokenizer(iter, opts) + filters := []paginator.Filter{ + paginator.NamespaceFilter{ + AllowableNamespaces: allowableNamespaces, + }, } - return false - }) - tokenizer := paginator.NewStructsTokenizer(iter, opts) + var evals []*structs.Evaluation + 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) + } - var evals []*structs.Evaluation - paginator, err := paginator.NewPaginator(iter, tokenizer, nil, 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) + } - 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 } - reply.QueryMeta.NextToken = nextToken - reply.Evaluations = evals - // Use the last index that affected the jobs table index, err := store.Index("evals") if err != nil { diff --git a/nomad/eval_endpoint_test.go b/nomad/eval_endpoint_test.go index bf05ce4d91a5..59b821125277 100644 --- a/nomad/eval_endpoint_test.go +++ b/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{ - QueryOptions: structs.QueryOptions{ - Region: "global", - Namespace: structs.DefaultNamespace, + testCases := []struct { + name string + namespace string + 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 - { - var resp structs.EvalListResponse - err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp) - assert.NotNil(err) - assert.Contains(err.Error(), structs.ErrPermissionDenied.Error()) - } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + get := &structs.EvalListRequest{ + QueryOptions: structs.QueryOptions{ + AuthToken: tc.token, + Region: "global", + Namespace: tc.namespace, + }, + } - // Try with an invalid token and expect permission denied - { - 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()) - } + var resp structs.EvalListResponse + err := msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp) - // List evals with a valid token - { - get.AuthToken = validToken.SecretID - var resp structs.EvalListResponse - assert.Nil(msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)) - assert.Equal(uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000) - assert.Lenf(resp.Evaluations, 2, "bad: %#v", resp.Evaluations) - } + if tc.expectedError != "" { + require.Contains(t, err.Error(), tc.expectedError) + } else { + require.NoError(t, err) + require.Equal(t, uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000) - // List evals with a root token - { - get.AuthToken = root.SecretID - var resp structs.EvalListResponse - assert.Nil(msgpackrpc.CallWithCodec(codec, "Eval.List", get, &resp)) - assert.Equal(uint64(1000), resp.Index, "Bad index: %d %d", resp.Index, 1000) - assert.Lenf(resp.Evaluations, 2, "bad: %#v", resp.Evaluations) + got := make([]string, len(resp.Evaluations)) + for i, eval := range resp.Evaluations { + got[i] = eval.ID + } + require.ElementsMatch(t, got, tc.expectedEvals) + } + }) } } @@ -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 diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 723865d2d930..07c758a0e98b 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -10900,6 +10900,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 { diff --git a/ui/app/adapters/evaluation.js b/ui/app/adapters/evaluation.js index 7013b58a3e4d..d927b71cb192 100644 --- a/ui/app/adapters/evaluation.js +++ b/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; } diff --git a/ui/app/controllers/evaluations/index.js b/ui/app/controllers/evaluations/index.js index 11afc9f12532..cb13f3a4945a 100644 --- a/ui/app/controllers/evaluations/index.js +++ b/ui/app/controllers/evaluations/index.js @@ -35,7 +35,7 @@ export default class EvaluationsController extends Controller { 'currentEval', 'pageSize', 'status', - 'qpNamespace', + { qpNamespace: 'namespace' }, 'type', 'searchTerm', ]; diff --git a/ui/tests/acceptance/evaluations-test.js b/ui/tests/acceptance/evaluations-test.js index f79a08cf52fd..e94bb51b3b5f 100644 --- a/ui/tests/acceptance/evaluations-test.js +++ b/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 }; });