From 0abd96c0d9b8c1c1cee684b322b98a6a626d67d3 Mon Sep 17 00:00:00 2001 From: Semir Patel Date: Fri, 27 Oct 2023 08:55:02 -0500 Subject: [PATCH] resource: resource service now checks for `v2tenancy` feature flag (#19400) --- agent/consul/options.go | 11 ++++++- agent/consul/server.go | 22 ++++++++++---- .../grpc-external/services/resource/delete.go | 4 +++ agent/grpc-external/services/resource/list.go | 4 +++ .../services/resource/list_by_owner.go | 4 +++ agent/grpc-external/services/resource/read.go | 4 +++ .../grpc-external/services/resource/server.go | 5 ++++ .../services/resource/server_ce.go | 13 ++++++++ .../services/resource/testing/testing.go | 6 ++-- .../services/resource/testing/testing_ce.go | 8 +++-- .../grpc-external/services/resource/watch.go | 4 +++ .../grpc-external/services/resource/write.go | 4 +++ internal/tenancy/exports.go | 8 +++++ .../internal/controllers/register_ce.go | 14 +++++++++ internal/tenancy/internal/types/namespace.go | 6 ---- internal/tenancy/internal/types/types_test.go | 30 +++---------------- .../tenancy/tenancytest/namespace_test.go | 23 +++++++------- 17 files changed, 115 insertions(+), 55 deletions(-) create mode 100644 internal/tenancy/internal/controllers/register_ce.go diff --git a/agent/consul/options.go b/agent/consul/options.go index 6dc754b3ae..88c16bd1a9 100644 --- a/agent/consul/options.go +++ b/agent/consul/options.go @@ -49,7 +49,7 @@ type Deps struct { EnterpriseDeps } -// useV2Resources returns true if "resource-apis" is present in the Experiments +// UseV2Resources returns true if "resource-apis" is present in the Experiments // array of the agent config. func (d Deps) UseV2Resources() bool { if stringslice.Contains(d.Experiments, CatalogResourceExperimentName) { @@ -58,6 +58,15 @@ func (d Deps) UseV2Resources() bool { return false } +// UseV2Tenancy returns true if "v2tenancy" is present in the Experiments +// array of the agent config. +func (d Deps) UseV2Tenancy() bool { + if stringslice.Contains(d.Experiments, V2TenancyExperimentName) { + return true + } + return false +} + type GRPCClientConner interface { ClientConn(datacenter string) (*grpc.ClientConn, error) ClientConnLeader() (*grpc.ClientConn, error) diff --git a/agent/consul/server.go b/agent/consul/server.go index 73a26bd805..0dfe48b4bf 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -8,7 +8,6 @@ import ( "crypto/x509" "errors" "fmt" - "github.com/hashicorp/consul/internal/tenancy" "io" "net" "os" @@ -80,6 +79,7 @@ import ( "github.com/hashicorp/consul/internal/resource/demo" "github.com/hashicorp/consul/internal/resource/reaper" raftstorage "github.com/hashicorp/consul/internal/storage/raft" + "github.com/hashicorp/consul/internal/tenancy" "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/lib/routine" "github.com/hashicorp/consul/lib/stringslice" @@ -468,6 +468,9 @@ type Server struct { registry resource.Registry useV2Resources bool + + // useV2Tenancy is tied to the "v2tenancy" feature flag. + useV2Tenancy bool } func (s *Server) DecrementBlockingQueries() uint64 { @@ -557,6 +560,7 @@ func NewServer(config *Config, flat Deps, externalGRPCServer *grpc.Server, routineManager: routine.NewManager(logger.Named(logging.ConsulServer)), registry: flat.Registry, useV2Resources: flat.UseV2Resources(), + useV2Tenancy: flat.UseV2Tenancy(), } incomingRPCLimiter.Register(s) @@ -834,7 +838,7 @@ func NewServer(config *Config, flat Deps, externalGRPCServer *grpc.Server, go s.reportingManager.Run(&lib.StopChannelContext{StopCh: s.shutdownCh}) // Setup insecure resource service client. - if err := s.setupInsecureResourceServiceClient(flat.Registry, logger, flat); err != nil { + if err := s.setupInsecureResourceServiceClient(flat.Registry, logger); err != nil { return nil, err } @@ -931,6 +935,11 @@ func isV1CatalogRequest(rpcName string) bool { } func (s *Server) registerControllers(deps Deps, proxyUpdater ProxyUpdater) error { + // When not enabled, the v1 tenancy bridge is used by default. + if s.useV2Tenancy { + tenancy.RegisterControllers(s.controllerManager) + } + if s.useV2Resources { catalog.RegisterControllers(s.controllerManager, catalog.DefaultControllerDependencies()) @@ -1456,7 +1465,7 @@ func (s *Server) setupExternalGRPC(config *Config, deps Deps, logger hclog.Logge s.peerStreamServer.Register(s.externalGRPCServer) tenancyBridge := NewV1TenancyBridge(s) - if stringslice.Contains(deps.Experiments, V2TenancyExperimentName) { + if s.useV2Tenancy { tenancyBridgeV2 := tenancy.NewV2TenancyBridge() tenancyBridge = tenancyBridgeV2.WithClient(s.insecureResourceServiceClient) } @@ -1467,20 +1476,22 @@ func (s *Server) setupExternalGRPC(config *Config, deps Deps, logger hclog.Logge ACLResolver: s.ACLResolver, Logger: logger.Named("grpc-api.resource"), TenancyBridge: tenancyBridge, + UseV2Tenancy: s.useV2Tenancy, }) s.resourceServiceServer.Register(s.externalGRPCServer) reflection.Register(s.externalGRPCServer) } -func (s *Server) setupInsecureResourceServiceClient(typeRegistry resource.Registry, logger hclog.Logger, deps Deps) error { +func (s *Server) setupInsecureResourceServiceClient(typeRegistry resource.Registry, logger hclog.Logger) error { if s.raftStorageBackend == nil { return fmt.Errorf("raft storage backend cannot be nil") } + // Can't use interface type var here since v2 specific "WithClient(...)" is called futher down. tenancyBridge := NewV1TenancyBridge(s) tenancyBridgeV2 := tenancy.NewV2TenancyBridge() - if stringslice.Contains(deps.Experiments, V2TenancyExperimentName) { + if s.useV2Tenancy { tenancyBridge = tenancyBridgeV2 } server := resourcegrpc.NewServer(resourcegrpc.Config{ @@ -1489,6 +1500,7 @@ func (s *Server) setupInsecureResourceServiceClient(typeRegistry resource.Regist ACLResolver: resolver.DANGER_NO_AUTH{}, Logger: logger.Named("grpc-api.resource"), TenancyBridge: tenancyBridge, + UseV2Tenancy: s.useV2Tenancy, }) conn, err := s.runInProcessGRPCServer(server.Register) diff --git a/agent/grpc-external/services/resource/delete.go b/agent/grpc-external/services/resource/delete.go index a2d3bec995..f19d4a5249 100644 --- a/agent/grpc-external/services/resource/delete.go +++ b/agent/grpc-external/services/resource/delete.go @@ -159,6 +159,10 @@ func (s *Server) validateDeleteRequest(req *pbresource.DeleteRequest) (*resource return nil, err } + if err = checkV2Tenancy(s.UseV2Tenancy, req.Id.Type); err != nil { + return nil, err + } + // Check scope if reg.Scope == resource.ScopePartition && req.Id.Tenancy.Namespace != "" { return nil, status.Errorf( diff --git a/agent/grpc-external/services/resource/list.go b/agent/grpc-external/services/resource/list.go index befb619eec..8bdfc4fb3c 100644 --- a/agent/grpc-external/services/resource/list.go +++ b/agent/grpc-external/services/resource/list.go @@ -100,6 +100,10 @@ func (s *Server) validateListRequest(req *pbresource.ListRequest) (*resource.Reg return nil, err } + if err = checkV2Tenancy(s.UseV2Tenancy, req.Type); err != nil { + return nil, err + } + if err := validateWildcardTenancy(req.Tenancy, req.NamePrefix); err != nil { return nil, err } diff --git a/agent/grpc-external/services/resource/list_by_owner.go b/agent/grpc-external/services/resource/list_by_owner.go index dba7bc23d4..1f69787901 100644 --- a/agent/grpc-external/services/resource/list_by_owner.go +++ b/agent/grpc-external/services/resource/list_by_owner.go @@ -105,6 +105,10 @@ func (s *Server) validateListByOwnerRequest(req *pbresource.ListByOwnerRequest) return nil, err } + if err = checkV2Tenancy(s.UseV2Tenancy, req.Owner.Type); err != nil { + return nil, err + } + // Error when partition scoped and namespace not empty. if reg.Scope == resource.ScopePartition && req.Owner.Tenancy.Namespace != "" { return nil, status.Errorf( diff --git a/agent/grpc-external/services/resource/read.go b/agent/grpc-external/services/resource/read.go index cab8a2d6f2..3c413bc138 100644 --- a/agent/grpc-external/services/resource/read.go +++ b/agent/grpc-external/services/resource/read.go @@ -102,6 +102,10 @@ func (s *Server) validateReadRequest(req *pbresource.ReadRequest) (*resource.Reg return nil, err } + if err = checkV2Tenancy(s.UseV2Tenancy, req.Id.Type); err != nil { + return nil, err + } + // Check scope if reg.Scope == resource.ScopePartition && req.Id.Tenancy.Namespace != "" { return nil, status.Errorf( diff --git a/agent/grpc-external/services/resource/server.go b/agent/grpc-external/services/resource/server.go index 0b6419fe6f..88237633ed 100644 --- a/agent/grpc-external/services/resource/server.go +++ b/agent/grpc-external/services/resource/server.go @@ -35,6 +35,11 @@ type Config struct { // TenancyBridge temporarily allows us to use V1 implementations of // partitions and namespaces until V2 implementations are available. TenancyBridge TenancyBridge + + // UseV2Tenancy is true if the "v2tenancy" experiement is active, false otherwise. + // Attempts to create v2 tenancy resources (partition or namespace) will fail when the + // flag is false. + UseV2Tenancy bool } //go:generate mockery --name Registry --inpackage diff --git a/agent/grpc-external/services/resource/server_ce.go b/agent/grpc-external/services/resource/server_ce.go index bc48194574..2e3f792fe1 100644 --- a/agent/grpc-external/services/resource/server_ce.go +++ b/agent/grpc-external/services/resource/server_ce.go @@ -6,9 +6,13 @@ package resource import ( + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/proto-public/pbresource" + pbtenancy "github.com/hashicorp/consul/proto-public/pbtenancy/v2beta1" ) func v2TenancyToV1EntMeta(tenancy *pbresource.Tenancy) *acl.EnterpriseMeta { @@ -24,3 +28,12 @@ func v1EntMetaToV2Tenancy(reg *resource.Registration, entMeta *acl.EnterpriseMet tenancy.Namespace = entMeta.NamespaceOrDefault() } } + +// checkV2Tenancy returns FailedPrecondition error for namespace resource type +// when the "v2tenancy" feature flag is not enabled. +func checkV2Tenancy(useV2Tenancy bool, rtype *pbresource.Type) error { + if resource.EqualType(rtype, pbtenancy.NamespaceType) && !useV2Tenancy { + return status.Errorf(codes.FailedPrecondition, "use of the v2 namespace resource requires the \"v2tenancy\" feature flag") + } + return nil +} diff --git a/agent/grpc-external/services/resource/testing/testing.go b/agent/grpc-external/services/resource/testing/testing.go index e7c3fb7a05..c9f03bea1a 100644 --- a/agent/grpc-external/services/resource/testing/testing.go +++ b/agent/grpc-external/services/resource/testing/testing.go @@ -5,13 +5,13 @@ package testing import ( "context" - "github.com/hashicorp/consul/internal/tenancy" + "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" - "testing" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/acl/resolver" @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/consul/agent/structs" "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" ) @@ -93,6 +94,7 @@ func RunResourceServiceWithConfig(t *testing.T, config svc.Config, registerFns . 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) diff --git a/agent/grpc-external/services/resource/testing/testing_ce.go b/agent/grpc-external/services/resource/testing/testing_ce.go index 2d1c22b561..da20be3533 100644 --- a/agent/grpc-external/services/resource/testing/testing_ce.go +++ b/agent/grpc-external/services/resource/testing/testing_ce.go @@ -8,15 +8,17 @@ package testing import ( "context" "errors" + "time" + + "github.com/oklog/ulid/v2" + "google.golang.org/protobuf/types/known/anypb" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/storage" "github.com/hashicorp/consul/internal/storage/inmem" "github.com/hashicorp/consul/proto-public/pbresource" pbtenancy "github.com/hashicorp/consul/proto-public/pbtenancy/v2beta1" - "github.com/oklog/ulid/v2" - "google.golang.org/protobuf/types/known/anypb" - "time" ) func FillEntMeta(entMeta *acl.EnterpriseMeta) { diff --git a/agent/grpc-external/services/resource/watch.go b/agent/grpc-external/services/resource/watch.go index 44b0a83caa..a984194ca2 100644 --- a/agent/grpc-external/services/resource/watch.go +++ b/agent/grpc-external/services/resource/watch.go @@ -110,6 +110,10 @@ func (s *Server) validateWatchListRequest(req *pbresource.WatchListRequest) (*re return nil, err } + if err = checkV2Tenancy(s.UseV2Tenancy, req.Type); err != nil { + return nil, err + } + if err := validateWildcardTenancy(req.Tenancy, req.NamePrefix); err != nil { return nil, err } diff --git a/agent/grpc-external/services/resource/write.go b/agent/grpc-external/services/resource/write.go index aacd1ec859..7b1bd8a73d 100644 --- a/agent/grpc-external/services/resource/write.go +++ b/agent/grpc-external/services/resource/write.go @@ -294,6 +294,10 @@ func (s *Server) validateWriteRequest(req *pbresource.WriteRequest) (*resource.R return nil, err } + if err = checkV2Tenancy(s.UseV2Tenancy, req.Resource.Id.Type); err != nil { + return nil, err + } + // Check scope if reg.Scope == resource.ScopePartition && req.Resource.Id.Tenancy.Namespace != "" { return nil, status.Errorf( diff --git a/internal/tenancy/exports.go b/internal/tenancy/exports.go index c07b25903c..806e85b7b0 100644 --- a/internal/tenancy/exports.go +++ b/internal/tenancy/exports.go @@ -4,8 +4,10 @@ package tenancy import ( + "github.com/hashicorp/consul/internal/controller" "github.com/hashicorp/consul/internal/resource" "github.com/hashicorp/consul/internal/tenancy/internal/bridge" + "github.com/hashicorp/consul/internal/tenancy/internal/controllers" "github.com/hashicorp/consul/internal/tenancy/internal/types" ) @@ -19,6 +21,12 @@ func RegisterTypes(r resource.Registry) { types.Register(r) } +// RegisterControllers registers controllers for the tenancy types with +// the given controller manager. +func RegisterControllers(mgr *controller.Manager) { + controllers.Register(mgr) +} + func NewV2TenancyBridge() *V2TenancyBridge { return bridge.NewV2TenancyBridge() } diff --git a/internal/tenancy/internal/controllers/register_ce.go b/internal/tenancy/internal/controllers/register_ce.go new file mode 100644 index 0000000000..324f1bcfc0 --- /dev/null +++ b/internal/tenancy/internal/controllers/register_ce.go @@ -0,0 +1,14 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build !consulent + +package controllers + +import ( + "github.com/hashicorp/consul/internal/controller" +) + +func Register(mgr *controller.Manager) { + //mgr.Register(namespace.NamespaceController()) +} diff --git a/internal/tenancy/internal/types/namespace.go b/internal/tenancy/internal/types/namespace.go index 88bf215125..c45b405e8b 100644 --- a/internal/tenancy/internal/types/namespace.go +++ b/internal/tenancy/internal/types/namespace.go @@ -19,16 +19,10 @@ func RegisterNamespace(r resource.Registry) { Proto: &pbtenancy.Namespace{}, Scope: resource.ScopePartition, Validate: ValidateNamespace, - Mutate: MutateNamespace, // ACLs: TODO }) } -func MutateNamespace(res *pbresource.Resource) error { - res.Id.Name = strings.ToLower(res.Id.Name) - return nil -} - func ValidateNamespace(res *pbresource.Resource) error { var ns pbtenancy.Namespace diff --git a/internal/tenancy/internal/types/types_test.go b/internal/tenancy/internal/types/types_test.go index a82d5b9e6c..df22b71f13 100644 --- a/internal/tenancy/internal/types/types_test.go +++ b/internal/tenancy/internal/types/types_test.go @@ -4,16 +4,16 @@ package types import ( - "errors" "testing" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/known/anypb" + "github.com/hashicorp/consul/internal/resource" pbcatalog "github.com/hashicorp/consul/proto-public/pbcatalog/v2beta1" "github.com/hashicorp/consul/proto-public/pbresource" pbtenancy "github.com/hashicorp/consul/proto-public/pbtenancy/v2beta1" - "github.com/stretchr/testify/require" - "google.golang.org/protobuf/reflect/protoreflect" - "google.golang.org/protobuf/types/known/anypb" ) func createNamespaceResource(t *testing.T, data protoreflect.ProtoMessage) *pbresource.Resource { @@ -86,28 +86,6 @@ func TestValidateNamespace_ParseError(t *testing.T) { require.ErrorAs(t, err, &resource.ErrDataParse{}) } -func TestMutateNamespace(t *testing.T) { - tests := []struct { - name string - namespaceName string - expectedName string - err error - }{ - {"lower", "lower", "lower", nil}, - {"mixed", "MiXeD", "mixed", nil}, - {"upper", "UPPER", "upper", nil}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - res := &pbresource.Resource{Id: &pbresource.ID{Name: tt.namespaceName}} - if err := MutateNamespace(res); !errors.Is(err, tt.err) { - t.Errorf("MutateNamespace() error = %v", err) - } - require.Equal(t, res.Id.Name, tt.expectedName) - }) - } -} - func TestValidateNamespace(t *testing.T) { tests := []struct { name string diff --git a/internal/tenancy/tenancytest/namespace_test.go b/internal/tenancy/tenancytest/namespace_test.go index 14ee7942ad..e2461c254c 100644 --- a/internal/tenancy/tenancytest/namespace_test.go +++ b/internal/tenancy/tenancytest/namespace_test.go @@ -7,24 +7,23 @@ import ( "context" "testing" - 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/tenancy" - - rtest "github.com/hashicorp/consul/internal/resource/resourcetest" - "github.com/hashicorp/consul/proto/private/prototest" + "github.com/stretchr/testify/require" "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/stretchr/testify/require" + "github.com/hashicorp/consul/proto/private/prototest" ) func TestWriteNamespace_Success(t *testing.T) { v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge} + config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) cl := rtest.NewClient(client) @@ -43,7 +42,7 @@ func TestWriteNamespace_Success(t *testing.T) { func TestReadNamespace_Success(t *testing.T) { v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge} + config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) cl := rtest.NewClient(client) @@ -76,7 +75,7 @@ func TestReadNamespace_Success(t *testing.T) { func TestDeleteNamespace_Success(t *testing.T) { v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge} + config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) cl := rtest.NewClient(client) @@ -98,7 +97,7 @@ func TestDeleteNamespace_Success(t *testing.T) { func TestListNamespace_Success(t *testing.T) { v2TenancyBridge := tenancy.NewV2TenancyBridge() - config := svc.Config{TenancyBridge: v2TenancyBridge} + config := svc.Config{TenancyBridge: v2TenancyBridge, UseV2Tenancy: true} client := svctest.RunResourceServiceWithConfig(t, config, tenancy.RegisterTypes) cl := rtest.NewClient(client)