diff --git a/CHANGELOG.md b/CHANGELOG.md index a07114b0d31..5057e1ac914 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ __BACKWARDS INCOMPATIBILITIES:__ BUG FIXES: * core: Fixed a bug where blocking queries would not include the query's maximum wait time when calculating whether it was safe to retry. [[GH-8921](https://github.com/hashicorp/nomad/issues/8921)] + * core: Fixed a bug where ACL handling prevented cross-namespace allocation listing [[GH-9278](https://github.com/hashicorp/nomad/issues/9278)] * config (Enterprise): Fixed default enterprise config merging. [[GH-9083](https://github.com/hashicorp/nomad/pull/9083)] * client: Fixed an issue with the Java fingerprinter on macOS causing pop-up notifications when no JVM installed. [[GH-9225](https://github.com/hashicorp/nomad/pull/9225)] * consul: Fixed a bug to correctly validate task when using script-checks in group-level services [[GH-8952](https://github.com/hashicorp/nomad/issues/8952)] diff --git a/nomad/alloc_endpoint.go b/nomad/alloc_endpoint.go index 0d6d7210ca4..3a32b5f1964 100644 --- a/nomad/alloc_endpoint.go +++ b/nomad/alloc_endpoint.go @@ -29,6 +29,10 @@ func (a *Alloc) List(args *structs.AllocListRequest, reply *structs.AllocListRes } defer metrics.MeasureSince([]string{"nomad", "alloc", "list"}, time.Now()) + if args.RequestNamespace() == structs.AllNamespacesSentinel { + return a.listAllNamespaces(args, reply) + } + // Check namespace read-job permissions aclObj, err := a.srv.ResolveToken(args.AuthToken) if err != nil { @@ -37,10 +41,6 @@ func (a *Alloc) List(args *structs.AllocListRequest, reply *structs.AllocListRes return structs.ErrPermissionDenied } - allow := func(ns string) bool { - return aclObj.AllowNsOp(ns, acl.NamespaceCapabilityListJobs) - } - // Setup the blocking query opts := blockingOptions{ queryOpts: &args.QueryOptions, @@ -51,16 +51,7 @@ func (a *Alloc) List(args *structs.AllocListRequest, reply *structs.AllocListRes var iter memdb.ResultIterator prefix := args.QueryOptions.Prefix - if args.RequestNamespace() == structs.AllNamespacesSentinel { - allowedNSes, err := allowedNSes(aclObj, state, allow) - if err != nil { - return err - } - iter, err = state.AllocsByIDPrefixInNSes(ws, allowedNSes, prefix) - if err != nil { - return err - } - } else if prefix != "" { + if prefix != "" { iter, err = state.AllocsByIDPrefix(ws, args.RequestNamespace(), prefix) } else { iter, err = state.AllocsByNamespace(ws, args.RequestNamespace()) @@ -94,6 +85,68 @@ func (a *Alloc) List(args *structs.AllocListRequest, reply *structs.AllocListRes return a.srv.blockingRPC(&opts) } +// listAllNamespaces lists all allocations across all namespaces +func (a *Alloc) listAllNamespaces(args *structs.AllocListRequest, reply *structs.AllocListResponse) error { + // Check for read-job permissions + aclObj, err := a.srv.ResolveToken(args.AuthToken) + if err != nil { + return err + } + prefix := args.QueryOptions.Prefix + allow := func(ns string) bool { + return aclObj.AllowNsOp(ns, acl.NamespaceCapabilityReadJob) + } + + // Setup the blocking query + opts := blockingOptions{ + queryOpts: &args.QueryOptions, + queryMeta: &reply.QueryMeta, + run: func(ws memdb.WatchSet, state *state.StateStore) error { + // get list of accessible namespaces + allowedNSes, err := allowedNSes(aclObj, state, allow) + if err == structs.ErrPermissionDenied { + // return empty allocations if token isn't authorized for any + // namespace, matching other endpoints + reply.Allocations = []*structs.AllocListStub{} + } else if err != nil { + return err + } else { + var iter memdb.ResultIterator + var err error + if prefix != "" { + iter, err = state.AllocsByIDPrefixAllNSs(ws, prefix) + } else { + iter, err = state.Allocs(ws) + } + if err != nil { + return err + } + + var allocs []*structs.AllocListStub + for raw := iter.Next(); raw != nil; raw = iter.Next() { + alloc := raw.(*structs.Allocation) + if allowedNSes != nil && !allowedNSes[alloc.Namespace] { + continue + } + allocs = append(allocs, alloc.Stub(args.Fields)) + } + reply.Allocations = allocs + } + + // Use the last index that affected the jobs table + index, err := state.Index("allocs") + if err != nil { + return err + } + reply.Index = index + + // Set the query response + a.srv.setQueryMeta(&reply.QueryMeta) + return nil + }} + return a.srv.blockingRPC(&opts) +} + // GetAlloc is used to lookup a particular allocation func (a *Alloc) GetAlloc(args *structs.AllocSpecificRequest, reply *structs.SingleAllocResponse) error { diff --git a/nomad/alloc_endpoint_test.go b/nomad/alloc_endpoint_test.go index ad76bf51d07..d34abfbbf2d 100644 --- a/nomad/alloc_endpoint_test.go +++ b/nomad/alloc_endpoint_test.go @@ -6,14 +6,15 @@ import ( "time" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/hashicorp/nomad/acl" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/testutil" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestAllocEndpoint_List(t *testing.T) { @@ -318,16 +319,26 @@ func TestAllocEndpoint_List_AllNamespaces_OSS(t *testing.T) { defer cleanupS1() codec := rpcClient(t, s1) testutil.WaitForLeader(t, s1.RPC) - - // Create the register request - alloc := mock.Alloc() - summary := mock.JobSummary(alloc.JobID) state := s1.fsm.State() - err := state.UpsertJobSummary(999, summary) - require.NoError(t, err) - err = state.UpsertAllocs(structs.MsgTypeTestSetup, 1000, []*structs.Allocation{alloc}) - require.NoError(t, err) + // two namespaces + ns1 := mock.Namespace() + ns2 := mock.Namespace() + require.NoError(t, state.UpsertNamespaces(900, []*structs.Namespace{ns1, ns2})) + + // Create the allocations + alloc1 := mock.Alloc() + alloc1.ID = "a" + alloc1.ID[1:] + alloc1.Namespace = ns1.Name + alloc2 := mock.Alloc() + alloc2.ID = "b" + alloc2.ID[1:] + alloc2.Namespace = ns2.Name + summary1 := mock.JobSummary(alloc1.JobID) + summary2 := mock.JobSummary(alloc2.JobID) + + require.NoError(t, state.UpsertJobSummary(999, summary1)) + require.NoError(t, state.UpsertJobSummary(999, summary2)) + require.NoError(t, state.UpsertAllocs(structs.MsgTypeTestSetup, 1000, []*structs.Allocation{alloc1, alloc2})) t.Run("looking up all allocations", func(t *testing.T) { get := &structs.AllocListRequest{ @@ -337,12 +348,12 @@ func TestAllocEndpoint_List_AllNamespaces_OSS(t *testing.T) { }, } var resp structs.AllocListResponse - err = msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp) - require.NoError(t, err) + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp)) require.Equal(t, uint64(1000), resp.Index) - require.Len(t, resp.Allocations, 1) - require.Equal(t, alloc.ID, resp.Allocations[0].ID) - require.Equal(t, structs.DefaultNamespace, resp.Allocations[0].Namespace) + require.Len(t, resp.Allocations, 2) + require.ElementsMatch(t, + []string{resp.Allocations[0].ID, resp.Allocations[1].ID}, + []string{alloc1.ID, alloc2.ID}) }) t.Run("looking up allocations with prefix", func(t *testing.T) { @@ -350,26 +361,21 @@ func TestAllocEndpoint_List_AllNamespaces_OSS(t *testing.T) { QueryOptions: structs.QueryOptions{ Region: "global", Namespace: "*", - Prefix: alloc.ID[:4], + // allocations were constructed above to have non-matching prefix + Prefix: alloc1.ID[:4], }, } var resp structs.AllocListResponse - err = msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp) - require.NoError(t, err) + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp)) require.Equal(t, uint64(1000), resp.Index) require.Len(t, resp.Allocations, 1) - require.Equal(t, alloc.ID, resp.Allocations[0].ID) - require.Equal(t, structs.DefaultNamespace, resp.Allocations[0].Namespace) + require.Equal(t, alloc1.ID, resp.Allocations[0].ID) + require.Equal(t, alloc1.Namespace, resp.Allocations[0].Namespace) }) t.Run("looking up allocations with mismatch prefix", func(t *testing.T) { - // ensure that prefix doesn't match the alloc - badPrefix := alloc.ID[:4] - if badPrefix[0] == '0' { - badPrefix = "1" + badPrefix[1:] - } else { - badPrefix = "0" + badPrefix[1:] - } + // allocations were constructed above to have prefix starting with "a" or "b" + badPrefix := "cc" get := &structs.AllocListRequest{ QueryOptions: structs.QueryOptions{ @@ -379,12 +385,10 @@ func TestAllocEndpoint_List_AllNamespaces_OSS(t *testing.T) { }, } var resp structs.AllocListResponse - err = msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp) - require.NoError(t, err) + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp)) require.Equal(t, uint64(1000), resp.Index) require.Empty(t, resp.Allocations) }) - } func TestAllocEndpoint_GetAlloc(t *testing.T) { @@ -844,3 +848,189 @@ func TestAllocEndpoint_Stop_ACL(t *testing.T) { require.True(*out1.DesiredTransition.Migrate) require.True(*out2.DesiredTransition.Migrate) } + +func TestAllocEndpoint_List_AllNamespaces_ACL_OSS(t *testing.T) { + t.Parallel() + + s1, root, cleanupS1 := TestACLServer(t, nil) + defer cleanupS1() + codec := rpcClient(t, s1) + testutil.WaitForLeader(t, s1.RPC) + state := s1.fsm.State() + + // two namespaces + ns1 := mock.Namespace() + ns2 := mock.Namespace() + require.NoError(t, state.UpsertNamespaces(900, []*structs.Namespace{ns1, ns2})) + + // Create the allocations + alloc1 := mock.Alloc() + alloc1.ID = "a" + alloc1.ID[1:] + alloc1.Namespace = ns1.Name + alloc2 := mock.Alloc() + alloc2.ID = "b" + alloc2.ID[1:] + alloc2.Namespace = ns2.Name + summary1 := mock.JobSummary(alloc1.JobID) + summary2 := mock.JobSummary(alloc2.JobID) + + require.NoError(t, state.UpsertJobSummary(999, summary1)) + require.NoError(t, state.UpsertJobSummary(999, summary2)) + require.NoError(t, state.UpsertAllocs(structs.MsgTypeTestSetup, 1000, []*structs.Allocation{alloc1, alloc2})) + alloc1.CreateIndex = 1000 + alloc1.ModifyIndex = 1000 + alloc2.CreateIndex = 1000 + alloc2.ModifyIndex = 1000 + + everythingButReadJob := []string{ + acl.NamespaceCapabilityDeny, + acl.NamespaceCapabilityListJobs, + // acl.NamespaceCapabilityReadJob, + acl.NamespaceCapabilitySubmitJob, + acl.NamespaceCapabilityDispatchJob, + acl.NamespaceCapabilityReadLogs, + acl.NamespaceCapabilityReadFS, + acl.NamespaceCapabilityAllocExec, + acl.NamespaceCapabilityAllocNodeExec, + acl.NamespaceCapabilityAllocLifecycle, + acl.NamespaceCapabilitySentinelOverride, + acl.NamespaceCapabilityCSIRegisterPlugin, + acl.NamespaceCapabilityCSIWriteVolume, + acl.NamespaceCapabilityCSIReadVolume, + acl.NamespaceCapabilityCSIListVolume, + acl.NamespaceCapabilityCSIMountVolume, + acl.NamespaceCapabilityListScalingPolicies, + acl.NamespaceCapabilityReadScalingPolicy, + acl.NamespaceCapabilityReadJobScaling, + acl.NamespaceCapabilityScaleJob, + acl.NamespaceCapabilitySubmitRecommendation, + } + + ns1token := mock.CreatePolicyAndToken(t, state, 1001, "ns1", + mock.NamespacePolicy(ns1.Name, "", []string{acl.NamespaceCapabilityReadJob})) + ns1tokenInsufficient := mock.CreatePolicyAndToken(t, state, 1001, "ns1-insufficient", + mock.NamespacePolicy(ns1.Name, "", everythingButReadJob)) + ns2token := mock.CreatePolicyAndToken(t, state, 1001, "ns2", + mock.NamespacePolicy(ns2.Name, "", []string{acl.NamespaceCapabilityReadJob})) + bothToken := mock.CreatePolicyAndToken(t, state, 1001, "nsBoth", + mock.NamespacePolicy(ns1.Name, "", []string{acl.NamespaceCapabilityReadJob})+ + mock.NamespacePolicy(ns2.Name, "", []string{acl.NamespaceCapabilityReadJob})) + + cases := []struct { + Label string + Namespace string + Token string + Allocs []*structs.Allocation + Error bool + Message string + Prefix string + }{ + { + Label: "all namespaces with sufficient token", + Namespace: "*", + Token: bothToken.SecretID, + Allocs: []*structs.Allocation{alloc1, alloc2}, + }, + { + Label: "all namespaces with root token", + Namespace: "*", + Token: root.SecretID, + Allocs: []*structs.Allocation{alloc1, alloc2}, + }, + { + Label: "all namespaces with ns1 token", + Namespace: "*", + Token: ns1token.SecretID, + Allocs: []*structs.Allocation{alloc1}, + }, + { + Label: "all namespaces with ns2 token", + Namespace: "*", + Token: ns2token.SecretID, + Allocs: []*structs.Allocation{alloc2}, + }, + { + Label: "all namespaces with bad token", + Namespace: "*", + Token: uuid.Generate(), + Error: true, + Message: structs.ErrTokenNotFound.Error(), + }, + { + Label: "all namespaces with insufficient token", + Namespace: "*", + Allocs: []*structs.Allocation{}, + Token: ns1tokenInsufficient.SecretID, + }, + { + Label: "ns1 with ns1 token", + Namespace: ns1.Name, + Token: ns1token.SecretID, + Allocs: []*structs.Allocation{alloc1}, + }, + { + Label: "ns1 with root token", + Namespace: ns1.Name, + Token: root.SecretID, + Allocs: []*structs.Allocation{alloc1}, + }, + { + Label: "ns1 with ns2 token", + Namespace: ns1.Name, + Token: ns2token.SecretID, + Error: true, + }, + { + Label: "ns1 with invalid token", + Namespace: ns1.Name, + Token: uuid.Generate(), + Error: true, + Message: structs.ErrTokenNotFound.Error(), + }, + { + Label: "bad namespace with root token", + Namespace: uuid.Generate(), + Token: root.SecretID, + Allocs: []*structs.Allocation{}, + }, + { + Label: "all namespaces with prefix", + Namespace: "*", + Prefix: alloc1.ID[:2], + Token: root.SecretID, + Allocs: []*structs.Allocation{alloc1}, + }, + } + + for _, tc := range cases { + t.Run(tc.Label, func(t *testing.T) { + + get := &structs.AllocListRequest{ + QueryOptions: structs.QueryOptions{ + Region: "global", + Namespace: tc.Namespace, + Prefix: tc.Prefix, + AuthToken: tc.Token, + }, + } + var resp structs.AllocListResponse + err := msgpackrpc.CallWithCodec(codec, "Alloc.List", get, &resp) + if tc.Error { + require.Error(t, err) + if tc.Message != "" { + require.Equal(t, err.Error(), tc.Message) + } else { + require.Equal(t, err.Error(), structs.ErrPermissionDenied.Error()) + } + } else { + require.NoError(t, err) + require.Equal(t, uint64(1000), resp.Index) + exp := make([]*structs.AllocListStub, len(tc.Allocs)) + for i, a := range tc.Allocs { + exp[i] = a.Stub(nil) + } + require.ElementsMatch(t, exp, resp.Allocations) + } + }) + } + +} diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go index d5e5f12f9b0..e159a0db448 100644 --- a/nomad/state/state_store.go +++ b/nomad/state/state_store.go @@ -3329,35 +3329,17 @@ func allocNamespaceFilter(namespace string) func(interface{}) bool { } // AllocsByIDPrefix is used to lookup allocs by prefix -func (s *StateStore) AllocsByIDPrefixInNSes(ws memdb.WatchSet, namespaces map[string]bool, prefix string) (memdb.ResultIterator, error) { +func (s *StateStore) AllocsByIDPrefixAllNSs(ws memdb.WatchSet, prefix string) (memdb.ResultIterator, error) { txn := s.db.ReadTxn() - var iter memdb.ResultIterator - var err error - if prefix != "" { - iter, err = txn.Get("allocs", "id_prefix", prefix) - } else { - iter, err = txn.Get("allocs", "id") - - } + iter, err := txn.Get("allocs", "id_prefix", prefix) if err != nil { return nil, fmt.Errorf("alloc lookup failed: %v", err) } ws.Add(iter.WatchCh()) - // Wrap the iterator in a filter - nsesFilter := func(raw interface{}) bool { - alloc, ok := raw.(*structs.Allocation) - if !ok { - return true - } - - return namespaces[alloc.Namespace] - } - - wrap := memdb.NewFilterIterator(iter, nsesFilter) - return wrap, nil + return iter, nil } // AllocsByNode returns all the allocations by node