From c90836f83a55897e6b03811a376a66aba2d3157b Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Mon, 21 Aug 2023 15:54:10 -0500 Subject: [PATCH] add more grpc route tests --- internal/mesh/internal/types/grpc_route.go | 36 +- .../mesh/internal/types/grpc_route_test.go | 397 +++++++++++++++++- internal/mesh/internal/types/http_route.go | 2 +- .../mesh/internal/types/http_route_test.go | 190 ++++----- 4 files changed, 509 insertions(+), 116 deletions(-) diff --git a/internal/mesh/internal/types/grpc_route.go b/internal/mesh/internal/types/grpc_route.go index eb12fd733ba42..1520288276945 100644 --- a/internal/mesh/internal/types/grpc_route.go +++ b/internal/mesh/internal/types/grpc_route.go @@ -39,8 +39,6 @@ func RegisterGRPCRoute(r resource.Registry) { func ValidateGRPCRoute(res *pbresource.Resource) error { var route pbmesh.GRPCRoute - // TODO(rb):sync common stuff from HTTPRoute - if err := res.Data.UnmarshalTo(&route); err != nil { return resource.NewErrDataParse(&route, err) } @@ -66,8 +64,6 @@ func ValidateGRPCRoute(res *pbresource.Resource) error { } } - // TODO(rb): port a bunch of validation from ServiceRouterConfigEntry.Validate - for j, match := range rule.Matches { wrapMatchErr := func(err error) error { return wrapRuleErr(resource.ErrInvalidListElement{ @@ -77,13 +73,23 @@ func ValidateGRPCRoute(res *pbresource.Resource) error { }) } - if match.Method != nil && match.Method.Type == pbmesh.GRPCMethodMatchType_GRPC_METHOD_MATCH_TYPE_UNSPECIFIED { - merr = multierror.Append(merr, wrapMatchErr( - resource.ErrInvalidField{ - Name: "method", - Wrapped: resource.ErrMissing, - }, - )) + if match.Method != nil { + if match.Method.Type == pbmesh.GRPCMethodMatchType_GRPC_METHOD_MATCH_TYPE_UNSPECIFIED { + merr = multierror.Append(merr, wrapMatchErr( + resource.ErrInvalidField{ + Name: "type", + Wrapped: resource.ErrMissing, + }, + )) + } + if match.Method.Service == "" && match.Method.Method == "" { + merr = multierror.Append(merr, wrapMatchErr( + resource.ErrInvalidField{ + Name: "service", + Wrapped: errors.New("at least one of \"service\" or \"method\" must be set"), + }, + )) + } } for k, header := range match.Headers { @@ -103,6 +109,14 @@ func ValidateGRPCRoute(res *pbresource.Resource) error { }, )) } + if header.Name == "" { + merr = multierror.Append(merr, wrapMatchHeaderErr( + resource.ErrInvalidField{ + Name: "name", + Wrapped: resource.ErrMissing, + }), + ) + } } } diff --git a/internal/mesh/internal/types/grpc_route_test.go b/internal/mesh/internal/types/grpc_route_test.go index 81f795ad60cfe..e5499864b77e9 100644 --- a/internal/mesh/internal/types/grpc_route_test.go +++ b/internal/mesh/internal/types/grpc_route_test.go @@ -49,10 +49,403 @@ func TestValidateGRPCRoute(t *testing.T) { }, expectErr: `invalid "hostnames" field: should not populate hostnames`, }, + "no rules": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + }, + }, + "rules with no matches": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "rules with matches that are empty": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + // none + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "method match with no type is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Method: &pbmesh.GRPCMethodMatch{ + Service: "foo", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "matches": invalid "type" field: missing required field`, + }, + "method match with no service nor method is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Method: &pbmesh.GRPCMethodMatch{ + Type: pbmesh.GRPCMethodMatchType_GRPC_METHOD_MATCH_TYPE_EXACT, + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "matches": invalid "service" field: at least one of "service" or "method" must be set`, + }, + "method match is good (1)": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Method: &pbmesh.GRPCMethodMatch{ + Type: pbmesh.GRPCMethodMatchType_GRPC_METHOD_MATCH_TYPE_EXACT, + Service: "foo", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "method match is good (2)": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Method: &pbmesh.GRPCMethodMatch{ + Type: pbmesh.GRPCMethodMatchType_GRPC_METHOD_MATCH_TYPE_EXACT, + Method: "bar", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "method match is good (3)": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Method: &pbmesh.GRPCMethodMatch{ + Type: pbmesh.GRPCMethodMatchType_GRPC_METHOD_MATCH_TYPE_EXACT, + Service: "foo", + Method: "bar", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "header match with no type is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Headers: []*pbmesh.GRPCHeaderMatch{{ + Name: "x-foo", + }}, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "headers": invalid "type" field: missing required field`, + }, + "header match with no name is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Headers: []*pbmesh.GRPCHeaderMatch{{ + Type: pbmesh.HeaderMatchType_HEADER_MATCH_TYPE_EXACT, + }}, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "headers": invalid "name" field: missing required field`, + }, + "header match is good": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Matches: []*pbmesh.GRPCRouteMatch{{ + Headers: []*pbmesh.GRPCHeaderMatch{{ + Type: pbmesh.HeaderMatchType_HEADER_MATCH_TYPE_EXACT, + Name: "x-foo", + }}, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "filter empty is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + // none + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": exactly one of request_header_modifier, response_header_modifier, or url_rewrite`, + }, + "filter req header mod is ok": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + RequestHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "filter resp header mod is ok": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + ResponseHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "filter rewrite header mod missing path prefix": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + UrlRewrite: &pbmesh.HTTPURLRewriteFilter{}, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": invalid "url_rewrite" field: invalid "path_prefix" field: field should not be empty if enclosing section is set`, + }, + "filter rewrite header mod is ok": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + UrlRewrite: &pbmesh.HTTPURLRewriteFilter{ + PathPrefix: "/blah", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + }, + "filter req+resp header mod is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + RequestHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + ResponseHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": exactly one of request_header_modifier, response_header_modifier, or url_rewrite`, + }, + "filter req+rewrite header mod is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + RequestHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + UrlRewrite: &pbmesh.HTTPURLRewriteFilter{ + PathPrefix: "/blah", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": exactly one of request_header_modifier, response_header_modifier, or url_rewrite`, + }, + "filter resp+rewrite header mod is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + ResponseHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + UrlRewrite: &pbmesh.HTTPURLRewriteFilter{ + PathPrefix: "/blah", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": exactly one of request_header_modifier, response_header_modifier, or url_rewrite`, + }, + "filter req+resp+rewrite header mod is bad": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Filters: []*pbmesh.GRPCRouteFilter{{ + RequestHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + ResponseHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + UrlRewrite: &pbmesh.HTTPURLRewriteFilter{ + PathPrefix: "/blah", + }, + }}, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": exactly one of request_header_modifier, response_header_modifier, or url_rewrite`, + }, + "backend ref with filters is unsupported": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + Filters: []*pbmesh.GRPCRouteFilter{{ + RequestHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + }}, + }}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "backend_refs": invalid "filters" field: filters are not supported at this level yet`, + }, + "nil backend ref": { + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + BackendRefs: []*pbmesh.GRPCBackendRef{nil}, + }}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "backend_refs": invalid "backend_ref" field: missing required field`, + }, + } + + // Add common timeouts test cases. + for name, timeoutsTC := range getXRouteTimeoutsTestCases() { + cases["timeouts: "+name] = testcase{ + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Timeouts: timeoutsTC.timeouts, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: timeoutsTC.expectErr, + } } - // TODO(rb): add rest of tests for the matching logic - // TODO(rb): add rest of tests for the retry and timeout logic + // Add common retries test cases. + for name, retriesTC := range getXRouteRetriesTestCases() { + cases["retries: "+name] = testcase{ + route: &pbmesh.GRPCRoute{ + ParentRefs: []*pbmesh.ParentReference{ + newParentRef(catalog.ServiceType, "web", ""), + }, + Rules: []*pbmesh.GRPCRouteRule{{ + Retries: retriesTC.retries, + BackendRefs: []*pbmesh.GRPCBackendRef{{ + BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + }}, + }}, + }, + expectErr: retriesTC.expectErr, + } + } // Add common parent refs test cases. for name, parentTC := range getXRouteParentRefTestCases() { diff --git a/internal/mesh/internal/types/http_route.go b/internal/mesh/internal/types/http_route.go index 7834c7169e100..4ac846bc81614 100644 --- a/internal/mesh/internal/types/http_route.go +++ b/internal/mesh/internal/types/http_route.go @@ -228,7 +228,7 @@ func ValidateHTTPRoute(res *pbresource.Resource) error { Name: "url_rewrite", Wrapped: resource.ErrInvalidField{ Name: "path_prefix", - Wrapped: resource.ErrMissing, + Wrapped: errors.New("field should not be empty if enclosing section is set"), }, }, )) diff --git a/internal/mesh/internal/types/http_route_test.go b/internal/mesh/internal/types/http_route_test.go index 3d7027f679475..d94c7a36336d5 100644 --- a/internal/mesh/internal/types/http_route_test.go +++ b/internal/mesh/internal/types/http_route_test.go @@ -506,7 +506,7 @@ func TestValidateHTTPRoute(t *testing.T) { }}, }}, }, - expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": invalid "url_rewrite" field: invalid "path_prefix" field: missing required field`, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": invalid "url_rewrite" field: invalid "path_prefix" field: field should not be empty if enclosing section is set`, }, "filter rewrite header mod is ok": { route: &pbmesh.HTTPRoute{ @@ -600,146 +600,69 @@ func TestValidateHTTPRoute(t *testing.T) { }, expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "filters": exactly one of request_header_modifier, response_header_modifier, or url_rewrite`, }, - "timeout: bad request": { - route: &pbmesh.HTTPRoute{ - ParentRefs: []*pbmesh.ParentReference{ - newParentRef(catalog.ServiceType, "web", ""), - }, - Rules: []*pbmesh.HTTPRouteRule{{ - Timeouts: &pbmesh.HTTPRouteTimeouts{ - Request: durationpb.New(-1 * time.Second), - }, - BackendRefs: []*pbmesh.HTTPBackendRef{{ - BackendRef: newBackendRef(catalog.ServiceType, "api", ""), - }}, - }}, - }, - expectErr: `invalid element at index 0 of list "rules": invalid "timeouts" field: invalid "request" field: timeout cannot be negative: -1s`, - }, - "timeout: bad backend request": { - route: &pbmesh.HTTPRoute{ - ParentRefs: []*pbmesh.ParentReference{ - newParentRef(catalog.ServiceType, "web", ""), - }, - Rules: []*pbmesh.HTTPRouteRule{{ - Timeouts: &pbmesh.HTTPRouteTimeouts{ - BackendRequest: durationpb.New(-1 * time.Second), - }, - BackendRefs: []*pbmesh.HTTPBackendRef{{ - BackendRef: newBackendRef(catalog.ServiceType, "api", ""), - }}, - }}, - }, - expectErr: `invalid element at index 0 of list "rules": invalid "timeouts" field: invalid "backend_request" field: timeout cannot be negative: -1s`, - }, - "timeout: bad idle": { - route: &pbmesh.HTTPRoute{ - ParentRefs: []*pbmesh.ParentReference{ - newParentRef(catalog.ServiceType, "web", ""), - }, - Rules: []*pbmesh.HTTPRouteRule{{ - Timeouts: &pbmesh.HTTPRouteTimeouts{ - Idle: durationpb.New(-1 * time.Second), - }, - BackendRefs: []*pbmesh.HTTPBackendRef{{ - BackendRef: newBackendRef(catalog.ServiceType, "api", ""), - }}, - }}, - }, - expectErr: `invalid element at index 0 of list "rules": invalid "timeouts" field: invalid "idle" field: timeout cannot be negative: -1s`, - }, - "timeout: good all": { - route: &pbmesh.HTTPRoute{ - ParentRefs: []*pbmesh.ParentReference{ - newParentRef(catalog.ServiceType, "web", ""), - }, - Rules: []*pbmesh.HTTPRouteRule{{ - Timeouts: &pbmesh.HTTPRouteTimeouts{ - Request: durationpb.New(1 * time.Second), - BackendRequest: durationpb.New(2 * time.Second), - Idle: durationpb.New(3 * time.Second), - }, - BackendRefs: []*pbmesh.HTTPBackendRef{{ - BackendRef: newBackendRef(catalog.ServiceType, "api", ""), - }}, - }}, - }, - }, - "retries: bad number": { + "backend ref with filters is unsupported": { route: &pbmesh.HTTPRoute{ ParentRefs: []*pbmesh.ParentReference{ newParentRef(catalog.ServiceType, "web", ""), }, Rules: []*pbmesh.HTTPRouteRule{{ - Retries: &pbmesh.HTTPRouteRetries{ - Number: -5, - }, BackendRefs: []*pbmesh.HTTPBackendRef{{ BackendRef: newBackendRef(catalog.ServiceType, "api", ""), + Filters: []*pbmesh.HTTPRouteFilter{{ + RequestHeaderModifier: &pbmesh.HTTPHeaderFilter{}, + }}, }}, }}, }, - expectErr: `invalid element at index 0 of list "rules": invalid "retries" field: invalid "number" field: cannot be negative: -5`, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "backend_refs": invalid "filters" field: filters are not supported at this level yet`, }, - "retries: bad conditions": { + "nil backend ref": { route: &pbmesh.HTTPRoute{ ParentRefs: []*pbmesh.ParentReference{ newParentRef(catalog.ServiceType, "web", ""), }, Rules: []*pbmesh.HTTPRouteRule{{ - Retries: &pbmesh.HTTPRouteRetries{ - OnConditions: []string{"garbage"}, - }, - BackendRefs: []*pbmesh.HTTPBackendRef{{ - BackendRef: newBackendRef(catalog.ServiceType, "api", ""), - }}, + BackendRefs: []*pbmesh.HTTPBackendRef{nil}, }}, }, - expectErr: `invalid element at index 0 of list "rules": invalid "retries" field: invalid element at index 0 of list "on_conditions": not a valid retry condition: "garbage"`, + expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "backend_refs": invalid "backend_ref" field: missing required field`, }, - "retries: good all": { + } + + // Add common timeouts test cases. + for name, timeoutsTC := range getXRouteTimeoutsTestCases() { + cases["timeouts: "+name] = testcase{ route: &pbmesh.HTTPRoute{ ParentRefs: []*pbmesh.ParentReference{ newParentRef(catalog.ServiceType, "web", ""), }, Rules: []*pbmesh.HTTPRouteRule{{ - Retries: &pbmesh.HTTPRouteRetries{ - Number: 5, - OnConditions: []string{"internal"}, - }, + Timeouts: timeoutsTC.timeouts, BackendRefs: []*pbmesh.HTTPBackendRef{{ BackendRef: newBackendRef(catalog.ServiceType, "api", ""), }}, }}, }, - }, - "backend ref with filters is unsupported": { + expectErr: timeoutsTC.expectErr, + } + } + + // Add common retries test cases. + for name, retriesTC := range getXRouteRetriesTestCases() { + cases["retries: "+name] = testcase{ route: &pbmesh.HTTPRoute{ ParentRefs: []*pbmesh.ParentReference{ newParentRef(catalog.ServiceType, "web", ""), }, Rules: []*pbmesh.HTTPRouteRule{{ + Retries: retriesTC.retries, BackendRefs: []*pbmesh.HTTPBackendRef{{ BackendRef: newBackendRef(catalog.ServiceType, "api", ""), - Filters: []*pbmesh.HTTPRouteFilter{{ - RequestHeaderModifier: &pbmesh.HTTPHeaderFilter{}, - }}, }}, }}, }, - expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "backend_refs": invalid "filters" field: filters are not supported at this level yet`, - }, - "nil backend ref": { - route: &pbmesh.HTTPRoute{ - ParentRefs: []*pbmesh.ParentReference{ - newParentRef(catalog.ServiceType, "web", ""), - }, - Rules: []*pbmesh.HTTPRouteRule{{ - BackendRefs: []*pbmesh.HTTPBackendRef{nil}, - }}, - }, - expectErr: `invalid element at index 0 of list "rules": invalid element at index 0 of list "backend_refs": invalid "backend_ref" field: missing required field`, - }, + expectErr: retriesTC.expectErr, + } } // Add common parent refs test cases. @@ -918,6 +841,69 @@ func getXRouteBackendRefTestCases() map[string]xRouteBackendRefTestcase { } } +type xRouteTimeoutsTestcase struct { + timeouts *pbmesh.HTTPRouteTimeouts + expectErr string +} + +func getXRouteTimeoutsTestCases() map[string]xRouteTimeoutsTestcase { + return map[string]xRouteTimeoutsTestcase{ + "bad request": { + timeouts: &pbmesh.HTTPRouteTimeouts{ + Request: durationpb.New(-1 * time.Second), + }, + expectErr: `invalid element at index 0 of list "rules": invalid "timeouts" field: invalid "request" field: timeout cannot be negative: -1s`, + }, + "bad backend request": { + timeouts: &pbmesh.HTTPRouteTimeouts{ + BackendRequest: durationpb.New(-1 * time.Second), + }, + expectErr: `invalid element at index 0 of list "rules": invalid "timeouts" field: invalid "backend_request" field: timeout cannot be negative: -1s`, + }, + "bad idle": { + timeouts: &pbmesh.HTTPRouteTimeouts{ + Idle: durationpb.New(-1 * time.Second), + }, + expectErr: `invalid element at index 0 of list "rules": invalid "timeouts" field: invalid "idle" field: timeout cannot be negative: -1s`, + }, + "good all": { + timeouts: &pbmesh.HTTPRouteTimeouts{ + Request: durationpb.New(1 * time.Second), + BackendRequest: durationpb.New(2 * time.Second), + Idle: durationpb.New(3 * time.Second), + }, + }, + } +} + +type xRouteRetriesTestcase struct { + retries *pbmesh.HTTPRouteRetries + expectErr string +} + +func getXRouteRetriesTestCases() map[string]xRouteRetriesTestcase { + return map[string]xRouteRetriesTestcase{ + "bad number": { + retries: &pbmesh.HTTPRouteRetries{ + Number: -5, + }, + expectErr: `invalid element at index 0 of list "rules": invalid "retries" field: invalid "number" field: cannot be negative: -5`, + }, + "bad conditions": { + retries: &pbmesh.HTTPRouteRetries{ + OnConditions: []string{"garbage"}, + }, + expectErr: `invalid element at index 0 of list "rules": invalid "retries" field: invalid element at index 0 of list "on_conditions": not a valid retry condition: "garbage"`, + }, + "good all": { + retries: &pbmesh.HTTPRouteRetries{ + Number: 5, + OnConditions: []string{"internal"}, + }, + }, + } +} + func newRef(typ *pbresource.Type, name string) *pbresource.Reference { return resourcetest.Resource(typ, name).Reference("") }