diff --git a/apis/v1alpha2/shared_types.go b/apis/v1alpha2/shared_types.go index bba8e95160..788fef9a06 100644 --- a/apis/v1alpha2/shared_types.go +++ b/apis/v1alpha2/shared_types.go @@ -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 diff --git a/apis/v1beta1/httproute_types.go b/apis/v1beta1/httproute_types.go index e8216ed33c..fb6809ddb3 100644 --- a/apis/v1beta1/httproute_types.go +++ b/apis/v1beta1/httproute_types.go @@ -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 + // + 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 { + // 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. diff --git a/apis/v1beta1/shared_types.go b/apis/v1beta1/shared_types.go index b18af1d0e0..b879d3a1fd 100644 --- a/apis/v1beta1/shared_types.go +++ b/apis/v1beta1/shared_types.go @@ -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 diff --git a/apis/v1beta1/validation/httproute.go b/apis/v1beta1/validation/httproute.go index 8dbe4e8f3d..a1c96b8722 100644 --- a/apis/v1beta1/validation/httproute.go +++ b/apis/v1beta1/validation/httproute.go @@ -21,6 +21,7 @@ import ( "net/http" "regexp" "strings" + "time" "k8s.io/apimachinery/pkg/util/validation/field" @@ -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"))...) @@ -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 diff --git a/apis/v1beta1/validation/httproute_test.go b/apis/v1beta1/validation/httproute_test.go index c157dc3704..ca29f0b067 100644 --- a/apis/v1beta1/validation/httproute_test.go +++ b/apis/v1beta1/validation/httproute_test.go @@ -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) + } + }) + } +} diff --git a/apis/v1beta1/zz_generated.deepcopy.go b/apis/v1beta1/zz_generated.deepcopy.go index d1d46269d2..b4c5028366 100644 --- a/apis/v1beta1/zz_generated.deepcopy.go +++ b/apis/v1beta1/zz_generated.deepcopy.go @@ -815,6 +815,11 @@ func (in *HTTPRouteRule) DeepCopyInto(out *HTTPRouteRule) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Timeouts != nil { + in, out := &in.Timeouts, &out.Timeouts + *out = new(HTTPRouteTimeouts) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HTTPRouteRule. @@ -871,6 +876,31 @@ func (in *HTTPRouteStatus) DeepCopy() *HTTPRouteStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HTTPRouteTimeouts) DeepCopyInto(out *HTTPRouteTimeouts) { + *out = *in + if in.Request != nil { + in, out := &in.Request, &out.Request + *out = new(Duration) + **out = **in + } + if in.BackendRequest != nil { + in, out := &in.BackendRequest, &out.BackendRequest + *out = new(Duration) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HTTPRouteTimeouts. +func (in *HTTPRouteTimeouts) DeepCopy() *HTTPRouteTimeouts { + if in == nil { + return nil + } + out := new(HTTPRouteTimeouts) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HTTPURLRewriteFilter) DeepCopyInto(out *HTTPURLRewriteFilter) { *out = *in diff --git a/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml b/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml index 49f324c1f6..020cdc1ac3 100644 --- a/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml +++ b/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml @@ -2113,6 +2113,48 @@ spec: type: object maxItems: 8 type: array + timeouts: + description: "Timeouts defines the timeouts that can be configured + for an HTTP request. \n Support: Extended \n " + properties: + backendRequest: + description: "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. \n 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. + \n Because the Request timeout encompasses the BackendRequest + timeout, the value of BackendRequest must be <= the value + of Request timeout. \n Support: Extended" + pattern: ^([0-9]{1,5}(h|m|s|ms)){1,4}$ + type: string + request: + description: "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. \n 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. + \n 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. \n When this + field is unspecified, request timeout behavior is implementation-specific. + \n Support: Extended" + pattern: ^([0-9]{1,5}(h|m|s|ms)){1,4}$ + type: string + type: object + x-kubernetes-validations: + - 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: object x-kubernetes-validations: - message: RequestRedirect filter must not be used together with @@ -4512,6 +4554,48 @@ spec: type: object maxItems: 8 type: array + timeouts: + description: "Timeouts defines the timeouts that can be configured + for an HTTP request. \n Support: Extended \n " + properties: + backendRequest: + description: "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. \n 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. + \n Because the Request timeout encompasses the BackendRequest + timeout, the value of BackendRequest must be <= the value + of Request timeout. \n Support: Extended" + pattern: ^([0-9]{1,5}(h|m|s|ms)){1,4}$ + type: string + request: + description: "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. \n 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. + \n 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. \n When this + field is unspecified, request timeout behavior is implementation-specific. + \n Support: Extended" + pattern: ^([0-9]{1,5}(h|m|s|ms)){1,4}$ + type: string + type: object + x-kubernetes-validations: + - 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: object x-kubernetes-validations: - message: RequestRedirect filter must not be used together with diff --git a/pkg/test/cel/httproute_experimental_test.go b/pkg/test/cel/httproute_experimental_test.go index d77ea3638a..7cbb064c4a 100644 --- a/pkg/test/cel/httproute_experimental_test.go +++ b/pkg/test/cel/httproute_experimental_test.go @@ -233,3 +233,139 @@ func TestHTTPRouteParentRefExperimental(t *testing.T) { }) } } + +func toDuration(durationString string) *gatewayv1b1.Duration { + return (*gatewayv1b1.Duration)(&durationString) +} + +func TestHTTPRouteTimeouts(t *testing.T) { + tests := []struct { + name string + wantErrors []string + rules []gatewayv1b1.HTTPRouteRule + }{ + { + name: "invalid timeout unit us is not supported", + wantErrors: []string{"Invalid value: \"100us\": spec.rules[0].timeouts.request in body should match '^([0-9]{1,5}(h|m|s|ms)){1,4}$'"}, + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("100us"), + }, + }, + }, + }, + { + name: "invalid timeout unit ns is not supported", + wantErrors: []string{"Invalid value: \"500ns\": spec.rules[0].timeouts.request in body should match '^([0-9]{1,5}(h|m|s|ms)){1,4}$'"}, + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("500ns"), + }, + }, + }, + }, + { + name: "valid timeout request and backendRequest", + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("4s"), + BackendRequest: toDuration("2s"), + }, + }, + }, + }, + { + name: "valid timeout request", + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("0s"), + }, + }, + }, + }, + { + name: "invalid timeout request day unit not supported", + wantErrors: []string{"Invalid value: \"1d\": spec.rules[0].timeouts.request in body should match '^([0-9]{1,5}(h|m|s|ms)){1,4}$'"}, + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("1d"), + }, + }, + }, + }, + { + name: "invalid timeout request decimal not supported ", + wantErrors: []string{"Invalid value: \"0.5s\": spec.rules[0].timeouts.request in body should match '^([0-9]{1,5}(h|m|s|ms)){1,4}$'"}, + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("0.5s"), + }, + }, + }, + }, + { + name: "valid timeout request infinite greater than backendRequest 1ms", + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("0s"), + BackendRequest: toDuration("1ms"), + }, + }, + }, + }, + { + name: "valid timeout request 1s greater than backendRequest 200ms", + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("1s"), + BackendRequest: toDuration("200ms"), + }, + }, + }, + }, + { + name: "valid timeout request 10s equal backendRequest 10s", + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("10s"), + BackendRequest: toDuration("10s"), + }, + }, + }, + }, + { + name: "invalid timeout request 200ms less than backendRequest 1s", + wantErrors: []string{"Invalid value: \"object\": backendRequest timeout cannot be longer than request timeout"}, + rules: []gatewayv1b1.HTTPRouteRule{ + { + Timeouts: &gatewayv1b1.HTTPRouteTimeouts{ + Request: toDuration("200ms"), + BackendRequest: toDuration("1s"), + }, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + route := &gatewayv1b1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("foo-%v", time.Now().UnixNano()), + Namespace: metav1.NamespaceDefault, + }, + Spec: gatewayv1b1.HTTPRouteSpec{Rules: tc.rules}, + } + validateHTTPRoute(t, route, tc.wantErrors) + }) + } +}