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.

Sign up for GitHub

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

De-scope tenenacy requirements to OSS only for now. #17087

Merged
merged 1 commit into from
Apr 24, 2023
Merged

De-scope tenenacy requirements to OSS only for now. #17087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions agent/grpc-external/services/resource/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions agent/grpc-external/services/resource/read_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 9 additions & 7 deletions agent/grpc-external/services/resource/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 1 addition & 4 deletions agent/grpc-external/services/resource/write.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
51 changes: 12 additions & 39 deletions agent/grpc-external/services/resource/write_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
},
Expand Down Expand Up @@ -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

Expand Down