Skip to content

HTTPRoute timeout #2013

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

Merged
merged 11 commits into from
Sep 12, 2023
4 changes: 4 additions & 0 deletions apis/v1alpha2/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,10 @@ type AnnotationValue = v1beta1.AnnotationValue
// +kubebuilder:validation:Pattern=`^Hostname|IPAddress|NamedAddress|[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$`
type AddressType = v1beta1.AddressType

// Duration is a string value representing a duration in time. The format is as specified
// in GEP-2257, a strict subset of the syntax parsed by Golang time.ParseDuration.
type Duration = v1beta1.Duration

const (
// A textual representation of a numeric IP address. IPv4
// addresses must be in dotted-decimal form. IPv6 addresses
Expand Down
51 changes: 51 additions & 0 deletions apis/v1beta1/httproute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,57 @@ type HTTPRouteRule struct {
// +optional
// +kubebuilder:validation:MaxItems=16
BackendRefs []HTTPBackendRef `json:"backendRefs,omitempty"`

// Timeouts defines the timeouts that can be configured for an HTTP request.
//
// Support: Extended
//
// +optional
// <gateway:experimental>
Timeouts *HTTPRouteTimeouts `json:"timeouts,omitempty"`
}

// HTTPRouteTimeouts defines timeouts that can be configured for an HTTPRoute.
// Timeout values are represented with Gateway API Duration formatting.
// Specifying a zero value such as "0s" is interpreted as no timeout.
//
// +kubebuilder:validation:XValidation:message="backendRequest timeout cannot be longer than request timeout",rule="!(has(self.request) && has(self.backendRequest) && duration(self.request) != duration('0s') && duration(self.backendRequest) > duration(self.request))"
type HTTPRouteTimeouts struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need type HTTPRouteTimeouts = v1beta1.HTTPRouteTimeouts in apis/v1alpha2/httproute_types.go?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave it out. We're moving away from v1alpha2 types (they're no longer served in v0.8.0 CRDs) and I'd expect most/all implementations to have moved away from them before we hit v1.0.0. I haven't had time to figure out if we should leave v1alpha2 type definitions in place for one more release or drop them in v1.0, but in either case, I think I'd rather avoid adding anything new to them.

// Request specifies the maximum duration for a gateway to respond to an HTTP request.
// If the gateway has not been able to respond before this deadline is met, the gateway
// MUST return a timeout error.
//
// For example, setting the `rules.timeouts.request` field to the value `10s` in an
// `HTTPRoute` will cause a timeout if a client request is taking longer than 10 seconds
// to complete.
//
// This timeout is intended to cover as close to the whole request-response transaction
// as possible although an implementation MAY choose to start the timeout after the entire
// request stream has been received instead of immediately after the transaction is
// initiated by the client.
//
// When this field is unspecified, request timeout behavior is implementation-specific.
//
// Support: Extended
//
// +optional
Request *Duration `json:"request,omitempty"`

// BackendRequest specifies a timeout for an individual request from the gateway
// to a backend. This covers the time from when the request first starts being
// sent from the gateway to when the full response has been received from the backend.
//
// An entire client HTTP transaction with a gateway, covered by the Request timeout,
// may result in more than one call from the gateway to the destination backend,
// for example, if automatic retries are supported.
//
// Because the Request timeout encompasses the BackendRequest timeout, the value of
// BackendRequest must be <= the value of Request timeout.
//
// Support: Extended
//
// +optional
BackendRequest *Duration `json:"backendRequest,omitempty"`
}

// PathMatchType specifies the semantics of how HTTP paths should be compared.
Expand Down
6 changes: 6 additions & 0 deletions apis/v1beta1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,12 @@ type AddressType string
// +k8s:deepcopy-gen=false
type HeaderName string

// Duration is a string value representing a duration in time. The format is as specified
// in GEP-2257, a strict subset of the syntax parsed by Golang time.ParseDuration.
//
// +kubebuilder:validation:Pattern=`^([0-9]{1,5}(h|m|s|ms)){1,4}$`
type Duration string

const (
// A textual representation of a numeric IP address. IPv4
// addresses must be in dotted-decimal form. IPv6 addresses
Expand Down
20 changes: 20 additions & 0 deletions apis/v1beta1/validation/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"net/http"
"regexp"
"strings"
"time"

"k8s.io/apimachinery/pkg/util/validation/field"

Expand Down Expand Up @@ -73,6 +74,10 @@ func ValidateHTTPRouteSpec(spec *gatewayv1b1.HTTPRouteSpec, path *field.Path) fi
errs = append(errs, validateHTTPQueryParamMatches(m.QueryParams, matchPath.Child("queryParams"))...)
}
}

