feat(policy): 1651 move GetAttributesByValueFqns RPC request validation to protovalidate (#1657)

fix #1651

---------

Co-authored-by: Jake Van Vorhis <[email protected]>
Co-authored-by: jakedoublev <[email protected]>

Author: ryanulit

docs/grpc/index.html

@@ -1472,7 +1472,7 @@ func (s *AttributeFqnSuite) TestGetAttributesByValueFqns_Fails_WithNonValueFqns(
 	})
 	s.Require().Error(err)
 	s.Nil(v)
-	s.Require().ErrorIs(err, db.ErrFqnMissingValue)
+	s.Require().ErrorIs(err, db.ErrNotFound)
 
 	v, err = s.db.PolicyClient.GetAttributesByValueFqns(s.ctx, &attributes.GetAttributeValuesByFqnsRequest{
 		Fqns: []string{attrFqn},
@@ -1482,5 +1482,5 @@ func (s *AttributeFqnSuite) TestGetAttributesByValueFqns_Fails_WithNonValueFqns(
 	})
 	s.Require().Error(err)
 	s.Nil(v)
-	s.Require().ErrorIs(err, db.ErrFqnMissingValue)
+	s.Require().ErrorIs(err, db.ErrNotFound)
 }

protocol/go/policy/attributes/attributes.pb.go

@@ -21,7 +21,6 @@ var (
 	ErrNotFound                  = errors.New("ErrNotFound: value not found")
 	ErrEnumValueInvalid          = errors.New("ErrEnumValueInvalid: not a valid enum value")
 	ErrUUIDInvalid               = errors.New("ErrUUIDInvalid: value not a valid UUID")
-	ErrFqnMissingValue           = errors.New("ErrFqnMissingValue: FQN must include a value")
 	ErrMissingValue              = errors.New("ErrMissingValue: value must be included")
 )
 
@@ -127,10 +126,6 @@ func StatusifyError(err error, fallbackErr string, log ...any) error {
 		slog.Error(ErrTextRestrictViolation, l...)
 		return status.Error(codes.InvalidArgument, ErrTextRestrictViolation)
 	}
-	if errors.Is(err, ErrFqnMissingValue) {
-		slog.Error(ErrTextFqnMissingValue, l...)
-		return status.Error(codes.InvalidArgument, ErrTextFqnMissingValue)
-	}
 	slog.Error(err.Error(), l...)
 	return status.Error(codes.Internal, fallbackErr)
 }

service/integration/attribute_fqns_test.go

@@ -214,8 +214,16 @@ message DeactivateAttributeValueResponse {
 message GetAttributeValuesByFqnsRequest {
   // Required
   // Fully Qualified Names of attribute values (i.e. https://<namespace>/attr/<attribute_name>/value/<value_name>), normalized to lower case.
-  repeated string fqns = 1 [(buf.validate.field).required = true];
-  policy.AttributeValueSelector with_value = 2 [(buf.validate.field).required = true];
+  repeated string fqns = 1 [
+    (buf.validate.field).repeated = {
+      min_items: 1,
+      max_items: 250
+    }
+  ];
+
+  // Optional
+  // This attribute value selector is not used currently, but left here for future use.
+  policy.AttributeValueSelector with_value = 2;
 }
 message GetAttributeValuesByFqnsResponse {
   message AttributeAndValue {

service/pkg/db/errors.go

@@ -1,6 +1,7 @@
 package attributes
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -451,3 +452,59 @@ func TestDeactivateAttributeValueRequest(t *testing.T) {
 	err = getValidator().Validate(req)
 	require.NoError(t, err)
 }
+
+func TestGetAttributeValuesByFqns_Valid_Succeeds(t *testing.T) {
+	req := &attributes.GetAttributeValuesByFqnsRequest{
+		Fqns: []string{
+			"any_value",
+		},
+	}
+
+	v := getValidator()
+	err := v.Validate(req)
+
+	require.NoError(t, err)
+}
+
+func TestGetAttributeValuesByFqns_FQNsNil_Fails(t *testing.T) {
+	req := &attributes.GetAttributeValuesByFqnsRequest{}
+
+	v := getValidator()
+	err := v.Validate(req)
+
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "fqns")
+	require.Contains(t, err.Error(), "[repeated.min_items]")
+}
+
+func TestGetAttributeValuesByFqns_FQNsEmpty_Fails(t *testing.T) {
+	req := &attributes.GetAttributeValuesByFqnsRequest{
+		Fqns: []string{},
+	}
+
+	v := getValidator()
+	err := v.Validate(req)
+
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "fqns")
+	require.Contains(t, err.Error(), "[repeated.min_items]")
+}
+
+func TestGetAttributeValuesByFqns_FQNsOutsideMaxItemsRange_Fails(t *testing.T) {
+	outsideRange := 251
+	fqns := make([]string, outsideRange)
+	for i := 0; i < outsideRange; i++ {
+		fqns[i] = fmt.Sprintf("fqn_%d", i)
+	}
+
+	req := &attributes.GetAttributeValuesByFqnsRequest{
+		Fqns: fqns,
+	}
+
+	v := getValidator()
+	err := v.Validate(req)
+
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "fqns")
+	require.Contains(t, err.Error(), "[repeated.max_items]")
+}

service/policy/attributes/attributes.proto

@@ -2,7 +2,6 @@ package db
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strings"
 
@@ -71,23 +70,12 @@ func (c *PolicyDBClient) AttrFqnReindex(ctx context.Context) (res struct { //nol
 func (c *PolicyDBClient) GetAttributesByValueFqns(ctx context.Context, r *attributes.GetAttributeValuesByFqnsRequest) (map[string]*attributes.GetAttributeValuesByFqnsResponse_AttributeAndValue, error) {
 	fqns := r.GetFqns()
 
-	// todo: move to proto validation
-	if fqns == nil || r.GetWithValue() == nil {
-		return nil, errors.Join(db.ErrMissingValue, errors.New("error: one or more FQNs and a WithValue selector must be provided"))
-	}
-
 	list := make(map[string]*attributes.GetAttributeValuesByFqnsResponse_AttributeAndValue, len(fqns))
 
 	for i, fqn := range fqns {
 		// normalize to lower case
 		fqn = strings.ToLower(fqn)
 
-		// ensure the FQN corresponds to an attribute value and not a definition or namespace alone
-		// todo: move to proto validation
-		if !strings.Contains(fqn, "/value/") {
-			return nil, db.ErrFqnMissingValue
-		}
-
 		// update array with normalized FQN
 		fqns[i] = fqn