diff --git a/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params_test.go b/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params_test.go index 3ae0670e5c..2a50094029 100644 --- a/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params_test.go +++ b/agent/grpc-external/services/dataplane/get_envoy_bootstrap_params_test.go @@ -7,29 +7,29 @@ import ( "context" "testing" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" + "github.com/hashicorp/go-hclog" + + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/acl/resolver" + external "github.com/hashicorp/consul/agent/grpc-external" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" + "github.com/hashicorp/consul/agent/grpc-external/testutils" + "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/internal/catalog" "github.com/hashicorp/consul/internal/mesh" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" + "github.com/hashicorp/consul/proto-public/pbdataplane" pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/proto/private/prototest" - - "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/acl/resolver" - external "github.com/hashicorp/consul/agent/grpc-external" - "github.com/hashicorp/consul/agent/grpc-external/testutils" - "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/proto-public/pbdataplane" ) const ( @@ -262,7 +262,9 @@ func TestGetEnvoyBootstrapParams_Success_EnableV2(t *testing.T) { } run := func(t *testing.T, tc testCase) { - resourceClient := svctest.RunResourceService(t, catalog.RegisterTypes, mesh.RegisterTypes) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(catalog.RegisterTypes, mesh.RegisterTypes). + Run(t) options := structs.QueryOptions{Token: testToken} ctx, err := external.ContextWithQueryOptions(context.Background(), options) @@ -490,7 +492,9 @@ func TestGetEnvoyBootstrapParams_Error_EnableV2(t *testing.T) { } run := func(t *testing.T, tc testCase) { - resourceClient := svctest.RunResourceService(t, catalog.RegisterTypes, mesh.RegisterTypes) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(catalog.RegisterTypes, mesh.RegisterTypes). + Run(t) options := structs.QueryOptions{Token: testToken} ctx, err := external.ContextWithQueryOptions(context.Background(), options) diff --git a/agent/grpc-external/services/resource/delete.go b/agent/grpc-external/services/resource/delete.go index d9b76d8043..582cc5dbc0 100644 --- a/agent/grpc-external/services/resource/delete.go +++ b/agent/grpc-external/services/resource/delete.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/storage" "github.com/hashicorp/consul/proto-public/pbresource" + pbtenancy "github.com/hashicorp/consul/proto-public/pbtenancy/v2beta1" ) // Delete deletes a resource. @@ -188,16 +189,13 @@ func (s *Server) ensureDeleteRequestValid(req *pbresource.DeleteRequest) (*resou return nil, err } - // Check scope - if reg.Scope == resource.ScopePartition && req.Id.Tenancy.Namespace != "" { - return nil, status.Errorf( - codes.InvalidArgument, - "partition scoped resource %s cannot have a namespace. got: %s", - resource.ToGVK(req.Id.Type), - req.Id.Tenancy.Namespace, - ) + if err := validateScopedTenancy(reg.Scope, reg.Type, req.Id.Tenancy); err != nil { + return nil, err } + if err := blockBuiltinsDeletion(reg.Type, req.Id); err != nil { + return nil, err + } return reg, nil } @@ -207,3 +205,12 @@ func TombstoneNameFor(deleteId *pbresource.ID) string { // deleteId.Name is just included for easier identification return fmt.Sprintf("tombstone-%v-%v", deleteId.Name, strings.ToLower(deleteId.Uid)) } + +func blockDefaultNamespaceDeletion(rtype *pbresource.Type, id *pbresource.ID) error { + if id.Name == resource.DefaultNamespaceName && + id.Tenancy.Partition == resource.DefaultPartitionName && + resource.EqualType(rtype, pbtenancy.NamespaceType) { + return status.Errorf(codes.InvalidArgument, "cannot delete default namespace") + } + return nil +} diff --git a/agent/grpc-external/services/resource/delete_ce.go b/agent/grpc-external/services/resource/delete_ce.go new file mode 100644 index 0000000000..d2ff805a24 --- /dev/null +++ b/agent/grpc-external/services/resource/delete_ce.go @@ -0,0 +1,15 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build !consulent + +package resource + +import "github.com/hashicorp/consul/proto-public/pbresource" + +func blockBuiltinsDeletion(rtype *pbresource.Type, id *pbresource.ID) error { + if err := blockDefaultNamespaceDeletion(rtype, id); err != nil { + return err + } + return nil +} diff --git a/agent/grpc-external/services/resource/delete_test.go b/agent/grpc-external/services/resource/delete_test.go index f8afeb3a38..fd5ed321c8 100644 --- a/agent/grpc-external/services/resource/delete_test.go +++ b/agent/grpc-external/services/resource/delete_test.go @@ -1,10 +1,11 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "context" + "fmt" "strings" "testing" @@ -15,123 +16,158 @@ import ( "google.golang.org/protobuf/proto" "github.com/hashicorp/consul/acl/resolver" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" rtest "github.com/hashicorp/consul/internal/resource/resourcetest" "github.com/hashicorp/consul/proto-public/pbresource" + pbtenancy "github.com/hashicorp/consul/proto-public/pbtenancy/v2beta1" pbdemo "github.com/hashicorp/consul/proto/private/pbdemo/v1" ) func TestDelete_InputValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) - type testCase struct { - modFn func(artistId, recordLabelId *pbresource.ID) *pbresource.ID + modFn func(artistId, recordLabelId, executiveId *pbresource.ID) *pbresource.ID errContains string } + run := func(t *testing.T, client pbresource.ResourceServiceClient, tc testCase) { + executive, err := demo.GenerateV1Executive("marvin", "CEO") + require.NoError(t, err) + + recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") + require.NoError(t, err) + + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) + + req := &pbresource.DeleteRequest{Id: tc.modFn(artist.Id, recordLabel.Id, executive.Id), Version: ""} + _, err = client.Delete(context.Background(), req) + require.Error(t, err) + require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String()) + require.ErrorContains(t, err, tc.errContains) + } + testCases := map[string]testCase{ "no id": { - modFn: func(_, _ *pbresource.ID) *pbresource.ID { + modFn: func(_, _, _ *pbresource.ID) *pbresource.ID { return nil }, errContains: "id is required", }, "no type": { - modFn: func(artistId, _ *pbresource.ID) *pbresource.ID { + 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 { + 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 { + 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 { + 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 { + 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 { + 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 { + 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 { + 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 { + modFn: func(_, recordLabelId, _ *pbresource.ID) *pbresource.ID { recordLabelId.Tenancy.Namespace = "ishouldnothaveanamespace" return recordLabelId }, errContains: "cannot have a namespace", }, + "cluster scoped resource with partition": { + modFn: func(_, _, executiveId *pbresource.ID) *pbresource.ID { + executiveId.Tenancy.Partition = "ishouldnothaveapartition" + executiveId.Tenancy.Namespace = "" + return executiveId + }, + errContains: "cannot have a partition", + }, + "cluster scoped resource with namespace": { + modFn: func(_, _, executiveId *pbresource.ID) *pbresource.ID { + executiveId.Tenancy.Partition = "" + executiveId.Tenancy.Namespace = "ishouldnothaveanamespace" + return executiveId + }, + errContains: "cannot have a namespace", + }, } - for desc, tc := range testCases { - t.Run(desc, func(t *testing.T) { - recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") - require.NoError(t, err) - artist, err := demo.GenerateV2Artist() - require.NoError(t, err) + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(useV2Tenancy). + WithRegisterFns(demo.RegisterTypes). + Run(t) - 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) + for desc, tc := range testCases { + t.Run(desc, func(t *testing.T) { + run(t, client, tc) + }) + } }) } } func TestDelete_TypeNotRegistered(t *testing.T) { - t.Parallel() + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + client := svctest.NewResourceServiceBuilder().WithV2Tenancy(useV2Tenancy).Run(t) - _, client, ctx := testDeps(t) - artist, err := demo.GenerateV2Artist() - require.NoError(t, err) + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) - // delete artist with unregistered type - _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: artist.Id, Version: ""}) - require.Error(t, err) - require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String()) + // delete artist with unregistered type + _, err = client.Delete(context.Background(), &pbresource.DeleteRequest{Id: artist.Id, Version: ""}) + require.Error(t, err) + require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String()) + require.ErrorContains(t, err, "not registered") + }) + } } func TestDelete_ACLs(t *testing.T) { @@ -157,9 +193,8 @@ func TestDelete_ACLs(t *testing.T) { for desc, tc := range testcases { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + builder := svctest.NewResourceServiceBuilder().WithRegisterFns(demo.RegisterTypes) + client := builder.Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -169,10 +204,10 @@ func TestDelete_ACLs(t *testing.T) { require.NoError(t, err) // Mock is put in place after the above "write" since the "write" must also pass the ACL check. - mockACLResolver := &MockACLResolver{} + mockACLResolver := &svc.MockACLResolver{} mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). Return(tc.authz, nil) - server.ACLResolver = mockACLResolver + builder.ServiceImpl().Config.ACLResolver = mockACLResolver // Exercise ACL. _, err = client.Delete(testContext(t), &pbresource.DeleteRequest{Id: rsp.Resource.Id}) @@ -184,59 +219,70 @@ func TestDelete_ACLs(t *testing.T) { func TestDelete_Success(t *testing.T) { t.Parallel() + run := func(t *testing.T, client pbresource.ResourceServiceClient, tc deleteTestCase, modFn func(artistId, recordlabelId *pbresource.ID) *pbresource.ID) { + ctx := context.Background() + + recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") + require.NoError(t, err) + writeRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel}) + require.NoError(t, err) + recordLabel = writeRsp.Resource + originalRecordLabelId := clone(recordLabel.Id) + + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) + writeRsp, err = client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) + require.NoError(t, err) + artist = writeRsp.Resource + originalArtistId := clone(artist.Id) + + // Pick the resource to be deleted based on type's scope and mod tenancy + // based on the tenancy test case. + deleteId := modFn(artist.Id, recordLabel.Id) + deleteReq := tc.deleteReqFn(recordLabel) + if proto.Equal(deleteId.Type, demo.TypeV2Artist) { + deleteReq = tc.deleteReqFn(artist) + } + + // Delete + _, err = client.Delete(ctx, deleteReq) + require.NoError(t, err) + + // Verify deleted + _, err = client.Read(ctx, &pbresource.ReadRequest{Id: deleteId}) + require.Error(t, err) + require.Equal(t, codes.NotFound.String(), status.Code(err).String()) + + // Derive tombstone name from resource that was deleted. + tname := svc.TombstoneNameFor(originalRecordLabelId) + if proto.Equal(deleteId.Type, demo.TypeV2Artist) { + tname = svc.TombstoneNameFor(originalArtistId) + } + + // Verify tombstone created + _, err = client.Read(ctx, &pbresource.ReadRequest{ + Id: &pbresource.ID{ + Name: tname, + Type: resource.TypeV1Tombstone, + Tenancy: deleteReq.Id.Tenancy, + }, + }) + require.NoError(t, err, "expected tombstone to be found") + } + for desc, tc := range deleteTestCases() { t.Run(desc, func(t *testing.T) { for tenancyDesc, modFn := range tenancyCases() { t.Run(tenancyDesc, func(t *testing.T) { - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) - - recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") - require.NoError(t, err) - writeRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel}) - require.NoError(t, err) - recordLabel = writeRsp.Resource - originalRecordLabelId := clone(recordLabel.Id) - - artist, err := demo.GenerateV2Artist() - require.NoError(t, err) - writeRsp, err = client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) - require.NoError(t, err) - artist = writeRsp.Resource - originalArtistId := clone(artist.Id) - - // Pick the resource to be deleted based on type's scope and mod tenancy - // based on the tenancy test case. - deleteId := modFn(artist.Id, recordLabel.Id) - deleteReq := tc.deleteReqFn(recordLabel) - if proto.Equal(deleteId.Type, demo.TypeV2Artist) { - deleteReq = tc.deleteReqFn(artist) + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(useV2Tenancy). + WithRegisterFns(demo.RegisterTypes). + Run(t) + run(t, client, tc, modFn) + }) } - - // Delete - _, err = client.Delete(ctx, deleteReq) - require.NoError(t, err) - - // Verify deleted - _, err = client.Read(ctx, &pbresource.ReadRequest{Id: deleteId}) - require.Error(t, err) - require.Equal(t, codes.NotFound.String(), status.Code(err).String()) - - // Derive tombstone name from resource that was deleted. - tname := TombstoneNameFor(originalRecordLabelId) - if proto.Equal(deleteId.Type, demo.TypeV2Artist) { - tname = TombstoneNameFor(originalArtistId) - } - - // Verify tombstone created - _, err = client.Read(ctx, &pbresource.ReadRequest{ - Id: &pbresource.ID{ - Name: tname, - Type: resource.TypeV1Tombstone, - Tenancy: deleteReq.Id.Tenancy, - }, - }) - require.NoError(t, err, "expected tombstone to be found") }) } }) @@ -246,54 +292,72 @@ func TestDelete_Success(t *testing.T) { func TestDelete_TombstoneDeletionDoesNotCreateNewTombstone(t *testing.T) { t.Parallel() - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + ctx := context.Background() + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(useV2Tenancy). + WithRegisterFns(demo.RegisterTypes). + Run(t) - artist, err := demo.GenerateV2Artist() - require.NoError(t, err) + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) - rsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) - require.NoError(t, err) - artist = rsp.Resource + rsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) + require.NoError(t, err) + artist = rsp.Resource - // delete artist - _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: artist.Id, Version: ""}) - require.NoError(t, err) + // delete artist + _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: artist.Id, Version: ""}) + require.NoError(t, err) - // verify artist's tombstone created - rsp2, err := client.Read(ctx, &pbresource.ReadRequest{ - Id: &pbresource.ID{ - Name: TombstoneNameFor(artist.Id), - Type: resource.TypeV1Tombstone, - Tenancy: artist.Id.Tenancy, - }, - }) - require.NoError(t, err) - tombstone := rsp2.Resource + // verify artist's tombstone created + rsp2, err := client.Read(ctx, &pbresource.ReadRequest{ + Id: &pbresource.ID{ + Name: svc.TombstoneNameFor(artist.Id), + Type: resource.TypeV1Tombstone, + Tenancy: artist.Id.Tenancy, + }, + }) + require.NoError(t, err) + tombstone := rsp2.Resource - // delete artist's tombstone - _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: tombstone.Id, Version: tombstone.Version}) - require.NoError(t, err) + // delete artist's tombstone + _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: tombstone.Id, Version: tombstone.Version}) + require.NoError(t, err) - // verify no new tombstones created and artist's existing tombstone deleted - rsp3, err := client.List(ctx, &pbresource.ListRequest{Type: resource.TypeV1Tombstone, Tenancy: artist.Id.Tenancy}) - require.NoError(t, err) - require.Empty(t, rsp3.Resources) + // verify no new tombstones created and artist's existing tombstone deleted + rsp3, err := client.List(ctx, &pbresource.ListRequest{Type: resource.TypeV1Tombstone, Tenancy: artist.Id.Tenancy}) + require.NoError(t, err) + require.Empty(t, rsp3.Resources) + }) + } } func TestDelete_NotFound(t *testing.T) { t.Parallel() - for desc, tc := range deleteTestCases() { - t.Run(desc, func(t *testing.T) { - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) - artist, err := demo.GenerateV2Artist() - require.NoError(t, err) + run := func(t *testing.T, client pbresource.ResourceServiceClient, tc deleteTestCase) { + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) - // verify delete of non-existant or already deleted resource is a no-op - _, err = client.Delete(ctx, tc.deleteReqFn(artist)) - require.NoError(t, err) + // verify delete of non-existant or already deleted resource is a no-op + _, err = client.Delete(context.Background(), tc.deleteReqFn(artist)) + require.NoError(t, err) + } + + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(useV2Tenancy). + WithRegisterFns(demo.RegisterTypes). + Run(t) + + for desc, tc := range deleteTestCases() { + t.Run(desc, func(t *testing.T) { + run(t, client, tc) + }) + } }) } } @@ -301,86 +365,115 @@ func TestDelete_NotFound(t *testing.T) { func TestDelete_VersionMismatch(t *testing.T) { t.Parallel() - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) - artist, err := demo.GenerateV2Artist() - require.NoError(t, err) - rsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) - require.NoError(t, err) + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(useV2Tenancy). + WithRegisterFns(demo.RegisterTypes). + Run(t) - // delete with a version that is different from the stored version - _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: rsp.Resource.Id, Version: "non-existent-version"}) - require.Error(t, err) - require.Equal(t, codes.Aborted.String(), status.Code(err).String()) - require.ErrorContains(t, err, "CAS operation failed") + artist, err := demo.GenerateV2Artist() + require.NoError(t, err) + rsp, err := client.Write(context.Background(), &pbresource.WriteRequest{Resource: artist}) + require.NoError(t, err) + + // delete with a version that is different from the stored version + _, err = client.Delete(context.Background(), &pbresource.DeleteRequest{Id: rsp.Resource.Id, Version: "non-existent-version"}) + require.Error(t, err) + require.Equal(t, codes.Aborted.String(), status.Code(err).String()) + require.ErrorContains(t, err, "CAS operation failed") + }) + } } func TestDelete_MarkedForDeletionWhenFinalizersPresent(t *testing.T) { - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + ctx := context.Background() + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(useV2Tenancy). + WithRegisterFns(demo.RegisterTypes). + Run(t) - // Create a resource with a finalizer - res := rtest.Resource(demo.TypeV1Artist, "manwithnoname"). - WithTenancy(resource.DefaultClusteredTenancy()). - WithData(t, &pbdemo.Artist{Name: "Man With No Name"}). - WithMeta(resource.FinalizerKey, "finalizer1"). - Write(t, client) + // Create a resource with a finalizer + res := rtest.Resource(demo.TypeV1Artist, "manwithnoname"). + WithTenancy(resource.DefaultClusteredTenancy()). + WithData(t, &pbdemo.Artist{Name: "Man With No Name"}). + WithMeta(resource.FinalizerKey, "finalizer1"). + Write(t, client) - // Delete it - _, err := client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) - require.NoError(t, err) + // Delete it + _, err := client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) + require.NoError(t, err) - // Verify resource has been marked for deletion - rsp, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) - require.NoError(t, err) - require.True(t, resource.IsMarkedForDeletion(rsp.Resource)) + // Verify resource has been marked for deletion + rsp, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) + require.NoError(t, err) + require.True(t, resource.IsMarkedForDeletion(rsp.Resource)) - // Delete again - should be no-op - _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) - require.NoError(t, err) + // Delete again - should be no-op + _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) + require.NoError(t, err) - // Verify no-op by checking version still the same - rsp2, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) - require.NoError(t, err) - rtest.RequireVersionUnchanged(t, rsp2.Resource, rsp.Resource.Version) + // Verify no-op by checking version still the same + rsp2, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) + require.NoError(t, err) + rtest.RequireVersionUnchanged(t, rsp2.Resource, rsp.Resource.Version) + }) + } } func TestDelete_ImmediatelyDeletedAfterFinalizersRemoved(t *testing.T) { - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) + for _, useV2Tenancy := range []bool{false, true} { + t.Run(fmt.Sprintf("v2tenancy %v", useV2Tenancy), func(t *testing.T) { + ctx := context.Background() + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(useV2Tenancy). + WithRegisterFns(demo.RegisterTypes). + Run(t) - // Create a resource with a finalizer - res := rtest.Resource(demo.TypeV1Artist, "manwithnoname"). - WithTenancy(resource.DefaultClusteredTenancy()). - WithData(t, &pbdemo.Artist{Name: "Man With No Name"}). - WithMeta(resource.FinalizerKey, "finalizer1"). - Write(t, client) + // Create a resource with a finalizer + res := rtest.Resource(demo.TypeV1Artist, "manwithnoname"). + WithTenancy(resource.DefaultClusteredTenancy()). + WithData(t, &pbdemo.Artist{Name: "Man With No Name"}). + WithMeta(resource.FinalizerKey, "finalizer1"). + Write(t, client) - // Delete should mark it for deletion - _, err := client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) - require.NoError(t, err) + // Delete should mark it for deletion + _, err := client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) + require.NoError(t, err) - // Remove the finalizer - rsp, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) - require.NoError(t, err) - resource.RemoveFinalizer(rsp.Resource, "finalizer1") - _, err = client.Write(ctx, &pbresource.WriteRequest{Resource: rsp.Resource}) - require.NoError(t, err) + // Remove the finalizer + rsp, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) + require.NoError(t, err) + resource.RemoveFinalizer(rsp.Resource, "finalizer1") + _, err = client.Write(ctx, &pbresource.WriteRequest{Resource: rsp.Resource}) + require.NoError(t, err) - // Delete should be immediate - _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: rsp.Resource.Id}) - require.NoError(t, err) + // Delete should be immediate + _, err = client.Delete(ctx, &pbresource.DeleteRequest{Id: rsp.Resource.Id}) + require.NoError(t, err) - // Verify deleted - _, err = client.Read(ctx, &pbresource.ReadRequest{Id: rsp.Resource.Id}) - require.Error(t, err) - require.Equal(t, codes.NotFound.String(), status.Code(err).String()) + // Verify deleted + _, err = client.Read(ctx, &pbresource.ReadRequest{Id: rsp.Resource.Id}) + require.Error(t, err) + require.Equal(t, codes.NotFound.String(), status.Code(err).String()) + }) + } } -func testDeps(t *testing.T) (*Server, pbresource.ResourceServiceClient, context.Context) { - server := testServer(t) - client := testClient(t, server) - return server, client, context.Background() +func TestDelete_BlockDeleteDefaultNamespace(t *testing.T) { + client := svctest.NewResourceServiceBuilder().WithV2Tenancy(true).Run(t) + + id := &pbresource.ID{ + Name: resource.DefaultNamespaceName, + Type: pbtenancy.NamespaceType, + Tenancy: &pbresource.Tenancy{Partition: resource.DefaultPartitionName}, + } + _, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: id}) + require.Error(t, err) + require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String()) + require.ErrorContains(t, err, "cannot delete default namespace") } type deleteTestCase struct { 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 e24d8ab24b..92167042ea 100644 --- a/agent/grpc-external/services/resource/list_by_owner_test.go +++ b/agent/grpc-external/services/resource/list_by_owner_test.go @@ -1,7 +1,7 @@ // // Copyright (c) HashiCorp, Inc. // // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "context" @@ -17,6 +17,8 @@ import ( "google.golang.org/protobuf/proto" "github.com/hashicorp/consul/acl" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/internal/resource/resourcetest" @@ -25,10 +27,12 @@ import ( "github.com/hashicorp/consul/proto/private/prototest" ) +// TODO: Update all tests to use true/false table test for v2tenancy + func TestListByOwner_InputValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) type testCase struct { modFn func(artistId, recordlabelId, executiveId *pbresource.ID) *pbresource.ID @@ -152,8 +156,7 @@ func TestListByOwner_InputValidation(t *testing.T) { } func TestListByOwner_TypeNotRegistered(t *testing.T) { - server := testServer(t) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder().Run(t) _, err := client.ListByOwner(context.Background(), &pbresource.ListByOwnerRequest{ Owner: &pbresource.ID{ @@ -169,9 +172,9 @@ func TestListByOwner_TypeNotRegistered(t *testing.T) { } func TestListByOwner_Empty(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -185,9 +188,9 @@ func TestListByOwner_Empty(t *testing.T) { } func TestListByOwner_Many(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -245,9 +248,9 @@ func TestListByOwner_OwnerTenancyDoesNotExist(t *testing.T) { } for desc, tc := range tenancyCases { t.Run(desc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) recordLabel := resourcetest.Resource(demo.TypeV1RecordLabel, "looney-tunes"). WithTenancy(resource.DefaultPartitionedTenancy()). @@ -271,9 +274,9 @@ func TestListByOwner_OwnerTenancyDoesNotExist(t *testing.T) { func TestListByOwner_Tenancy_Defaults_And_Normalization(t *testing.T) { for tenancyDesc, modFn := range tenancyCases() { t.Run(tenancyDesc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Create partition scoped recordLabel. recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") @@ -343,9 +346,9 @@ func TestListByOwner_ACL_PerTypeAllowed(t *testing.T) { // roundtrip a ListByOwner which attempts to return a single resource func roundTripListByOwner(t *testing.T, authz acl.Authorizer) (*pbresource.Resource, *pbresource.ListByOwnerResponse, error) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + builder := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes) + client := builder.Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -361,10 +364,11 @@ func roundTripListByOwner(t *testing.T, authz acl.Authorizer) (*pbresource.Resou album = rsp2.Resource require.NoError(t, err) - mockACLResolver := &MockACLResolver{} + // Mock has to be put in place after the above writes so writes will succeed. + mockACLResolver := &svc.MockACLResolver{} mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). Return(authz, nil) - server.ACLResolver = mockACLResolver + builder.ServiceImpl().ACLResolver = mockACLResolver rsp3, err := client.ListByOwner(testContext(t), &pbresource.ListByOwnerRequest{Owner: artist.Id}) return album, rsp3, err diff --git a/agent/grpc-external/services/resource/list_test.go b/agent/grpc-external/services/resource/list_test.go index 5af6747f84..780497ec0f 100644 --- a/agent/grpc-external/services/resource/list_test.go +++ b/agent/grpc-external/services/resource/list_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "context" @@ -10,25 +10,29 @@ import ( "strings" "testing" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + "github.com/hashicorp/consul/acl" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/agent/grpc-external/testutils" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/internal/storage" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/proto/private/prototest" - - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/status" ) +// TODO: Update all tests to use true/false table test for v2tenancy + func TestList_InputValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) type testCase struct { modReqFn func(req *pbresource.ListRequest) @@ -93,8 +97,7 @@ func TestList_InputValidation(t *testing.T) { } func TestList_TypeNotFound(t *testing.T) { - server := testServer(t) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder().Run(t) _, err := client.List(context.Background(), &pbresource.ListRequest{ Type: demo.TypeV2Artist, @@ -109,9 +112,9 @@ func TestList_TypeNotFound(t *testing.T) { func TestList_Empty(t *testing.T) { for desc, tc := range listTestCases() { t.Run(desc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) rsp, err := client.List(tc.ctx, &pbresource.ListRequest{ Type: demo.TypeV1Artist, @@ -127,9 +130,9 @@ func TestList_Empty(t *testing.T) { func TestList_Many(t *testing.T) { for desc, tc := range listTestCases() { t.Run(desc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) resources := make([]*pbresource.Resource, 10) for i := 0; i < len(resources); i++ { @@ -159,9 +162,9 @@ func TestList_Many(t *testing.T) { func TestList_NamePrefix(t *testing.T) { for desc, tc := range listTestCases() { t.Run(desc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) expectedResources := []*pbresource.Resource{} @@ -201,9 +204,9 @@ func TestList_Tenancy_Defaults_And_Normalization(t *testing.T) { ctx := context.Background() for desc, tc := range wildcardTenancyCases() { t.Run(desc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Write partition scoped record label recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") @@ -236,14 +239,14 @@ func TestList_Tenancy_Defaults_And_Normalization(t *testing.T) { func TestList_GroupVersionMismatch(t *testing.T) { for desc, tc := range listTestCases() { t.Run(desc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) - _, err = server.Backend.WriteCAS(tc.ctx, artist) + _, err = client.Write(tc.ctx, &pbresource.WriteRequest{Resource: artist}) require.NoError(t, err) rsp, err := client.List(tc.ctx, &pbresource.ListRequest{ @@ -261,7 +264,7 @@ func TestList_VerifyReadConsistencyArg(t *testing.T) { // Uses a mockBackend instead of the inmem Backend to verify the ReadConsistency argument is set correctly. for desc, tc := range listTestCases() { t.Run(desc, func(t *testing.T) { - mockBackend := NewMockBackend(t) + mockBackend := svc.NewMockBackend(t) server := testServer(t) server.Backend = mockBackend demo.RegisterTypes(server.Registry) @@ -322,25 +325,24 @@ func TestList_ACL_ListAllowed_ReadAllowed(t *testing.T) { prototest.AssertDeepEqual(t, artist, rsp.Resources[0]) } -// roundtrip a List which attempts to return a single resource func roundTripList(t *testing.T, authz acl.Authorizer) (*pbresource.Resource, *pbresource.ListResponse, error) { - server := testServer(t) - client := testClient(t, server) ctx := testContext(t) - - mockACLResolver := &MockACLResolver{} - mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(authz, nil) - server.ACLResolver = mockACLResolver - demo.RegisterTypes(server.Registry) + builder := svctest.NewResourceServiceBuilder().WithRegisterFns(demo.RegisterTypes) + client := builder.Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) - artist, err = server.Backend.WriteCAS(ctx, artist) + rsp1, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) require.NoError(t, err) - rsp, err := client.List( + // Put ACLResolver in place after above writes so writes not subject to ACLs + mockACLResolver := &svc.MockACLResolver{} + mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). + Return(authz, nil) + builder.ServiceImpl().Config.ACLResolver = mockACLResolver + + rsp2, err := client.List( ctx, &pbresource.ListRequest{ Type: artist.Id.Type, @@ -348,8 +350,7 @@ func roundTripList(t *testing.T, authz acl.Authorizer) (*pbresource.Resource, *p NamePrefix: "", }, ) - - return artist, rsp, err + return rsp1.Resource, rsp2, err } type listTestCase struct { diff --git a/agent/grpc-external/services/resource/read_test.go b/agent/grpc-external/services/resource/read_test.go index 2afdfeab0e..b7367e6390 100644 --- a/agent/grpc-external/services/resource/read_test.go +++ b/agent/grpc-external/services/resource/read_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "context" @@ -19,21 +19,23 @@ import ( "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl/resolver" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/agent/grpc-external/testutils" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/internal/storage" - "github.com/hashicorp/consul/internal/tenancy" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/proto/private/prototest" "github.com/hashicorp/consul/sdk/testutil" ) +// TODO: Update all tests to use true/false table test for v2tenancy + func TestRead_InputValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - tenancy.RegisterTypes(server.Registry) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) type testCase struct { modFn func(artistId, recordlabelId, executiveId *pbresource.ID) *pbresource.ID @@ -148,7 +150,7 @@ func TestRead_InputValidation(t *testing.T) { } func TestRead_TypeNotFound(t *testing.T) { - server := NewServer(Config{Registry: resource.NewRegistry()}) + server := svc.NewServer(svc.Config{Registry: resource.NewRegistry()}) client := testClient(t, server) artist, err := demo.GenerateV2Artist() @@ -202,18 +204,19 @@ func TestRead_ResourceNotFound(t *testing.T) { } for tenancyDesc, tenancyCase := range tenancyCases { t.Run(tenancyDesc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes). + Run(t) recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") require.NoError(t, err) - recordLabel, err = server.Backend.WriteCAS(tc.ctx, recordLabel) + _, err = client.Write(context.Background(), &pbresource.WriteRequest{Resource: recordLabel}) require.NoError(t, err) artist, err := demo.GenerateV2Artist() require.NoError(t, err) - artist, err = server.Backend.WriteCAS(tc.ctx, artist) + _, err = client.Write(context.Background(), &pbresource.WriteRequest{Resource: artist}) require.NoError(t, err) // Each tenancy test case picks which resource to use based on the resource type's scope. @@ -230,15 +233,14 @@ func TestRead_ResourceNotFound(t *testing.T) { func TestRead_GroupVersionMismatch(t *testing.T) { for desc, tc := range readTestCases() { t.Run(desc, func(t *testing.T) { - server := testServer(t) - - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) - _, err = server.Backend.WriteCAS(tc.ctx, artist) + _, err = client.Write(tc.ctx, &pbresource.WriteRequest{Resource: artist}) require.NoError(t, err) id := clone(artist.Id) @@ -257,18 +259,20 @@ func TestRead_Success(t *testing.T) { t.Run(desc, func(t *testing.T) { for tenancyDesc, modFn := range tenancyCases() { t.Run(tenancyDesc, func(t *testing.T) { - server := testServer(t) - demo.RegisterTypes(server.Registry) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") require.NoError(t, err) - recordLabel, err = server.Backend.WriteCAS(tc.ctx, recordLabel) + rsp1, err := client.Write(tc.ctx, &pbresource.WriteRequest{Resource: recordLabel}) + recordLabel = rsp1.Resource require.NoError(t, err) artist, err := demo.GenerateV2Artist() require.NoError(t, err) - artist, err = server.Backend.WriteCAS(tc.ctx, artist) + rsp2, err := client.Write(tc.ctx, &pbresource.WriteRequest{Resource: artist}) + artist = rsp2.Resource require.NoError(t, err) // Each tenancy test case picks which resource to use based on the resource type's scope. @@ -295,7 +299,7 @@ func TestRead_VerifyReadConsistencyArg(t *testing.T) { for desc, tc := range readTestCases() { t.Run(desc, func(t *testing.T) { server := testServer(t) - mockBackend := NewMockBackend(t) + mockBackend := svc.NewMockBackend(t) server.Backend = mockBackend demo.RegisterTypes(server.Registry) @@ -364,15 +368,14 @@ func TestRead_ACLs(t *testing.T) { for desc, tc := range testcases { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - dr := &dummyACLResolver{ result: testutils.ACLsDisabled(t), } - server.ACLResolver = dr - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + WithACLResolver(dr). + Run(t) dr.SetResult(tc.authz) testutil.RunStep(t, "does not exist", func(t *testing.T) { @@ -410,7 +413,7 @@ type dummyACLResolver struct { result resolver.Result } -var _ ACLResolver = (*dummyACLResolver)(nil) +var _ svc.ACLResolver = (*dummyACLResolver)(nil) func (r *dummyACLResolver) SetResult(result resolver.Result) { r.lock.Lock() diff --git a/agent/grpc-external/services/resource/server_ce_test.go b/agent/grpc-external/services/resource/server_ce_test.go index c385b65f3c..f48ff3b52b 100644 --- a/agent/grpc-external/services/resource/server_ce_test.go +++ b/agent/grpc-external/services/resource/server_ce_test.go @@ -3,7 +3,7 @@ //go:build !consulent -package resource +package resource_test import "github.com/hashicorp/consul/acl" diff --git a/agent/grpc-external/services/resource/server_test.go b/agent/grpc-external/services/resource/server_test.go index e0b5226390..6abb4564d8 100644 --- a/agent/grpc-external/services/resource/server_test.go +++ b/agent/grpc-external/services/resource/server_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "context" @@ -11,12 +11,14 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/go-uuid" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl/resolver" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" "github.com/hashicorp/consul/agent/grpc-external/testutils" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/internal/resource" @@ -51,7 +53,8 @@ func AuthorizerFrom(t *testing.T, policyStrs ...string) resolver.Result { } } -func testServer(t *testing.T) *Server { +// Deprecated: use NewResourceServiceBuilder instead +func testServer(t *testing.T) *svc.Server { t.Helper() backend, err := inmem.NewBackend() @@ -59,7 +62,7 @@ func testServer(t *testing.T) *Server { go backend.Run(testContext(t)) // Mock the ACL Resolver to "allow all" for testing. - mockACLResolver := &MockACLResolver{} + mockACLResolver := &svc.MockACLResolver{} mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). Return(testutils.ACLsDisabled(t), nil). Run(func(args mock.Arguments) { @@ -76,7 +79,7 @@ func testServer(t *testing.T) *Server { }) // Mock the tenancy bridge since we can't use the real thing. - mockTenancyBridge := &MockTenancyBridge{} + mockTenancyBridge := &svc.MockTenancyBridge{} mockTenancyBridge.On("PartitionExists", resource.DefaultPartitionName).Return(true, nil) mockTenancyBridge.On("NamespaceExists", resource.DefaultPartitionName, resource.DefaultNamespaceName).Return(true, nil) mockTenancyBridge.On("PartitionExists", mock.Anything).Return(false, nil) @@ -84,7 +87,7 @@ func testServer(t *testing.T) *Server { mockTenancyBridge.On("IsPartitionMarkedForDeletion", resource.DefaultPartitionName).Return(false, nil) mockTenancyBridge.On("IsNamespaceMarkedForDeletion", resource.DefaultPartitionName, resource.DefaultNamespaceName).Return(false, nil) - return NewServer(Config{ + return svc.NewServer(svc.Config{ Logger: testutil.Logger(t), Registry: resource.NewRegistry(), Backend: backend, @@ -93,7 +96,8 @@ func testServer(t *testing.T) *Server { }) } -func testClient(t *testing.T, server *Server) pbresource.ResourceServiceClient { +// Deprecated: use NewResourceServiceBuilder instead +func testClient(t *testing.T, server *svc.Server) pbresource.ResourceServiceClient { t.Helper() addr := testutils.RunTestServer(t, server) @@ -253,3 +257,5 @@ func tenancyCases() map[string]func(artistId, recordlabelId *pbresource.ID) *pbr } return tenancyCases } + +func clone[T proto.Message](v T) T { return proto.Clone(v).(T) } diff --git a/agent/grpc-external/services/resource/testing/builder.go b/agent/grpc-external/services/resource/testing/builder.go new file mode 100644 index 0000000000..96c8379c0e --- /dev/null +++ b/agent/grpc-external/services/resource/testing/builder.go @@ -0,0 +1,201 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package testing + +import ( + "context" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + + "github.com/hashicorp/consul/acl" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + "github.com/hashicorp/consul/agent/grpc-external/testutils" + internal "github.com/hashicorp/consul/agent/grpc-internal" + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/internal/storage/inmem" + "github.com/hashicorp/consul/internal/tenancy" + "github.com/hashicorp/consul/proto-public/pbresource" + "github.com/hashicorp/consul/sdk/testutil" +) + +type builder struct { + registry resource.Registry + registerFns []func(resource.Registry) + useV2Tenancy bool + tenancies []*pbresource.Tenancy + aclResolver svc.ACLResolver + serviceImpl *svc.Server +} + +// NewResourceServiceBuilder is the preferred way to configure and run +// an isolated in-process instance of the resource service for unit +// testing. The final call to `Run()` returns a client you can use for +// making requests. +func NewResourceServiceBuilder() *builder { + b := &builder{ + useV2Tenancy: false, + registry: resource.NewRegistry(), + // Regardless of whether using mock of v2tenancy, always make sure + // the builtin tenancy exists. + tenancies: []*pbresource.Tenancy{resource.DefaultNamespacedTenancy()}, + } + return b +} + +// WithV2Tenancy configures which tenancy bridge is used. +// +// true => real v2 default partition and namespace via v2 tenancy bridge +// false => mock default partition and namespace since v1 tenancy bridge can't be used (not spinning up an entire server here) +func (b *builder) WithV2Tenancy(useV2Tenancy bool) *builder { + b.useV2Tenancy = useV2Tenancy + return b +} + +// Registry provides access to the constructed registry post-Run() when +// needed by other test dependencies. +func (b *builder) Registry() resource.Registry { + return b.registry +} + +// ServiceImpl provides access to the actual server side implemenation of the resource service. This should never be used +// used/accessed without good reason. The current justifying use case is to monkeypatch the ACL resolver post-creation +// to allow unfettered writes which some ACL related tests require to put test data in place. +func (b *builder) ServiceImpl() *svc.Server { + return b.serviceImpl +} + +func (b *builder) WithRegisterFns(registerFns ...func(resource.Registry)) *builder { + for _, registerFn := range registerFns { + b.registerFns = append(b.registerFns, registerFn) + } + return b +} + +func (b *builder) WithACLResolver(aclResolver svc.ACLResolver) *builder { + b.aclResolver = aclResolver + return b +} + +// WithTenancies adds additional partitions and namespaces if default/default +// is not sufficient. +func (b *builder) WithTenancies(tenancies ...*pbresource.Tenancy) *builder { + for _, tenancy := range tenancies { + b.tenancies = append(b.tenancies, tenancy) + } + return b +} + +// Run starts the resource service and returns a client. +func (b *builder) Run(t *testing.T) pbresource.ResourceServiceClient { + // backend cannot be customized + backend, err := inmem.NewBackend() + require.NoError(t, err) + + // start the backend and add teardown hook + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + go backend.Run(ctx) + + // Automatically add tenancy types if v2 tenancy enabled + if b.useV2Tenancy { + b.registerFns = append(b.registerFns, tenancy.RegisterTypes) + } + + for _, registerFn := range b.registerFns { + registerFn(b.registry) + } + + var tenancyBridge resource.TenancyBridge + if !b.useV2Tenancy { + // use mock tenancy bridge. default/default has already been added out of the box + mockTenancyBridge := &svc.MockTenancyBridge{} + + for _, tenancy := range b.tenancies { + mockTenancyBridge.On("PartitionExists", tenancy.Partition).Return(true, nil) + mockTenancyBridge.On("NamespaceExists", tenancy.Partition, tenancy.Namespace).Return(true, nil) + mockTenancyBridge.On("IsPartitionMarkedForDeletion", tenancy.Partition).Return(false, nil) + mockTenancyBridge.On("IsNamespaceMarkedForDeletion", tenancy.Partition, tenancy.Namespace).Return(false, nil) + } + + tenancyBridge = mockTenancyBridge + } else { + // use v2 tenancy bridge. population comes later after client injected. + tenancyBridge = tenancy.NewV2TenancyBridge() + } + + if b.aclResolver == nil { + // When not provided (regardless of V1 tenancy or V2 tenancy), configure an ACL resolver + // that has ACLs disabled and fills in "default" for the partition and namespace when + // not provided. This is similar to user initiated requests. + // + // Controllers under test should be providing full tenancy since they will run with the DANGER_NO_AUTH. + mockACLResolver := &svc.MockACLResolver{} + mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). + Return(testutils.ACLsDisabled(t), nil). + Run(func(args mock.Arguments) { + // Caller expecting passed in tokenEntMeta and authorizerContext to be filled in. + tokenEntMeta := args.Get(1).(*acl.EnterpriseMeta) + if tokenEntMeta != nil { + FillEntMeta(tokenEntMeta) + } + + authzContext := args.Get(2).(*acl.AuthorizerContext) + if authzContext != nil { + FillAuthorizerContext(authzContext) + } + }) + b.aclResolver = mockACLResolver + } + + config := svc.Config{ + Logger: testutil.Logger(t), + Registry: b.registry, + Backend: backend, + ACLResolver: b.aclResolver, + TenancyBridge: tenancyBridge, + UseV2Tenancy: b.useV2Tenancy, + } + + server := grpc.NewServer() + + b.serviceImpl = svc.NewServer(config) + b.serviceImpl.Register(server) + + pipe := internal.NewPipeListener() + go server.Serve(pipe) + t.Cleanup(server.Stop) + + conn, err := grpc.Dial("", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithContextDialer(pipe.DialContext), + grpc.WithBlock(), + ) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + client := pbresource.NewResourceServiceClient(conn) + + // HACK ALERT: The client needs to be injected into the V2TenancyBridge + // after it has been created due the the circular dependency. This will + // go away when the tenancy bridge is removed and V1 is no more, however + // long that takes. + switch config.TenancyBridge.(type) { + case *tenancy.V2TenancyBridge: + config.TenancyBridge.(*tenancy.V2TenancyBridge).WithClient(client) + // Default partition namespace can finally be created + require.NoError(t, initTenancy(ctx, backend)) + + for _, tenancy := range b.tenancies { + if tenancy.Partition == resource.DefaultPartitionName && tenancy.Namespace == resource.DefaultNamespaceName { + continue + } + t.Fatalf("TODO: implement creation of passed in v2 tenancy: %v", tenancy) + } + } + return client +} diff --git a/agent/grpc-external/services/resource/testing/testing.go b/agent/grpc-external/services/resource/testing/testing.go index f38d745321..906953b034 100644 --- a/agent/grpc-external/services/resource/testing/testing.go +++ b/agent/grpc-external/services/resource/testing/testing.go @@ -4,27 +4,15 @@ package testing import ( - "context" "testing" - "github.com/hashicorp/go-uuid" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" + + "github.com/hashicorp/go-uuid" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl/resolver" - svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" - "github.com/hashicorp/consul/agent/grpc-external/testutils" - internal "github.com/hashicorp/consul/agent/grpc-internal" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/internal/resource" - rtest "github.com/hashicorp/consul/internal/resource/resourcetest" - "github.com/hashicorp/consul/internal/storage/inmem" - "github.com/hashicorp/consul/internal/tenancy" - "github.com/hashicorp/consul/proto-public/pbresource" - "github.com/hashicorp/consul/sdk/testutil" ) func randomACLIdentity(t *testing.T) structs.ACLIdentity { @@ -50,129 +38,3 @@ func AuthorizerFrom(t *testing.T, policyStrs ...string) resolver.Result { ACLIdentity: randomACLIdentity(t), } } - -// RunResourceService runs a Resource Service for the duration of the test and -// returns a client to interact with it. ACLs will be disabled and only the -// default partition and namespace are available. -func RunResourceService(t *testing.T, registerFns ...func(resource.Registry)) pbresource.ResourceServiceClient { - return RunResourceServiceWithConfig(t, svc.Config{}, registerFns...) -} - -// RunResourceServiceWithTenancies runs a Resource Service with tenancies returned from TestTenancies. -func RunResourceServiceWithTenancies(t *testing.T, registerFns ...func(resource.Registry)) pbresource.ResourceServiceClient { - mockTenancyBridge := &svc.MockTenancyBridge{} - - for _, tenant := range rtest.TestTenancies() { - mockTenancyBridge.On("PartitionExists", tenant.Partition).Return(true, nil) - mockTenancyBridge.On("NamespaceExists", tenant.Partition, tenant.Namespace).Return(true, nil) - mockTenancyBridge.On("IsPartitionMarkedForDeletion", tenant.Partition).Return(false, nil) - mockTenancyBridge.On("IsNamespaceMarkedForDeletion", tenant.Partition, tenant.Namespace).Return(false, nil) - } - - cfg := &svc.Config{ - TenancyBridge: mockTenancyBridge, - } - return RunResourceServiceWithConfig(t, *cfg, registerFns...) -} - -// RunResourceServiceWithConfig runs a ResourceService with caller injectable config to ease mocking dependencies. -// Any nil config field is replaced with a reasonable default with the following behavior: -// -// config.Backend - cannot be configured and must be nil -// config.Registry - empty registry -// config.TenancyBridge - mock provided with only the default partition and namespace -// config.ACLResolver - mock provided with ACLs disabled. Fills entMeta and authzContext with default partition and namespace -func RunResourceServiceWithConfig(t *testing.T, config svc.Config, registerFns ...func(resource.Registry)) pbresource.ResourceServiceClient { - t.Helper() - - if config.Backend != nil { - panic("backend can not be configured") - } - - backend, err := inmem.NewBackend() - require.NoError(t, err) - - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - go backend.Run(ctx) - config.Backend = backend - - if config.Registry == nil { - config.Registry = resource.NewRegistry() - } - - for _, fn := range registerFns { - fn(config.Registry) - } - - server := grpc.NewServer() - - if config.TenancyBridge == nil { - mockTenancyBridge := &svc.MockTenancyBridge{} - mockTenancyBridge.On("PartitionExists", resource.DefaultPartitionName).Return(true, nil) - mockTenancyBridge.On("PartitionExists", "foo").Return(true, nil) - mockTenancyBridge.On("NamespaceExists", resource.DefaultPartitionName, resource.DefaultNamespaceName).Return(true, nil) - mockTenancyBridge.On("PartitionExists", "foo").Return(true, nil) - mockTenancyBridge.On("IsPartitionMarkedForDeletion", resource.DefaultPartitionName).Return(false, nil) - mockTenancyBridge.On("IsPartitionMarkedForDeletion", "foo").Return(false, nil) - mockTenancyBridge.On("IsNamespaceMarkedForDeletion", resource.DefaultPartitionName, resource.DefaultNamespaceName).Return(false, nil) - config.TenancyBridge = mockTenancyBridge - } else { - switch config.TenancyBridge.(type) { - case *tenancy.V2TenancyBridge: - err = initTenancy(ctx, backend) - require.NoError(t, err) - } - } - - if config.ACLResolver == nil { - // Provide a resolver which will default partition and namespace when not provided. This is similar to user - // initiated requests. - // - // Controllers under test should be providing full tenancy since they will run with the DANGER_NO_AUTH. - mockACLResolver := &svc.MockACLResolver{} - mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). - Return(testutils.ACLsDisabled(t), nil). - Run(func(args mock.Arguments) { - // Caller expecting passed in tokenEntMeta and authorizerContext to be filled in. - tokenEntMeta := args.Get(1).(*acl.EnterpriseMeta) - if tokenEntMeta != nil { - FillEntMeta(tokenEntMeta) - } - - authzContext := args.Get(2).(*acl.AuthorizerContext) - if authzContext != nil { - FillAuthorizerContext(authzContext) - } - }) - config.ACLResolver = mockACLResolver - } - - if config.Logger == nil { - config.Logger = testutil.Logger(t) - } - - svc.NewServer(config).Register(server) - - pipe := internal.NewPipeListener() - go server.Serve(pipe) - t.Cleanup(server.Stop) - - conn, err := grpc.Dial("", - grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithContextDialer(pipe.DialContext), - grpc.WithBlock(), - ) - require.NoError(t, err) - t.Cleanup(func() { _ = conn.Close() }) - client := pbresource.NewResourceServiceClient(conn) - if config.TenancyBridge != nil { - switch config.TenancyBridge.(type) { - case *tenancy.V2TenancyBridge: - config.TenancyBridge.(*tenancy.V2TenancyBridge).WithClient(client) - } - - } - - return client -} diff --git a/agent/grpc-external/services/resource/testing/testing_ce.go b/agent/grpc-external/services/resource/testing/testing_ce.go index da20be3533..8042eba550 100644 --- a/agent/grpc-external/services/resource/testing/testing_ce.go +++ b/agent/grpc-external/services/resource/testing/testing_ce.go @@ -31,8 +31,6 @@ func FillAuthorizerContext(authzContext *acl.AuthorizerContext) { // initTenancy create the base tenancy objects (default/default) func initTenancy(ctx context.Context, b *inmem.Backend) error { - //TODO(dhiaayachi): This is now called for testing purpose but at some point we need to add something similar - // when bootstrapping a server, probably in the tenancy controllers. nsData, err := anypb.New(&pbtenancy.Namespace{Description: "default namespace in default partition"}) if err != nil { return err @@ -61,5 +59,4 @@ func initTenancy(ctx context.Context, b *inmem.Backend) error { } } return nil - } diff --git a/agent/grpc-external/services/resource/watch_test.go b/agent/grpc-external/services/resource/watch_test.go index 1c9da4b68d..2c596e01b3 100644 --- a/agent/grpc-external/services/resource/watch_test.go +++ b/agent/grpc-external/services/resource/watch_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "context" @@ -11,24 +11,28 @@ import ( "testing" "time" - "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/agent/grpc-external/testutils" - "github.com/hashicorp/consul/internal/resource" - "github.com/hashicorp/consul/internal/resource/demo" - "github.com/hashicorp/consul/proto-public/pbresource" - "github.com/hashicorp/consul/proto/private/prototest" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" + + "github.com/hashicorp/consul/acl" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" + "github.com/hashicorp/consul/agent/grpc-external/testutils" + "github.com/hashicorp/consul/internal/resource" + "github.com/hashicorp/consul/internal/resource/demo" + "github.com/hashicorp/consul/proto-public/pbresource" + "github.com/hashicorp/consul/proto/private/prototest" ) +// TODO: Update all tests to use true/false table test for v2tenancy + func TestWatchList_InputValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) type testCase struct { modFn func(*pbresource.WatchListRequest) @@ -108,8 +112,7 @@ func TestWatchList_InputValidation(t *testing.T) { func TestWatchList_TypeNotFound(t *testing.T) { t.Parallel() - server := testServer(t) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder().Run(t) stream, err := client.WatchList(context.Background(), &pbresource.WatchListRequest{ Type: demo.TypeV2Artist, @@ -171,9 +174,9 @@ func TestWatchList_Tenancy_Defaults_And_Normalization(t *testing.T) { for desc, tc := range wildcardTenancyCases() { t.Run(desc, func(t *testing.T) { ctx := context.Background() - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Create a watch. stream, err := client.WatchList(ctx, &pbresource.WatchListRequest{ @@ -191,17 +194,17 @@ func TestWatchList_Tenancy_Defaults_And_Normalization(t *testing.T) { require.NoError(t, err) // Create and verify upsert event received. - recordLabel, err = server.Backend.WriteCAS(ctx, recordLabel) + rlRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel}) require.NoError(t, err) - artist, err = server.Backend.WriteCAS(ctx, artist) + artistRsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist}) require.NoError(t, err) var expected *pbresource.Resource switch { case proto.Equal(tc.typ, demo.TypeV1RecordLabel): - expected = recordLabel + expected = rlRsp.Resource case proto.Equal(tc.typ, demo.TypeV2Artist): - expected = artist + expected = artistRsp.Resource default: require.Fail(t, "unsupported type", tc.typ) } @@ -210,7 +213,6 @@ func TestWatchList_Tenancy_Defaults_And_Normalization(t *testing.T) { require.Equal(t, pbresource.WatchEvent_OPERATION_UPSERT, rsp.Operation) prototest.AssertDeepEqual(t, expected, rsp.Resource) }) - } } @@ -304,7 +306,7 @@ func roundTripACL(t *testing.T, authz acl.Authorizer) (<-chan resourceOrError, * server := testServer(t) client := testClient(t, server) - mockACLResolver := &MockACLResolver{} + mockACLResolver := &svc.MockACLResolver{} mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). Return(authz, nil) server.ACLResolver = mockACLResolver @@ -395,10 +397,11 @@ type resourceOrError struct { func TestWatchList_NoTenancy(t *testing.T) { t.Parallel() + ctx := context.Background() - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Create a watch. stream, err := client.WatchList(ctx, &pbresource.WatchListRequest{ @@ -411,11 +414,11 @@ func TestWatchList_NoTenancy(t *testing.T) { require.NoError(t, err) // Create and verify upsert event received. - recordLabel, err = server.Backend.WriteCAS(ctx, recordLabel) + rsp1, err := client.Write(ctx, &pbresource.WriteRequest{Resource: recordLabel}) require.NoError(t, err) - rsp := mustGetResource(t, rspCh) + rsp2 := mustGetResource(t, rspCh) - require.Equal(t, pbresource.WatchEvent_OPERATION_UPSERT, rsp.Operation) - prototest.AssertDeepEqual(t, recordLabel, rsp.Resource) + require.Equal(t, pbresource.WatchEvent_OPERATION_UPSERT, rsp2.Operation) + prototest.AssertDeepEqual(t, rsp1.Resource, rsp2.Resource) } diff --git a/agent/grpc-external/services/resource/write.go b/agent/grpc-external/services/resource/write.go index b0dc27e2f0..87091ed920 100644 --- a/agent/grpc-external/services/resource/write.go +++ b/agent/grpc-external/services/resource/write.go @@ -357,8 +357,9 @@ func ensureDataUnchanged(input *pbresource.Resource, existing *pbresource.Resour return nil } -// ensureFinalizerRemoved ensures at least one finalizer was removed. -func ensureFinalizerRemoved(input *pbresource.Resource, existing *pbresource.Resource) error { +// EnsureFinalizerRemoved ensures at least one finalizer was removed. +// TODO: only public for test to access +func EnsureFinalizerRemoved(input *pbresource.Resource, existing *pbresource.Resource) error { inputFinalizers := resource.GetFinalizers(input) existingFinalizers := resource.GetFinalizers(existing) if !inputFinalizers.IsProperSubset(existingFinalizers) { @@ -419,13 +420,13 @@ func vetIfDeleteRelated(input, existing *pbresource.Resource, tenancyMarkedForDe if err := ensureDataUnchanged(input, existing); err != nil { return err } - if err := ensureFinalizerRemoved(input, existing); err != nil { + if err := EnsureFinalizerRemoved(input, existing); err != nil { return err } } // Classify writes that just remove finalizer as deleteRelated regardless of deletion state. - if err := ensureFinalizerRemoved(input, existing); err == nil { + if err := EnsureFinalizerRemoved(input, existing); err == nil { if err := ensureDataUnchanged(input, existing); err == nil { deleteRelated = deleteRelated || true } diff --git a/agent/grpc-external/services/resource/write_status_test.go b/agent/grpc-external/services/resource/write_status_test.go index 1ddf738632..e84266673b 100644 --- a/agent/grpc-external/services/resource/write_status_test.go +++ b/agent/grpc-external/services/resource/write_status_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "fmt" @@ -16,11 +16,15 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" "github.com/hashicorp/consul/acl/resolver" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/proto-public/pbresource" ) +// TODO: Update all tests to use true/false table test for v2tenancy + func TestWriteStatus_ACL(t *testing.T) { type testCase struct { authz resolver.Result @@ -44,9 +48,8 @@ func TestWriteStatus_ACL(t *testing.T) { for desc, tc := range testcases { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + builder := svctest.NewResourceServiceBuilder().WithRegisterFns(demo.RegisterTypes) + client := builder.Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -56,10 +59,10 @@ func TestWriteStatus_ACL(t *testing.T) { artist = rsp.Resource // Defer mocking out authz since above write is necessary to set up the test resource. - mockACLResolver := &MockACLResolver{} + mockACLResolver := &svc.MockACLResolver{} mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). Return(tc.authz, nil) - server.ACLResolver = mockACLResolver + builder.ServiceImpl().Config.ACLResolver = mockACLResolver // exercise ACL _, err = client.WriteStatus(testContext(t), validWriteStatusRequest(t, artist)) @@ -69,9 +72,9 @@ func TestWriteStatus_ACL(t *testing.T) { } func TestWriteStatus_InputValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) testCases := map[string]struct { typ *pbresource.Type @@ -259,9 +262,9 @@ func TestWriteStatus_Success(t *testing.T) { "Non CAS": func(req *pbresource.WriteStatusRequest) { req.Version = "" }, } { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -331,9 +334,9 @@ func TestWriteStatus_Tenancy_Defaults(t *testing.T) { }, } { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Pick resource based on scope of type in testcase. var res *pbresource.Resource @@ -395,9 +398,10 @@ func TestWriteStatus_Tenancy_NotFound(t *testing.T) { }, } { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Pick resource based on scope of type in testcase. var res *pbresource.Resource @@ -428,10 +432,9 @@ func TestWriteStatus_Tenancy_NotFound(t *testing.T) { } func TestWriteStatus_CASFailure(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -449,8 +452,7 @@ func TestWriteStatus_CASFailure(t *testing.T) { } func TestWriteStatus_TypeNotFound(t *testing.T) { - server := testServer(t) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder().Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -464,9 +466,9 @@ func TestWriteStatus_TypeNotFound(t *testing.T) { } func TestWriteStatus_ResourceNotFound(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -479,9 +481,9 @@ func TestWriteStatus_ResourceNotFound(t *testing.T) { } func TestWriteStatus_WrongUid(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() 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 81392b8543..5c6c773210 100644 --- a/agent/grpc-external/services/resource/write_test.go +++ b/agent/grpc-external/services/resource/write_test.go @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc. // SPDX-License-Identifier: BUSL-1.1 -package resource +package resource_test import ( "context" @@ -18,6 +18,8 @@ import ( "google.golang.org/protobuf/types/known/anypb" "github.com/hashicorp/consul/acl/resolver" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" rtest "github.com/hashicorp/consul/internal/resource/resourcetest" @@ -29,10 +31,12 @@ import ( "github.com/hashicorp/consul/proto/private/prototest" ) +// TODO: Update all tests to use true/false table test for v2tenancy + func TestWrite_InputValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) type testCase struct { modFn func(artist, recordLabel *pbresource.Resource) *pbresource.Resource @@ -154,9 +158,9 @@ func TestWrite_InputValidation(t *testing.T) { } func TestWrite_OwnerValidation(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) type testCase struct { modReqFn func(req *pbresource.WriteRequest) @@ -230,8 +234,7 @@ func TestWrite_OwnerValidation(t *testing.T) { } func TestWrite_TypeNotFound(t *testing.T) { - server := testServer(t) - client := testClient(t, server) + client := svctest.NewResourceServiceBuilder().Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -265,14 +268,14 @@ func TestWrite_ACLs(t *testing.T) { for desc, tc := range testcases { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - mockACLResolver := &MockACLResolver{} + mockACLResolver := &svc.MockACLResolver{} mockACLResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything). Return(tc.authz, nil) - server.ACLResolver = mockACLResolver - demo.RegisterTypes(server.Registry) + + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + WithACLResolver(mockACLResolver). + Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -285,9 +288,9 @@ func TestWrite_ACLs(t *testing.T) { } func TestWrite_Mutate(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -389,9 +392,9 @@ func TestWrite_Create_Success(t *testing.T) { } for desc, tc := range testCases { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") require.NoError(t, err) @@ -442,9 +445,10 @@ func TestWrite_Create_Tenancy_NotFound(t *testing.T) { } for desc, tc := range testCases { t.Run(desc, func(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes). + Run(t) recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") require.NoError(t, err) @@ -461,9 +465,10 @@ func TestWrite_Create_Tenancy_NotFound(t *testing.T) { } func TestWrite_Create_With_DeletionTimestamp_Fails(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes). + Run(t) res := rtest.Resource(demo.TypeV1Artist, "blur"). WithTenancy(resource.DefaultNamespacedTenancy()). @@ -480,18 +485,18 @@ func TestWrite_Create_With_DeletionTimestamp_Fails(t *testing.T) { func TestWrite_Create_With_TenancyMarkedForDeletion_Fails(t *testing.T) { // Verify resource write fails when its partition or namespace is marked for deletion. testCases := map[string]struct { - modFn func(artist, recordLabel *pbresource.Resource, mockTenancyBridge *MockTenancyBridge) *pbresource.Resource + modFn func(artist, recordLabel *pbresource.Resource, mockTenancyBridge *svc.MockTenancyBridge) *pbresource.Resource errContains string }{ "namespaced resources partition marked for deletion": { - modFn: func(artist, _ *pbresource.Resource, mockTenancyBridge *MockTenancyBridge) *pbresource.Resource { + modFn: func(artist, _ *pbresource.Resource, mockTenancyBridge *svc.MockTenancyBridge) *pbresource.Resource { mockTenancyBridge.On("IsPartitionMarkedForDeletion", "ap1").Return(true, nil) return artist }, errContains: "tenancy marked for deletion", }, "namespaced resources namespace marked for deletion": { - modFn: func(artist, _ *pbresource.Resource, mockTenancyBridge *MockTenancyBridge) *pbresource.Resource { + modFn: func(artist, _ *pbresource.Resource, mockTenancyBridge *svc.MockTenancyBridge) *pbresource.Resource { mockTenancyBridge.On("IsPartitionMarkedForDeletion", "ap1").Return(false, nil) mockTenancyBridge.On("IsNamespaceMarkedForDeletion", "ap1", "ns1").Return(true, nil) return artist @@ -499,7 +504,7 @@ func TestWrite_Create_With_TenancyMarkedForDeletion_Fails(t *testing.T) { errContains: "tenancy marked for deletion", }, "partitioned resources partition marked for deletion": { - modFn: func(_, recordLabel *pbresource.Resource, mockTenancyBridge *MockTenancyBridge) *pbresource.Resource { + modFn: func(_, recordLabel *pbresource.Resource, mockTenancyBridge *svc.MockTenancyBridge) *pbresource.Resource { mockTenancyBridge.On("IsPartitionMarkedForDeletion", "ap1").Return(true, nil) return recordLabel }, @@ -511,6 +516,7 @@ func TestWrite_Create_With_TenancyMarkedForDeletion_Fails(t *testing.T) { server := testServer(t) client := testClient(t, server) demo.RegisterTypes(server.Registry) + recordLabel, err := demo.GenerateV1RecordLabel("looney-tunes") require.NoError(t, err) recordLabel.Id.Tenancy.Partition = "ap1" @@ -520,7 +526,7 @@ func TestWrite_Create_With_TenancyMarkedForDeletion_Fails(t *testing.T) { artist.Id.Tenancy.Partition = "ap1" artist.Id.Tenancy.Namespace = "ns1" - mockTenancyBridge := &MockTenancyBridge{} + mockTenancyBridge := &svc.MockTenancyBridge{} mockTenancyBridge.On("PartitionExists", "ap1").Return(true, nil) mockTenancyBridge.On("NamespaceExists", "ap1", "ns1").Return(true, nil) server.TenancyBridge = mockTenancyBridge @@ -534,10 +540,9 @@ func TestWrite_Create_With_TenancyMarkedForDeletion_Fails(t *testing.T) { } func TestWrite_CASUpdate_Success(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -556,10 +561,9 @@ func TestWrite_CASUpdate_Success(t *testing.T) { } func TestWrite_ResourceCreation_StatusProvided(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -575,10 +579,9 @@ func TestWrite_ResourceCreation_StatusProvided(t *testing.T) { } func TestWrite_CASUpdate_Failure(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -596,10 +599,9 @@ func TestWrite_CASUpdate_Failure(t *testing.T) { } func TestWrite_Update_WrongUid(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -617,10 +619,9 @@ func TestWrite_Update_WrongUid(t *testing.T) { } func TestWrite_Update_StatusModified(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -647,10 +648,9 @@ func TestWrite_Update_StatusModified(t *testing.T) { } func TestWrite_Update_NilStatus(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -671,10 +671,9 @@ func TestWrite_Update_NilStatus(t *testing.T) { } func TestWrite_Update_NoUid(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -690,10 +689,9 @@ func TestWrite_Update_NoUid(t *testing.T) { } func TestWrite_Update_GroupVersion(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -720,10 +718,9 @@ func TestWrite_Update_GroupVersion(t *testing.T) { } func TestWrite_NonCASUpdate_Success(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -743,7 +740,6 @@ func TestWrite_NonCASUpdate_Success(t *testing.T) { func TestWrite_NonCASUpdate_Retry(t *testing.T) { server := testServer(t) client := testClient(t, server) - demo.RegisterTypes(server.Registry) res, err := demo.GenerateV2Artist() @@ -788,10 +784,9 @@ func TestWrite_NonCASUpdate_Retry(t *testing.T) { } func TestWrite_NoData(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) res, err := demo.GenerateV1Concept("jazz") require.NoError(t, err) @@ -806,10 +801,9 @@ func TestWrite_Owner_Immutable(t *testing.T) { // Use of proto.Equal(..) in implementation covers all permutations // (nil -> non-nil, non-nil -> nil, owner1 -> owner2) so only the first one // is tested. - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) artist, err := demo.GenerateV2Artist() require.NoError(t, err) @@ -834,10 +828,9 @@ func TestWrite_Owner_Immutable(t *testing.T) { } func TestWrite_Owner_Uid(t *testing.T) { - server := testServer(t) - client := testClient(t, server) - - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) t.Run("uid given", func(t *testing.T) { artist, err := demo.GenerateV2Artist() @@ -991,7 +984,7 @@ func TestEnsureFinalizerRemoved(t *testing.T) { tc.mod(input, existing) - err := ensureFinalizerRemoved(input, existing) + err := svc.EnsureFinalizerRemoved(input, existing) if tc.errContains != "" { require.Error(t, err) require.Equal(t, codes.InvalidArgument.String(), status.Code(err).String()) @@ -1054,8 +1047,10 @@ func TestWrite_ResourceFrozenAfterMarkedForDeletion(t *testing.T) { for desc, tc := range testCases { t.Run(desc, func(t *testing.T) { - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Create a resource with finalizers res := rtest.Resource(demo.TypeV1Artist, "joydivision"). @@ -1065,11 +1060,11 @@ func TestWrite_ResourceFrozenAfterMarkedForDeletion(t *testing.T) { Write(t, client) // Mark for deletion - resource should now be frozen - _, err := client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) + _, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: res.Id}) require.NoError(t, err) // Verify marked for deletion - rsp, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) + rsp, err := client.Read(context.Background(), &pbresource.ReadRequest{Id: res.Id}) require.NoError(t, err) require.True(t, resource.IsMarkedForDeletion(rsp.Resource)) @@ -1077,7 +1072,7 @@ func TestWrite_ResourceFrozenAfterMarkedForDeletion(t *testing.T) { tc.modFn(rsp.Resource) // Verify write results - _, err = client.Write(ctx, &pbresource.WriteRequest{Resource: rsp.Resource}) + _, err = client.Write(context.Background(), &pbresource.WriteRequest{Resource: rsp.Resource}) if tc.errContains == "" { require.NoError(t, err) } else { @@ -1120,8 +1115,10 @@ func TestWrite_NonCASWritePreservesFinalizers(t *testing.T) { for desc, tc := range testCases { t.Run(desc, func(t *testing.T) { - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Create the resource based on tc.existingMetadata builder := rtest.Resource(demo.TypeV1Artist, "joydivision"). @@ -1148,7 +1145,7 @@ func TestWrite_NonCASWritePreservesFinalizers(t *testing.T) { userRes := builder.Build() // Perform the user write - rsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: userRes}) + rsp, err := client.Write(context.Background(), &pbresource.WriteRequest{Resource: userRes}) require.NoError(t, err) // Verify write result preserved metadata based on testcase.expecteMetadata @@ -1184,8 +1181,10 @@ func TestWrite_NonCASWritePreservesDeletionTimestamp(t *testing.T) { for desc, tc := range testCases { t.Run(desc, func(t *testing.T) { - server, client, ctx := testDeps(t) - demo.RegisterTypes(server.Registry) + client := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes). + Run(t) // Create the resource based on tc.existingMetadata builder := rtest.Resource(demo.TypeV1Artist, "joydivision"). @@ -1200,11 +1199,11 @@ func TestWrite_NonCASWritePreservesDeletionTimestamp(t *testing.T) { res := builder.Write(t, client) // Mark for deletion - _, err := client.Delete(ctx, &pbresource.DeleteRequest{Id: res.Id}) + _, err := client.Delete(context.Background(), &pbresource.DeleteRequest{Id: res.Id}) require.NoError(t, err) // Re-read the deleted res for future comparison of deletionTimestamp - delRsp, err := client.Read(ctx, &pbresource.ReadRequest{Id: res.Id}) + delRsp, err := client.Read(context.Background(), &pbresource.ReadRequest{Id: res.Id}) require.NoError(t, err) // Build resource for user write based on tc.inputMetadata @@ -1220,7 +1219,7 @@ func TestWrite_NonCASWritePreservesDeletionTimestamp(t *testing.T) { userRes := builder.Build() // Perform the non-CAS user write - rsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: userRes}) + rsp, err := client.Write(context.Background(), &pbresource.WriteRequest{Resource: userRes}) require.NoError(t, err) // Verify write result preserved metadata based on testcase.expecteMetadata diff --git a/internal/auth/internal/controllers/trafficpermissions/controller_test.go b/internal/auth/internal/controllers/trafficpermissions/controller_test.go index dde95cb76f..8f0fd99860 100644 --- a/internal/auth/internal/controllers/trafficpermissions/controller_test.go +++ b/internal/auth/internal/controllers/trafficpermissions/controller_test.go @@ -41,7 +41,11 @@ func (suite *controllerSuite) SetupTest() { suite.isEnterprise = versiontest.IsEnterprise() suite.tenancies = resourcetest.TestTenancies() suite.ctx = testutil.TestContext(suite.T()) - client := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register). + WithTenancies(suite.tenancies...). + Run(suite.T()) + suite.client = rtest.NewClient(client) suite.rt = controller.Runtime{ Client: suite.client, diff --git a/internal/catalog/catalogtest/run_test.go b/internal/catalog/catalogtest/run_test.go index 554900559e..f6e9610e0b 100644 --- a/internal/catalog/catalogtest/run_test.go +++ b/internal/catalog/catalogtest/run_test.go @@ -26,7 +26,9 @@ func runInMemResourceServiceAndControllers(t *testing.T, deps controllers.Depend ctx := testutil.TestContext(t) // Create the in-mem resource service - client := svctest.RunResourceService(t, catalog.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(catalog.RegisterTypes). + Run(t) // Setup/Run the controller manager mgr := controller.NewManager(client, testutil.Logger(t)) diff --git a/internal/catalog/internal/controllers/endpoints/controller_test.go b/internal/catalog/internal/controllers/endpoints/controller_test.go index 6bf54a1435..35c2352601 100644 --- a/internal/catalog/internal/controllers/endpoints/controller_test.go +++ b/internal/catalog/internal/controllers/endpoints/controller_test.go @@ -449,7 +449,10 @@ type controllerSuite struct { func (suite *controllerSuite) SetupTest() { suite.tenancies = resourcetest.TestTenancies() suite.ctx = testutil.TestContext(suite.T()) - client := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register). + WithTenancies(suite.tenancies...). + Run(suite.T()) suite.rt = controller.Runtime{ Client: client, Logger: testutil.Logger(suite.T()), diff --git a/internal/catalog/internal/controllers/endpoints/reconciliation_data_test.go b/internal/catalog/internal/controllers/endpoints/reconciliation_data_test.go index 96ddceb481..14c729e2cf 100644 --- a/internal/catalog/internal/controllers/endpoints/reconciliation_data_test.go +++ b/internal/catalog/internal/controllers/endpoints/reconciliation_data_test.go @@ -49,9 +49,11 @@ type reconciliationDataSuite struct { func (suite *reconciliationDataSuite) SetupTest() { suite.ctx = testutil.TestContext(suite.T()) - suite.tenancies = rtest.TestTenancies() - resourceClient := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register). + WithTenancies(suite.tenancies...). + Run(suite.T()) suite.client = resourcetest.NewClient(resourceClient) suite.rt = controller.Runtime{ Client: suite.client, diff --git a/internal/catalog/internal/controllers/failover/controller_test.go b/internal/catalog/internal/controllers/failover/controller_test.go index 4f1dbc3db6..30e19b4a17 100644 --- a/internal/catalog/internal/controllers/failover/controller_test.go +++ b/internal/catalog/internal/controllers/failover/controller_test.go @@ -36,17 +36,19 @@ type controllerSuite struct { } func (suite *controllerSuite) SetupTest() { - suite.ctx = testutil.TestContext(suite.T()) - client := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register, types.RegisterDNSPolicy) + suite.tenancies = rtest.TestTenancies() + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, types.RegisterDNSPolicy). + WithTenancies(suite.tenancies...). + Run(suite.T()) + suite.rt = controller.Runtime{ Client: client, Logger: testutil.Logger(suite.T()), } suite.client = rtest.NewClient(client) - suite.failoverMapper = failovermapper.New() - - suite.tenancies = rtest.TestTenancies() + suite.ctx = testutil.TestContext(suite.T()) } func (suite *controllerSuite) TestController() { diff --git a/internal/catalog/internal/controllers/nodehealth/controller_test.go b/internal/catalog/internal/controllers/nodehealth/controller_test.go index e228a2d6e3..33814b8243 100644 --- a/internal/catalog/internal/controllers/nodehealth/controller_test.go +++ b/internal/catalog/internal/controllers/nodehealth/controller_test.go @@ -14,7 +14,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - mockres "github.com/hashicorp/consul/agent/grpc-external/services/resource" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/catalog/internal/types" "github.com/hashicorp/consul/internal/controller" @@ -82,18 +81,12 @@ func (suite *nodeHealthControllerTestSuite) writeNode(name string, tenancy *pbre } func (suite *nodeHealthControllerTestSuite) SetupTest() { - mockTenancyBridge := &mockres.MockTenancyBridge{} suite.tenancies = resourcetest.TestTenancies() - for _, tenancy := range suite.tenancies { - mockTenancyBridge.On("PartitionExists", tenancy.Partition).Return(true, nil) - mockTenancyBridge.On("NamespaceExists", tenancy.Partition, tenancy.Namespace).Return(true, nil) - mockTenancyBridge.On("IsPartitionMarkedForDeletion", tenancy.Partition).Return(false, nil) - mockTenancyBridge.On("IsNamespaceMarkedForDeletion", tenancy.Partition, tenancy.Namespace).Return(false, nil) - } - cfg := mockres.Config{ - TenancyBridge: mockTenancyBridge, - } - client := svctest.RunResourceServiceWithConfig(suite.T(), cfg, types.Register, types.RegisterDNSPolicy) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, types.RegisterDNSPolicy). + WithTenancies(suite.tenancies...). + Run(suite.T()) + suite.resourceClient = resourcetest.NewClient(client) suite.runtime = controller.Runtime{Client: suite.resourceClient, Logger: testutil.Logger(suite.T())} suite.isEnterprise = versiontest.IsEnterprise() diff --git a/internal/catalog/internal/controllers/workloadhealth/controller_test.go b/internal/catalog/internal/controllers/workloadhealth/controller_test.go index 9e3d2283e8..fd15404cdf 100644 --- a/internal/catalog/internal/controllers/workloadhealth/controller_test.go +++ b/internal/catalog/internal/controllers/workloadhealth/controller_test.go @@ -15,7 +15,6 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/testing/protocmp" - svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/catalog/internal/controllers/nodehealth" "github.com/hashicorp/consul/internal/catalog/internal/mappers/nodemapper" @@ -93,15 +92,10 @@ type controllerSuite struct { func (suite *controllerSuite) SetupTest() { suite.tenancies = resourcetest.TestTenancies() - mockTenancyBridge := &svc.MockTenancyBridge{} - for _, tenancy := range suite.tenancies { - mockTenancyBridge.On("PartitionExists", tenancy.Partition).Return(true, nil) - mockTenancyBridge.On("IsPartitionMarkedForDeletion", tenancy.Partition).Return(false, nil) - mockTenancyBridge.On("NamespaceExists", tenancy.Partition, tenancy.Namespace).Return(true, nil) - mockTenancyBridge.On("IsNamespaceMarkedForDeletion", tenancy.Partition, tenancy.Namespace).Return(false, nil) - } - - suite.client = svctest.RunResourceServiceWithConfig(suite.T(), svc.Config{TenancyBridge: mockTenancyBridge}, types.Register) + suite.client = svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register). + WithTenancies(suite.tenancies...). + Run(suite.T()) suite.runtime = controller.Runtime{Client: suite.client, Logger: testutil.Logger(suite.T())} suite.isEnterprise = versiontest.IsEnterprise() } diff --git a/internal/controller/api_test.go b/internal/controller/api_test.go index e804585410..e6d2b8a034 100644 --- a/internal/controller/api_test.go +++ b/internal/controller/api_test.go @@ -23,7 +23,9 @@ func TestController_API(t *testing.T) { t.Parallel() rec := newTestReconciler() - client := svctest.RunResourceService(t, demo.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) concertsChan := make(chan controller.Event) defer close(concertsChan) @@ -164,7 +166,9 @@ func TestController_Placement(t *testing.T) { t.Run("singleton", func(t *testing.T) { rec := newTestReconciler() - client := svctest.RunResourceService(t, demo.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) ctrl := controller. ForType(demo.TypeV2Artist). @@ -197,7 +201,9 @@ func TestController_Placement(t *testing.T) { t.Run("each server", func(t *testing.T) { rec := newTestReconciler() - client := svctest.RunResourceService(t, demo.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) ctrl := controller. ForType(demo.TypeV2Artist). @@ -233,7 +239,10 @@ func TestController_String(t *testing.T) { } func TestController_NoReconciler(t *testing.T) { - client := svctest.RunResourceService(t, demo.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) + mgr := controller.NewManager(client, testutil.Logger(t)) ctrl := controller.ForType(demo.TypeV2Artist) @@ -248,7 +257,9 @@ func TestController_Watch(t *testing.T) { t.Run("partitioned scoped resources", func(t *testing.T) { rec := newTestReconciler() - client := svctest.RunResourceService(t, demo.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) ctrl := controller. ForType(demo.TypeV1RecordLabel). @@ -274,7 +285,9 @@ func TestController_Watch(t *testing.T) { t.Run("cluster scoped resources", func(t *testing.T) { rec := newTestReconciler() - client := svctest.RunResourceService(t, demo.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) ctrl := controller. ForType(demo.TypeV1Executive). @@ -299,7 +312,9 @@ func TestController_Watch(t *testing.T) { t.Run("namespace scoped resources", func(t *testing.T) { rec := newTestReconciler() - client := svctest.RunResourceService(t, demo.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) ctrl := controller. ForType(demo.TypeV2Artist). diff --git a/internal/mesh/internal/controllers/explicitdestinations/controller_test.go b/internal/mesh/internal/controllers/explicitdestinations/controller_test.go index 95d9bd108b..d23f884fd5 100644 --- a/internal/mesh/internal/controllers/explicitdestinations/controller_test.go +++ b/internal/mesh/internal/controllers/explicitdestinations/controller_test.go @@ -192,7 +192,10 @@ func TestFindDuplicates(t *testing.T) { func (suite *controllerTestSuite) SetupTest() { suite.tenancies = resourcetest.TestTenancies() - resourceClient := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register, catalog.RegisterTypes) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + WithTenancies(suite.tenancies...). + Run(suite.T()) suite.runtime = controller.Runtime{Client: resourceClient, Logger: testutil.Logger(suite.T())} suite.ctx = testutil.TestContext(suite.T()) suite.client = resourcetest.NewClient(resourceClient) diff --git a/internal/mesh/internal/controllers/proxyconfiguration/controller_test.go b/internal/mesh/internal/controllers/proxyconfiguration/controller_test.go index c8a6f585f4..fe53607568 100644 --- a/internal/mesh/internal/controllers/proxyconfiguration/controller_test.go +++ b/internal/mesh/internal/controllers/proxyconfiguration/controller_test.go @@ -53,7 +53,10 @@ type controllerTestSuite struct { func (suite *controllerTestSuite) SetupTest() { suite.tenancies = resourcetest.TestTenancies() - resourceClient := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register, catalog.RegisterTypes) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + WithTenancies(suite.tenancies...). + Run(suite.T()) suite.client = resourcetest.NewClient(resourceClient) suite.runtime = controller.Runtime{Client: resourceClient, Logger: testutil.Logger(suite.T())} suite.ctx = testutil.TestContext(suite.T()) diff --git a/internal/mesh/internal/controllers/proxyconfiguration/sort_test.go b/internal/mesh/internal/controllers/proxyconfiguration/sort_test.go index 1fbf8254ee..4345443b18 100644 --- a/internal/mesh/internal/controllers/proxyconfiguration/sort_test.go +++ b/internal/mesh/internal/controllers/proxyconfiguration/sort_test.go @@ -94,7 +94,9 @@ func TestSortProxyConfigurations(t *testing.T) { for name, c := range cases { t.Run(name, func(t *testing.T) { - resourceClient := svctest.RunResourceService(t, types.Register) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register). + Run(t) var decProxyCfgs []*types.DecodedProxyConfiguration for i, ws := range c.selectors { diff --git a/internal/mesh/internal/controllers/routes/controller_test.go b/internal/mesh/internal/controllers/routes/controller_test.go index f7fbfa4ccc..e8cb1d64c2 100644 --- a/internal/mesh/internal/controllers/routes/controller_test.go +++ b/internal/mesh/internal/controllers/routes/controller_test.go @@ -37,7 +37,11 @@ type controllerSuite struct { func (suite *controllerSuite) SetupTest() { suite.ctx = testutil.TestContext(suite.T()) suite.tenancies = rtest.TestTenancies() - client := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register, catalog.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + WithTenancies(suite.tenancies...). + Run(suite.T()) + suite.rt = controller.Runtime{ Client: client, Logger: testutil.Logger(suite.T()), diff --git a/internal/mesh/internal/controllers/routes/loader/loader_test.go b/internal/mesh/internal/controllers/routes/loader/loader_test.go index a665635d05..0db050fee6 100644 --- a/internal/mesh/internal/controllers/routes/loader/loader_test.go +++ b/internal/mesh/internal/controllers/routes/loader/loader_test.go @@ -7,11 +7,10 @@ import ( "testing" "time" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/durationpb" - pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v2beta1" + "github.com/hashicorp/go-hclog" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/catalog" @@ -21,6 +20,7 @@ import ( "github.com/hashicorp/consul/internal/resource" rtest "github.com/hashicorp/consul/internal/resource/resourcetest" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" + pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/proto/private/prototest" "github.com/hashicorp/consul/sdk/testutil" @@ -28,7 +28,9 @@ import ( func TestLoadResourcesForComputedRoutes(t *testing.T) { ctx := testutil.TestContext(t) - rclient := svctest.RunResourceService(t, types.Register, catalog.RegisterTypes) + rclient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + Run(t) rt := controller.Runtime{ Client: rclient, Logger: testutil.Logger(t), diff --git a/internal/mesh/internal/controllers/sidecarproxy/cache/cache_test.go b/internal/mesh/internal/controllers/sidecarproxy/cache/cache_test.go index f49feccb18..e0ade738e2 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/cache/cache_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/cache/cache_test.go @@ -5,7 +5,6 @@ package cache import ( "context" - "github.com/hashicorp/consul/internal/mesh/internal/controllers/routes/routestest" "testing" "github.com/stretchr/testify/require" @@ -13,6 +12,7 @@ import ( svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/catalog" "github.com/hashicorp/consul/internal/controller" + "github.com/hashicorp/consul/internal/mesh/internal/controllers/routes/routestest" "github.com/hashicorp/consul/internal/mesh/internal/types" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/resourcetest" @@ -82,7 +82,10 @@ func TestIdentities(t *testing.T) { func TestMapComputedTrafficPermissions(t *testing.T) { resourcetest.RunWithTenancies(func(tenancy *pbresource.Tenancy) { - client := svctest.RunResourceService(t, types.Register, catalog.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + Run(t) + ctp := resourcetest.Resource(pbauth.ComputedTrafficPermissionsType, "workload-identity-1"). WithTenancy(tenancy). WithData(t, &pbauth.ComputedTrafficPermissions{}). @@ -137,7 +140,9 @@ func TestUnified_AllMappingsToProxyStateTemplate(t *testing.T) { resourcetest.RunWithTenancies(func(tenancy *pbresource.Tenancy) { var ( cache = New() - client = svctest.RunResourceService(t, types.Register, catalog.RegisterTypes) + client = svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + Run(t) ) anyServiceData := &pbcatalog.Service{ diff --git a/internal/mesh/internal/controllers/sidecarproxy/controller_test.go b/internal/mesh/internal/controllers/sidecarproxy/controller_test.go index 5262c61fe8..8a2fe05a46 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/controller_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/controller_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - mockres "github.com/hashicorp/consul/agent/grpc-external/services/resource" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/envoyextensions/xdscommon" "github.com/hashicorp/consul/internal/auth" @@ -85,17 +84,11 @@ type apiData struct { func (suite *controllerTestSuite) SetupTest() { suite.tenancies = resourcetest.TestTenancies() - mockTenancyBridge := &mockres.MockTenancyBridge{} - for _, tenancy := range suite.tenancies { - mockTenancyBridge.On("PartitionExists", tenancy.Partition).Return(true, nil) - mockTenancyBridge.On("NamespaceExists", tenancy.Partition, tenancy.Namespace).Return(true, nil) - mockTenancyBridge.On("IsPartitionMarkedForDeletion", tenancy.Partition).Return(false, nil) - mockTenancyBridge.On("IsNamespaceMarkedForDeletion", tenancy.Partition, tenancy.Namespace).Return(false, nil) - } - cfg := mockres.Config{ - TenancyBridge: mockTenancyBridge, - } - resourceClient := svctest.RunResourceServiceWithConfig(suite.T(), cfg, types.Register, catalog.RegisterTypes, auth.RegisterTypes) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes, auth.RegisterTypes). + WithTenancies(suite.tenancies...). + Run(suite.T()) + suite.client = resourcetest.NewClient(resourceClient) suite.runtime = controller.Runtime{Client: resourceClient, Logger: testutil.Logger(suite.T())} suite.ctx = testutil.TestContext(suite.T()) diff --git a/internal/mesh/internal/controllers/sidecarproxy/fetcher/data_fetcher_test.go b/internal/mesh/internal/controllers/sidecarproxy/fetcher/data_fetcher_test.go index 401518e077..c9cdd54751 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/fetcher/data_fetcher_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/fetcher/data_fetcher_test.go @@ -53,8 +53,10 @@ type dataFetcherSuite struct { func (suite *dataFetcherSuite) SetupTest() { suite.ctx = testutil.TestContext(suite.T()) suite.tenancies = resourcetest.TestTenancies() - suite.tenancies = resourcetest.TestTenancies() - suite.client = svctest.RunResourceServiceWithTenancies(suite.T(), types.Register, catalog.RegisterTypes) + suite.client = svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + WithTenancies(suite.tenancies...). + Run(suite.T()) suite.resourceClient = resourcetest.NewClient(suite.client) suite.rt = controller.Runtime{ Client: suite.client, diff --git a/internal/mesh/internal/controllers/xds/controller_test.go b/internal/mesh/internal/controllers/xds/controller_test.go index f0aca598f0..6ef4eefd7f 100644 --- a/internal/mesh/internal/controllers/xds/controller_test.go +++ b/internal/mesh/internal/controllers/xds/controller_test.go @@ -70,7 +70,12 @@ type xdsControllerTestSuite struct { func (suite *xdsControllerTestSuite) SetupTest() { suite.ctx = testutil.TestContext(suite.T()) - resourceClient := svctest.RunResourceServiceWithTenancies(suite.T(), types.Register, catalog.RegisterTypes) + suite.tenancies = resourcetest.TestTenancies() + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + WithTenancies(suite.tenancies...). + Run(suite.T()) + suite.runtime = controller.Runtime{Client: resourceClient, Logger: testutil.Logger(suite.T())} suite.client = resourcetest.NewClient(resourceClient) suite.fetcher = mockFetcher @@ -100,8 +105,6 @@ func (suite *xdsControllerTestSuite) SetupTest() { leafCertEvents: suite.leafCertEvents, datacenter: "dc1", } - - suite.tenancies = resourcetest.TestTenancies() } func mockFetcher() (*pbproxystate.TrustBundle, error) { diff --git a/internal/mesh/internal/mappers/common/workload_selector_util_test.go b/internal/mesh/internal/mappers/common/workload_selector_util_test.go index 6f9e71e1af..6b178d8922 100644 --- a/internal/mesh/internal/mappers/common/workload_selector_util_test.go +++ b/internal/mesh/internal/mappers/common/workload_selector_util_test.go @@ -21,7 +21,9 @@ import ( ) func TestMapSelector(t *testing.T) { - client := svctest.RunResourceService(t, types.Register, catalog.RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + Run(t) // Create some workloads. // For this test, we don't care about the workload data, so we will re-use diff --git a/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper_test.go b/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper_test.go index 071be1f60d..5c90fa2463 100644 --- a/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper_test.go +++ b/internal/mesh/internal/mappers/workloadselectionmapper/workload_selection_mapper_test.go @@ -22,7 +22,10 @@ import ( ) func TestMapToComputedType(t *testing.T) { - resourceClient := svctest.RunResourceService(t, types.Register, catalog.RegisterTypes) + resourceClient := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + Run(t) + mapper := New[*pbmesh.ProxyConfiguration](pbmesh.ComputedProxyConfigurationType) workloadData := &pbcatalog.Workload{ diff --git a/internal/multicluster/internal/controllers/exportedservices/controller_test.go b/internal/multicluster/internal/controllers/exportedservices/controller_test.go index 1afec40760..667f4ef3a9 100644 --- a/internal/multicluster/internal/controllers/exportedservices/controller_test.go +++ b/internal/multicluster/internal/controllers/exportedservices/controller_test.go @@ -11,9 +11,8 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" - cat "github.com/hashicorp/consul/internal/catalog" + "github.com/hashicorp/consul/internal/catalog" "github.com/hashicorp/consul/internal/controller" "github.com/hashicorp/consul/internal/multicluster/internal/types" "github.com/hashicorp/consul/internal/resource" @@ -39,20 +38,12 @@ type controllerSuite struct { func (suite *controllerSuite) SetupTest() { suite.ctx = testutil.TestContext(suite.T()) suite.tenancies = rtest.TestTenancies() - mockTenancyBridge := &svc.MockTenancyBridge{} - for _, tenancy := range suite.tenancies { - mockTenancyBridge.On("PartitionExists", tenancy.Partition).Return(true, nil) - mockTenancyBridge.On("IsPartitionMarkedForDeletion", tenancy.Partition).Return(false, nil) - mockTenancyBridge.On("NamespaceExists", tenancy.Partition, tenancy.Namespace).Return(true, nil) - mockTenancyBridge.On("IsNamespaceMarkedForDeletion", tenancy.Partition, tenancy.Namespace).Return(false, nil) - mockTenancyBridge.On("NamespaceExists", tenancy.Partition, "app").Return(true, nil) - mockTenancyBridge.On("IsNamespaceMarkedForDeletion", tenancy.Partition, "app").Return(false, nil) - } + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(types.Register, catalog.RegisterTypes). + WithTenancies(suite.tenancies...). + WithTenancies(rtest.Tenancy("default.app"), rtest.Tenancy("foo.app")). + Run(suite.T()) - config := svc.Config{ - TenancyBridge: mockTenancyBridge, - } - client := svctest.RunResourceServiceWithConfig(suite.T(), config, types.Register, cat.RegisterTypes) suite.client = rtest.NewClient(client) suite.rt = controller.Runtime{ Client: suite.client, diff --git a/internal/resource/decode_test.go b/internal/resource/decode_test.go index 02e86fddfb..31db247c70 100644 --- a/internal/resource/decode_test.go +++ b/internal/resource/decode_test.go @@ -22,7 +22,7 @@ import ( func TestGetDecodedResource(t *testing.T) { var ( - baseClient = svctest.RunResourceService(t, demo.RegisterTypes) + baseClient = svctest.NewResourceServiceBuilder().WithRegisterFns(demo.RegisterTypes).Run(t) client = rtest.NewClient(baseClient) ctx = testutil.TestContext(t) ) diff --git a/internal/resource/demo/controller_test.go b/internal/resource/demo/controller_test.go index a8c2640d61..303ecfe235 100644 --- a/internal/resource/demo/controller_test.go +++ b/internal/resource/demo/controller_test.go @@ -16,7 +16,9 @@ import ( ) func TestArtistReconciler(t *testing.T) { - client := svctest.RunResourceService(t, RegisterTypes) + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(RegisterTypes). + Run(t) // Seed the database with an artist. res, err := GenerateV2Artist() diff --git a/internal/resource/http/http_test.go b/internal/resource/http/http_test.go index aeb85f0b83..aa36b8a38c 100644 --- a/internal/resource/http/http_test.go +++ b/internal/resource/http/http_test.go @@ -12,17 +12,17 @@ import ( "strings" "testing" - "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - resourceSvc "github.com/hashicorp/consul/agent/grpc-external/services/resource" - svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" - pbdemov1 "github.com/hashicorp/consul/proto/private/pbdemo/v1" + "github.com/hashicorp/go-hclog" + svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" + svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/proto-public/pbresource" + pbdemov1 "github.com/hashicorp/consul/proto/private/pbdemo/v1" pbdemov2 "github.com/hashicorp/consul/proto/private/pbdemo/v2" "github.com/hashicorp/consul/sdk/testutil" ) @@ -44,7 +44,11 @@ func TestResourceHandler_InputValidation(t *testing.T) { expectedResponseCode int responseBodyContains string } - client := svctest.RunResourceService(t, demo.RegisterTypes) + + client := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + Run(t) + resourceHandler := resourceHandler{ resource.Registration{ Type: demo.TypeV2Artist, @@ -125,17 +129,17 @@ func TestResourceHandler_InputValidation(t *testing.T) { } func TestResourceWriteHandler(t *testing.T) { - aclResolver := &resourceSvc.MockACLResolver{} + aclResolver := &svc.MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistReadPolicy, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, demo.ArtistV1ReadPolicy, demo.ArtistV2ReadPolicy), nil) aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, demo.ArtistV1WritePolicy, demo.ArtistV2WritePolicy), nil) - client := svctest.RunResourceServiceWithConfig(t, resourceSvc.Config{ACLResolver: aclResolver}, demo.RegisterTypes) - - r := resource.NewRegistry() - demo.RegisterTypes(r) - handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) + builder := svctest.NewResourceServiceBuilder(). + WithACLResolver(aclResolver). + WithRegisterFns(demo.RegisterTypes) + client := builder.Run(t) + handler := NewHandler(client, builder.Registry(), parseToken, hclog.NewNullLogger()) t.Run("should be blocked if the token is not authorized", func(t *testing.T) { rsp := httptest.NewRecorder() @@ -157,6 +161,7 @@ func TestResourceWriteHandler(t *testing.T) { require.Equal(t, http.StatusForbidden, rsp.Result().StatusCode) }) + var readRsp *pbresource.ReadResponse t.Run("should write to the resource backend", func(t *testing.T) { rsp := httptest.NewRecorder() @@ -352,7 +357,7 @@ func deleteResource(t *testing.T, artistHandler http.Handler, resourceUri *Resou } func TestResourceReadHandler(t *testing.T) { - aclResolver := &resourceSvc.MockACLResolver{} + aclResolver := &svc.MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistReadPolicy, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, demo.ArtistV1ReadPolicy, demo.ArtistV2ReadPolicy), nil) aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything). @@ -360,11 +365,11 @@ func TestResourceReadHandler(t *testing.T) { aclResolver.On("ResolveTokenAndDefaultMeta", fakeToken, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, ""), nil) - client := svctest.RunResourceServiceWithConfig(t, resourceSvc.Config{ACLResolver: aclResolver}, demo.RegisterTypes) - - r := resource.NewRegistry() - demo.RegisterTypes(r) - handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) + builder := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + WithACLResolver(aclResolver) + client := builder.Run(t) + handler := NewHandler(client, builder.Registry(), parseToken, hclog.NewNullLogger()) createdResource := createResource(t, handler, nil) @@ -407,18 +412,17 @@ func TestResourceReadHandler(t *testing.T) { } func TestResourceDeleteHandler(t *testing.T) { - aclResolver := &resourceSvc.MockACLResolver{} + aclResolver := &svc.MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistReadPolicy, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, demo.ArtistV2ReadPolicy), nil) aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, demo.ArtistV2WritePolicy), nil) - client := svctest.RunResourceServiceWithConfig(t, resourceSvc.Config{ACLResolver: aclResolver}, demo.RegisterTypes) - - r := resource.NewRegistry() - demo.RegisterTypes(r) - - handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) + builder := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + WithACLResolver(aclResolver) + client := builder.Run(t) + handler := NewHandler(client, builder.Registry(), parseToken, hclog.NewNullLogger()) t.Run("should surface PermissionDenied error from resource service", func(t *testing.T) { createResource(t, handler, nil) @@ -484,18 +488,17 @@ func TestResourceDeleteHandler(t *testing.T) { } func TestResourceListHandler(t *testing.T) { - aclResolver := &resourceSvc.MockACLResolver{} + aclResolver := &svc.MockACLResolver{} aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistListPolicy, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, demo.ArtistV2ListPolicy), nil) aclResolver.On("ResolveTokenAndDefaultMeta", testACLTokenArtistWritePolicy, mock.Anything, mock.Anything). Return(svctest.AuthorizerFrom(t, demo.ArtistV2WritePolicy), nil) - client := svctest.RunResourceServiceWithConfig(t, resourceSvc.Config{ACLResolver: aclResolver}, demo.RegisterTypes) - - r := resource.NewRegistry() - demo.RegisterTypes(r) - - handler := NewHandler(client, r, parseToken, hclog.NewNullLogger()) + builder := svctest.NewResourceServiceBuilder(). + WithRegisterFns(demo.RegisterTypes). + WithACLResolver(aclResolver) + client := builder.Run(t) + handler := NewHandler(client, builder.Registry(), parseToken, hclog.NewNullLogger()) t.Run("should return MethodNotAllowed", func(t *testing.T) { rsp := httptest.NewRecorder() diff --git a/internal/resource/reaper/controller_test.go b/internal/resource/reaper/controller_test.go index d78a105d21..58ed2564fe 100644 --- a/internal/resource/reaper/controller_test.go +++ b/internal/resource/reaper/controller_test.go @@ -4,7 +4,6 @@ package reaper import ( - "github.com/hashicorp/consul/internal/resource/resourcetest" "testing" "time" @@ -16,13 +15,14 @@ import ( "github.com/hashicorp/consul/internal/controller" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" + "github.com/hashicorp/consul/internal/resource/resourcetest" + rtest "github.com/hashicorp/consul/internal/resource/resourcetest" "github.com/hashicorp/consul/proto-public/pbresource" "github.com/hashicorp/consul/sdk/testutil" ) func TestReconcile_ResourceWithNoChildren(t *testing.T) { - client := svctest.RunResourceServiceWithTenancies(t, demo.RegisterTypes) - + client := setupResourceService(t) runReaperTestCaseWithTenancies(func(tenancy *pbresource.Tenancy) { // Seed the database with an artist. res, err := demo.GenerateV2Artist() @@ -79,8 +79,7 @@ func TestReconcile_ResourceWithNoChildren(t *testing.T) { } func TestReconcile_ResourceWithChildren(t *testing.T) { - client := svctest.RunResourceServiceWithTenancies(t, demo.RegisterTypes) - + client := setupResourceService(t) runReaperTestCaseWithTenancies(func(tenancy *pbresource.Tenancy) { // Seed the database with an artist res, err := demo.GenerateV2Artist() @@ -162,8 +161,7 @@ func TestReconcile_ResourceWithChildren(t *testing.T) { } func TestReconcile_RequeueWithDelayWhenSecondPassDelayNotElapsed(t *testing.T) { - client := svctest.RunResourceServiceWithTenancies(t, demo.RegisterTypes) - + client := setupResourceService(t) runReaperTestCaseWithTenancies(func(tenancy *pbresource.Tenancy) { // Seed the database with an artist. res, err := demo.GenerateV2Artist() @@ -216,3 +214,10 @@ func runReaperTestCaseWithTenancies(testCase func(tenancy *pbresource.Tenancy)) testCase(tenancy) } } + +func setupResourceService(t *testing.T) pbresource.ResourceServiceClient { + return svctest.NewResourceServiceBuilder(). + WithTenancies(rtest.TestTenancies()...). + WithRegisterFns(demo.RegisterTypes). + Run(t) +} diff --git a/internal/tenancy/tenancytest/namespace_controller_test.go b/internal/tenancy/tenancytest/namespace_controller_test.go index 886224ed94..574ee92cd4 100644 --- a/internal/tenancy/tenancytest/namespace_controller_test.go +++ b/internal/tenancy/tenancytest/namespace_controller_test.go @@ -10,13 +10,11 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/controller" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/resource/demo" rtest "github.com/hashicorp/consul/internal/resource/resourcetest" - "github.com/hashicorp/consul/internal/tenancy" "github.com/hashicorp/consul/internal/tenancy/internal/controllers/common" "github.com/hashicorp/consul/internal/tenancy/internal/controllers/namespace" "github.com/hashicorp/consul/proto-public/pbresource" @@ -31,24 +29,21 @@ import ( type nsTestSuite struct { suite.Suite - client *rtest.Client - runtime controller.Runtime - ctx context.Context - registry resource.Registry + client *rtest.Client + runtime controller.Runtime + ctx context.Context } func (ts *nsTestSuite) SetupTest() { - ts.registry = resource.NewRegistry() - tenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: tenancyBridge, Registry: ts.registry, UseV2Tenancy: true} - resourceClient := svctest.RunResourceServiceWithConfig(ts.T(), config, tenancy.RegisterTypes, demo.RegisterTypes) - tenancyBridge.WithClient(resourceClient) - ts.client = rtest.NewClient(resourceClient) - ts.runtime = controller.Runtime{Client: resourceClient, Logger: testutil.Logger(ts.T())} + builder := svctest.NewResourceServiceBuilder(). + WithV2Tenancy(true). + WithRegisterFns(demo.RegisterTypes) + ts.client = rtest.NewClient(builder.Run(ts.T())) + ts.runtime = controller.Runtime{Client: ts.client, Logger: testutil.Logger(ts.T())} ts.ctx = testutil.TestContext(ts.T()) mgr := controller.NewManager(ts.client, testutil.Logger(ts.T())) - mgr.Register(namespace.Controller(ts.registry)) + mgr.Register(namespace.Controller(builder.Registry())) mgr.SetRaftLeader(true) ctx, cancel := context.WithCancel(context.Background()) ts.T().Cleanup(cancel) diff --git a/internal/tenancy/tenancytest/namespace_test.go b/internal/tenancy/tenancytest/namespace_test.go index e2461c254c..45beca9a6b 100644 --- a/internal/tenancy/tenancytest/namespace_test.go +++ b/internal/tenancy/tenancytest/namespace_test.go @@ -11,21 +11,16 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - svc "github.com/hashicorp/consul/agent/grpc-external/services/resource" svctest "github.com/hashicorp/consul/agent/grpc-external/services/resource/testing" "github.com/hashicorp/consul/internal/resource" rtest "github.com/hashicorp/consul/internal/resource/resourcetest" - "github.com/hashicorp/consul/internal/tenancy" "github.com/hashicorp/consul/proto-public/pbresource" pbtenancy "github.com/hashicorp/consul/proto-public/pbtenancy/v2beta1" "github.com/hashicorp/consul/proto/private/prototest" ) func TestWriteNamespace_Success(t *testing.T) { - v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} - client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) - cl := rtest.NewClient(client) + cl := rtest.NewClient(svctest.NewResourceServiceBuilder().WithV2Tenancy(true).Run(t)) res := rtest.Resource(pbtenancy.NamespaceType, "ns1"). WithTenancy(resource.DefaultPartitionedTenancy()). @@ -41,10 +36,7 @@ func TestWriteNamespace_Success(t *testing.T) { } func TestReadNamespace_Success(t *testing.T) { - v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} - client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) - cl := rtest.NewClient(client) + cl := rtest.NewClient(svctest.NewResourceServiceBuilder().WithV2Tenancy(true).Run(t)) res := rtest.Resource(pbtenancy.NamespaceType, "ns1"). WithData(t, validNamespace()). @@ -74,10 +66,7 @@ func TestReadNamespace_Success(t *testing.T) { } func TestDeleteNamespace_Success(t *testing.T) { - v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} - client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) - cl := rtest.NewClient(client) + cl := rtest.NewClient(svctest.NewResourceServiceBuilder().WithV2Tenancy(true).Run(t)) res := rtest.Resource(pbtenancy.NamespaceType, "ns1"). WithData(t, validNamespace()).Write(t, cl) @@ -96,10 +85,7 @@ func TestDeleteNamespace_Success(t *testing.T) { } func TestListNamespace_Success(t *testing.T) { - v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} - client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) - cl := rtest.NewClient(client) + cl := rtest.NewClient(svctest.NewResourceServiceBuilder().WithV2Tenancy(true).Run(t)) res := rtest.Resource(pbtenancy.NamespaceType, "ns1"). WithData(t, validNamespace()).Write(t, cl)