resource: Make resource watchlist tenancy aware (#18539)

agent/grpc-external/services/resource/list_test.go

@@ -119,83 +119,7 @@ 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 {
+	for desc, tc := range wildcardTenancyCases() {
-		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)

agent/grpc-external/services/resource/server_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/hashicorp/consul/agent/grpc-external/testutils"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/internal/resource"
+	"github.com/hashicorp/consul/internal/resource/demo"
 	"github.com/hashicorp/consul/internal/storage/inmem"
 	"github.com/hashicorp/consul/proto-public/pbresource"
 	pbdemov2 "github.com/hashicorp/consul/proto/private/pbdemo/v2"
@@ -131,6 +132,90 @@ func modifyArtist(t *testing.T, res *pbresource.Resource) *pbresource.Resource {
 	return res
 }
 
+// wildcardTenancyCases returns permutations of tenancy and type scope used as input
+// to endpoints that accept wildcards for tenancy.
+func wildcardTenancyCases() map[string]struct {
+	typ     *pbresource.Type
+	tenancy *pbresource.Tenancy
+} {
+	return 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",
+			},
+		},
+	}
+}
+
 // tenancyCases returns permutations of valid tenancy structs in a resource id to use as inputs.
 // - the id is for a recordLabel when the resource is partition scoped
 // - the id is for an artist when the resource is namespace scoped

agent/grpc-external/services/resource/watch.go

@@ -10,28 +10,26 @@ 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) WatchList(req *pbresource.WatchListRequest, stream pbresource.ResourceService_WatchListServer) error {
-	if err := validateWatchListRequest(req); err != nil {
+	reg, err := s.validateWatchListRequest(req)
-		return err
-	}
-
-	// check type exists
-	reg, err := s.resolveType(req.Type)
 	if err != nil {
 		return err
 	}
 
-	// TODO(spatel): Refactor _ and entMeta as part of NET-4914
+	// v1 ACL subsystem is "wildcard" aware so just pass on through.
-	authz, authzContext, err := s.getAuthorizer(tokenFromContext(stream.Context()), acl.DefaultEnterpriseMeta())
+	entMeta := v2TenancyToV1EntMeta(req.Tenancy)
+	token := tokenFromContext(stream.Context())
+	authz, authzContext, err := s.getAuthorizer(token, entMeta)
 	if err != nil {
 		return err
 	}
 
-	// check acls
+	// Check list ACL.
 	err = reg.ACLs.List(authz, authzContext)
 	switch {
 	case acl.IsErrPermissionDenied(err):
@@ -40,6 +38,9 @@ func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.R
 		return 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)
+
 	unversionedType := storage.UnversionedTypeFrom(req.Type)
 	watch, err := s.Backend.WatchList(
 		stream.Context(),
@@ -66,6 +67,15 @@ func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.R
 			continue
 		}
 
+		// 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(event.Resource.Id.Tenancy)
+		authz, authzContext, err = s.getAuthorizer(token, entMeta)
+		if err != nil {
+			return err
+		}
+
 		// filter out items that don't pass read ACLs
 		err = reg.ACLs.Read(authz, authzContext, event.Resource.Id)
 		switch {
@@ -81,15 +91,37 @@ func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.R
 	}
 }
 
