diff --git a/.changelog/19218.txt b/.changelog/19218.txt new file mode 100644 index 000000000000..a3dde32317b4 --- /dev/null +++ b/.changelog/19218.txt @@ -0,0 +1,3 @@ +```release-note:improvement +resource: lowercase names enforced for v2 resources only. +``` \ No newline at end of file diff --git a/agent/grpc-external/services/resource/delete.go b/agent/grpc-external/services/resource/delete.go index 2f30e27f983f..a2d3bec995d4 100644 --- a/agent/grpc-external/services/resource/delete.go +++ b/agent/grpc-external/services/resource/delete.go @@ -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)) } diff --git a/agent/grpc-external/services/resource/delete_test.go b/agent/grpc-external/services/resource/delete_test.go index 5f5d7d7e2192..3bdbb0581d10 100644 --- a/agent/grpc-external/services/resource/delete_test.go +++ b/agent/grpc-external/services/resource/delete_test.go @@ -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": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Type = nil + return artistId + }, + errContains: "id.type is required", + }, + "no name": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Name = "" + return artistId + }, + errContains: "id.name invalid", + }, + "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", }, - "no type": func(artistId, _ *pbresource.ID) *pbresource.ID { - artistId.Type = nil - return artistId + "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", }, - "no name": func(artistId, _ *pbresource.ID) *pbresource.ID { - artistId.Name = "" - return artistId + "namespace mixed case": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Tenancy.Namespace = "Default" + return artistId + }, + errContains: "id.tenancy.namespace invalid", }, - "partition scoped resource with namespace": func(_, recordLabelId *pbresource.ID) *pbresource.ID { - recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace" - return recordLabelId + "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) diff --git a/agent/grpc-external/services/resource/list.go b/agent/grpc-external/services/resource/list.go index c1ecb253448c..befb619eec53 100644 --- a/agent/grpc-external/services/resource/list.go +++ b/agent/grpc-external/services/resource/list.go @@ -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 != "" { diff --git a/agent/grpc-external/services/resource/list_by_owner.go b/agent/grpc-external/services/resource/list_by_owner.go index 2310a5b50eda..a9b1754498fe 100644 --- a/agent/grpc-external/services/resource/list_by_owner.go +++ b/agent/grpc-external/services/resource/list_by_owner.go @@ -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( diff --git a/agent/grpc-external/services/resource/list_by_owner_test.go b/agent/grpc-external/services/resource/list_by_owner_test.go index 11c6027c0b64..78024e68d0fb 100644 --- a/agent/grpc-external/services/resource/list_by_owner_test.go +++ b/agent/grpc-external/services/resource/list_by_owner_test.go @@ -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": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Type = nil + return artistId + }, + errContains: "owner.type is required", + }, + "no name": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Name = "" + return artistId + }, + errContains: "owner.name invalid", + }, + "name mixed case": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Name = "U2" + return artistId + }, + errContains: "owner.name invalid", }, - "no type": func(artistId, _ *pbresource.ID) *pbresource.ID { - artistId.Type = nil - return artistId + "name too long": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Name = strings.Repeat("n", resource.MaxNameLength+1) + return artistId + }, + errContains: "owner.name invalid", }, - "no name": func(artistId, _ *pbresource.ID) *pbresource.ID { - artistId.Name = "" - return artistId + "partition mixed case": { + modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + artistId.Tenancy.Partition = "Default" + return artistId + }, + errContains: "owner.tenancy.partition invalid", }, - "no uid": func(artistId, _ *pbresource.ID) *pbresource.ID { - artistId.Uid = "" - return artistId + "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", }, - "partition scope with non-empty namespace": func(_, recordLabelId *pbresource.ID) *pbresource.ID { - recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace" - return recordLabelId + "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) diff --git a/agent/grpc-external/services/resource/list_test.go b/agent/grpc-external/services/resource/list_test.go index 64026b7d34e5..a80685b0834e 100644 --- a/agent/grpc-external/services/resource/list_test.go +++ b/agent/grpc-external/services/resource/list_test.go @@ -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]) } }) - } } diff --git a/agent/grpc-external/services/resource/read_test.go b/agent/grpc-external/services/resource/read_test.go index 2601689bc6c4..2afdfeab0e1e 100644 --- a/agent/grpc-external/services/resource/read_test.go +++ b/agent/grpc-external/services/resource/read_test.go @@ -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 type": { + modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID { + artistId.Type = nil + return artistId + }, + errContains: "id.type is required", + }, + "no name": { + modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID { + artistId.Name = "" + return artistId + }, + errContains: "id.name invalid", + }, + "name is mixed case": { + modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID { + artistId.Name = "MixedCaseNotAllowed" + return artistId + }, + errContains: "id.name invalid", }, - "no name": func(artistId, _, _ *pbresource.ID) *pbresource.ID { - artistId.Name = "" - return artistId + "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 scope with non-empty namespace": func(_, recordLabelId, _ *pbresource.ID) *pbresource.ID { - recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace" - return recordLabelId + "partition is mixed case": { + modFn: func(artistId, _, _ *pbresource.ID) *pbresource.ID { + artistId.Tenancy.Partition = "Default" + return artistId + }, + errContains: "id.tenancy.partition invalid", }, - "cluster scope with non-empty partition": func(_, _, executiveId *pbresource.ID) *pbresource.ID { - executiveId.Tenancy = &pbresource.Tenancy{Partition: resource.DefaultPartitionName} - return executiveId + "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", }, - "cluster scope with non-empty namespace": func(_, _, executiveId *pbresource.ID) *pbresource.ID { - executiveId.Tenancy = &pbresource.Tenancy{Namespace: resource.DefaultNamespaceName} - return executiveId + "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) diff --git a/agent/grpc-external/services/resource/server.go b/agent/grpc-external/services/resource/server.go index 5fc5a01fafd7..1084fd3860a1 100644 --- a/agent/grpc-external/services/resource/server.go +++ b/agent/grpc-external/services/resource/server.go @@ -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 diff --git a/agent/grpc-external/services/resource/server_test.go b/agent/grpc-external/services/resource/server_test.go index 99add6497121..ffe7df52c401 100644 --- a/agent/grpc-external/services/resource/server_test.go +++ b/agent/grpc-external/services/resource/server_test.go @@ -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 = "" diff --git a/agent/grpc-external/services/resource/watch.go b/agent/grpc-external/services/resource/watch.go index f20d3f00f875..44b0a83caa94 100644 --- a/agent/grpc-external/services/resource/watch.go +++ b/agent/grpc-external/services/resource/watch.go @@ -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 != "" { diff --git a/agent/grpc-external/services/resource/watch_test.go b/agent/grpc-external/services/resource/watch_test.go index 051264441bbc..5e5590d3f9fd 100644 --- a/agent/grpc-external/services/resource/watch_test.go +++ b/agent/grpc-external/services/resource/watch_test.go @@ -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) diff --git a/agent/grpc-external/services/resource/write_status.go b/agent/grpc-external/services/resource/write_status.go index 0d3b68bb0876..993c8382e2c4 100644 --- a/agent/grpc-external/services/resource/write_status.go +++ b/agent/grpc-external/services/resource/write_status.go @@ -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) diff --git a/agent/grpc-external/services/resource/write_status_test.go b/agent/grpc-external/services/resource/write_status_test.go index 5b71983475d9..1ddf73863236 100644 --- a/agent/grpc-external/services/resource/write_status_test.go +++ b/agent/grpc-external/services/resource/write_status_test.go @@ -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) diff --git a/agent/grpc-external/services/resource/write_test.go b/agent/grpc-external/services/resource/write_test.go index 3828ff9753f2..9f7704b52b97 100644 --- a/agent/grpc-external/services/resource/write_test.go +++ b/agent/grpc-external/services/resource/write_test.go @@ -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 + type testCase struct { + modFn func(artist, recordLabel *pbresource.Resource) *pbresource.Resource + errContains string + } + + testCases := map[string]testCase{ + "no resource": { + modFn: func(_, _ *pbresource.Resource) *pbresource.Resource { + return nil + }, + errContains: "resource is required", }, - "no type": func(artist, _ *pbresource.Resource) *pbresource.Resource { - artist.Id.Type = nil - return artist + "no id": { + modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource { + artist.Id = nil + return artist + }, + errContains: "resource.id is required", }, - "no name": func(artist, _ *pbresource.Resource) *pbresource.Resource { - artist.Id.Name = "" - return artist + "no type": { + modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource { + artist.Id.Type = nil + return artist + }, + errContains: "resource.id.type is required", }, - "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 + "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", }, - "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 + "namespace is mixed case": { + modFn: func(artist, _ *pbresource.Resource) *pbresource.Resource { + artist.Id.Tenancy.Namespace = "Default" + return artist + }, + errContains: "resource.id.tenancy.namespace invalid", }, - "partition scope with non-empty namespace": func(_, recordLabel *pbresource.Resource) *pbresource.Resource { - recordLabel.Id.Tenancy.Namespace = "bogus" - return recordLabel + "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", }, - // TODO(spatel): add cluster scope tests when we have an actual cluster scoped resource (e.g. partition) } - 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) - 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)}) diff --git a/internal/mesh/internal/controllers/xds/controller_test.go b/internal/mesh/internal/controllers/xds/controller_test.go index 6bb5f85c9908..8d4799e2da28 100644 --- a/internal/mesh/internal/controllers/xds/controller_test.go +++ b/internal/mesh/internal/controllers/xds/controller_test.go @@ -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 diff --git a/internal/resource/demo/controller.go b/internal/resource/demo/controller.go index 7f1bba902ea5..a8757fcae262 100644 --- a/internal/resource/demo/controller.go +++ b/internal/resource/demo/controller.go @@ -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 diff --git a/internal/resource/demo/demo.go b/internal/resource/demo/demo.go index 8e978c9fb49a..12fced6718e6 100644 --- a/internal/resource/demo/demo.go +++ b/internal/resource/demo/demo.go @@ -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, diff --git a/internal/resource/http/http_test.go b/internal/resource/http/http_test.go index 50f50fbe3948..46d2af363590 100644 --- a/internal/resource/http/http_test.go +++ b/internal/resource/http/http_test.go @@ -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) }) } } diff --git a/internal/resource/resource.go b/internal/resource/resource.go new file mode 100644 index 000000000000..b5100f002955 --- /dev/null +++ b/internal/resource/resource.go @@ -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 +} diff --git a/internal/resource/tenancy.go b/internal/resource/tenancy.go index 126e12413f6a..99756d503ff3 100644 --- a/internal/resource/tenancy.go +++ b/internal/resource/tenancy.go @@ -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") diff --git a/internal/tenancy/internal/types/namespace_test.go b/internal/tenancy/internal/types/namespace_test.go index b64f86d5212b..10d70ba82480 100644 --- a/internal/tenancy/internal/types/namespace_test.go +++ b/internal/tenancy/internal/types/namespace_test.go @@ -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",