Resource `Delete` endpoint (#16756)

agent/grpc-external/services/resource/delete.go

@@ -0,0 +1,55 @@
+package resource
+
+import (
+	"context"
+	"errors"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
+	"github.com/hashicorp/consul/internal/storage"
+	"github.com/hashicorp/consul/proto-public/pbresource"
+)
+
+// TODO(spatel): Move docs to the proto file
+// Deletes a resource with the given Id and Version.
+//
+// Pass an empty Version to delete a resource regardless of the stored Version.
+// Deletes of previously deleted or non-existent resource are no-ops.
+// Returns an Aborted error if the requested Version does not match the stored Version.
+func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pbresource.DeleteResponse, error) {
+	// check type registered
+	reg, err := s.resolveType(req.Id.Type)
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO(spatel): reg will be used for ACL hooks
+	_ = reg
+
+	versionToDelete := req.Version
+	if versionToDelete == "" {
+		// Delete resource regardless of the stored Version. Hence, strong read
+		// necessary to get latest Version
+		existing, err := s.Backend.Read(ctx, storage.StrongConsistency, req.Id)
+		switch {
+		case err == nil:
+			versionToDelete = existing.Version
+		case errors.Is(err, storage.ErrNotFound):
+			// deletes are idempotent so no-op if resource not found
+			return &pbresource.DeleteResponse{}, nil
+		default:
+			return nil, status.Errorf(codes.Internal, "failed read: %v", err)
+		}
+	}
+
+	err = s.Backend.DeleteCAS(ctx, req.Id, versionToDelete)
+	switch {
+	case err == nil:
+		return &pbresource.DeleteResponse{}, nil
+	case errors.Is(err, storage.ErrCASFailure):
+		return nil, status.Error(codes.Aborted, err.Error())
+	default:
+		return nil, status.Errorf(codes.Internal, "failed delete: %v", err)
+	}
+}

agent/grpc-external/services/resource/delete_test.go

@@ -0,0 +1,114 @@
+package resource
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
+	"github.com/hashicorp/consul/internal/resource/demo"
+	"github.com/hashicorp/consul/internal/storage"
+	"github.com/hashicorp/consul/proto-public/pbresource"
+)
+
+func TestDelete_TypeNotRegistered(t *testing.T) {
+	t.Parallel()
+
+	_, client, ctx := testDeps(t)
+	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())
+}
+
+func TestDelete_Success(t *testing.T) {
+	t.Parallel()
+
+	for desc, tc := range deleteTestCases() {
+		t.Run(desc, func(t *testing.T) {
+			server, client, ctx := testDeps(t)
+			demo.Register(server.Registry)
+			artist, err := demo.GenerateV2Artist()
+			require.NoError(t, err)
+			rsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist})
+			require.NoError(t, err)
+
+			// delete
+			_, err = client.Delete(ctx, &pbresource.DeleteRequest{
+				Id:      rsp.Resource.Id,
+				Version: tc.versionFn(rsp.Resource),
+			})
+			require.NoError(t, err)
+
+			// verify deleted
+			_, err = server.Backend.Read(ctx, storage.StrongConsistency, rsp.Resource.Id)
+			require.Error(t, err)
+			require.ErrorIs(t, err, storage.ErrNotFound)
+		})
+	}
+}
+
+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.Register(server.Registry)
+			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, &pbresource.DeleteRequest{Id: artist.Id, Version: tc.versionFn(artist)})
+			require.NoError(t, err)
+		})
+	}
+}
+
+func TestDelete_VersionMismatch(t *testing.T) {
+	t.Parallel()
+
+	server, client, ctx := testDeps(t)
+	demo.Register(server.Registry)
+	artist, err := demo.GenerateV2Artist()
+	require.NoError(t, err)
+	rsp, err := client.Write(ctx, &pbresource.WriteRequest{Resource: artist})
+	require.NoError(t, err)
+
+	// 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")
+}
+
+func testDeps(t *testing.T) (*Server, pbresource.ResourceServiceClient, context.Context) {
+	server := testServer(t)
+	client := testClient(t, server)
+	return server, client, context.Background()
+}
+
+type deleteTestCase struct {
+	// returns the version to use in the test given the passed in resource
+	versionFn func(*pbresource.Resource) string
+}
+
+func deleteTestCases() map[string]deleteTestCase {
+	return map[string]deleteTestCase{
+		"specific version": {
+			versionFn: func(r *pbresource.Resource) string {
+				return r.Version
+			},
+		},
+		"empty version": {
+			versionFn: func(r *pbresource.Resource) string {
+				return ""
+			},
+		},
+	}
+}

agent/grpc-external/services/resource/server.go

@@ -55,11 +55,6 @@ func (s *Server) WriteStatus(ctx context.Context, req *pbresource.WriteStatusReq
 	return &pbresource.WriteStatusResponse{}, nil
 }
 
-func (s *Server) Delete(ctx context.Context, req *pbresource.DeleteRequest) (*pbresource.DeleteResponse, error) {
-	// TODO
-	return &pbresource.DeleteResponse{}, nil
-}
-
 //nolint:unparam
 func (s *Server) resolveType(typ *pbresource.Type) (*resource.Registration, error) {
 	v, ok := s.Registry.Resolve(typ)

agent/grpc-external/services/resource/server_test.go

@@ -28,14 +28,6 @@ func TestWriteStatus_TODO(t *testing.T) {
 	require.NotNil(t, resp)
 }
 
-func TestDelete_TODO(t *testing.T) {
-	server := testServer(t)
-	client := testClient(t, server)
-	resp, err := client.Delete(context.Background(), &pbresource.DeleteRequest{})
-	require.NoError(t, err)
-	require.NotNil(t, resp)
-}
-
 func testServer(t *testing.T) *Server {
 	t.Helper()