Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/grpc/index.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

506 changes: 254 additions & 252 deletions protocol/go/policy/attributes/attributes.pb.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions service/integration/attribute_fqns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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)
}
5 changes: 0 additions & 5 deletions service/pkg/db/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)

Expand Down Expand Up @@ -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)
}
12 changes: 10 additions & 2 deletions service/policy/attributes/attributes.proto
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,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 {
Expand Down
57 changes: 57 additions & 0 deletions service/policy/attributes/attributes_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package attributes

import (
"fmt"
"strings"
"testing"

Expand Down Expand Up @@ -274,3 +275,59 @@ func TestCreateAttributeValue_ValueMissing_Fails(t *testing.T) {
require.Contains(t, err.Error(), "value")
require.Contains(t, err.Error(), "[required]")
}

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]")
}
12 changes: 0 additions & 12 deletions service/policy/db/attribute_fqn.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package db

import (
"context"
"errors"
"fmt"
"log/slog"
"strings"
Expand Down Expand Up @@ -119,23 +118,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

Expand Down
Loading