Resource `Delete` endpoint (#16756)
This commit is contained in:
parent
4e8ab7a390
commit
2b0a5b52c2
|
@ -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)
|
||||
}
|
||||
}
|
|
@ -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 ""
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
|
@ -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)
|
||||
|
|
|
@ -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()
|
||||
|
||||
|
|
Loading…
Reference in New Issue