-func validateWatchListRequest(req *pbresource.WatchListRequest) error {
+func (s *Server) validateWatchListRequest(req *pbresource.WatchListRequest) (*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
 }

agent/grpc-external/services/resource/watch_test.go

@@ -12,6 +12,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/proto-public/pbresource"
 	"github.com/hashicorp/consul/proto/private/prototest"
@@ -20,6 +21,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/proto"
 )
 
 func TestWatchList_InputValidation(t *testing.T) {
@@ -31,12 +33,16 @@ func TestWatchList_InputValidation(t *testing.T) {
 	testCases := map[string]func(*pbresource.WatchListRequest){
 		"no type":    func(req *pbresource.WatchListRequest) { req.Type = nil },
 		"no tenancy": func(req *pbresource.WatchListRequest) { req.Tenancy = nil },
+		"partitioned type provides non-empty namespace": func(req *pbresource.WatchListRequest) {
+			req.Type = demo.TypeV1RecordLabel
+			req.Tenancy.Namespace = "bad"
+		},
 	}
 	for desc, modFn := range testCases {
 		t.Run(desc, func(t *testing.T) {
 			req := &pbresource.WatchListRequest{
 				Type:    demo.TypeV2Album,
-				Tenancy: demo.TenancyDefault,
+				Tenancy: resource.DefaultNamespacedTenancy(),
 			}
 			modFn(req)
 
@@ -58,7 +64,7 @@ func TestWatchList_TypeNotFound(t *testing.T) {
 
 	stream, err := client.WatchList(context.Background(), &pbresource.WatchListRequest{
 		Type:       demo.TypeV2Artist,
-		Tenancy:    demo.TenancyDefault,
+		Tenancy:    resource.DefaultNamespacedTenancy(),
 		NamePrefix: "",
 	})
 	require.NoError(t, err)
@@ -80,7 +86,7 @@ func TestWatchList_GroupVersionMatches(t *testing.T) {
 	// create a watch
 	stream, err := client.WatchList(ctx, &pbresource.WatchListRequest{
 		Type:       demo.TypeV2Artist,
-		Tenancy:    demo.TenancyDefault,
+		Tenancy:    resource.DefaultNamespacedTenancy(),
 		NamePrefix: "",
 	})
 	require.NoError(t, err)
@@ -111,6 +117,54 @@ func TestWatchList_GroupVersionMatches(t *testing.T) {
 	require.Equal(t, pbresource.WatchEvent_OPERATION_DELETE, rsp.Operation)
 }
 
+func TestWatchList_Tenancy_Defaults_And_Normalization(t *testing.T) {
+	// Test units of tenancy get lowercased and defaulted correctly when empty.
+	for desc, tc := range wildcardTenancyCases() {
+		t.Run(desc, func(t *testing.T) {
+			ctx := context.Background()
+			server := testServer(t)
+			client := testClient(t, server)
+			demo.RegisterTypes(server.Registry)
+
+			// Create a watch.
+			stream, err := client.WatchList(ctx, &pbresource.WatchListRequest{
+				Type:       tc.typ,
+				Tenancy:    tc.tenancy,
+				NamePrefix: "",
+			})
+			require.NoError(t, err)
+			rspCh := handleResourceStream(t, stream)
+
+			// Testcase will pick one of recordLabel or artist based on scope of type.
+			recordLabel, err := demo.GenerateV1RecordLabel("LooneyTunes")
+			require.NoError(t, err)
+			artist, err := demo.GenerateV2Artist()
+			require.NoError(t, err)
+
+			// Create and verify upsert event received.
+			recordLabel, err = server.Backend.WriteCAS(ctx, recordLabel)
+			require.NoError(t, err)
+			artist, err = server.Backend.WriteCAS(ctx, artist)
+			require.NoError(t, err)
+
+			var expected *pbresource.Resource
+			switch {
+			case proto.Equal(tc.typ, demo.TypeV1RecordLabel):
+				expected = recordLabel
+			case proto.Equal(tc.typ, demo.TypeV2Artist):
+				expected = artist
+			default:
+				require.Fail(t, "unsupported type", tc.typ)
+			}
+
+			rsp := mustGetResource(t, rspCh)
+			require.Equal(t, pbresource.WatchEvent_OPERATION_UPSERT, rsp.Operation)
+			prototest.AssertDeepEqual(t, expected, rsp.Resource)
+		})
+
+	}
+}
+
 func TestWatchList_GroupVersionMismatch(t *testing.T) {
 	// Given a watch on TypeArtistV1 that only differs from TypeArtistV2 by GroupVersion
 	// When a resource of TypeArtistV2 is created/updated/deleted
@@ -125,7 +179,7 @@ func TestWatchList_GroupVersionMismatch(t *testing.T) {
 	// create a watch for TypeArtistV1
 	stream, err := client.WatchList(ctx, &pbresource.WatchListRequest{
 		Type:       demo.TypeV1Artist,
-		Tenancy:    demo.TenancyDefault,
+		Tenancy:    resource.DefaultNamespacedTenancy(),
 		NamePrefix: "",
 	})
 	require.NoError(t, err)