From 2ba2db24a9338737668aa946f6d5cc50d2c0e95a Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Fri, 15 Sep 2017 14:27:11 -0700 Subject: [PATCH] Filter Node.GetAllocs results by readable namespaces --- nomad/node_endpoint.go | 41 +++++++++++++++++++++++++------ nomad/node_endpoint_test.go | 49 +++++++++++++++++++++++++++++-------- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 051be49d0..7c7d94e50 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -559,15 +559,33 @@ func (n *Node) GetAllocs(args *structs.NodeSpecificRequest, defer metrics.MeasureSince([]string{"nomad", "client", "get_allocs"}, time.Now()) // Check node read and namespace job read permissions - if aclObj, err := n.srv.resolveToken(args.SecretID); err != nil { + aclObj, err := n.srv.resolveToken(args.SecretID) + if err != nil { return err - } else if aclObj != nil { - if !aclObj.AllowNodeRead() { - return structs.ErrPermissionDenied + } + if aclObj != nil && !aclObj.AllowNodeRead() { + return structs.ErrPermissionDenied + } + + // cache namespace perms + readableNamespaces := map[string]bool{} + + // readNS is a caching namespace read-job helper + readNS := func(ns string) bool { + if aclObj == nil { + // ACLs are disabled; everything is readable + return true } - if !aclObj.AllowNsOp(args.RequestNamespace(), acl.NamespaceCapabilityReadJob) { - return structs.ErrPermissionDenied + + if readable, ok := readableNamespaces[ns]; ok { + // cache hit + return readable } + + // cache miss + readable := aclObj.AllowNsOp(ns, acl.NamespaceCapabilityReadJob) + readableNamespaces[ns] = readable + return readable } // Verify the arguments @@ -587,9 +605,16 @@ func (n *Node) GetAllocs(args *structs.NodeSpecificRequest, } // Setup the output - if len(allocs) != 0 { - reply.Allocs = allocs + if n := len(allocs); n != 0 { + reply.Allocs = make([]*structs.Allocation, 0, n) for _, alloc := range allocs { + if readNS(alloc.Namespace) { + reply.Allocs = append(reply.Allocs, alloc) + } + + // Get the max of all allocs since + // subsequent requests need to start + // from the latest index reply.Index = maxUint64(reply.Index, alloc.ModifyIndex) } } else { diff --git a/nomad/node_endpoint_test.go b/nomad/node_endpoint_test.go index d31d5fee3..d20d8157c 100644 --- a/nomad/node_endpoint_test.go +++ b/nomad/node_endpoint_test.go @@ -1138,18 +1138,30 @@ func TestClientEndpoint_GetAllocs_ACL(t *testing.T) { assert := assert.New(t) // Create the node - alloc := mock.Alloc() + allocDefaultNS := mock.Alloc() + allocAltNS := mock.Alloc() + allocAltNS.Namespace = "altnamespace" + allocOtherNS := mock.Alloc() + allocOtherNS.Namespace = "should-only-be-displayed-for-root-ns" + node := mock.Node() - alloc.NodeID = node.ID + allocDefaultNS.NodeID = node.ID + allocAltNS.NodeID = node.ID + allocOtherNS.NodeID = node.ID state := s1.fsm.State() assert.Nil(state.UpsertNode(1, node), "UpsertNode") - assert.Nil(state.UpsertJobSummary(2, mock.JobSummary(alloc.JobID)), "UpsertJobSummary") - assert.Nil(state.UpsertAllocs(3, []*structs.Allocation{alloc}), "UpsertAllocs") + assert.Nil(state.UpsertJobSummary(2, mock.JobSummary(allocDefaultNS.JobID)), "UpsertJobSummary") + assert.Nil(state.UpsertJobSummary(3, mock.JobSummary(allocAltNS.JobID)), "UpsertJobSummary") + assert.Nil(state.UpsertJobSummary(4, mock.JobSummary(allocOtherNS.JobID)), "UpsertJobSummary") + allocs := []*structs.Allocation{allocDefaultNS, allocAltNS, allocOtherNS} + assert.Nil(state.UpsertAllocs(5, allocs), "UpsertAllocs") // Create the namespace policy and tokens - validToken := CreatePolicyAndToken(t, state, 1001, "test-valid", NodePolicy(acl.PolicyRead)+ + validDefaultToken := CreatePolicyAndToken(t, state, 1001, "test-default-valid", NodePolicy(acl.PolicyRead)+ + NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob})) + validNoNSToken := CreatePolicyAndToken(t, state, 1003, "test-alt-valid", NodePolicy(acl.PolicyRead)) + invalidToken := CreatePolicyAndToken(t, state, 1004, "test-invalid", NamespacePolicy(structs.DefaultNamespace, "", []string{acl.NamespaceCapabilityReadJob})) - invalidToken := CreatePolicyAndToken(t, state, 1003, "test-invalid", NodePolicy(acl.PolicyRead)) // Lookup the node without a token and expect failure req := &structs.NodeSpecificRequest{ @@ -1163,12 +1175,21 @@ func TestClientEndpoint_GetAllocs_ACL(t *testing.T) { assert.Equal(err.Error(), structs.ErrPermissionDenied.Error()) } - // Try with a valid token - req.SecretID = validToken.SecretID + // Try with a valid token for the default namespace + req.SecretID = validDefaultToken.SecretID { var resp structs.NodeAllocsResponse assert.Nil(msgpackrpc.CallWithCodec(codec, "Node.GetAllocs", req, &resp), "RPC") - assert.Equal(alloc.ID, resp.Allocs[0].ID) + assert.Len(resp.Allocs, 1) + assert.Equal(allocDefaultNS.ID, resp.Allocs[0].ID) + } + + // Try with a valid token for a namespace with no allocs on this node + req.SecretID = validNoNSToken.SecretID + { + var resp structs.NodeAllocsResponse + assert.Nil(msgpackrpc.CallWithCodec(codec, "Node.GetAllocs", req, &resp), "RPC") + assert.Len(resp.Allocs, 0) } // Try with a invalid token @@ -1185,7 +1206,15 @@ func TestClientEndpoint_GetAllocs_ACL(t *testing.T) { { var resp structs.NodeAllocsResponse assert.Nil(msgpackrpc.CallWithCodec(codec, "Node.GetAllocs", req, &resp), "RPC") - assert.Equal(alloc.ID, resp.Allocs[0].ID) + assert.Len(resp.Allocs, 3) + for _, alloc := range resp.Allocs { + switch alloc.ID { + case allocDefaultNS.ID, allocAltNS.ID, allocOtherNS.ID: + // expected + default: + t.Errorf("unexpected alloc %q for namespace %q", alloc.ID, alloc.Namespace) + } + } } }