Skip to content

Commit

Permalink
Merge branch 'main' into integ-test-deregister-service-dp
Browse files Browse the repository at this point in the history
  • Loading branch information
huikang authored Nov 6, 2023
2 parents ca1c833 + e5948e8 commit 293973c
Show file tree
Hide file tree
Showing 113 changed files with 2,388 additions and 4,208 deletions.
3 changes: 3 additions & 0 deletions .changelog/19342.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
Replaces UI Side Nav with Helios Design System Side Nav. Adds dc/partition/namespace searching in Side Nav.
```
3 changes: 3 additions & 0 deletions .changelog/19503.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
wan-federation: Fix a bug where servers wan-federated through mesh-gateways could crash due to overlapping LAN IP addresses.
```
2 changes: 1 addition & 1 deletion .github/scripts/set_test_package_matrix.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ set -euo pipefail
export RUNNER_COUNT=$1

# set matrix var to list of unique packages containing tests
matrix="$(go list -json="ImportPath,TestGoFiles" ./... | jq --compact-output '. | select(.TestGoFiles != null) | .ImportPath' | jq --slurp --compact-output '.' | jq --argjson runnercount $RUNNER_COUNT -cM '[_nwise(length / $runnercount | floor)]'))"
matrix="$(go list -json="ImportPath,TestGoFiles" ./... | jq --compact-output '. | select(.TestGoFiles != null) | .ImportPath' | shuf | jq --slurp --compact-output '.' | jq --argjson runnercount $RUNNER_COUNT -cM '[_nwise(length / $runnercount | floor)]'))"

echo "matrix=${matrix}" >> "${GITHUB_OUTPUT}"
163 changes: 163 additions & 0 deletions CHANGELOG.md

Large diffs are not rendered by default.

30 changes: 28 additions & 2 deletions agent/grpc-external/services/resource/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
// - Errors with Aborted if the requested Version does not match the stored Version.
// - Errors with PermissionDenied if ACL check fails
func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pbresource.DeleteResponse, error) {
reg, err := s.validateDeleteRequest(req)
reg, err := s.ensureDeleteRequestValid(req)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -74,6 +74,18 @@ func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pb
deleteId = existing.Id
}

// Check finalizers for a deferred delete
if resource.HasFinalizers(existing) {
if resource.IsMarkedForDeletion(existing) {
// Delete previously requested and finalizers still present so nothing to do
return &pbresource.DeleteResponse{}, nil
}

// Mark for deletion and let controllers that put finalizers in place do their thing
return s.markForDeletion(ctx, existing)
}

// Continue with an immediate delete
if err := s.maybeCreateTombstone(ctx, deleteId); err != nil {
return nil, err
}
Expand All @@ -89,6 +101,20 @@ func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pb
}
}

func (s *Server) markForDeletion(ctx context.Context, res *pbresource.Resource) (*pbresource.DeleteResponse, error) {
if res.Metadata == nil {
res.Metadata = map[string]string{}
}
res.Metadata[resource.DeletionTimestampKey] = time.Now().Format(time.RFC3339)

// Write the deletion timestamp
_, err := s.Write(ctx, &pbresource.WriteRequest{Resource: res})
if err != nil {
return nil, err
}
return &pbresource.DeleteResponse{}, nil
}

// Create a tombstone to capture the intent to delete child resources.
// Tombstones are created preemptively to prevent partial failures even though
// we are currently unaware of the success/failure/no-op of DeleteCAS. In
Expand Down Expand Up @@ -145,7 +171,7 @@ func (s *Server) maybeCreateTombstone(ctx context.Context, deleteId *pbresource.
}
}

