From 46816071df2edb0e05cf527c8cedf3bdd61bda6b Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Mon, 24 Apr 2023 08:14:51 -0500 Subject: [PATCH] De-scope tenenacy requirements to OSS only for now. (#17087) Partition and namespace must be "default" Peername must be "local" --- .../services/resource/delete_test.go | 12 ++--- .../services/resource/read_test.go | 12 ++--- .../grpc-external/services/resource/server.go | 16 +++--- .../grpc-external/services/resource/write.go | 5 +- .../services/resource/write_test.go | 51 +++++-------------- 5 files changed, 34 insertions(+), 62 deletions(-) diff --git a/agent/grpc-external/services/resource/delete_test.go b/agent/grpc-external/services/resource/delete_test.go index ddbefbd1166d..3147fb5b3000 100644 --- a/agent/grpc-external/services/resource/delete_test.go +++ b/agent/grpc-external/services/resource/delete_test.go @@ -30,17 +30,17 @@ func TestDelete_InputValidation(t *testing.T) { "no tenancy": func(req *pbresource.DeleteRequest) { req.Id.Tenancy = nil }, "no name": func(req *pbresource.DeleteRequest) { req.Id.Name = "" }, // clone necessary to not pollute DefaultTenancy - "tenancy partition wildcard": func(req *pbresource.DeleteRequest) { + "tenancy partition not default": func(req *pbresource.DeleteRequest) { req.Id.Tenancy = clone(req.Id.Tenancy) - req.Id.Tenancy.Partition = storage.Wildcard + req.Id.Tenancy.Partition = "" }, - "tenancy namespace wildcard": func(req *pbresource.DeleteRequest) { + "tenancy namespace not default": func(req *pbresource.DeleteRequest) { req.Id.Tenancy = clone(req.Id.Tenancy) - req.Id.Tenancy.Namespace = storage.Wildcard + req.Id.Tenancy.Namespace = "" }, - "tenancy peername wildcard": func(req *pbresource.DeleteRequest) { + "tenancy peername not local": func(req *pbresource.DeleteRequest) { req.Id.Tenancy = clone(req.Id.Tenancy) - req.Id.Tenancy.PeerName = storage.Wildcard + req.Id.Tenancy.PeerName = "" }, } for desc, modFn := range testCases { diff --git a/agent/grpc-external/services/resource/read_test.go b/agent/grpc-external/services/resource/read_test.go index 6f7f80a090ab..e7265043f3ce 100644 --- a/agent/grpc-external/services/resource/read_test.go +++ b/agent/grpc-external/services/resource/read_test.go @@ -33,17 +33,17 @@ func TestRead_InputValidation(t *testing.T) { "no tenancy": func(req *pbresource.ReadRequest) { req.Id.Tenancy = nil }, "no name": func(req *pbresource.ReadRequest) { req.Id.Name = "" }, // clone necessary to not pollute DefaultTenancy - "tenancy partition wildcard": func(req *pbresource.ReadRequest) { + "tenancy partition not default": func(req *pbresource.ReadRequest) { req.Id.Tenancy = clone(req.Id.Tenancy) - req.Id.Tenancy.Partition = storage.Wildcard + req.Id.Tenancy.Partition = "" }, - "tenancy namespace wildcard": func(req *pbresource.ReadRequest) { + "tenancy namespace not default": func(req *pbresource.ReadRequest) { req.Id.Tenancy = clone(req.Id.Tenancy) - req.Id.Tenancy.Namespace = storage.Wildcard + req.Id.Tenancy.Namespace = "" }, - "tenancy peername wildcard": func(req *pbresource.ReadRequest) { + "tenancy peername not local": func(req *pbresource.ReadRequest) { req.Id.Tenancy = clone(req.Id.Tenancy) - req.Id.Tenancy.PeerName = storage.Wildcard + req.Id.Tenancy.PeerName = "" }, } for desc, modFn := range testCases { diff --git a/agent/grpc-external/services/resource/server.go b/agent/grpc-external/services/resource/server.go index 46bbbef17e22..51bb4610d527 100644 --- a/agent/grpc-external/services/resource/server.go +++ b/agent/grpc-external/services/resource/server.go @@ -131,17 +131,19 @@ func validateId(id *pbresource.ID, errorPrefix string) error { return status.Errorf(codes.InvalidArgument, "%s.%s is required", errorPrefix, field) } + // Revisit defaulting and non-namespaced resources post-1.16 + var expected string switch { - case id.Tenancy.Namespace == storage.Wildcard: - field = "tenancy.namespace" - case id.Tenancy.Partition == storage.Wildcard: - field = "tenancy.partition" - case id.Tenancy.PeerName == storage.Wildcard: - field = "tenancy.peername" + case id.Tenancy.Partition != "default": + field, expected = "partition", "default" + case id.Tenancy.Namespace != "default": + field, expected = "namespace", "default" + case id.Tenancy.PeerName != "local": + field, expected = "peername", "local" } if field != "" { - return status.Errorf(codes.InvalidArgument, "%s.%s cannot be a wildcard", errorPrefix, field) + return status.Errorf(codes.InvalidArgument, "%s.tenancy.%s must be %s", errorPrefix, field, expected) } return nil } diff --git a/agent/grpc-external/services/resource/write.go b/agent/grpc-external/services/resource/write.go index e5c4f3db73a3..121c7ce39ccc 100644 --- a/agent/grpc-external/services/resource/write.go +++ b/agent/grpc-external/services/resource/write.go @@ -119,10 +119,7 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre return errUseWriteStatus } - // Enforce same tenancy for owner - if input.Owner != nil && !proto.Equal(input.Id.Tenancy, input.Owner.Tenancy) { - return status.Errorf(codes.InvalidArgument, "owner and resource tenancy must be the same") - } + // TODO(spatel): Revisit owner<->resource tenancy rules post-1.16 // Update path. case err == nil: diff --git a/agent/grpc-external/services/resource/write_test.go b/agent/grpc-external/services/resource/write_test.go index 6d0645d1fe33..666937c12ee5 100644 --- a/agent/grpc-external/services/resource/write_test.go +++ b/agent/grpc-external/services/resource/write_test.go @@ -36,17 +36,17 @@ func TestWrite_InputValidation(t *testing.T) { "no name": func(req *pbresource.WriteRequest) { req.Resource.Id.Name = "" }, "no data": func(req *pbresource.WriteRequest) { req.Resource.Data = nil }, // clone necessary to not pollute DefaultTenancy - "tenancy partition wildcard": func(req *pbresource.WriteRequest) { + "tenancy partition not default": func(req *pbresource.WriteRequest) { req.Resource.Id.Tenancy = clone(req.Resource.Id.Tenancy) - req.Resource.Id.Tenancy.Partition = storage.Wildcard + req.Resource.Id.Tenancy.Partition = "" }, - "tenancy namespace wildcard": func(req *pbresource.WriteRequest) { + "tenancy namespace not default": func(req *pbresource.WriteRequest) { req.Resource.Id.Tenancy = clone(req.Resource.Id.Tenancy) - req.Resource.Id.Tenancy.Namespace = storage.Wildcard + req.Resource.Id.Tenancy.Namespace = "" }, - "tenancy peername wildcard": func(req *pbresource.WriteRequest) { + "tenancy peername not local": func(req *pbresource.WriteRequest) { req.Resource.Id.Tenancy = clone(req.Resource.Id.Tenancy) - req.Resource.Id.Tenancy.PeerName = storage.Wildcard + req.Resource.Id.Tenancy.PeerName = "" }, "wrong data type": func(req *pbresource.WriteRequest) { var err error @@ -99,24 +99,24 @@ func TestWrite_OwnerValidation(t *testing.T) { errorContains: "resource.owner.name", }, // clone necessary to not pollute DefaultTenancy - "owner tenancy partition wildcard": { + "owner tenancy partition not default": { modReqFn: func(req *pbresource.WriteRequest) { req.Resource.Owner.Tenancy = clone(req.Resource.Owner.Tenancy) - req.Resource.Owner.Tenancy.Partition = storage.Wildcard + req.Resource.Owner.Tenancy.Partition = "" }, errorContains: "resource.owner.tenancy.partition", }, - "owner tenancy namespace wildcard": { + "owner tenancy namespace not default": { modReqFn: func(req *pbresource.WriteRequest) { req.Resource.Owner.Tenancy = clone(req.Resource.Owner.Tenancy) - req.Resource.Owner.Tenancy.Namespace = storage.Wildcard + req.Resource.Owner.Tenancy.Namespace = "" }, errorContains: "resource.owner.tenancy.namespace", }, - "owner tenancy peername wildcard": { + "owner tenancy peername not local": { modReqFn: func(req *pbresource.WriteRequest) { req.Resource.Owner.Tenancy = clone(req.Resource.Owner.Tenancy) - req.Resource.Owner.Tenancy.PeerName = storage.Wildcard + req.Resource.Owner.Tenancy.PeerName = "" }, errorContains: "resource.owner.tenancy.peername", }, @@ -491,33 +491,6 @@ func TestWrite_Owner_Immutable(t *testing.T) { require.ErrorContains(t, err, "owner cannot be changed") } -func TestWrite_Owner_RequireSameTenancy(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.Register(server.Registry) - - artist, err := demo.GenerateV2Artist() - require.NoError(t, err) - rsp1, err := client.Write(testContext(t), &pbresource.WriteRequest{Resource: artist}) - require.NoError(t, err) - - // change album tenancy to be different from artist tenancy - album, err := demo.GenerateV2Album(rsp1.Resource.Id) - require.NoError(t, err) - album.Owner.Tenancy = &pbresource.Tenancy{ - Partition: "some", - Namespace: "other", - PeerName: "tenancy", - } - - // verify create fails - _, err = client.Write(testContext(t), &pbresource.WriteRequest{Resource: album}) - require.Error(t, err) - require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String()) - require.ErrorContains(t, err, "tenancy must be the same") -} - type blockOnceBackend struct { storage.Backend