Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource: Make resource list tenancy aware #18475

Merged
merged 1 commit into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 48 additions & 16 deletions agent/grpc-external/services/resource/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,37 @@ import (
"google.golang.org/grpc/status"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/storage"
"github.com/hashicorp/consul/proto-public/pbresource"
)

func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbresource.ListResponse, error) {
if err := validateListRequest(req); err != nil {
return nil, err
}

// check type
reg, err := s.resolveType(req.Type)
reg, err := s.validateListRequest(req)
if err != nil {
return nil, err
}

// TODO(spatel): Refactor _ and entMeta in NET-4915
authz, authzContext, err := s.getAuthorizer(tokenFromContext(ctx), acl.DefaultEnterpriseMeta())
// v1 ACL subsystem is "wildcard" aware so just pass on through.
entMeta := v2TenancyToV1EntMeta(req.Tenancy)
token := tokenFromContext(ctx)
authz, authzContext, err := s.getAuthorizer(token, entMeta)
if err != nil {
return nil, err
}

// check acls
err = reg.ACLs.List(authz, req.Tenancy)
// Check ACLs.
err = reg.ACLs.List(authz, authzContext)
switch {
case acl.IsErrPermissionDenied(err):
return nil, status.Error(codes.PermissionDenied, err.Error())
case err != nil:
return nil, status.Errorf(codes.Internal, "failed list acl: %v", err)
}

// Ensure we're defaulting correctly when request tenancy units are empty.
v1EntMetaToV2Tenancy(reg, entMeta, req.Tenancy)

resources, err := s.Backend.List(
ctx,
readConsistencyFrom(ctx),
Expand All @@ -53,12 +54,21 @@ func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbreso

result := make([]*pbresource.Resource, 0)
for _, resource := range resources {
// filter out non-matching GroupVersion
// Filter out non-matching GroupVersion.
if resource.Id.Type.GroupVersion != req.Type.GroupVersion {
continue
}

// filter out items that don't pass read ACLs
// Need to rebuild authorizer per resource since wildcard inputs may
// result in different tenancies. Consider caching per tenancy if this
// is deemed expensive.
entMeta = v2TenancyToV1EntMeta(resource.Id.Tenancy)
authz, authzContext, err = s.getAuthorizer(token, entMeta)
if err != nil {
return nil, err
}

// Filter out items that don't pass read ACLs.
err = reg.ACLs.Read(authz, authzContext, resource.Id)
switch {
case acl.IsErrPermissionDenied(err):
Expand All @@ -71,15 +81,37 @@ func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbreso
return &pbresource.ListResponse{Resources: result}, nil
}

func validateListRequest(req *pbresource.ListRequest) error {
func (s *Server) validateListRequest(req *pbresource.ListRequest) (*resource.Registration, error) {
var field string
switch {
case req.Type == nil:
field = "type"
case req.Tenancy == nil:
field = "tenancy"
default:
return nil
}
return status.Errorf(codes.InvalidArgument, "%s is required", field)

if field != "" {
return nil, status.Errorf(codes.InvalidArgument, "%s is required", field)
}

// Check type exists.
reg, err := s.resolveType(req.Type)
if err != nil {
return nil, err
}

// Lowercase
resource.Normalize(req.Tenancy)

// Error when partition scoped and namespace not empty.
if reg.Scope == resource.ScopePartition && req.Tenancy.Namespace != "" {
return nil, status.Errorf(
codes.InvalidArgument,
"partition scoped type %s cannot have a namespace. got: %s",
resource.ToGVK(req.Type),
req.Tenancy.Namespace,
)
}

return reg, nil
}
127 changes: 123 additions & 4 deletions agent/grpc-external/services/resource/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/grpc-external/testutils"
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/resource/demo"
"github.com/hashicorp/consul/internal/storage"
"github.com/hashicorp/consul/proto-public/pbresource"
Expand All @@ -31,12 +32,16 @@ func TestList_InputValidation(t *testing.T) {
testCases := map[string]func(*pbresource.ListRequest){
"no type": func(req *pbresource.ListRequest) { req.Type = nil },
"no tenancy": func(req *pbresource.ListRequest) { req.Tenancy = nil },
"partitioned resource provides non-empty namespace": func(req *pbresource.ListRequest) {
req.Type = demo.TypeV1RecordLabel
req.Tenancy.Namespace = "bad"
},
}
for desc, modFn := range testCases {
t.Run(desc, func(t *testing.T) {
req := &pbresource.ListRequest{
Type: demo.TypeV2Album,
Tenancy: demo.TenancyDefault,
Tenancy: resource.DefaultNamespacedTenancy(),
}
modFn(req)

Expand All @@ -53,7 +58,7 @@ func TestList_TypeNotFound(t *testing.T) {

_, err := client.List(context.Background(), &pbresource.ListRequest{
Type: demo.TypeV2Artist,
Tenancy: demo.TenancyDefault,
Tenancy: resource.DefaultNamespacedTenancy(),
NamePrefix: "",
})
require.Error(t, err)
Expand All @@ -70,7 +75,7 @@ func TestList_Empty(t *testing.T) {

rsp, err := client.List(tc.ctx, &pbresource.ListRequest{
Type: demo.TypeV1Artist,
Tenancy: demo.TenancyDefault,
Tenancy: resource.DefaultNamespacedTenancy(),
NamePrefix: "",
})
require.NoError(t, err)
Expand Down Expand Up @@ -102,7 +107,7 @@ func TestList_Many(t *testing.T) {

rsp, err := client.List(tc.ctx, &pbresource.ListRequest{
Type: demo.TypeV2Artist,
Tenancy: demo.TenancyDefault,
Tenancy: resource.DefaultNamespacedTenancy(),
NamePrefix: "",
})
require.NoError(t, err)
Expand All @@ -111,6 +116,120 @@ func TestList_Many(t *testing.T) {
}
}

func TestList_Tenancy_Defaults_And_Normalization(t *testing.T) {
// Test units of tenancy get defaulted correctly when empty.
ctx := context.Background()
testCases := map[string]struct {
typ *pbresource.Type
tenancy *pbresource.Tenancy
}{
"namespaced type with empty partition": {
typ: demo.TypeV2Artist,
tenancy: &pbresource.Tenancy{
Partition: "",
Namespace: resource.DefaultNamespaceName,
PeerName: "local",
},
},
"namespaced type with empty namespace": {
typ: demo.TypeV2Artist,
tenancy: &pbresource.Tenancy{
Partition: resource.DefaultPartitionName,
Namespace: "",
PeerName: "local",
},
},
"namespaced type with empty partition and namespace": {
typ: demo.TypeV2Artist,
tenancy: &pbresource.Tenancy{
Partition: "",
Namespace: "",
PeerName: "local",
},
},
"namespaced type with uppercase partition and namespace": {
typ: demo.TypeV2Artist,
tenancy: &pbresource.Tenancy{
Partition: "DEFAULT",
Namespace: "DEFAULT",
PeerName: "local",
},
},
"namespaced type with wildcard partition and empty namespace": {
typ: demo.TypeV2Artist,
tenancy: &pbresource.Tenancy{
Partition: "*",
Namespace: "",
PeerName: "local",
},
},
"namespaced type with empty partition and wildcard namespace": {
typ: demo.TypeV2Artist,
tenancy: &pbresource.Tenancy{
Partition: "",
Namespace: "*",
PeerName: "local",
},
},
"partitioned type with empty partition": {
typ: demo.TypeV1RecordLabel,
tenancy: &pbresource.Tenancy{
Partition: "",
Namespace: "",
PeerName: "local",
},
},
"partitioned type with uppercase partition": {
typ: demo.TypeV1RecordLabel,
tenancy: &pbresource.Tenancy{
Partition: "DEFAULT",
Namespace: "",
PeerName: "local",
},
},
"partitioned type with wildcard partition": {
typ: demo.TypeV1RecordLabel,
tenancy: &pbresource.Tenancy{
Partition: "*",
PeerName: "local",
},
},
}
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
server := testServer(t)
demo.RegisterTypes(server.Registry)
client := testClient(t, server)

// Write partition scoped record label
recordLabel, err := demo.GenerateV1RecordLabel("LooneyTunes")
require.NoError(t, err)
recordLabelRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel})
require.NoError(t, err)

// Write namespace scoped artist
artist, err := demo.GenerateV2Artist()
require.NoError(t, err)
artistRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist})
require.NoError(t, err)

// List and verify correct resource returned for empty tenancy units.
listRsp, err := client.List(ctx, &pbresource.ListRequest{
Type: tc.typ,
Tenancy: tc.tenancy,
})
require.NoError(t, err)
require.Len(t, listRsp.Resources, 1)
if tc.typ == demo.TypeV1RecordLabel {
prototest.AssertDeepEqual(t, recordLabelRsp.Resource, listRsp.Resources[0])
} else {
prototest.AssertDeepEqual(t, artistRsp.Resource, listRsp.Resources[0])
}
})

}
}

func TestList_GroupVersionMismatch(t *testing.T) {
for desc, tc := range listTestCases() {
t.Run(desc, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion agent/grpc-external/services/resource/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.R
}

// check acls
err = reg.ACLs.List(authz, req.Tenancy)
err = reg.ACLs.List(authz, authzContext)
switch {
case acl.IsErrPermissionDenied(err):
return status.Error(codes.PermissionDenied, err.Error())
Expand Down
2 changes: 1 addition & 1 deletion internal/mesh/internal/types/proxy_state_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func RegisterProxyStateTemplate(r resource.Registry) {
// managed by a controller.
return authorizer.ToAllowAuthorizer().OperatorWriteAllowed(authzContext)
},
List: func(authorizer acl.Authorizer, tenancy *pbresource.Tenancy) error {
List: func(authorizer acl.Authorizer, authzContext *acl.AuthorizerContext) error {
// No-op List permission as we want to default to filtering resources
// from the list using the Read enforcement.
return nil
Expand Down
8 changes: 4 additions & 4 deletions internal/resource/demo/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ func RegisterTypes(r resource.Registry) {

writeACL := func(authz acl.Authorizer, authzContext *acl.AuthorizerContext, res *pbresource.Resource) error {
key := fmt.Sprintf("resource/%s/%s", resource.ToGVK(res.Id.Type), res.Id.Name)
return authz.ToAllowAuthorizer().KeyWriteAllowed(key, &acl.AuthorizerContext{})
return authz.ToAllowAuthorizer().KeyWriteAllowed(key, authzContext)
}

makeListACL := func(typ *pbresource.Type) func(acl.Authorizer, *pbresource.Tenancy) error {
return func(authz acl.Authorizer, tenancy *pbresource.Tenancy) error {
makeListACL := func(typ *pbresource.Type) func(acl.Authorizer, *acl.AuthorizerContext) error {
return func(authz acl.Authorizer, authzContext *acl.AuthorizerContext) error {
key := fmt.Sprintf("resource/%s", resource.ToGVK(typ))
return authz.ToAllowAuthorizer().KeyListAllowed(key, &acl.AuthorizerContext{})
return authz.ToAllowAuthorizer().KeyListAllowed(key, authzContext)
}
}

Expand Down
4 changes: 2 additions & 2 deletions internal/resource/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ type ACLHooks struct {
// List is used to authorize List RPCs.
//
// If it is omitted, we only filter the results using Read.
List func(acl.Authorizer, *pbresource.Tenancy) error
List func(acl.Authorizer, *acl.AuthorizerContext) error
}

// Resource type registry
Expand Down Expand Up @@ -130,7 +130,7 @@ func (r *TypeRegistry) Register(registration Registration) {
}
}
if registration.ACLs.List == nil {
registration.ACLs.List = func(authz acl.Authorizer, tenancy *pbresource.Tenancy) error {
registration.ACLs.List = func(authz acl.Authorizer, authzContext *acl.AuthorizerContext) error {
return authz.ToAllowAuthorizer().OperatorReadAllowed(&acl.AuthorizerContext{})
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/resource/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ func TestRegister_Defaults(t *testing.T) {
require.True(t, acl.IsErrPermissionDenied(reg.ACLs.Write(testutils.ACLNoPermissions(t), nil, artist)))

// verify default list hook requires operator:read
require.NoError(t, reg.ACLs.List(testutils.ACLOperatorRead(t), artist.Id.Tenancy))
require.True(t, acl.IsErrPermissionDenied(reg.ACLs.List(testutils.ACLNoPermissions(t), artist.Id.Tenancy)))
require.NoError(t, reg.ACLs.List(testutils.ACLOperatorRead(t), nil))
require.True(t, acl.IsErrPermissionDenied(reg.ACLs.List(testutils.ACLNoPermissions(t), nil)))

// verify default validate is a no-op
require.NoError(t, reg.Validate(nil))
Expand Down