func (s *Server) validateDeleteRequest(req *pbresource.DeleteRequest) (*resource.Registration, error) {
func (s *Server) ensureDeleteRequestValid(req *pbresource.DeleteRequest) (*resource.Registration, error) {
if req.Id == nil {
return nil, status.Errorf(codes.InvalidArgument, "id is required")
}
Expand Down
64 changes: 64 additions & 0 deletions agent/grpc-external/services/resource/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import (
"github.com/hashicorp/consul/acl/resolver"
"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"
pbdemo "github.com/hashicorp/consul/proto/private/pbdemo/v1"
)

func TestDelete_InputValidation(t *testing.T) {
Expand Down Expand Up @@ -313,6 +315,68 @@ func TestDelete_VersionMismatch(t *testing.T) {
require.ErrorContains(t, err, "CAS operation failed")
}

func TestDelete_MarkedForDeletionWhenFinalizersPresent(t *testing.T) {
server, client, ctx := testDeps(t)
demo.RegisterTypes(server.Registry)

// 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)

// 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)

// 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)

// 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)

// 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)

// 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)
Expand Down
4 changes: 2 additions & 2 deletions agent/grpc-external/services/resource/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbresource.ListResponse, error) {
reg, err := s.validateListRequest(req)
reg, err := s.ensureListRequestValid(req)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -81,7 +81,7 @@ func (s *Server) List(ctx context.Context, req *pbresource.ListRequest) (*pbreso
return &pbresource.ListResponse{Resources: result}, nil
}

func (s *Server) validateListRequest(req *pbresource.ListRequest) (*resource.Registration, error) {
func (s *Server) ensureListRequestValid(req *pbresource.ListRequest) (*resource.Registration, error) {
var field string
switch {
case req.Type == 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 @@ -15,7 +15,7 @@ import (
)

func (s *Server) ListByOwner(ctx context.Context, req *pbresource.ListByOwnerRequest) (*pbresource.ListByOwnerResponse, error) {
reg, err := s.validateListByOwnerRequest(req)
reg, err := s.ensureListByOwnerRequestValid(req)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -87,7 +87,7 @@ func (s *Server) ListByOwner(ctx context.Context, req *pbresource.ListByOwnerReq
return &pbresource.ListByOwnerResponse{Resources: result}, nil
}

func (s *Server) validateListByOwnerRequest(req *pbresource.ListByOwnerRequest) (*resource.Registration, error) {
func (s *Server) ensureListByOwnerRequestValid(req *pbresource.ListByOwnerRequest) (*resource.Registration, error) {
if req.Owner == nil {
return nil, status.Errorf(codes.InvalidArgument, "owner is required")
}
Expand Down
32 changes: 5 additions & 27 deletions agent/grpc-external/services/resource/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (

func (s *Server) Read(ctx context.Context, req *pbresource.ReadRequest) (*pbresource.ReadResponse, error) {
// Light first pass validation based on what user passed in and not much more.
reg, err := s.validateReadRequest(req)
reg, err := s.ensureReadRequestValid(req)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -87,7 +87,7 @@ func (s *Server) Read(ctx context.Context, req *pbresource.ReadRequest) (*pbreso
return &pbresource.ReadResponse{Resource: resource}, nil
}

func (s *Server) validateReadRequest(req *pbresource.ReadRequest) (*resource.Registration, error) {
func (s *Server) ensureReadRequestValid(req *pbresource.ReadRequest) (*resource.Registration, error) {
if req.Id == nil {
return nil, status.Errorf(codes.InvalidArgument, "id is required")
}
Expand All @@ -107,31 +107,9 @@ func (s *Server) validateReadRequest(req *pbresource.ReadRequest) (*resource.Reg
}

// 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 reg.Scope == resource.ScopeCluster {
if req.Id.Tenancy.Partition != "" {
return nil, status.Errorf(
codes.InvalidArgument,
"cluster scoped resource %s cannot have a partition: %s",
resource.ToGVK(req.Id.Type),
req.Id.Tenancy.Partition,
)
}
if req.Id.Tenancy.Namespace != "" {
return nil, status.Errorf(
codes.InvalidArgument,
"cluster scoped resource %s cannot have a namespace: %s",
resource.ToGVK(req.Id.Type),
req.Id.Tenancy.Namespace,
)
}
if err = validateScopedTenancy(reg.Scope, req.Id.Type, req.Id.Tenancy); err != nil {
return nil, err
}

return reg, nil
}
30 changes: 30 additions & 0 deletions agent/grpc-external/services/resource/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,36 @@ func tenancyExists(reg *resource.Registration, tenancyBridge TenancyBridge, tena
return nil
}

func validateScopedTenancy(scope resource.Scope, resourceType *pbresource.Type, tenancy *pbresource.Tenancy) error {
if scope == resource.ScopePartition && tenancy.Namespace != "" {
return status.Errorf(
codes.InvalidArgument,
"partition scoped resource %s cannot have a namespace. got: %s",
resource.ToGVK(resourceType),
tenancy.Namespace,
)
}
if scope == resource.ScopeCluster {
if tenancy.Partition != "" {
return status.Errorf(
codes.InvalidArgument,
"cluster scoped resource %s cannot have a partition: %s",
resource.ToGVK(resourceType),
tenancy.Partition,
)
}
if tenancy.Namespace != "" {
return status.Errorf(
codes.InvalidArgument,
"cluster scoped resource %s cannot have a namespace: %s",
resource.ToGVK(resourceType),
tenancy.Namespace,
)
}
}
return nil
}

// 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 {
Expand Down
55 changes: 35 additions & 20 deletions agent/grpc-external/services/resource/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.ResourceService_WatchListServer) error {
reg, err := s.validateWatchListRequest(req)
reg, err := s.ensureWatchListRequestValid(req)
if err != nil {
return err
}
Expand Down Expand Up @@ -91,17 +91,9 @@ func (s *Server) WatchList(req *pbresource.WatchListRequest, stream pbresource.R
}
}

func (s *Server) validateWatchListRequest(req *pbresource.WatchListRequest) (*resource.Registration, error) {
var field string
switch {
case req.Type == nil:
field = "type"
case req.Tenancy == nil:
field = "tenancy"
}

if field != "" {
return nil, status.Errorf(codes.InvalidArgument, "%s is required", field)
func (s *Server) ensureWatchListRequestValid(req *pbresource.WatchListRequest) (*resource.Registration, error) {
if req.Type == nil {
return nil, status.Errorf(codes.InvalidArgument, "type is required")
}

// Check type exists.
Expand All @@ -110,6 +102,11 @@ func (s *Server) validateWatchListRequest(req *pbresource.WatchListRequest) (*re
return nil, err
}

// if no tenancy is passed defaults to wildcard
if req.Tenancy == nil {
req.Tenancy = wildcardTenancyFor(reg.Scope)
}

if err = checkV2Tenancy(s.UseV2Tenancy, req.Type); err != nil {
return nil, err
}
Expand All @@ -118,15 +115,33 @@ func (s *Server) validateWatchListRequest(req *pbresource.WatchListRequest) (*re
return nil, err
}

// Error when partition scoped and namespace not empty.
if reg.Scope == resource.ScopePartition && req.Tenancy.Namespace != "" {
return nil, status.Errorf(
codes.InvalidArgument,
"partition scoped type %s cannot have a namespace. got: %s",
resource.ToGVK(req.Type),
req.Tenancy.Namespace,
)
// Check scope
if err = validateScopedTenancy(reg.Scope, req.Type, req.Tenancy); err != nil {
return nil, err
}

return reg, nil
}

func wildcardTenancyFor(scope resource.Scope) *pbresource.Tenancy {
var defaultTenancy *pbresource.Tenancy

switch scope {
case resource.ScopeCluster:
defaultTenancy = &pbresource.Tenancy{
PeerName: storage.Wildcard,
}
case resource.ScopePartition:
defaultTenancy = &pbresource.Tenancy{
Partition: storage.Wildcard,
PeerName: storage.Wildcard,
}
default:
defaultTenancy = &pbresource.Tenancy{
Partition: storage.Wildcard,
PeerName: storage.Wildcard,
Namespace: storage.Wildcard,
}
}
return defaultTenancy
}
Loading

0 comments on commit 293973c

Please sign in to comment.