Skip to content

Commit

Permalink
De-scope tenenacy requirements to OSS only for now. (#17087)
Browse files Browse the repository at this point in the history
Partition and namespace must be "default"
Peername must be "local"
  • Loading branch information
analogue authored Apr 24, 2023
1 parent a011d8c commit 4681607
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 62 deletions.
12 changes: 6 additions & 6 deletions agent/grpc-external/services/resource/delete_test.go
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
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
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
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
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

0 comments on commit 4681607

Please sign in to comment.