Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource: resource service now checks for v2tenancy feature flag #19400

Merged
merged 1 commit into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion agent/consul/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down
22 changes: 17 additions & 5 deletions agent/consul/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"crypto/x509"
"errors"
"fmt"
"github.com/hashicorp/consul/internal/tenancy"
"io"
"net"
"os"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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())

Expand Down Expand Up @@ -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)
}
Expand All @@ -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{
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions agent/grpc-external/services/resource/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions agent/grpc-external/services/resource/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions agent/grpc-external/services/resource/list_by_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions agent/grpc-external/services/resource/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 5 additions & 0 deletions agent/grpc-external/services/resource/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions agent/grpc-external/services/resource/server_ce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
6 changes: 4 additions & 2 deletions agent/grpc-external/services/resource/testing/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions agent/grpc-external/services/resource/testing/testing_ce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions agent/grpc-external/services/resource/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions agent/grpc-external/services/resource/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions internal/tenancy/exports.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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()
}
14 changes: 14 additions & 0 deletions internal/tenancy/internal/controllers/register_ce.go
Original file line number Diff line number Diff line change
@@ -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())
}
6 changes: 0 additions & 6 deletions internal/tenancy/internal/types/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
30 changes: 4 additions & 26 deletions internal/tenancy/internal/types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading