Skip to content

Commit

Permalink
tenancy bridge v2 for v2 resources
Browse files Browse the repository at this point in the history
  • Loading branch information
dhiaayachi committed Oct 20, 2023
1 parent 1280f45 commit b7f899e
Show file tree
Hide file tree
Showing 16 changed files with 264 additions and 112 deletions.
9 changes: 6 additions & 3 deletions agent/consul/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/x509"
"errors"
"fmt"
"github.com/hashicorp/consul/internal/tenancy"
"io"
"net"
"os"
Expand Down Expand Up @@ -1456,7 +1457,8 @@ func (s *Server) setupExternalGRPC(config *Config, deps Deps, logger hclog.Logge

tenancyBridge := NewV1TenancyBridge(s)
if stringslice.Contains(deps.Experiments, V2TenancyExperimentName) {
tenancyBridge = resource.NewV2TenancyBridge()
tenancyBridgeV2 := tenancy.NewV2TenancyBridge()
tenancyBridge = tenancyBridgeV2.WithClient(s.insecureResourceServiceClient)
}

s.resourceServiceServer = resourcegrpc.NewServer(resourcegrpc.Config{
Expand All @@ -1477,8 +1479,9 @@ func (s *Server) setupInsecureResourceServiceClient(typeRegistry resource.Regist
}

tenancyBridge := NewV1TenancyBridge(s)
tenancyBridgeV2 := tenancy.NewV2TenancyBridge()
if stringslice.Contains(deps.Experiments, V2TenancyExperimentName) {
tenancyBridge = resource.NewV2TenancyBridge()
tenancyBridge = tenancyBridgeV2
}
server := resourcegrpc.NewServer(resourcegrpc.Config{
Registry: typeRegistry,
Expand All @@ -1493,7 +1496,7 @@ func (s *Server) setupInsecureResourceServiceClient(typeRegistry resource.Regist
return err
}
s.insecureResourceServiceClient = pbresource.NewResourceServiceClient(conn)

tenancyBridgeV2.WithClient(s.insecureResourceServiceClient)
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions agent/grpc-external/services/resource/list_by_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ func (s *Server) ListByOwner(ctx context.Context, req *pbresource.ListByOwnerReq
return nil, status.Errorf(codes.Internal, "failed list acl: %v", err)
}

// Check v1 tenancy exists for the v2 resource.
if err = v1TenancyExists(reg, s.TenancyBridge, req.Owner.Tenancy, codes.InvalidArgument); err != nil {
// Check tenancy exists for the v2 resource.
if err = tenancyExists(reg, s.TenancyBridge, req.Owner.Tenancy, codes.InvalidArgument); err != nil {
return nil, err
}

Expand Down
4 changes: 2 additions & 2 deletions agent/grpc-external/services/resource/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ func (s *Server) Read(ctx context.Context, req *pbresource.ReadRequest) (*pbreso
return nil, status.Errorf(codes.Internal, "failed read acl: %v", err)
}

// Check V1 tenancy exists for the V2 resource.
if err = v1TenancyExists(reg, s.TenancyBridge, req.Id.Tenancy, codes.NotFound); err != nil {
// Check tenancy exists for the V2 resource.
if err = tenancyExists(reg, s.TenancyBridge, req.Id.Tenancy, codes.NotFound); err != nil {
return nil, err
}

Expand Down
16 changes: 8 additions & 8 deletions agent/grpc-external/services/resource/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,10 @@ func validateWildcardTenancy(tenancy *pbresource.Tenancy, namePrefix string) err
return nil
}

// v1TenancyExists return an error with the passed in gRPC status code when tenancy partition or namespace do not exist.
func v1TenancyExists(reg *resource.Registration, v1Bridge TenancyBridge, tenancy *pbresource.Tenancy, errCode codes.Code) error {
// tenancyExists return an error with the passed in gRPC status code when tenancy partition or namespace do not exist.
func tenancyExists(reg *resource.Registration, tenancyBridge TenancyBridge, tenancy *pbresource.Tenancy, errCode codes.Code) error {
if reg.Scope == resource.ScopePartition || reg.Scope == resource.ScopeNamespace {
exists, err := v1Bridge.PartitionExists(tenancy.Partition)
exists, err := tenancyBridge.PartitionExists(tenancy.Partition)
switch {
case err != nil:
return err
Expand All @@ -221,7 +221,7 @@ func v1TenancyExists(reg *resource.Registration, v1Bridge TenancyBridge, tenancy
}

if reg.Scope == resource.ScopeNamespace {
exists, err := v1Bridge.NamespaceExists(tenancy.Partition, tenancy.Namespace)
exists, err := tenancyBridge.NamespaceExists(tenancy.Partition, tenancy.Namespace)
switch {
case err != nil:
return err
Expand All @@ -232,10 +232,10 @@ func v1TenancyExists(reg *resource.Registration, v1Bridge TenancyBridge, tenancy
return nil
}

// v1TenancyMarkedForDeletion returns a gRPC InvalidArgument when either partition or namespace is marked for deletion.
func v1TenancyMarkedForDeletion(reg *resource.Registration, v1Bridge TenancyBridge, tenancy *pbresource.Tenancy) error {
// tenancyMarkedForDeletion returns a gRPC InvalidArgument when either partition or namespace is marked for deletion.
func tenancyMarkedForDeletion(reg *resource.Registration, tenancyBridge TenancyBridge, tenancy *pbresource.Tenancy) error {
if reg.Scope == resource.ScopePartition || reg.Scope == resource.ScopeNamespace {
marked, err := v1Bridge.IsPartitionMarkedForDeletion(tenancy.Partition)
marked, err := tenancyBridge.IsPartitionMarkedForDeletion(tenancy.Partition)
switch {
case err != nil:
return err
Expand All @@ -245,7 +245,7 @@ func v1TenancyMarkedForDeletion(reg *resource.Registration, v1Bridge TenancyBrid
}

if reg.Scope == resource.ScopeNamespace {
marked, err := v1Bridge.IsNamespaceMarkedForDeletion(tenancy.Partition, tenancy.Namespace)
marked, err := tenancyBridge.IsNamespaceMarkedForDeletion(tenancy.Partition, tenancy.Namespace)
switch {
case err != nil:
return err
Expand Down
2 changes: 1 addition & 1 deletion agent/grpc-external/services/resource/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func testServer(t *testing.T) *Server {
}
})

// Mock the V1 tenancy bridge since we can't use the real thing.
// Mock the tenancy bridge since we can't use the real thing.
mockTenancyBridge := &MockTenancyBridge{}
mockTenancyBridge.On("PartitionExists", resource.DefaultPartitionName).Return(true, nil)
mockTenancyBridge.On("NamespaceExists", resource.DefaultPartitionName, resource.DefaultNamespaceName).Return(true, nil)
Expand Down
23 changes: 18 additions & 5 deletions agent/grpc-external/services/resource/testing/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ package testing

import (
"context"
"testing"

"github.com/hashicorp/consul/internal/tenancy"
"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"
"testing"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/acl/resolver"
Expand Down Expand Up @@ -98,6 +97,12 @@ func RunResourceServiceWithConfig(t *testing.T, config svc.Config, registerFns .
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 {
Expand Down Expand Up @@ -140,6 +145,14 @@ func RunResourceServiceWithConfig(t *testing.T, config svc.Config, registerFns .
)
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 pbresource.NewResourceServiceClient(conn)
return client
}
45 changes: 45 additions & 0 deletions agent/grpc-external/services/resource/testing/testing_ce.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,17 @@
package testing

import (
"context"
"errors"
"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 All @@ -16,3 +26,38 @@ func FillEntMeta(entMeta *acl.EnterpriseMeta) {
func FillAuthorizerContext(authzContext *acl.AuthorizerContext) {
// nothing to to in CE.
}

// 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
}
nsID := &pbresource.ID{
Type: pbtenancy.NamespaceType,
Name: resource.DefaultNamespaceName,
Tenancy: resource.DefaultPartitionedTenancy(),
Uid: ulid.Make().String(),
}
read, err := b.Read(ctx, storage.StrongConsistency, nsID)
if err != nil && !errors.Is(err, storage.ErrNotFound) {
return err
}
if read == nil && errors.Is(err, storage.ErrNotFound) {
_, err = b.WriteCAS(ctx, &pbresource.Resource{
Id: nsID,
Generation: ulid.Make().String(),
Data: nsData,
Metadata: map[string]string{
"generated_at": time.Now().Format(time.RFC3339),
},
})
if err != nil {
return err
}
}
return nil

}
8 changes: 4 additions & 4 deletions agent/grpc-external/services/resource/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre
return nil, status.Errorf(codes.Internal, "failed write acl: %v", err)
}

// Check V1 tenancy exists for the V2 resource
if err = v1TenancyExists(reg, s.TenancyBridge, req.Resource.Id.Tenancy, codes.InvalidArgument); err != nil {
// Check tenancy exists for the V2 resource
if err = tenancyExists(reg, s.TenancyBridge, req.Resource.Id.Tenancy, codes.InvalidArgument); err != nil {
return nil, err
}

// Check V1 tenancy not marked for deletion.
if err = v1TenancyMarkedForDeletion(reg, s.TenancyBridge, req.Resource.Id.Tenancy); err != nil {
// Check tenancy not marked for deletion.
if err = tenancyMarkedForDeletion(reg, s.TenancyBridge, req.Resource.Id.Tenancy); err != nil {
return nil, err
}

Expand Down
4 changes: 2 additions & 2 deletions agent/grpc-external/services/resource/write_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ func (s *Server) WriteStatus(ctx context.Context, req *pbresource.WriteStatusReq
// Apply defaults when tenancy units empty.
v1EntMetaToV2Tenancy(reg, entMeta, req.Id.Tenancy)

// Check V1 tenancy exists for the V2 resource. Ignore "marked for deletion" since status updates
// Check tenancy exists for the V2 resource. Ignore "marked for deletion" since status updates
// should still work regardless.
if err = v1TenancyExists(reg, s.TenancyBridge, req.Id.Tenancy, codes.InvalidArgument); err != nil {
if err = tenancyExists(reg, s.TenancyBridge, req.Id.Tenancy, codes.InvalidArgument); err != nil {
return nil, err
}

Expand Down
7 changes: 4 additions & 3 deletions internal/resource/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,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()
req := httptest.NewRequest("PUT", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default", strings.NewReader(`
Expand All @@ -183,7 +183,8 @@ func TestResourceWriteHandler(t *testing.T) {
require.Equal(t, "Keith Urban", result["data"].(map[string]any)["name"])
require.Equal(t, "keith-urban", result["id"].(map[string]any)["name"])

readRsp, err := client.Read(testutil.TestContext(t), &pbresource.ReadRequest{
var err error
readRsp, err = client.Read(testutil.TestContext(t), &pbresource.ReadRequest{
Id: &pbresource.ID{
Type: demo.TypeV2Artist,
Tenancy: resource.DefaultNamespacedTenancy(),
Expand All @@ -200,7 +201,7 @@ func TestResourceWriteHandler(t *testing.T) {

t.Run("should update the record with version parameter", func(t *testing.T) {
rsp := httptest.NewRecorder()
req := httptest.NewRequest("PUT", "/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&version=1", strings.NewReader(`
req := httptest.NewRequest("PUT", fmt.Sprintf("/demo/v2/artist/keith-urban?partition=default&peer_name=local&namespace=default&version=%s", readRsp.Resource.Version), strings.NewReader(`
{
"metadata": {
"foo": "bar"
Expand Down
9 changes: 0 additions & 9 deletions internal/resource/tenancy.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@ const (
DefaultPeerName = "local"
)

// V2TenancyBridge is used by the resource service to access V2 implementations of
// partitions and namespaces.
type V2TenancyBridge struct {
}

func NewV2TenancyBridge() TenancyBridge {
return &V2TenancyBridge{}
}

// Scope describes the tenancy scope of a resource.
type Scope int

Expand Down
9 changes: 9 additions & 0 deletions internal/tenancy/exports.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package tenancy

import (
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/tenancy/internal/bridge"
"github.com/hashicorp/consul/internal/tenancy/internal/types"
)

Expand All @@ -21,8 +22,16 @@ var (
NamespaceV2Beta1Type = types.NamespaceV2Beta1Type
)

type (
V2TenancyBridge = bridge.V2TenancyBridge
)

// RegisterTypes adds all resource types within the "tenancy" API group
// to the given type registry
func RegisterTypes(r resource.Registry) {
types.Register(r)
}

func NewV2TenancyBridge() *V2TenancyBridge {
return bridge.NewV2TenancyBridge()
}
51 changes: 51 additions & 0 deletions internal/tenancy/internal/bridge/tenancy_bridge.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package bridge

import (
"context"
"github.com/hashicorp/consul/internal/tenancy/internal/types"
"github.com/hashicorp/consul/proto-public/pbresource"
)

// V2TenancyBridge is used by the resource service to access V2 implementations of
// partitions and namespaces.
type V2TenancyBridge struct {
client pbresource.ResourceServiceClient
}

// WithClient inject a ResourceServiceClient in the V2TenancyBridge.
// This is needed to break a circular dependency between
// the ResourceServiceServer, ResourceServiceClient and the TenancyBridge
func (b *V2TenancyBridge) WithClient(client pbresource.ResourceServiceClient) *V2TenancyBridge {
b.client = client
return b
}

func NewV2TenancyBridge() *V2TenancyBridge {
return &V2TenancyBridge{}
}

func (b *V2TenancyBridge) NamespaceExists(partition, namespace string) (bool, error) {
read, err := b.client.Read(context.Background(), &pbresource.ReadRequest{
Id: &pbresource.ID{
Name: namespace,
Tenancy: &pbresource.Tenancy{
Partition: partition,
},
Type: types.NamespaceType,
},
})
return read != nil && read.Resource != nil, err
}

func (b *V2TenancyBridge) IsNamespaceMarkedForDeletion(partition, namespace string) (bool, error) {
read, err := b.client.Read(context.Background(), &pbresource.ReadRequest{
Id: &pbresource.ID{
Name: namespace,
Tenancy: &pbresource.Tenancy{
Partition: partition,
},
Type: types.NamespaceType,
},
})
return read.Resource != nil, err
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

//go:build !consulent

package resource
package bridge

func (b *V2TenancyBridge) PartitionExists(partition string) (bool, error) {
if partition == "default" {
Expand All @@ -15,14 +15,3 @@ func (b *V2TenancyBridge) PartitionExists(partition string) (bool, error) {
func (b *V2TenancyBridge) IsPartitionMarkedForDeletion(partition string) (bool, error) {
return false, nil
}

func (b *V2TenancyBridge) NamespaceExists(partition, namespace string) (bool, error) {
if partition == "default" && namespace == "default" {
return true, nil
}
return false, nil
}

func (b *V2TenancyBridge) IsNamespaceMarkedForDeletion(partition, namespace string) (bool, error) {
return false, nil
}
Loading

0 comments on commit b7f899e

Please sign in to comment.