Resource validation hook for `Write` endpoint (#16950)

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

@@ -37,6 +37,10 @@ func (s *Server) Write(ctx context.Context, req *pbresource.WriteRequest) (*pbre
 		)
 	}
 
+	if err = reg.Validate(req.Resource); err != nil {
+		return nil, status.Error(codes.InvalidArgument, err.Error())
+	}
+
 	// At the storage backend layer, all writes are CAS operations.
 	//
 	// This makes it possible to *safely* do things like keeping the Uid stable

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

@@ -34,6 +34,12 @@ func TestWrite_InputValidation(t *testing.T) {
 			req.Resource.Data, err = anypb.New(&pbdemov2.Album{})
 			require.NoError(t, err)
 		},
+		"fail validation hook": func(req *pbresource.WriteRequest) {
+			artist := &pbdemov2.Artist{}
+			require.NoError(t, req.Resource.Data.UnmarshalTo(artist))
+			artist.Name = "" // name cannot be empty
+			require.NoError(t, req.Resource.Data.MarshalFrom(artist))
+		},
 	}
 	for desc, modFn := range testCases {
 		t.Run(desc, func(t *testing.T) {

internal/resource/demo/demo.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"time"
 
+	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/anypb"
 
 	"github.com/hashicorp/consul/acl"
@@ -85,6 +86,28 @@ func Register(r resource.Registry) {
 		}
 	}
 
+	validateV1ArtistFn := func(res *pbresource.Resource) error {
+		artist := &pbdemov1.Artist{}
+		if err := anypb.UnmarshalTo(res.Data, artist, proto.UnmarshalOptions{}); err != nil {
+			return err
+		}
+		if artist.Name == "" {
+			return fmt.Errorf("artist.name required")
+		}
+		return nil
+	}
+
+	validateV2ArtistFn := func(res *pbresource.Resource) error {
+		artist := &pbdemov2.Artist{}
+		if err := anypb.UnmarshalTo(res.Data, artist, proto.UnmarshalOptions{}); err != nil {
+			return err
+		}
+		if artist.Name == "" {
+			return fmt.Errorf("artist.name required")
+		}
+		return nil
+	}
+
 	r.Register(resource.Registration{
 		Type:  TypeV1Artist,
 		Proto: &pbdemov1.Artist{},
@@ -93,6 +116,7 @@ func Register(r resource.Registry) {
 			Write: writeACL,
 			List:  makeListACL(TypeV1Artist),
 		},
+		Validate: validateV1ArtistFn,
 	})
 
 	r.Register(resource.Registration{
@@ -113,6 +137,7 @@ func Register(r resource.Registry) {
 			Write: writeACL,
 			List:  makeListACL(TypeV2Artist),
 		},
+		Validate: validateV2ArtistFn,
 	})
 
 	r.Register(resource.Registration{

internal/resource/registry.go

@@ -31,6 +31,10 @@ type Registration struct {
 	// ACLs are hooks called to perform authorization on RPCs.
 	ACLs *ACLHooks
 
+	// Validate is called to structurally validate the resource (e.g.
+	// check for required fields).
+	Validate func(*pbresource.Resource) error
+
 	// In the future, we'll add hooks, the controller etc. here.
 	// TODO: https://github.com/hashicorp/consul/pull/16622#discussion_r1134515909
 }
@@ -99,6 +103,12 @@ func (r *TypeRegistry) Register(registration Registration) {
 			return authz.ToAllowAuthorizer().OperatorReadAllowed(&acl.AuthorizerContext{})
 		}
 	}
+
+	// default validation to a no-op
+	if registration.Validate == nil {
+		registration.Validate = func(resource *pbresource.Resource) error { return nil }
+	}
+
 	r.registrations[key] = registration
 }
 

internal/resource/registry_test.go

@@ -59,7 +59,7 @@ func TestRegister(t *testing.T) {
 	}, "type field(s) cannot be empty")
 }
 
-func TestRegister_DefaultACLs(t *testing.T) {
+func TestRegister_Defaults(t *testing.T) {
 	r := resource.NewRegistry()
 	r.Register(resource.Registration{
 		Type: demo.TypeV2Artist,
@@ -82,6 +82,9 @@ func TestRegister_DefaultACLs(t *testing.T) {
 	// verify default list hook requires operator:read
 	require.NoError(t, reg.ACLs.List(testutils.ACLOperatorRead(t), artist.Id.Tenancy))
 	require.True(t, acl.IsErrPermissionDenied(reg.ACLs.List(testutils.ACLNoPermissions(t), artist.Id.Tenancy)))
+
+	// verify default validate is a no-op
+	require.NoError(t, reg.Validate(nil))
 }
 
 func assertRegisterPanics(t *testing.T, registerFn func(reg resource.Registration), registration resource.Registration, panicString string) {