if rule.Timeouts != nil {
errs = append(errs, validateHTTPRouteTimeouts(rule.Timeouts, path.Child("rules").Child("timeouts"))...)
}
}
errs = append(errs, validateHTTPRouteBackendServicePorts(spec.Rules, path.Child("rules"))...)
errs = append(errs, ValidateParentRefs(spec.ParentRefs, path.Child("spec"))...)
Expand Down Expand Up @@ -348,6 +353,21 @@ func validateHTTPHeaderModifier(filter gatewayv1b1.HTTPHeaderFilter, path *field
return errs
}

func validateHTTPRouteTimeouts(timeouts *gatewayv1b1.HTTPRouteTimeouts, path *field.Path) field.ErrorList {
var errs field.ErrorList
if timeouts.BackendRequest != nil {
backendTimeout, _ := time.ParseDuration((string)(*timeouts.BackendRequest))
if timeouts.Request != nil {
timeout, _ := time.ParseDuration((string)(*timeouts.Request))
if backendTimeout > timeout && timeout != 0 {
errs = append(errs, field.Invalid(path.Child("backendRequest"), backendTimeout, "backendRequest timeout cannot be longer than request timeout"))
}
}
}

return errs
}

func hasExactlyOnePrefixMatch(matches []gatewayv1b1.HTTPRouteMatch) bool {
if len(matches) != 1 || matches[0].Path == nil {
return false
Expand Down
129 changes: 129 additions & 0 deletions apis/v1beta1/validation/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1176,3 +1176,132 @@ func TestValidateRequestRedirectFiltersWithNoBackendRef(t *testing.T) {
})
}
}

func toDuration(durationString string) *gatewayv1b1.Duration {
return (*gatewayv1b1.Duration)(&durationString)
}

func TestValidateHTTPTimeouts(t *testing.T) {
tests := []struct {
name string
rules []gatewayv1b1.HTTPRouteRule
errCount int
}{
{
name: "valid httpRoute Rules timeouts",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("1ms"),
},
},
},
}, {
name: "valid httpRoute Rules timeout set to 0s (disabled)",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("0s"),
},
},
},
}, {
name: "valid httpRoute Rules timeout set to 0ms (disabled)",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("0ms"),
},
},
},
}, {}, {
name: "valid httpRoute Rules timeout set to 0h (disabled)",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("0h"),
},
},
},
}, {
name: "valid httpRoute Rules timeout and backendRequest have the same value",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("1ms"),
BackendRequest: toDuration("1ms"),
},
},
},
}, {
name: "invalid httpRoute Rules backendRequest timeout cannot be longer than request timeout",
errCount: 1,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("1ms"),
BackendRequest: toDuration("2ms"),
},
},
},
}, {
name: "valid httpRoute Rules request timeout 1s and backendRequest timeout 200ms",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("1s"),
BackendRequest: toDuration("200ms"),
},
},
},
}, {
name: "valid httpRoute Rules request timeout 10s and backendRequest timeout 10s",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("10s"),
BackendRequest: toDuration("10s"),
},
},
},
}, {
name: "invalid httpRoute Rules backendRequest timeout cannot be greater than request timeout",
errCount: 1,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("200ms"),
BackendRequest: toDuration("1s"),
},
},
},
}, {
name: "valid httpRoute Rules request 0s (infinite) and backendRequest 100ms",
errCount: 0,
rules: []gatewayv1b1.HTTPRouteRule{
{
Timeouts: &gatewayv1b1.HTTPRouteTimeouts{
Request: toDuration("0s"),
BackendRequest: toDuration("100ms"),
},
},
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
route := gatewayv1b1.HTTPRoute{Spec: gatewayv1b1.HTTPRouteSpec{Rules: tc.rules}}
errs := ValidateHTTPRoute(&route)
if len(errs) != tc.errCount {
t.Errorf("got %d errors, want %d errors: %s", len(errs), tc.errCount, errs)
}
})
}
}
30 changes: 30 additions & 0 deletions apis/v1beta1/zz_generated.deepcopy.go

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

84 changes: 84 additions & 0 deletions config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml

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

Loading