Skip to content

Move proto validators to a separate validation package. #1409

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.

Sign up for GitHub

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

Draft
Julian Gutierrez Oschmann (juli4n) wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Move proto validators to a separate validation package. #1409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 3 additions & 17 deletions cmd/ateapi/internal/actoridentity/actoridentity.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import (
"time"

"github.com/agent-substrate/substrate/cmd/ateapi/internal/actoridjwt"
"github.com/agent-substrate/substrate/cmd/ateapi/internal/controlapi"
"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
"github.com/agent-substrate/substrate/cmd/ateapi/internal/validation"
"github.com/agent-substrate/substrate/cmd/ateapi/internal/workercache"
"github.com/agent-substrate/substrate/internal/localca"
"github.com/agent-substrate/substrate/internal/localjwtauthority"
Expand All @@ -41,8 +41,6 @@ import (
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/peer"
"google.golang.org/grpc/status"
"k8s.io/apimachinery/pkg/api/operation"
"k8s.io/apimachinery/pkg/util/validation/field"
)

// Server implements ateapipb.ActorIdentityServer
Expand Down Expand Up @@ -96,7 +94,7 @@ func (s *Server) MintJWT(ctx context.Context, req *ateapipb.MintJWTRequest) (*at
return nil, status.Errorf(codes.PermissionDenied, "caller is not permitted to mint actor JWTs")
}

