status-im
/
consul
mirror of https://github.com/status-im/consul.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

resource: enforce lowercase v2 resource names (#19218)

This commit is contained in:
Semir Patel 2023-10-16 12:55:30 -05:00 committed by GitHub
parent 6c7d0759e4
commit ad177698f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 822 additions and 325 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/19218.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
resource: lowercase names enforced for v2 resources only.
```

3
agent/grpc-external/services/resource/delete.go
View File

@ -7,6 +7,7 @@ import (
"context"
"errors"
"fmt"
"strings"
"time"
"github.com/oklog/ulid/v2"
@ -175,5 +176,5 @@ func (s *Server) validateDeleteRequest(req *pbresource.DeleteRequest) (*resource
// name by embedding the resources's Uid in the name.
func tombstoneName(deleteId *pbresource.ID) string {
// deleteId.Name is just included for easier identification
return fmt.Sprintf("tombstone-%v-%v", deleteId.Name, deleteId.Uid)
return fmt.Sprintf("tombstone-%v-%v", deleteId.Name, strings.ToLower(deleteId.Uid))
}

94
agent/grpc-external/services/resource/delete_test.go
View File

@ -5,6 +5,7 @@ package resource
import (
"context"
"strings"
"testing"
"github.com/stretchr/testify/mock"
@ -22,39 +23,98 @@ import (
func TestDelete_InputValidation(t *testing.T) {
server := testServer(t)
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
testCases := map[string]func(artistId, recordLabelId *pbresource.ID) *pbresource.ID{
"no id": func(artistId, recordLabelId *pbresource.ID) *pbresource.ID {
return nil
type testCase struct {
modFn func(artistId, recordLabelId *pbresource.ID) *pbresource.ID
errContains string
}
testCases := map[string]testCase{
"no id": {
modFn: func(_, _ *pbresource.ID) *pbresource.ID {
return nil
},
errContains: "id is required",
},
"no type": func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Type = nil
return artistId
"no type": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Type = nil
return artistId
},
errContains: "id.type is required",
},
"no name": func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = ""
return artistId
"no name": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = ""
return artistId
},
errContains: "id.name invalid",
},
"partition scoped resource with namespace": func(_, recordLabelId *pbresource.ID) *pbresource.ID {
recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace"
return recordLabelId
"mixed case name": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = "DepecheMode"
return artistId
},
errContains: "id.name invalid",
},
"name too long": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = strings.Repeat("n", resource.MaxNameLength+1)
return artistId
},
errContains: "id.name invalid",
},
"partition mixed case": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Partition = "Default"
return artistId
},
errContains: "id.tenancy.partition invalid",
},
"partition name too long": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
return artistId
},
errContains: "id.tenancy.partition invalid",
},
"namespace mixed case": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Namespace = "Default"
return artistId
},
errContains: "id.tenancy.namespace invalid",
},
"namespace name too long": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
return artistId
},
errContains: "id.tenancy.namespace invalid",
},
"partition scoped resource with namespace": {
modFn: func(_, recordLabelId *pbresource.ID) *pbresource.ID {
recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace"
return recordLabelId
},
errContains: "cannot have a namespace",
},
}
for desc, modFn := range testCases {
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
artist, err := demo.GenerateV2Artist()
require.NoError(t, err)
req := &pbresource.DeleteRequest{Id: modFn(artist.Id, recordLabel.Id), Version: ""}
req := &pbresource.DeleteRequest{Id: tc.modFn(artist.Id, recordLabel.Id), Version: ""}
_, err = client.Delete(testContext(t), req)
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -129,7 +189,7 @@ func TestDelete_Success(t *testing.T) {
server, client, ctx := testDeps(t)
demo.RegisterTypes(server.Registry)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
writeRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel})
require.NoError(t, err)

5
agent/grpc-external/services/resource/list.go
View File

@ -100,8 +100,9 @@ func (s *Server) validateListRequest(req *pbresource.ListRequest) (*resource.Reg
return nil, err
}
// Lowercase
resource.Normalize(req.Tenancy)
if err := validateWildcardTenancy(req.Tenancy, req.NamePrefix); err != nil {
return nil, err
}
// Error when partition scoped and namespace not empty.
if reg.Scope == resource.ScopePartition && req.Tenancy.Namespace != "" {

3
agent/grpc-external/services/resource/list_by_owner.go
View File

@ -105,9 +105,6 @@ func (s *Server) validateListByOwnerRequest(req *pbresource.ListByOwnerRequest)
return nil, err
}
// Lowercase
resource.Normalize(req.Owner.Tenancy)
// Error when partition scoped and namespace not empty.
if reg.Scope == resource.ScopePartition && req.Owner.Tenancy.Namespace != "" {
return nil, status.Errorf(

158
agent/grpc-external/services/resource/list_by_owner_test.go
View File

@ -6,6 +6,7 @@ package resource
import (
"context"
"fmt"
"strings"
"testing"
"github.com/hashicorp/consul/acl"
@ -13,6 +14,7 @@ import (
"github.com/hashicorp/consul/internal/resource/demo"
"github.com/hashicorp/consul/proto-public/pbresource"
"github.com/hashicorp/consul/proto/private/prototest"
"github.com/oklog/ulid/v2"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@ -26,41 +28,104 @@ func TestListByOwner_InputValidation(t *testing.T) {
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
testCases := map[string]func(artistId, recordlabelId *pbresource.ID) *pbresource.ID{
"no owner": func(artistId, recordLabelId *pbresource.ID) *pbresource.ID {
return nil
type testCase struct {
modFn func(artistId, recordlabelId *pbresource.ID) *pbresource.ID
errContains string
}
testCases := map[string]testCase{
"no owner": {
modFn: func(artistId, recordLabelId *pbresource.ID) *pbresource.ID {
return nil
},
errContains: "owner is required",
},
"no type": func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Type = nil
return artistId
"no type": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Type = nil
return artistId
},
errContains: "owner.type is required",
},
"no name": func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = ""
return artistId
"no name": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = ""
return artistId
},
errContains: "owner.name invalid",
},
"no uid": func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Uid = ""
return artistId
"name mixed case": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = "U2"
return artistId
},
errContains: "owner.name invalid",
},
"partition scope with non-empty namespace": func(_, recordLabelId *pbresource.ID) *pbresource.ID {
recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace"
return recordLabelId
"name too long": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = strings.Repeat("n", resource.MaxNameLength+1)
return artistId
},
errContains: "owner.name invalid",
},
"partition mixed case": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Partition = "Default"
return artistId
},
errContains: "owner.tenancy.partition invalid",
},
"partition too long": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
return artistId
},
errContains: "owner.tenancy.partition invalid",
},
"namespace mixed case": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Namespace = "Default"
return artistId
},
errContains: "owner.tenancy.namespace invalid",
},
"namespace too long": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
return artistId
},
errContains: "owner.tenancy.namespace invalid",
},
"no uid": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Uid = ""
return artistId
},
errContains: "owner uid is required",
},
"partition scope with non-empty namespace": {
modFn: func(_, recordLabelId *pbresource.ID) *pbresource.ID {
recordLabelId.Uid = ulid.Make().String()
recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace"
return recordLabelId
},
errContains: "cannot have a namespace",
},
}
for desc, modFn := range testCases {
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
artist, err := demo.GenerateV2Artist()
require.NoError(t, err)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
// Each test case picks which resource to use based on the resource type's scope.
req := &pbresource.ListByOwnerRequest{Owner: modFn(artist.Id, recordLabel.Id)}
req := &pbresource.ListByOwnerRequest{Owner: tc.modFn(artist.Id, recordLabel.Id)}
_, err = client.ListByOwner(testContext(t), req)
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -131,33 +196,46 @@ func TestListByOwner_Many(t *testing.T) {
}
func TestListByOwner_OwnerTenancyDoesNotExist(t *testing.T) {
tenancyCases := map[string]func(artistId, recordlabelId *pbresource.ID) *pbresource.ID{
"partition not found when namespace scoped": func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Uid = "doesnotmatter"
id.Tenancy.Partition = "boguspartition"
return id
type testCase struct {
modFn func(artistId, recordlabelId *pbresource.ID) *pbresource.ID
errContains string
}
tenancyCases := map[string]testCase{
"partition not found when namespace scoped": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Uid = "doesnotmatter"
id.Tenancy.Partition = "boguspartition"
return id
},
errContains: "partition not found",
},
"namespace not found when namespace scoped": func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Uid = "doesnotmatter"
id.Tenancy.Namespace = "bogusnamespace"
return id
"namespace not found when namespace scoped": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Uid = "doesnotmatter"
id.Tenancy.Namespace = "bogusnamespace"
return id
},
errContains: "namespace not found",
},
"partition not found when partition scoped": func(_, recordLabelId *pbresource.ID) *pbresource.ID {
id := clone(recordLabelId)
id.Uid = "doesnotmatter"
id.Tenancy.Partition = "boguspartition"
return id
"partition not found when partition scoped": {
modFn: func(_, recordLabelId *pbresource.ID) *pbresource.ID {
id := clone(recordLabelId)
id.Uid = "doesnotmatter"
id.Tenancy.Partition = "boguspartition"
return id
},
errContains: "partition not found",
},
}
for desc, modFn := range tenancyCases {
for desc, tc := range tenancyCases {
t.Run(desc, func(t *testing.T) {
server := testServer(t)
demo.RegisterTypes(server.Registry)
client := testClient(t, server)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
recordLabel, err = server.Backend.WriteCAS(testContext(t), recordLabel)
require.NoError(t, err)
@ -167,11 +245,11 @@ func TestListByOwner_OwnerTenancyDoesNotExist(t *testing.T) {
artist, err = server.Backend.WriteCAS(testContext(t), artist)
require.NoError(t, err)
// Verify non-existant tenancy units in owner err with not found.
_, err = client.ListByOwner(testContext(t), &pbresource.ListByOwnerRequest{Owner: modFn(artist.Id, recordLabel.Id)})
// Verify non-existant tenancy units in owner err with invalid arg.
_, err = client.ListByOwner(testContext(t), &pbresource.ListByOwnerRequest{Owner: tc.modFn(artist.Id, recordLabel.Id)})
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource not found")
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -184,7 +262,7 @@ func TestListByOwner_Tenancy_Defaults_And_Normalization(t *testing.T) {
client := testClient(t, server)
// Create partition scoped recordLabel.
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
rsp1, err := client.Write(testContext(t), &pbresource.WriteRequest{Resource: recordLabel})
require.NoError(t, err)

60
agent/grpc-external/services/resource/list_test.go
View File

@ -6,6 +6,7 @@ package resource
import (
"context"
"fmt"
"strings"
"testing"
"github.com/hashicorp/consul/acl"
@ -26,28 +27,66 @@ import (
func TestList_InputValidation(t *testing.T) {
server := testServer(t)
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
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"
type testCase struct {
modReqFn func(req *pbresource.ListRequest)
errContains string
}
testCases := map[string]testCase{
"no type": {
modReqFn: func(req *pbresource.ListRequest) { req.Type = nil },
errContains: "type is required",
},
"no tenancy": {
modReqFn: func(req *pbresource.ListRequest) { req.Tenancy = nil },
errContains: "tenancy is required",
},
"partition mixed case": {
modReqFn: func(req *pbresource.ListRequest) { req.Tenancy.Partition = "Default" },
errContains: "tenancy.partition invalid",
},
"partition too long": {
modReqFn: func(req *pbresource.ListRequest) {
req.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
},
errContains: "tenancy.partition invalid",
},
"namespace mixed case": {
modReqFn: func(req *pbresource.ListRequest) { req.Tenancy.Namespace = "Default" },
errContains: "tenancy.namespace invalid",
},
"namespace too long": {
modReqFn: func(req *pbresource.ListRequest) {
req.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
},
errContains: "tenancy.namespace invalid",
},
"name_prefix mixed case": {
modReqFn: func(req *pbresource.ListRequest) { req.NamePrefix = "Violator" },
errContains: "name_prefix invalid",
},
"partitioned resource provides non-empty namespace": {
modReqFn: func(req *pbresource.ListRequest) {
req.Type = demo.TypeV1RecordLabel
req.Tenancy.Namespace = "bad"
},
errContains: "cannot have a namespace",
},
}
for desc, modFn := range testCases {
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
req := &pbresource.ListRequest{
Type: demo.TypeV2Album,
Tenancy: resource.DefaultNamespacedTenancy(),
}
modFn(req)
tc.modReqFn(req)
_, err := client.List(testContext(t), req)
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -126,7 +165,7 @@ func TestList_Tenancy_Defaults_And_Normalization(t *testing.T) {
client := testClient(t, server)
// Write partition scoped record label
recordLabel, err := demo.GenerateV1RecordLabel("LooneyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
recordLabelRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel})
require.NoError(t, err)
@ -150,7 +189,6 @@ func TestList_Tenancy_Defaults_And_Normalization(t *testing.T) {
prototest.AssertDeepEqual(t, artistRsp.Resource, listRsp.Resources[0])
}
})
}
}

169
agent/grpc-external/services/resource/read_test.go
View File

@ -6,6 +6,7 @@ package resource
import (
"context"
"fmt"
"strings"
"sync"
"testing"
@ -34,46 +35,114 @@ func TestRead_InputValidation(t *testing.T) {
tenancy.RegisterTypes(server.Registry)
demo.RegisterTypes(server.Registry)
testCases := map[string]func(artistId, recordlabelId, executiveId *pbresource.ID) *pbresource.ID{
"no id": func(_, _, _ *pbresource.ID) *pbresource.ID { return nil },
"no type": func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Type = nil
return artistId
type testCase struct {
modFn func(artistId, recordlabelId, executiveId *pbresource.ID) *pbresource.ID
errContains string
}
testCases := map[string]testCase{
"no id": {
modFn: func(_, _, _ *pbresource.ID) *pbresource.ID {
return nil
},
errContains: "id is required",
},
"no name": func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Name = ""
return artistId
"no type": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Type = nil
return artistId
},
errContains: "id.type is required",
},
"partition scope with non-empty namespace": func(_, recordLabelId, _ *pbresource.ID) *pbresource.ID {
recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace"
return recordLabelId
"no name": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Name = ""
return artistId
},
errContains: "id.name invalid",
},
"cluster scope with non-empty partition": func(_, _, executiveId *pbresource.ID) *pbresource.ID {
executiveId.Tenancy = &pbresource.Tenancy{Partition: resource.DefaultPartitionName}
return executiveId
"name is mixed case": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Name = "MixedCaseNotAllowed"
return artistId
},
errContains: "id.name invalid",
},
"cluster scope with non-empty namespace": func(_, _, executiveId *pbresource.ID) *pbresource.ID {
executiveId.Tenancy = &pbresource.Tenancy{Namespace: resource.DefaultNamespaceName}
return executiveId
"name too long": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Name = strings.Repeat("a", resource.MaxNameLength+1)
return artistId
},
errContains: "id.name invalid",
},
"partition is mixed case": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Partition = "Default"
return artistId
},
errContains: "id.tenancy.partition invalid",
},
"partition too long": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
return artistId
},
errContains: "id.tenancy.partition invalid",
},
"namespace is mixed case": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Namespace = "Default"
return artistId
},
errContains: "id.tenancy.namespace invalid",
},
"namespace too long": {
modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID {
artistId.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
return artistId
},
errContains: "id.tenancy.namespace invalid",
},
"partition scope with non-empty namespace": {
modFn: func(_, recordLabelId, _ *pbresource.ID) *pbresource.ID {
recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace"
return recordLabelId
},
errContains: "cannot have a namespace",
},
"cluster scope with non-empty partition": {
modFn: func(_, _, executiveId *pbresource.ID) *pbresource.ID {
executiveId.Tenancy = &pbresource.Tenancy{Partition: resource.DefaultPartitionName}
return executiveId
},
errContains: "cannot have a partition",
},
"cluster scope with non-empty namespace": {
modFn: func(_, _, executiveId *pbresource.ID) *pbresource.ID {
executiveId.Tenancy = &pbresource.Tenancy{Namespace: resource.DefaultNamespaceName}
return executiveId
},
errContains: "cannot have a namespace",
},
}
for desc, modFn := range testCases {
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
artist, err := demo.GenerateV2Artist()
require.NoError(t, err)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
executive, err := demo.GenerateV1Executive("MusicMan", "CEO")
executive, err := demo.GenerateV1Executive("music-man", "CEO")
require.NoError(t, err)
// Each test case picks which resource to use based on the resource type's scope.
req := &pbresource.ReadRequest{Id: modFn(artist.Id, recordLabel.Id, executive.Id)}
req := &pbresource.ReadRequest{Id: tc.modFn(artist.Id, recordLabel.Id, executive.Id)}
_, err = client.Read(testContext(t), req)
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -94,34 +163,50 @@ func TestRead_TypeNotFound(t *testing.T) {
func TestRead_ResourceNotFound(t *testing.T) {
for desc, tc := range readTestCases() {
t.Run(desc, func(t *testing.T) {
tenancyCases := map[string]func(artistId, recordlabelId *pbresource.ID) *pbresource.ID{
"resource not found by name": func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = "bogusname"
return artistId
type tenancyCase struct {
modFn func(artistId, recordlabelId *pbresource.ID) *pbresource.ID
errContains string
}
tenancyCases := map[string]tenancyCase{
"resource not found by name": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
artistId.Name = "bogusname"
return artistId
},
errContains: "resource not found",
},
"partition not found when namespace scoped": func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Tenancy.Partition = "boguspartition"
return id
"partition not found when namespace scoped": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Tenancy.Partition = "boguspartition"
return id
},
errContains: "partition not found",
},
"namespace not found when namespace scoped": func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Tenancy.Namespace = "bogusnamespace"
return id
"namespace not found when namespace scoped": {
modFn: func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Tenancy.Namespace = "bogusnamespace"
return id
},
errContains: "namespace not found",
},
"partition not found when partition scoped": func(_, recordLabelId *pbresource.ID) *pbresource.ID {
id := clone(recordLabelId)
id.Tenancy.Partition = "boguspartition"
return id
"partition not found when partition scoped": {
modFn: func(_, recordLabelId *pbresource.ID) *pbresource.ID {
id := clone(recordLabelId)
id.Tenancy.Partition = "boguspartition"
return id
},
errContains: "partition not found",
},
}
for tenancyDesc, modFn := range tenancyCases {
for tenancyDesc, tenancyCase := range tenancyCases {
t.Run(tenancyDesc, func(t *testing.T) {
server := testServer(t)
demo.RegisterTypes(server.Registry)
client := testClient(t, server)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
recordLabel, err = server.Backend.WriteCAS(tc.ctx, recordLabel)
require.NoError(t, err)
@ -132,10 +217,10 @@ func TestRead_ResourceNotFound(t *testing.T) {
require.NoError(t, err)
// Each tenancy test case picks which resource to use based on the resource type's scope.
_, err = client.Read(tc.ctx, &pbresource.ReadRequest{Id: modFn(artist.Id, recordLabel.Id)})
_, err = client.Read(tc.ctx, &pbresource.ReadRequest{Id: tenancyCase.modFn(artist.Id, recordLabel.Id)})
require.Error(t, err)
require.Equal(t, codes.NotFound.String(), status.Code(err).String())
require.Contains(t, err.Error(), "resource not found")
require.ErrorContains(t, err, tenancyCase.errContains)
})
}
})
@ -176,7 +261,7 @@ func TestRead_Success(t *testing.T) {
demo.RegisterTypes(server.Registry)
client := testClient(t, server)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
recordLabel, err = server.Backend.WriteCAS(tc.ctx, recordLabel)
require.NoError(t, err)

73
agent/grpc-external/services/resource/server.go
View File

@ -5,6 +5,7 @@ package resource
import (
"context"
"strings"
"github.com/hashicorp/go-hclog"
"google.golang.org/grpc"
@ -129,16 +130,12 @@ func isGRPCStatusError(err error) bool {
}
func validateId(id *pbresource.ID, errorPrefix string) error {
var field string
switch {
case id.Type == nil:
field = "type"
case id.Name == "":
field = "name"
if id.Type == nil {
return status.Errorf(codes.InvalidArgument, "%s.type is required", errorPrefix)
}
if field != "" {
return status.Errorf(codes.InvalidArgument, "%s.%s is required", errorPrefix, field)
if err := resource.ValidateName(id.Name); err != nil {
return status.Errorf(codes.InvalidArgument, "%s.name invalid: %v", errorPrefix, err)
}
// Better UX: Allow callers to pass in nil tenancy. Defaulting and inheritance of tenancy
@ -152,7 +149,61 @@ func validateId(id *pbresource.ID, errorPrefix string) error {
}
}
resource.Normalize(id.Tenancy)
if id.Tenancy.Partition != "" {
if err := resource.ValidateName(id.Tenancy.Partition); err != nil {
return status.Errorf(codes.InvalidArgument, "%s.tenancy.partition invalid: %v", errorPrefix, err)
}
}
if id.Tenancy.Namespace != "" {
if err := resource.ValidateName(id.Tenancy.Namespace); err != nil {
return status.Errorf(codes.InvalidArgument, "%s.tenancy.namespace invalid: %v", errorPrefix, err)
}
}
// TODO(spatel): NET-5475 - Remove as part of peer_name moving to PeerTenancy
if id.Tenancy.PeerName == "" {
id.Tenancy.PeerName = resource.DefaultPeerName
}
return nil
}
func validateRef(ref *pbresource.Reference, errorPrefix string) error {
if ref.Type == nil {
return status.Errorf(codes.InvalidArgument, "%s.type is required", errorPrefix)
}
if err := resource.ValidateName(ref.Name); err != nil {
return status.Errorf(codes.InvalidArgument, "%s.name invalid: %v", errorPrefix, err)
}
if err := resource.ValidateName(ref.Tenancy.Partition); err != nil {
return status.Errorf(codes.InvalidArgument, "%s.tenancy.partition invalid: %v", errorPrefix, err)
}
if err := resource.ValidateName(ref.Tenancy.Namespace); err != nil {
return status.Errorf(codes.InvalidArgument, "%s.tenancy.namespace invalid: %v", errorPrefix, err)
}
return nil
}
func validateWildcardTenancy(tenancy *pbresource.Tenancy, namePrefix string) error {
// Partition has to be a valid name if not wildcard or empty
if tenancy.Partition != "" && tenancy.Partition != "*" {
if err := resource.ValidateName(tenancy.Partition); err != nil {
return status.Errorf(codes.InvalidArgument, "tenancy.partition invalid: %v", err)
}
}
// Namespace has to be a valid name if not wildcard or empty
if tenancy.Namespace != "" && tenancy.Namespace != "*" {
if err := resource.ValidateName(tenancy.Namespace); err != nil {
return status.Errorf(codes.InvalidArgument, "tenancy.namespace invalid: %v", err)
}
}
// Not doing a strict resource name validation here because the prefix can be
// something like "foo-" which is a valid prefix but not valid resource name.
// relax validation to just check for lowercasing
if namePrefix != strings.ToLower(namePrefix) {
return status.Errorf(codes.InvalidArgument, "name_prefix invalid: must be lowercase alphanumeric, got: %v", namePrefix)
}
return nil
}
@ -165,7 +216,7 @@ func v1TenancyExists(reg *resource.Registration, v1Bridge TenancyBridge, tenancy
case err != nil:
return err
case !exists:
return status.Errorf(errCode, "partition resource not found: %v", tenancy.Partition)
return status.Errorf(errCode, "partition not found: %v", tenancy.Partition)
}
}
@ -175,7 +226,7 @@ func v1TenancyExists(reg *resource.Registration, v1Bridge TenancyBridge, tenancy
case err != nil:
return err
case !exists:
return status.Errorf(errCode, "namespace resource not found: %v", tenancy.Namespace)
return status.Errorf(errCode, "namespace not found: %v", tenancy.Namespace)
}
}
return nil

28
agent/grpc-external/services/resource/server_test.go
View File

@ -6,7 +6,6 @@ package resource
import (
"context"
"fmt"
"strings"
"testing"
"github.com/stretchr/testify/mock"
@ -166,14 +165,6 @@ func wildcardTenancyCases() map[string]struct {
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{
@ -198,14 +189,6 @@ func wildcardTenancyCases() map[string]struct {
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{
@ -224,12 +207,6 @@ func tenancyCases() map[string]func(artistId, recordlabelId *pbresource.ID) *pbr
"namespaced resource provides nonempty partition and namespace": func(artistId, recordLabelId *pbresource.ID) *pbresource.ID {
return artistId
},
"namespaced resource provides uppercase partition and namespace": func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Tenancy.Partition = strings.ToUpper(artistId.Tenancy.Partition)
id.Tenancy.Namespace = strings.ToUpper(artistId.Tenancy.Namespace)
return id
},
"namespaced resource inherits tokens partition when empty": func(artistId, _ *pbresource.ID) *pbresource.ID {
id := clone(artistId)
id.Tenancy.Partition = ""
@ -254,11 +231,6 @@ func tenancyCases() map[string]func(artistId, recordlabelId *pbresource.ID) *pbr
"partitioned resource provides nonempty partition": func(_, recordLabelId *pbresource.ID) *pbresource.ID {
return recordLabelId
},
"partitioned resource provides uppercase partition": func(_, recordLabelId *pbresource.ID) *pbresource.ID {
id := clone(recordLabelId)
id.Tenancy.Partition = strings.ToUpper(recordLabelId.Tenancy.Partition)
return id
},
"partitioned resource inherits tokens partition when empty": func(_, recordLabelId *pbresource.ID) *pbresource.ID {
id := clone(recordLabelId)
id.Tenancy.Partition = ""

5
agent/grpc-external/services/resource/watch.go
View File

@ -110,8 +110,9 @@ func (s *Server) validateWatchListRequest(req *pbresource.WatchListRequest) (*re
return nil, err
}
// Lowercase
resource.Normalize(req.Tenancy)
if err := validateWildcardTenancy(req.Tenancy, req.NamePrefix); err != nil {
return nil, err
}
// Error when partition scoped and namespace not empty.
if reg.Scope == resource.ScopePartition && req.Tenancy.Namespace != "" {

59
agent/grpc-external/services/resource/watch_test.go
View File

@ -7,6 +7,7 @@ import (
"context"
"errors"
"io"
"strings"
"testing"
"time"
@ -27,24 +28,61 @@ import (
func TestWatchList_InputValidation(t *testing.T) {
server := testServer(t)
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
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"
type testCase struct {
modFn func(*pbresource.WatchListRequest)
errContains string
}
testCases := map[string]testCase{
"no type": {
modFn: func(req *pbresource.WatchListRequest) { req.Type = nil },
errContains: "type is required",
},
"no tenancy": {
modFn: func(req *pbresource.WatchListRequest) { req.Tenancy = nil },
errContains: "tenancy is required",
},
"partition mixed case": {
modFn: func(req *pbresource.WatchListRequest) { req.Tenancy.Partition = "Default" },
errContains: "tenancy.partition invalid",
},
"partition too long": {
modFn: func(req *pbresource.WatchListRequest) {
req.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
},
errContains: "tenancy.partition invalid",
},
"namespace mixed case": {
modFn: func(req *pbresource.WatchListRequest) { req.Tenancy.Namespace = "Default" },
errContains: "tenancy.namespace invalid",
},
"namespace too long": {
modFn: func(req *pbresource.WatchListRequest) {
req.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
},
errContains: "tenancy.namespace invalid",
},
"name_prefix mixed case": {
modFn: func(req *pbresource.WatchListRequest) { req.NamePrefix = "Smashing" },
errContains: "name_prefix invalid",
},
"partitioned type provides non-empty namespace": {
modFn: func(req *pbresource.WatchListRequest) {
req.Type = demo.TypeV1RecordLabel
req.Tenancy.Namespace = "bad"
},
errContains: "cannot have a namespace",
},
}
for desc, modFn := range testCases {
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
req := &pbresource.WatchListRequest{
Type: demo.TypeV2Album,
Tenancy: resource.DefaultNamespacedTenancy(),
}
modFn(req)
tc.modFn(req)
stream, err := client.WatchList(testContext(t), req)
require.NoError(t, err)
@ -52,6 +90,7 @@ func TestWatchList_InputValidation(t *testing.T) {
_, err = stream.Recv()
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -136,7 +175,7 @@ func TestWatchList_Tenancy_Defaults_And_Normalization(t *testing.T) {
rspCh := handleResourceStream(t, stream)
// Testcase will pick one of recordLabel or artist based on scope of type.
recordLabel, err := demo.GenerateV1RecordLabel("LooneyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
artist, err := demo.GenerateV2Artist()
require.NoError(t, err)

13
agent/grpc-external/services/resource/write_status.go
View File

@ -178,8 +178,17 @@ func (s *Server) validateWriteStatusRequest(req *pbresource.WriteStatusRequest)
}
}
// Lowercase
resource.Normalize(req.Id.Tenancy)
if err := validateId(req.Id, "id"); err != nil {
return nil, err
}
for i, condition := range req.Status.Conditions {
if condition.Resource != nil {
if err := validateRef(condition.Resource, fmt.Sprintf("status.conditions[%d].resource", i)); err != nil {
return nil, err
}
}
}
// Check type exists.
reg, err := s.resolveType(req.Id.Type)

173
agent/grpc-external/services/resource/write_status_test.go
View File

@ -74,64 +74,155 @@ func TestWriteStatus_InputValidation(t *testing.T) {
demo.RegisterTypes(server.Registry)
testCases := map[string]struct {
typ *pbresource.Type
modFn func(req *pbresource.WriteStatusRequest)
typ *pbresource.Type
modFn func(req *pbresource.WriteStatusRequest)
errContains string
}{
"no id": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id = nil },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id = nil },
errContains: "id is required",
},
"no type": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Type = nil },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Type = nil },
errContains: "id.type is required",
},
"no name": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Name = "" },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Name = "" },
errContains: "id.name is required",
},
"no uid": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Uid = "" },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Uid = "" },
errContains: "id.uid is required",
},
"name mixed case": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Name = "U2" },
errContains: "id.name invalid",
},
"name too long": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Id.Name = strings.Repeat("a", resource.MaxNameLength+1)
},
errContains: "id.name invalid",
},
"partition mixed case": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Tenancy.Partition = "Default" },
errContains: "id.tenancy.partition invalid",
},
"partition too long": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Id.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
},
errContains: "id.tenancy.partition invalid",
},
"namespace mixed case": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Tenancy.Namespace = "Default" },
errContains: "id.tenancy.namespace invalid",
},
"namespace too long": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Id.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
},
errContains: "id.tenancy.namespace invalid",
},
"no key": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Key = "" },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Key = "" },
errContains: "key is required",
},
"no status": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status = nil },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status = nil },
errContains: "status is required",
},
"no observed generation": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.ObservedGeneration = "" },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.ObservedGeneration = "" },
errContains: "status.observed_generation is required",
},
"bad observed generation": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.ObservedGeneration = "bogus" },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.ObservedGeneration = "bogus" },
errContains: "status.observed_generation is not valid",
},
"no condition type": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Type = "" },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Type = "" },
errContains: "status.conditions[0].type is required",
},
"no reference type": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Resource.Type = nil },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Resource.Type = nil },
errContains: "status.conditions[0].resource.type is required",
},
"no reference tenancy": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Resource.Tenancy = nil },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Resource.Tenancy = nil },
errContains: "status.conditions[0].resource.tenancy is required",
},
"no reference name": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Resource.Name = "" },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Resource.Name = "" },
errContains: "status.conditions[0].resource.name is required",
},
"reference name mixed case": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.Conditions[0].Resource.Name = "U2" },
errContains: "status.conditions[0].resource.name invalid",
},
"reference name too long": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Status.Conditions[0].Resource.Name = strings.Repeat("r", resource.MaxNameLength+1)
},
errContains: "status.conditions[0].resource.name invalid",
},
"reference partition mixed case": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Status.Conditions[0].Resource.Tenancy.Partition = "Default"
},
errContains: "status.conditions[0].resource.tenancy.partition invalid",
},
"reference partition too long": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Status.Conditions[0].Resource.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
},
errContains: "status.conditions[0].resource.tenancy.partition invalid",
},
"reference namespace mixed case": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Status.Conditions[0].Resource.Tenancy.Namespace = "Default"
},
errContains: "status.conditions[0].resource.tenancy.namespace invalid",
},
"reference namespace too long": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Status.Conditions[0].Resource.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
},
errContains: "status.conditions[0].resource.tenancy.namespace invalid",
},
"updated at provided": {
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.UpdatedAt = timestamppb.Now() },
typ: demo.TypeV2Artist,
modFn: func(req *pbresource.WriteStatusRequest) { req.Status.UpdatedAt = timestamppb.Now() },
errContains: "status.updated_at is automatically set and cannot be provided",
},
"partition scoped type provides namespace in tenancy": {
typ: demo.TypeV1RecordLabel,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Tenancy.Namespace = "bad" },
typ: demo.TypeV1RecordLabel,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Tenancy.Namespace = "bad" },
errContains: "cannot have a namespace",
},
}
for desc, tc := range testCases {
@ -142,7 +233,7 @@ func TestWriteStatus_InputValidation(t *testing.T) {
case resource.EqualType(demo.TypeV2Artist, tc.typ):
res, err = demo.GenerateV2Artist()
case resource.EqualType(demo.TypeV1RecordLabel, tc.typ):
res, err = demo.GenerateV1RecordLabel("Looney Tunes")
res, err = demo.GenerateV1RecordLabel("looney-tunes")
default:
t.Fatal("unsupported type", tc.typ)
}
@ -157,6 +248,7 @@ func TestWriteStatus_InputValidation(t *testing.T) {
_, err = client.WriteStatus(testContext(t), req)
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -210,13 +302,6 @@ func TestWriteStatus_Tenancy_Defaults(t *testing.T) {
scope: resource.ScopeNamespace,
modFn: func(req *pbresource.WriteStatusRequest) {},
},
"namespaced resource provides uppercase partition and namespace": {
scope: resource.ScopeNamespace,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Id.Tenancy.Partition = strings.ToUpper(req.Id.Tenancy.Partition)
req.Id.Tenancy.Namespace = strings.ToUpper(req.Id.Tenancy.Namespace)
},
},
"namespaced resource inherits tokens partition when empty": {
scope: resource.ScopeNamespace,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Tenancy.Partition = "" },
@ -240,12 +325,6 @@ func TestWriteStatus_Tenancy_Defaults(t *testing.T) {
scope: resource.ScopePartition,
modFn: func(req *pbresource.WriteStatusRequest) {},
},
"partitioned resource provides uppercase partition": {
scope: resource.ScopePartition,
modFn: func(req *pbresource.WriteStatusRequest) {
req.Id.Tenancy.Partition = strings.ToUpper(req.Id.Tenancy.Partition)
},
},
"partitioned resource inherits tokens partition when empty": {
scope: resource.ScopePartition,
modFn: func(req *pbresource.WriteStatusRequest) { req.Id.Tenancy.Partition = "" },
@ -263,7 +342,7 @@ func TestWriteStatus_Tenancy_Defaults(t *testing.T) {
case resource.ScopeNamespace:
res, err = demo.GenerateV2Artist()
case resource.ScopePartition:
res, err = demo.GenerateV1RecordLabel("Looney Tunes")
res, err = demo.GenerateV1RecordLabel("looney-tunes")
}
require.NoError(t, err)
@ -280,7 +359,7 @@ func TestWriteStatus_Tenancy_Defaults(t *testing.T) {
require.NoError(t, err)
res = rsp.Resource
// Re-read resoruce and verify status successfully written (not nil)
// Re-read resource and verify status successfully written (not nil)
_, err = client.Read(testContext(t), &pbresource.ReadRequest{Id: res.Id})
require.NoError(t, err)
res = rsp.Resource
@ -327,7 +406,7 @@ func TestWriteStatus_Tenancy_NotFound(t *testing.T) {
case resource.ScopeNamespace:
res, err = demo.GenerateV2Artist()
case resource.ScopePartition:
res, err = demo.GenerateV1RecordLabel("Looney Tunes")
res, err = demo.GenerateV1RecordLabel("looney-tunes")
}
require.NoError(t, err)

225
agent/grpc-external/services/resource/write_test.go
View File

@ -29,54 +29,123 @@ import (
func TestWrite_InputValidation(t *testing.T) {
server := testServer(t)
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
testCases := map[string]func(artist, recordLabel *pbresource.Resource) *pbresource.Resource{
"no resource": func(artist, recordLabel *pbresource.Resource) *pbresource.Resource { return nil },
"no id": func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id = nil
return artist
},
"no type": func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Type = nil
return artist
},
"no name": func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Name = ""
return artist
},
"wrong data type": func(artist, _ *pbresource.Resource) *pbresource.Resource {
var err error
artist.Data, err = anypb.New(&pbdemov2.Album{})
require.NoError(t, err)
return artist
},
"fail validation hook": func(artist, _ *pbresource.Resource) *pbresource.Resource {
buffer := &pbdemov2.Artist{}
require.NoError(t, artist.Data.UnmarshalTo(buffer))
buffer.Name = "" // name cannot be empty
require.NoError(t, artist.Data.MarshalFrom(buffer))
return artist
},
"partition scope with non-empty namespace": func(_, recordLabel *pbresource.Resource) *pbresource.Resource {
recordLabel.Id.Tenancy.Namespace = "bogus"
return recordLabel
},
// TODO(spatel): add cluster scope tests when we have an actual cluster scoped resource (e.g. partition)
type testCase struct {
modFn func(artist, recordLabel *pbresource.Resource) *pbresource.Resource
errContains string
}
for desc, modFn := range testCases {
testCases := map[string]testCase{
"no resource": {
modFn: func(_, _ *pbresource.Resource) *pbresource.Resource {
return nil
},
errContains: "resource is required",
},
"no id": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id = nil
return artist
},
errContains: "resource.id is required",
},
"no type": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Type = nil
return artist
},
errContains: "resource.id.type is required",
},
"no name": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Name = ""
return artist
},
errContains: "resource.id.name invalid",
},
"name is mixed case": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Name = "MixedCaseNotAllowed"
return artist
},
errContains: "resource.id.name invalid",
},
"name too long": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Name = strings.Repeat("a", resource.MaxNameLength+1)
return artist
},
errContains: "resource.id.name invalid",
},
"wrong data type": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
var err error
artist.Data, err = anypb.New(&pbdemov2.Album{})
require.NoError(t, err)
return artist
},
errContains: "resource.data is of wrong type",
},
"partition is mixed case": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Tenancy.Partition = "Default"
return artist
},
errContains: "resource.id.tenancy.partition invalid",
},
"partition too long": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
return artist
},
errContains: "resource.id.tenancy.partition invalid",
},
"namespace is mixed case": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Tenancy.Namespace = "Default"
return artist
},
errContains: "resource.id.tenancy.namespace invalid",
},
"namespace too long": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
return artist
},
errContains: "resource.id.tenancy.namespace invalid",
},
"fail validation hook": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
buffer := &pbdemov2.Artist{}
require.NoError(t, artist.Data.UnmarshalTo(buffer))
buffer.Name = "" // name cannot be empty
require.NoError(t, artist.Data.MarshalFrom(buffer))
return artist
},
errContains: "artist.name required",
},
"partition scope with non-empty namespace": {
modFn: func(_, recordLabel *pbresource.Resource) *pbresource.Resource {
recordLabel.Id.Tenancy.Namespace = "bogus"
return recordLabel
},
errContains: "cannot have a namespace",
},
}
for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
artist, err := demo.GenerateV2Artist()
require.NoError(t, err)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
req := &pbresource.WriteRequest{Resource: modFn(artist, recordLabel)}
req := &pbresource.WriteRequest{Resource: tc.modFn(artist, recordLabel)}
_, err = client.Write(testContext(t), req)
require.Error(t, err)
require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String())
require.ErrorContains(t, err, tc.errContains)
})
}
}
@ -84,7 +153,6 @@ func TestWrite_InputValidation(t *testing.T) {
func TestWrite_OwnerValidation(t *testing.T) {
server := testServer(t)
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
type testCase struct {
@ -94,15 +162,49 @@ func TestWrite_OwnerValidation(t *testing.T) {
testCases := map[string]testCase{
"no owner type": {
modReqFn: func(req *pbresource.WriteRequest) { req.Resource.Owner.Type = nil },
errorContains: "resource.owner.type",
errorContains: "resource.owner.type is required",
},
"no owner tenancy": {
modReqFn: func(req *pbresource.WriteRequest) { req.Resource.Owner.Tenancy = nil },
errorContains: "resource.owner",
errorContains: "resource.owner does not exist",
},
"no owner name": {
modReqFn: func(req *pbresource.WriteRequest) { req.Resource.Owner.Name = "" },
errorContains: "resource.owner.name",
errorContains: "resource.owner.name invalid",
},
"mixed case owner name": {
modReqFn: func(req *pbresource.WriteRequest) { req.Resource.Owner.Name = strings.ToUpper(req.Resource.Owner.Name) },
errorContains: "resource.owner.name invalid",
},
"owner name too long": {
modReqFn: func(req *pbresource.WriteRequest) {
req.Resource.Owner.Name = strings.Repeat("a", resource.MaxNameLength+1)
},
errorContains: "resource.owner.name invalid",
},
"owner partition is mixed case": {
modReqFn: func(req *pbresource.WriteRequest) {
req.Resource.Owner.Tenancy.Partition = "Default"
},
errorContains: "resource.owner.tenancy.partition invalid",
},
"owner partition too long": {
modReqFn: func(req *pbresource.WriteRequest) {
req.Resource.Owner.Tenancy.Partition = strings.Repeat("p", resource.MaxNameLength+1)
},
errorContains: "resource.owner.tenancy.partition invalid",
},
"owner namespace is mixed case": {
modReqFn: func(req *pbresource.WriteRequest) {
req.Resource.Owner.Tenancy.Namespace = "Default"
},
errorContains: "resource.owner.tenancy.namespace invalid",
},
"owner namespace too long": {
modReqFn: func(req *pbresource.WriteRequest) {
req.Resource.Owner.Tenancy.Namespace = strings.Repeat("n", resource.MaxNameLength+1)
},
errorContains: "resource.owner.tenancy.namespace invalid",
},
}
for desc, tc := range testCases {
@ -215,14 +317,6 @@ func TestWrite_Create_Success(t *testing.T) {
},
expectedTenancy: resource.DefaultNamespacedTenancy(),
},
"namespaced resource provides uppercase partition and namespace": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Tenancy.Partition = strings.ToUpper(artist.Id.Tenancy.Partition)
artist.Id.Tenancy.Namespace = strings.ToUpper(artist.Id.Tenancy.Namespace)
return artist
},
expectedTenancy: resource.DefaultNamespacedTenancy(),
},
"namespaced resource inherits tokens partition when empty": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
artist.Id.Tenancy.Partition = ""
@ -266,13 +360,6 @@ func TestWrite_Create_Success(t *testing.T) {
},
expectedTenancy: resource.DefaultPartitionedTenancy(),
},
"partitioned resource provides uppercase partition": {
modFn: func(_, recordLabel *pbresource.Resource) *pbresource.Resource {
recordLabel.Id.Tenancy.Partition = strings.ToUpper(recordLabel.Id.Tenancy.Partition)
return recordLabel
},
expectedTenancy: resource.DefaultPartitionedTenancy(),
},
"partitioned resource inherits tokens partition when empty": {
modFn: func(_, recordLabel *pbresource.Resource) *pbresource.Resource {
recordLabel.Id.Tenancy.Partition = ""
@ -303,7 +390,7 @@ func TestWrite_Create_Success(t *testing.T) {
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
artist, err := demo.GenerateV2Artist()
@ -331,7 +418,7 @@ func TestWrite_Create_Tenancy_NotFound(t *testing.T) {
return artist
},
errCode: codes.InvalidArgument,
errContains: "partition",
errContains: "partition not found",
},
"namespaced resource provides nonexistant namespace": {
modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource {
@ -339,7 +426,7 @@ func TestWrite_Create_Tenancy_NotFound(t *testing.T) {
return artist
},
errCode: codes.InvalidArgument,
errContains: "namespace",
errContains: "namespace not found",
},
"partitioned resource provides nonexistant partition": {
modFn: func(_, recordLabel *pbresource.Resource) *pbresource.Resource {
@ -347,7 +434,7 @@ func TestWrite_Create_Tenancy_NotFound(t *testing.T) {
return recordLabel
},
errCode: codes.InvalidArgument,
errContains: "partition",
errContains: "partition not found",
},
}
for desc, tc := range testCases {
@ -356,7 +443,7 @@ func TestWrite_Create_Tenancy_NotFound(t *testing.T) {
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
artist, err := demo.GenerateV2Artist()
@ -378,22 +465,22 @@ func TestWrite_Tenancy_MarkedForDeletion(t *testing.T) {
}{
"namespaced resources partition marked for deletion": {
modFn: func(artist, _ *pbresource.Resource, mockTenancyBridge *MockTenancyBridge) *pbresource.Resource {
mockTenancyBridge.On("IsPartitionMarkedForDeletion", "part1").Return(true, nil)
mockTenancyBridge.On("IsPartitionMarkedForDeletion", "ap1").Return(true, nil)
return artist
},
errContains: "partition marked for deletion",
},
"namespaced resources namespace marked for deletion": {
modFn: func(artist, _ *pbresource.Resource, mockTenancyBridge *MockTenancyBridge) *pbresource.Resource {
mockTenancyBridge.On("IsPartitionMarkedForDeletion", "part1").Return(false, nil)
mockTenancyBridge.On("IsNamespaceMarkedForDeletion", "part1", "ns1").Return(true, nil)
mockTenancyBridge.On("IsPartitionMarkedForDeletion", "ap1").Return(false, nil)
mockTenancyBridge.On("IsNamespaceMarkedForDeletion", "ap1", "ns1").Return(true, nil)
return artist
},
errContains: "namespace marked for deletion",
},
"partitioned resources partition marked for deletion": {
modFn: func(_, recordLabel *pbresource.Resource, mockTenancyBridge *MockTenancyBridge) *pbresource.Resource {
mockTenancyBridge.On("IsPartitionMarkedForDeletion", "part1").Return(true, nil)
mockTenancyBridge.On("IsPartitionMarkedForDeletion", "ap1").Return(true, nil)
return recordLabel
},
errContains: "partition marked for deletion",
@ -404,18 +491,18 @@ func TestWrite_Tenancy_MarkedForDeletion(t *testing.T) {
server := testServer(t)
client := testClient(t, server)
demo.RegisterTypes(server.Registry)
recordLabel, err := demo.GenerateV1RecordLabel("LoonyTunes")
recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes")
require.NoError(t, err)
recordLabel.Id.Tenancy.Partition = "part1"
recordLabel.Id.Tenancy.Partition = "ap1"
artist, err := demo.GenerateV2Artist()
require.NoError(t, err)
artist.Id.Tenancy.Partition = "part1"
artist.Id.Tenancy.Partition = "ap1"
artist.Id.Tenancy.Namespace = "ns1"
mockTenancyBridge := &MockTenancyBridge{}
mockTenancyBridge.On("PartitionExists", "part1").Return(true, nil)
mockTenancyBridge.On("NamespaceExists", "part1", "ns1").Return(true, nil)
mockTenancyBridge.On("PartitionExists", "ap1").Return(true, nil)
mockTenancyBridge.On("NamespaceExists", "ap1", "ns1").Return(true, nil)
server.TenancyBridge = mockTenancyBridge
_, err = client.Write(testContext(t), &pbresource.WriteRequest{Resource: tc.modFn(artist, recordLabel, mockTenancyBridge)})

5
internal/mesh/internal/controllers/xds/controller_test.go
View File

@ -237,7 +237,10 @@ func (suite *xdsControllerTestSuite) TestReconcile_ReadEndpointError() {
require.Error(suite.T(), err)
// Assert on the status reflecting endpoint couldn't be read.
suite.client.RequireStatusCondition(suite.T(), fooProxyStateTemplate.Id, ControllerName, status.ConditionRejectedErrorReadingEndpoints(status.KeyFromID(badID), "rpc error: code = InvalidArgument desc = id.name is required"))
suite.client.RequireStatusCondition(suite.T(), fooProxyStateTemplate.Id, ControllerName, status.ConditionRejectedErrorReadingEndpoints(
status.KeyFromID(badID),
"rpc error: code = InvalidArgument desc = id.name invalid: a resource name must consist of lower case alphanumeric characters or '-', must start and end with an alphanumeric character and be less than 64 characters, got: \"\"",
))
}
// This test is a happy path creation test to make sure pbproxystate.Endpoints are created in the computed

2
internal/resource/demo/controller.go
View File

@ -71,7 +71,7 @@ func (r *artistReconciler) Reconcile(ctx context.Context, rt controller.Runtime,
actualAlbums, err := rt.Client.List(ctx, &pbresource.ListRequest{
Type: TypeV2Album,
Tenancy: res.Id.Tenancy,
NamePrefix: fmt.Sprintf("%s/", res.Id.Name),
NamePrefix: fmt.Sprintf("%s-", res.Id.Name),
})
if err != nil {
return err

2
internal/resource/demo/demo.go
View File

@ -354,7 +354,7 @@ func generateV2Album(artistID *pbresource.ID, rand *rand.Rand) (*pbresource.Reso
Id: &pbresource.ID{
Type: TypeV2Album,
Tenancy: clone(artistID.Tenancy),
Name: fmt.Sprintf("%s/%s-%s", artistID.Name, strings.ToLower(adjective), strings.ToLower(noun)),
Name: fmt.Sprintf("%s-%s-%s", artistID.Name, strings.ToLower(adjective), strings.ToLower(noun)),
},
Owner: artistID,
Data: data,

12
internal/resource/http/http_test.go
View File

@ -42,7 +42,7 @@ func TestResourceHandler_InputValidation(t *testing.T) {
request *http.Request
response *httptest.ResponseRecorder
expectedResponseCode int
expectedErrorMessage string
responseBodyContains string
}
client := svctest.RunResourceService(t, demo.RegisterTypes)
resourceHandler := resourceHandler{
@ -72,7 +72,7 @@ func TestResourceHandler_InputValidation(t *testing.T) {
`)),
response: httptest.NewRecorder(),
expectedResponseCode: http.StatusBadRequest,
expectedErrorMessage: "rpc error: code = InvalidArgument desc = resource.id.name is required",
responseBodyContains: "resource.id.name invalid",
},
{
description: "wrong schema",
@ -89,21 +89,21 @@ func TestResourceHandler_InputValidation(t *testing.T) {
`)),
response: httptest.NewRecorder(),
expectedResponseCode: http.StatusBadRequest,
expectedErrorMessage: "Request body didn't follow the resource schema",
responseBodyContains: "Request body didn't follow the resource schema",
},
{
description: "invalid request body",
request: httptest.NewRequest("PUT", "/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader("bad-input")),
response: httptest.NewRecorder(),
expectedResponseCode: http.StatusBadRequest,
expectedErrorMessage: "Request body format is invalid",
responseBodyContains: "Request body format is invalid",
},
{
description: "no id",
request: httptest.NewRequest("DELETE", "/?partition=default&peer_name=local&namespace=default", strings.NewReader("")),
response: httptest.NewRecorder(),
expectedResponseCode: http.StatusBadRequest,
expectedErrorMessage: "rpc error: code = InvalidArgument desc = id.name is required",
responseBodyContains: "id.name invalid",
},
}
@ -119,7 +119,7 @@ func TestResourceHandler_InputValidation(t *testing.T) {
require.NoError(t, err)
require.Equal(t, tc.expectedResponseCode, tc.response.Result().StatusCode)
require.Equal(t, tc.expectedErrorMessage, string(b))
require.Contains(t, string(b), tc.responseBodyContains)
})
}
}

22
internal/resource/resource.go Normal file
View File

@ -0,0 +1,22 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package resource
import (
"fmt"
"strings"
"github.com/hashicorp/consul/agent/dns"
)
const MaxNameLength = 63
// ValidateName returns an error a name is not a valid resource name.
// The error will contain reference to what constitutes a valid resource name.
func ValidateName(name string) error {
if !dns.IsValidLabel(name) || strings.ToLower(name) != name || len(name) > MaxNameLength {
return fmt.Errorf("a resource name must consist of lower case alphanumeric characters or '-', must start and end with an alphanumeric character and be less than %d characters, got: %q", MaxNameLength+1, name)
}
return nil
}

17
internal/resource/tenancy.go
View File

@ -5,7 +5,6 @@ package resource
import (
"fmt"
"strings"
"google.golang.org/protobuf/proto"
@ -62,20 +61,6 @@ func (s Scope) String() string {
panic(fmt.Sprintf("string mapping missing for scope %v", int(s)))
}
// Normalize lowercases the partition and namespace.
func Normalize(tenancy *pbresource.Tenancy) {
if tenancy == nil {
return
}
tenancy.Partition = strings.ToLower(tenancy.Partition)
tenancy.Namespace = strings.ToLower(tenancy.Namespace)
// TODO(spatel): NET-5475 - Remove as part of peer_name moving to PeerTenancy
if tenancy.PeerName == "" {
tenancy.PeerName = DefaultPeerName
}
}
// DefaultClusteredTenancy returns the default tenancy for a cluster scoped resource.
func DefaultClusteredTenancy() *pbresource.Tenancy {
return &pbresource.Tenancy{
@ -156,7 +141,6 @@ func defaultTenancy(itemTenancy, parentTenancy, scopeTenancy *pbresource.Tenancy
if itemTenancy.PeerName == "" {
itemTenancy.PeerName = DefaultPeerName
}
Normalize(itemTenancy)
if parentTenancy != nil {
// Recursively normalize this tenancy as well.
@ -167,7 +151,6 @@ func defaultTenancy(itemTenancy, parentTenancy, scopeTenancy *pbresource.Tenancy
if parentTenancy == nil {
parentTenancy = scopeTenancy
}
Normalize(parentTenancy)
if !equalOrEmpty(itemTenancy.PeerName, DefaultPeerName) {
panic("peering is not supported yet for resource tenancies")

16
internal/tenancy/internal/types/namespace_test.go
View File

@ -6,13 +6,14 @@ package types
import (
"context"
"errors"
"testing"
svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing"
rtest "github.com/hashicorp/consul/internal/resource/resourcetest"
pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1"
"github.com/hashicorp/consul/proto/private/prototest"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"testing"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/reflect/protoreflect"
@ -188,19 +189,6 @@ func TestDelete_Success(t *testing.T) {
}
func TestRead_MixedCases_Success(t *testing.T) {
client := svctest.RunResourceService(t, Register)
client = rtest.NewClient(client)
res := rtest.Resource(NamespaceType, "nS1").
WithData(t, validNamespace()).Write(t, client)
readRsp, err := client.Read(context.Background(), &pbresource.ReadRequest{Id: res.Id})
require.NoError(t, err)
prototest.AssertDeepEqual(t, res.Id, readRsp.Resource.Id)
}
func validNamespace() *pbtenancy.Namespace {
return &pbtenancy.Namespace{
Description: "ns namespace",