if errs := validateMintJWTRequest(ctx, req); len(errs) > 0 {
if errs := validation.ValidateMintJWTRequest(ctx, req); len(errs) > 0 {
return nil, status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
}

Expand Down Expand Up @@ -152,7 +150,7 @@ func (s *Server) MintCert(ctx context.Context, req *ateapipb.MintCertRequest) (*
if err != nil {
return nil, err
}
if errs := validateMintCertRequest(ctx, req); len(errs) > 0 {
if errs := validation.ValidateMintCertRequest(ctx, req); len(errs) > 0 {
return nil, status.Error(codes.InvalidArgument, errs.ToAggregate().Error())
}
// Validation bounds purpose to the enum's range; which purposes this
Expand Down Expand Up @@ -286,18 +284,6 @@ func authenticateAtelet(ctx context.Context) (*ateletCaller, error) {
return &ateletCaller{podName: identity.PodName, nodeName: identity.NodeName}, nil
}

func validateMintJWTRequest(ctx context.Context, req *ateapipb.MintJWTRequest) field.ErrorList {
// Call the generated validation.
op := operation.Operation{Type: operation.Create}
return controlapi.Validate_MintJWTRequest(ctx, op, nil, req, nil)
}

func validateMintCertRequest(ctx context.Context, req *ateapipb.MintCertRequest) field.ErrorList {
// Call the generated validation.
op := operation.Operation{Type: operation.Create}
return controlapi.Validate_MintCertRequest(ctx, op, nil, req, nil)
}

// authorizeActor resolves the actor from the authenticated worker and verifies
// that the worker and actor still point at one another. Actor identity supplied
// by the requester never participates in this authorization decision.
Expand Down
159 changes: 0 additions & 159 deletions cmd/ateapi/internal/actoridentity/actoridentity_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,9 @@ import (
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"fmt"
"math/big"
"net/url"
"path"
"strings"
"testing"
"time"

Expand All @@ -41,14 +39,8 @@ import (
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/peer"
"google.golang.org/grpc/status"
"k8s.io/apimachinery/pkg/util/validation/field"
)

func assertValidateErr(t *testing.T, got field.ErrorList, want field.ErrorList) {
t.Helper()
field.ErrorMatcher{}.ByType().ByField().ByOrigin().Test(t, want, got)
}

const (
testAtespace = "team-alpha"
testActorName = "counter-1"
Expand Down Expand Up @@ -931,154 +923,3 @@ func TestMintCertAuthorizesBeforeSigning(t *testing.T) {
t.Errorf("MintCert() code = %v (err = %v), want %v", got, err, codes.PermissionDenied)
}
}

func TestValidateMintJWTRequest(t *testing.T) {
// This test verifies validation of user input for minting a JWT.
validReq := func(mods ...func(req *ateapipb.MintJWTRequest)) *ateapipb.MintJWTRequest {
req := &ateapipb.MintJWTRequest{
Audience: []string{"aud1"},
Atespace: "as1",
ActorName: "actor1",
ActorUid: "01234567-89ab-cdef-0123-456789abcdef",
}
for _, m := range mods {
m(req)
}
return req
}

tests := []struct {
name string
req *ateapipb.MintJWTRequest
want field.ErrorList
}{{
"valid",
validReq(),
nil,
}, {
"missing audience",
validReq(func(r *ateapipb.MintJWTRequest) { r.Audience = nil }),
field.ErrorList{field.Required(field.NewPath("audience"), "")},
}, {
"too many audiences",
validReq(func(r *ateapipb.MintJWTRequest) {
r.Audience = make([]string, 17)
for i := range r.Audience {
r.Audience[i] = fmt.Sprintf("https://svc-%d.example.com", i)
}
}),
field.ErrorList{field.TooMany(field.NewPath("audience"), 17, 16).WithOrigin("maxItems")},
}, {
"duplicate audience entry",
validReq(func(r *ateapipb.MintJWTRequest) {
r.Audience = []string{"https://a.example.com", "https://a.example.com"}
}),
field.ErrorList{field.Duplicate(field.NewPath("audience").Index(1), nil)},
}, {
"audience entry too long",
validReq(func(r *ateapipb.MintJWTRequest) { r.Audience = []string{strings.Repeat("a", 513)} }),
field.ErrorList{field.TooLong(field.NewPath("audience").Index(0), nil, 512).WithOrigin("maxLength")},
}, {
"missing atespace",
validReq(func(r *ateapipb.MintJWTRequest) { r.Atespace = "" }),
field.ErrorList{field.Required(field.NewPath("atespace"), "")},
}, {
"invalid atespace",
validReq(func(r *ateapipb.MintJWTRequest) { r.Atespace = "AS1" }),
field.ErrorList{field.Invalid(field.NewPath("atespace"), nil, "").WithOrigin("format=k8s-short-name")},
}, {
"missing actor_name",
validReq(func(r *ateapipb.MintJWTRequest) { r.ActorName = "" }),
field.ErrorList{field.Required(field.NewPath("actor_name"), "")},
}, {
"invalid actor_name",
validReq(func(r *ateapipb.MintJWTRequest) { r.ActorName = "invalid value" }),
field.ErrorList{field.Invalid(field.NewPath("actor_name"), nil, "").WithOrigin("format=k8s-short-name")},
}, {
"unspecified actor_uid",
validReq(func(r *ateapipb.MintJWTRequest) { r.ActorUid = "" }),
nil,
}, {
"invalid actor_uid",
validReq(func(r *ateapipb.MintJWTRequest) { r.ActorUid = "not a uid" }),
field.ErrorList{field.Invalid(field.NewPath("actor_uid"), nil, "").WithOrigin("format=k8s-uuid")},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assertValidateErr(t, validateMintJWTRequest(context.Background(), tt.req), tt.want)
})
}
}

func TestValidateMintCertRequest(t *testing.T) {
// This test verifies validation of user input for minting a certificate.
validReq := func(mods ...func(req *ateapipb.MintCertRequest)) *ateapipb.MintCertRequest {
req := &ateapipb.MintCertRequest{
Worker: &ateapipb.ObjectRef{Name: "worker1"},
CertificateSigningRequest: []byte{0x01},
ExpectedActorUid: "01234567-89ab-cdef-0123-456789abcdef",
Purpose: ateapipb.ActorCertificatePurpose_ACTOR_CERTIFICATE_PURPOSE_ATUNNEL,
}
for _, m := range mods {
m(req)
}
return req
}

tests := []struct {
name string
req *ateapipb.MintCertRequest
want field.ErrorList
}{{
"valid",
validReq(),
nil,
}, {
"oversized certificate_signing_request",
validReq(func(r *ateapipb.MintCertRequest) { r.CertificateSigningRequest = make([]byte, 16385) }),
field.ErrorList{field.TooLong(field.NewPath("certificate_signing_request"), nil, 16384)},
}, {
"missing worker",
validReq(func(r *ateapipb.MintCertRequest) { r.Worker = nil }),
field.ErrorList{field.Required(field.NewPath("worker"), "")},
}, {
"worker.atespace must be empty",
validReq(func(r *ateapipb.MintCertRequest) { r.Worker.Atespace = "as1" }),
field.ErrorList{field.Forbidden(field.NewPath("worker", "atespace"), "")},
}, {
"missing worker.name",
validReq(func(r *ateapipb.MintCertRequest) { r.Worker.Name = "" }),
field.ErrorList{field.Required(field.NewPath("worker", "name"), "")},
}, {
"invalid worker.name",
validReq(func(r *ateapipb.MintCertRequest) { r.Worker.Name = "invalid value" }),
field.ErrorList{field.Invalid(field.NewPath("worker", "name"), nil, "").WithOrigin("format=k8s-short-name")},
}, {
"missing certificate_signing_request",
validReq(func(r *ateapipb.MintCertRequest) { r.CertificateSigningRequest = nil }),
field.ErrorList{field.Required(field.NewPath("certificate_signing_request"), "")},
}, {
"missing expected_actor_uid",
validReq(func(r *ateapipb.MintCertRequest) { r.ExpectedActorUid = "" }),
field.ErrorList{field.Required(field.NewPath("expected_actor_uid"), "")},
}, {
"invalid expected_actor_uid",
validReq(func(r *ateapipb.MintCertRequest) { r.ExpectedActorUid = "not a uid" }),
field.ErrorList{field.Invalid(field.NewPath("expected_actor_uid"), nil, "").WithOrigin("format=k8s-uuid")},
}, {
"unspecified purpose",
validReq(func(r *ateapipb.MintCertRequest) {
r.Purpose = ateapipb.ActorCertificatePurpose_ACTOR_CERTIFICATE_PURPOSE_UNSPECIFIED
}),
field.ErrorList{field.Required(field.NewPath("purpose"), "")},
}, {
"out-of-range purpose",
validReq(func(r *ateapipb.MintCertRequest) { r.Purpose = ateapipb.ActorCertificatePurpose(99) }),
field.ErrorList{field.Invalid(field.NewPath("purpose"), nil, "").WithOrigin("maximum")},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assertValidateErr(t, validateMintCertRequest(context.Background(), tt.req), tt.want)
})
}
}
Loading
Loading