From f3fa8a584274d316f9bad7d6de5b9c4343fe4bc7 Mon Sep 17 00:00:00 2001 From: Michael Costello Date: Tue, 10 Nov 2026 14:34:24 -0500 Subject: [PATCH 1/2] feat: Let HTTPProxy rules set the response timeout The generated HTTPRoute carried no timeouts, so Envoy applied its own 15-second default to every response. Streams, large downloads and slow APIs were cut mid-transfer, and nothing in the HTTPProxy API could change it. Customers reached past the operator with a BackendTrafficPolicy on the generated route or gateway. Each rule now carries the Gateway API timeouts, copied to the route. The request timeout defaults to one hour, the cap the operator already enforces on BackendTrafficPolicy, so a timeout set on a proxy and one set on a policy meet the same limit. Zero is refused: Envoy Gateway drops the route's idle timeout along with a disabled request timeout. Key changes: - Add timeouts to HTTPProxyRule with a default request timeout of 1h - Copy the rule's timeouts onto the generated HTTPRoute rule - Cap request and backendRequest at the configured maximum and refuse 0s - Add an e2e scenario that streams 20 s through the edge with the default and sees the same stream cut by a 5 s rule timeout - Replace the BackendTrafficPolicy workaround in the plugin skills Fixes #459 --- api/v1alpha/httpproxy_types.go | 13 + api/v1alpha/zz_generated.deepcopy.go | 5 + claude-plugin/skills/alb/SKILL.md | 45 ++- claude-plugin/skills/httproute/SKILL.md | 2 +- .../networking.datumapis.com_httpproxies.yaml | 70 ++++ docs/agent/skills/access-log-triage.md | 11 +- docs/api/httpproxies.md | 99 ++++++ internal/controller/httpproxy_controller.go | 3 +- .../controller/httpproxy_controller_test.go | 12 + internal/validation/httpproxy_validation.go | 51 ++- .../validation/httpproxy_validation_test.go | 67 ++++ internal/webhook/v1alpha/httpproxy_webhook.go | 2 +- .../chainsaw-test.yaml | 322 ++++++++++++++++++ 13 files changed, 664 insertions(+), 38 deletions(-) create mode 100644 test/e2e/httpproxy-response-timeout/chainsaw-test.yaml diff --git a/api/v1alpha/httpproxy_types.go b/api/v1alpha/httpproxy_types.go index dac3aceb..67581075 100644 --- a/api/v1alpha/httpproxy_types.go +++ b/api/v1alpha/httpproxy_types.go @@ -266,6 +266,19 @@ type HTTPProxyRule struct { // +kubebuilder:validation:MinItems=0 // +kubebuilder:validation:MaxItems=16 Backends []HTTPProxyRuleBackend `json:"backends,omitempty"` + + // Timeouts bound how long a request matching this rule may take. + // + // The request timeout runs from the request reaching the edge until the + // last byte of the response is sent, so a streaming response or a large + // download must finish within it. It defaults to one hour, and the operator + // caps it at a configured maximum. It cannot be disabled. + // + // See documentation for the `timeouts` field in the `HTTPRouteRule` type at + // https://gateway-api.sigs.k8s.io/reference/spec/#httprouterule + // + // +kubebuilder:default={request: "1h"} + Timeouts *gatewayv1.HTTPRouteTimeouts `json:"timeouts,omitempty"` } // +kubebuilder:validation:XValidation:message="endpoint is required unless instance or networkService is set; instance and networkService are mutually exclusive with each other and with endpoint and connector",rule="has(self.instance) ? (!has(self.endpoint) && !has(self.connector) && !has(self.networkService)) : (has(self.networkService) ? (!has(self.endpoint) && !has(self.connector)) : has(self.endpoint))" diff --git a/api/v1alpha/zz_generated.deepcopy.go b/api/v1alpha/zz_generated.deepcopy.go index 7da540a5..77bca37a 100644 --- a/api/v1alpha/zz_generated.deepcopy.go +++ b/api/v1alpha/zz_generated.deepcopy.go @@ -561,6 +561,11 @@ func (in *HTTPProxyRule) DeepCopyInto(out *HTTPProxyRule) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Timeouts != nil { + in, out := &in.Timeouts, &out.Timeouts + *out = new(apisv1.HTTPRouteTimeouts) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HTTPProxyRule. diff --git a/claude-plugin/skills/alb/SKILL.md b/claude-plugin/skills/alb/SKILL.md index 8fc0864c..71d10f85 100644 --- a/claude-plugin/skills/alb/SKILL.md +++ b/claude-plugin/skills/alb/SKILL.md @@ -20,7 +20,7 @@ An Application Load Balancer is a **product built from several objects**, not a - Add routes and the origins behind them, by URL or by network service - Split a route's traffic across several origins by weight, choose the load balancing algorithm, and turn on passive health checks - Configure traffic protection (WAF), request headers and basic authentication -- Raise the 15-second response limit for streams and large downloads +- Set how long a response may run, up to an hour, per route - Read access logs for one load balancer - Preview every change before applying it - Attach advanced Envoy Gateway policies the plugin does not cover @@ -233,43 +233,36 @@ Datum attaches a `BackendTrafficPolicy` of its own to the load balancer's routin A Gateway takes one `BackendTrafficPolicy`. Put rate limiting, circuit breaking and the response limit below in the same policy. -### Responses that run longer than 15 seconds +### Responses that run longer than an hour -A response must finish within about 15 seconds of the request reaching the edge. After that the edge ends it: +Every route rule carries `timeouts.request`: the time from the request reaching the edge to the last byte of the response. It defaults to `1h`, so streams, large downloads and slow APIs run for up to an hour before the edge ends them: - If the origin has not started answering, the client gets a `504`. -- If the origin has started, the response is cut off mid-transfer with whatever status it began with. Large downloads stop partway, and Server-Sent Events, LLM token streams, log tails and long polls close. +- If the origin has started, the response is cut off mid-transfer with whatever status it began with. -WebSockets are not cut. In `alb logs`, a cut response shows a `DURATION` of about 15000 ms. +WebSockets are not cut. In `alb logs`, a cut response shows a `DURATION` equal to the rule's timeout. -`HTTPProxy` has no timeout field. Raise the limit, up to `1h`, with `timeout.http.requestTimeout` on the load balancer's Gateway: +The plugin does not set the timeout. Set it per rule in the `HTTPProxy` manifest, between `1ns` and `1h`. A lower value makes a hung origin fail fast; `0s` is refused, since it would also switch off the idle timeout and leave abandoned streams open: ```yaml -apiVersion: gateway.envoyproxy.io/v1alpha1 -kind: BackendTrafficPolicy +apiVersion: networking.datumapis.com/v1alpha +kind: HTTPProxy metadata: name: my-app namespace: default spec: - targetRefs: - - group: gateway.networking.k8s.io - kind: Gateway - name: my-app - timeout: - http: - requestTimeout: 1h + rules: + - matches: + - path: + type: PathPrefix + value: / + backends: + - endpoint: https://origin.example.com + timeouts: + request: 10m ``` -**Target the Gateway, never the HTTPRoute.** An HTTPRoute takes one `BackendTrafficPolicy`, and Datum attaches its own to the HTTPRoute when the load balancer has a network service origin, an algorithm or health checks. The older of the two wins and the other is rejected as `Conflicted`: - -- If Datum's is older, yours does nothing and responses are still cut at 15 seconds. -- If yours is older, Datum's is rejected and the load balancer silently loses its algorithm and health checks, while still reporting `Programmed`. - -A Gateway-level policy merges with Datum's, so both apply. Editing `timeouts` on the generated HTTPRoute does not work either: Datum rewrites that route from the `HTTPProxy` and the edit is reverted. - -`requestTimeout` covers everything from the request arriving to the last byte sent, so set it longer than the longest stream or download. A value above `1h` is rejected with `duration must be less than or equal to 1h0m0s`. - -To confirm it worked, check that `datumctl describe backendtrafficpolicy my-app` shows `Accepted` and not `Conflicted`, then make a request that runs past 15 seconds and check that it completes. To restore the default, delete the policy, or remove the `timeout` block if the policy carries other settings. +A value above `1h` is rejected with `duration must be less than or equal to 1h0m0s`. A `BackendTrafficPolicy` `requestTimeout` does not change a load balancer's limit any more: the rule's timeout takes precedence. Remove one you attached for that purpose. ### Authentication beyond basic auth @@ -302,7 +295,7 @@ Failures print a named exit code — `exit status 6 # ALB_INVALID`. The names ar - The portal manages one route: the pool of origins on `/`, with weights, the algorithm and health checks. Those are the same fields `datumctl alb` writes, so either can change them. Once a load balancer has a second route, a path match or a per-origin filter, the portal locks its origins, weights, algorithm, health checks, TLS, redirect and Host override and points to `datumctl`; hostnames, protection and auth stay editable there. Tell the user to expect that lock after you add a route - Traffic protection takes paranoia 1 to 4 here; the portal offers only 1 and 2 - Attach a `BackendTrafficPolicy` to the load balancer's Gateway, never its HTTPRoute. A route-level policy collides with the one Datum attaches for health checks and load balancing -- Responses end after about 15 seconds unless a `BackendTrafficPolicy` raises the limit, up to 1 hour +- Responses end after 1 hour by default. Lower it per rule with `timeouts.request` in the manifest; it cannot go above 1 hour or be switched off - Run `datumctl diff -f` before `apply`, and validate with `--dry-run=server` - `delete` has no confirmation prompt — verify the name first. Deleting a load balancer also removes its traffic protection policy and basic auth - Start traffic protection in `mode: Observe` on a live site, watch real traffic, then move to `Enforce`. Raise paranoia one level at a time; above 2 the false-positive rate climbs sharply diff --git a/claude-plugin/skills/httproute/SKILL.md b/claude-plugin/skills/httproute/SKILL.md index 34472b20..35a559e7 100644 --- a/claude-plugin/skills/httproute/SKILL.md +++ b/claude-plugin/skills/httproute/SKILL.md @@ -334,7 +334,7 @@ datumctl apply -f header-injection.yaml --project my-project - Use `sectionName` in `spec.parentRefs[]` to bind a route to a specific listener when a Gateway has both HTTP and HTTPS listeners; binding only to `http` is the standard pattern for redirect routes - `gateway.networking.k8s.io/v1` is stable GA — unlike the `gateway.envoyproxy.io` policy CRDs it is not subject to breaking alpha changes - Policy attachments from the alb skill (`SecurityPolicy`, `BackendTrafficPolicy`, `TrafficProtectionPolicy`) reference `HTTPRoute` resources by name via `targetRefs` — create the HTTPRoute first before attaching policies -- This skill is for HTTPRoutes you write yourself. An Application Load Balancer's HTTPRoute is generated from its `HTTPProxy` and rewritten on every change, so edits to it, `timeouts` included, are reverted. To let a load balancer's responses run longer than 15 seconds, attach a `BackendTrafficPolicy` to its Gateway as the [alb skill](../alb/SKILL.md) describes +- This skill is for HTTPRoutes you write yourself. An Application Load Balancer's HTTPRoute is generated from its `HTTPProxy` and rewritten on every change, so edits to it, `timeouts` included, are reverted. Set a load balancer's response limit with `timeouts.request` on the `HTTPProxy` rule instead, as the [alb skill](../alb/SKILL.md) describes - Run `datumctl diff -f` before `apply` for any changes - `--dry-run=server` validates the manifest against the API before committing - `delete` has no confirmation prompt — always verify the resource name first diff --git a/config/crd/bases/networking.datumapis.com_httpproxies.yaml b/config/crd/bases/networking.datumapis.com_httpproxies.yaml index 0ae6b601..9708a34e 100644 --- a/config/crd/bases/networking.datumapis.com_httpproxies.yaml +++ b/config/crd/bases/networking.datumapis.com_httpproxies.yaml @@ -3458,6 +3458,76 @@ spec: minLength: 1 pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ type: string + timeouts: + default: + request: 1h + description: |- + Timeouts bound how long a request matching this rule may take. + + The request timeout runs from the request reaching the edge until the + last byte of the response is sent, so a streaming response or a large + download must finish within it. It defaults to one hour, and the operator + caps it at a configured maximum. It cannot be disabled. + + See documentation for the `timeouts` field in the `HTTPRouteRule` type at + https://gateway-api.sigs.k8s.io/reference/spec/#httprouterule + 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. + + Setting a timeout to the zero duration (e.g. "0s") SHOULD disable the timeout + completely. Implementations that cannot completely disable the timeout MUST + instead interpret the zero duration as the longest possible value to which + the timeout can be set. + + 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. + + The value of BackendRequest must be a Gateway API Duration string as defined by + GEP-2257. When this field is unspecified, its behavior is implementation-specific; + when specified, the value of BackendRequest must be no more than the value of the + Request timeout (since the Request timeout encompasses the BackendRequest timeout). + + 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. + + 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. + + Setting a timeout to the zero duration (e.g. "0s") SHOULD disable the timeout + completely. Implementations that cannot completely disable the timeout MUST + instead interpret the zero duration as the longest possible value to which + the timeout can be set. + + 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. + + The value of Request is a Gateway API Duration string as defined by GEP-2257. When this + field is unspecified, request timeout behavior is implementation-specific. + + 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/docs/agent/skills/access-log-triage.md b/docs/agent/skills/access-log-triage.md index ac011886..456abc6d 100644 --- a/docs/agent/skills/access-log-triage.md +++ b/docs/agent/skills/access-log-triage.md @@ -56,11 +56,12 @@ Then widen the window, or send a request yourself and look again. is down, refusing connections, or too slow. - `429`: rate limited. -**Durations near 15 seconds** mean the response limit, not a slow origin. A -response must finish within about 15 seconds of the request reaching the edge. -A request whose origin had not started answering by then comes back `504`. One -whose origin had started keeps its status, `200` included, and is cut off -mid-transfer. So a `200` at about 15000 ms is a large download or a stream that +**Durations at the route's timeout** mean the response limit, not a slow +origin. A response must finish within the rule's `timeouts.request`, one hour +by default, of the request reaching the edge. A request whose origin had not +started answering by then comes back `504`. One whose origin had started keeps +its status, `200` included, and is cut off mid-transfer. So a `200` at about +3600000 ms, or at whatever the rule sets, is a large download or a stream that was cut short, not a success. WebSockets are not cut. The limit can be raised to at most one hour. `HTTPProxy` has no field for it. diff --git a/docs/api/httpproxies.md b/docs/api/httpproxies.md index c765aaad..be6ce4bb 100644 --- a/docs/api/httpproxies.md +++ b/docs/api/httpproxies.md @@ -235,6 +235,24 @@ https://gateway-api.sigs.k8s.io/reference/spec/#httprouterule
if it is set.
false + + timeouts + object + + Timeouts bound how long a request matching this rule may take. + +The request timeout runs from the request reaching the edge until the +last byte of the response is sent, so a streaming response or a large +download must finish within it. It defaults to one hour, and the operator +caps it at a configured maximum. It cannot be disabled. + +See documentation for the `timeouts` field in the `HTTPRouteRule` type at +https://gateway-api.sigs.k8s.io/reference/spec/#httprouterule
+
+ Validations:
  • !(has(self.request) && has(self.backendRequest) && duration(self.request) != duration('0s') && duration(self.backendRequest) > duration(self.request)): backendRequest timeout cannot be longer than request timeout
    • + Default: map[request:1h]
    + + false @@ -4324,6 +4342,87 @@ documentation to determine the supported dialect.
    +### HTTPProxy.spec.rules[index].timeouts +[↩ Parent](#httpproxyspecrulesindex) + + + +Timeouts bound how long a request matching this rule may take. + +The request timeout runs from the request reaching the edge until the +last byte of the response is sent, so a streaming response or a large +download must finish within it. It defaults to one hour, and the operator +caps it at a configured maximum. It cannot be disabled. + +See documentation for the `timeouts` field in the `HTTPRouteRule` type at +https://gateway-api.sigs.k8s.io/reference/spec/#httprouterule + + + + + + + + + + + + + + + + + + + + + +
    NameTypeDescriptionRequired
    backendRequeststring + 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. + +Setting a timeout to the zero duration (e.g. "0s") SHOULD disable the timeout +completely. Implementations that cannot completely disable the timeout MUST +instead interpret the zero duration as the longest possible value to which +the timeout can be set. + +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. + +The value of BackendRequest must be a Gateway API Duration string as defined by +GEP-2257. When this field is unspecified, its behavior is implementation-specific; +when specified, the value of BackendRequest must be no more than the value of the +Request timeout (since the Request timeout encompasses the BackendRequest timeout). + +Support: Extended
    +     		false
    requeststring + 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. + +Setting a timeout to the zero duration (e.g. "0s") SHOULD disable the timeout +completely. Implementations that cannot completely disable the timeout MUST +instead interpret the zero duration as the longest possible value to which +the timeout can be set. + +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. + +The value of Request is a Gateway API Duration string as defined by GEP-2257. When this +field is unspecified, request timeout behavior is implementation-specific. + +Support: Extended
    +     		false
    + + ### HTTPProxy.spec.healthCheck [↩ Parent](#httpproxyspec) diff --git a/internal/controller/httpproxy_controller.go b/internal/controller/httpproxy_controller.go index a3746ab5..dd098720 100644 --- a/internal/controller/httpproxy_controller.go +++ b/internal/controller/httpproxy_controller.go @@ -275,7 +275,7 @@ func (r *HTTPProxyReconciler) Reconcile(ctx context.Context, req mcreconcile.Req return ctrl.Result{}, addErr } - if errs := validation.ValidateHTTPProxy(&httpProxy, validation.HTTPProxyValidationOptions{Hostnames: validation.CustomHostnameOptions(r.Config.Gateway)}); len(errs) > 0 { + if errs := validation.ValidateHTTPProxy(&httpProxy, validation.HTTPProxyOptions(r.Config.Gateway)); len(errs) > 0 { acceptedCondition.Status = metav1.ConditionFalse acceptedCondition.Reason = networkingv1alpha.HTTPProxyReasonInvalid acceptedCondition.Message = fmt.Sprintf("The HTTPProxy is invalid and cannot be programmed: %s", errs.ToAggregate()) @@ -1370,6 +1370,7 @@ func (r *HTTPProxyReconciler) collectDesiredResources( Matches: rule.Matches, Filters: ruleFilters, BackendRefs: backendRefs, + Timeouts: rule.Timeouts, } } diff --git a/internal/controller/httpproxy_controller_test.go b/internal/controller/httpproxy_controller_test.go index 964bd309..7012677e 100644 --- a/internal/controller/httpproxy_controller_test.go +++ b/internal/controller/httpproxy_controller_test.go @@ -92,6 +92,18 @@ func TestHTTPProxyCollectDesiredResources(t *testing.T) { } }, }, + { + name: "rule timeouts reach the route", + httpProxy: newHTTPProxy(func(h *networkingv1alpha.HTTPProxy) { + h.Spec.Rules[0].Timeouts = &gatewayv1.HTTPRouteTimeouts{ + Request: ptr.To(gatewayv1.Duration("30m")), + BackendRequest: ptr.To(gatewayv1.Duration("5m")), + } + }), + assert: func(t *testing.T, httpProxy *networkingv1alpha.HTTPProxy, desiredResources *desiredHTTPProxyResources) { + assert.Equal(t, httpProxy.Spec.Rules[0].Timeouts, desiredResources.httpRoute.Spec.Rules[0].Timeouts) + }, + }, { name: "no URLRewrite filter in rule with hostname in endpoint", httpProxy: newHTTPProxy(func(h *networkingv1alpha.HTTPProxy) { diff --git a/internal/validation/httpproxy_validation.go b/internal/validation/httpproxy_validation.go index 6d29c3fd..01baf5aa 100644 --- a/internal/validation/httpproxy_validation.go +++ b/internal/validation/httpproxy_validation.go @@ -14,6 +14,7 @@ import ( gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + "go.datum.net/network-services-operator/internal/config" gatewayutil "go.datum.net/network-services-operator/internal/util/gateway" ) @@ -26,6 +27,21 @@ const ( // HTTPProxy may declare. type HTTPProxyValidationOptions struct { Hostnames HostnameOptions + + // MaxRequestTimeout caps every rule's request and backendRequest timeouts. + // Zero leaves them uncapped. + MaxRequestTimeout time.Duration +} + +// HTTPProxyOptions derives the HTTPProxy rules from operator settings. The +// timeout cap is the one BackendTrafficPolicy already enforces, so a timeout +// set on the proxy and one set on a policy meet the same limit. +func HTTPProxyOptions(gatewayConfig config.GatewayConfig) HTTPProxyValidationOptions { + opts := HTTPProxyValidationOptions{Hostnames: CustomHostnameOptions(gatewayConfig)} + if max := gatewayConfig.ExtensionAPIValidationOptions.BackendTrafficPolicies.ClusterSettings.HTTPMaxRequestTimeout; max != nil { + opts.MaxRequestTimeout = max.Duration + } + return opts } func ValidateHTTPProxy(httpProxy *networkingv1alpha.HTTPProxy, opts HTTPProxyValidationOptions) field.ErrorList { @@ -48,7 +64,7 @@ func ValidateHTTPProxy(httpProxy *networkingv1alpha.HTTPProxy, opts HTTPProxyVal allErrs = append(allErrs, field.Invalid(field.NewPath("metadata", "name"), httpProxy.Name, msg)) } - allErrs = append(allErrs, validateHTTPProxyRules(httpProxy, field.NewPath("spec", "rules"))...) + allErrs = append(allErrs, validateHTTPProxyRules(httpProxy, field.NewPath("spec", "rules"), opts)...) allErrs = append(allErrs, validateHTTPProxyHealthCheck(httpProxy.Spec.HealthCheck, field.NewPath("spec", "healthCheck"))...) return allErrs @@ -115,22 +131,49 @@ func validateHostHeaderOverride(filters []gatewayv1.HTTPRouteFilter, fldPath *fi return validateProgrammableHostname(override.Value, hostHeaderPath) } -func validateHTTPProxyRules(httpProxy *networkingv1alpha.HTTPProxy, fldPath *field.Path) field.ErrorList { +func validateHTTPProxyRules(httpProxy *networkingv1alpha.HTTPProxy, fldPath *field.Path, opts HTTPProxyValidationOptions) field.ErrorList { allErrs := field.ErrorList{} for i, rule := range httpProxy.Spec.Rules { - allErrs = append(allErrs, validateHTTPProxyRule(rule, fldPath.Index(i))...) + allErrs = append(allErrs, validateHTTPProxyRule(rule, fldPath.Index(i), opts)...) } return allErrs } -func validateHTTPProxyRule(rule networkingv1alpha.HTTPProxyRule, fldPath *field.Path) field.ErrorList { +func validateHTTPProxyRule(rule networkingv1alpha.HTTPProxyRule, fldPath *field.Path, opts HTTPProxyValidationOptions) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, validateFilters(rule.Filters, supportedHTTPRouteRuleFilters, fldPath.Child("filters"))...) allErrs = append(allErrs, validateHostHeaderOverride(rule.Filters, fldPath.Child("filters"))...) allErrs = append(allErrs, validateHTTPProxyRuleBackends(rule, fldPath.Child("backends"))...) + allErrs = append(allErrs, validateHTTPProxyRuleTimeouts(rule.Timeouts, fldPath.Child("timeouts"), opts.MaxRequestTimeout)...) + + return allErrs +} + +// validateHTTPProxyRuleTimeouts refuses a timeout above the operator's cap and +// refuses zero, which Gateway API defines as no timeout at all. Envoy Gateway +// drops the route's idle timeout along with a disabled request timeout, so a +// zero would leave an abandoned stream open for good. +func validateHTTPProxyRuleTimeouts(timeouts *gatewayv1.HTTPRouteTimeouts, fldPath *field.Path, maxRequestTimeout time.Duration) field.ErrorList { + if timeouts == nil { + return nil + } + + var upperLimit *time.Duration + if maxRequestTimeout > 0 { + upperLimit = &maxRequestTimeout + } + lowerLimit := ptr.To(time.Nanosecond) + + allErrs := field.ErrorList{} + if timeouts.Request != nil { + allErrs = append(allErrs, validateGatewayDuration(fldPath.Child("request"), timeouts.Request, lowerLimit, upperLimit)...) + } + if timeouts.BackendRequest != nil { + allErrs = append(allErrs, validateGatewayDuration(fldPath.Child("backendRequest"), timeouts.BackendRequest, lowerLimit, upperLimit)...) + } return allErrs } diff --git a/internal/validation/httpproxy_validation_test.go b/internal/validation/httpproxy_validation_test.go index ad640abf..1d17671c 100644 --- a/internal/validation/httpproxy_validation_test.go +++ b/internal/validation/httpproxy_validation_test.go @@ -3,6 +3,7 @@ package validation import ( "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -757,6 +758,72 @@ func TestValidateHTTPProxy(t *testing.T) { } } +func TestValidateHTTPProxyTimeouts(t *testing.T) { + timeoutsPath := rulePath().Child("timeouts") + scenarios := map[string]struct { + timeouts *gatewayv1.HTTPRouteTimeouts + expectedErrors field.ErrorList + }{ + "unset": { + expectedErrors: field.ErrorList{}, + }, + "within the maximum": { + timeouts: &gatewayv1.HTTPRouteTimeouts{ + Request: ptr.To(gatewayv1.Duration("30m")), + BackendRequest: ptr.To(gatewayv1.Duration("5m")), + }, + expectedErrors: field.ErrorList{}, + }, + "at the maximum": { + timeouts: &gatewayv1.HTTPRouteTimeouts{Request: ptr.To(gatewayv1.Duration("1h"))}, + expectedErrors: field.ErrorList{}, + }, + "request above the maximum": { + timeouts: &gatewayv1.HTTPRouteTimeouts{Request: ptr.To(gatewayv1.Duration("2h"))}, + expectedErrors: field.ErrorList{ + field.Invalid(timeoutsPath.Child("request"), "", ""), + }, + }, + "backendRequest above the maximum": { + timeouts: &gatewayv1.HTTPRouteTimeouts{BackendRequest: ptr.To(gatewayv1.Duration("90m"))}, + expectedErrors: field.ErrorList{ + field.Invalid(timeoutsPath.Child("backendRequest"), "", ""), + }, + }, + "zero disables the timeout and is refused": { + timeouts: &gatewayv1.HTTPRouteTimeouts{ + Request: ptr.To(gatewayv1.Duration("0s")), + BackendRequest: ptr.To(gatewayv1.Duration("0s")), + }, + expectedErrors: field.ErrorList{ + field.Invalid(timeoutsPath.Child("request"), "", ""), + field.Invalid(timeoutsPath.Child("backendRequest"), "", ""), + }, + }, + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { + proxy := &networkingv1alpha.HTTPProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: networkingv1alpha.HTTPProxySpec{ + Rules: []networkingv1alpha.HTTPProxyRule{ + { + Backends: []networkingv1alpha.HTTPProxyRuleBackend{{Endpoint: "http://example.com"}}, + Timeouts: scenario.timeouts, + }, + }, + }, + } + errs := ValidateHTTPProxy(proxy, HTTPProxyValidationOptions{MaxRequestTimeout: time.Hour}) + delta := cmp.Diff(scenario.expectedErrors, errs, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail")) + if delta != "" { + t.Errorf("expected errors '%v', got '%v', diff: '%v'", scenario.expectedErrors, errs, delta) + } + }) + } +} + func rulePath() *field.Path { return field.NewPath("spec", "rules").Index(0) } diff --git a/internal/webhook/v1alpha/httpproxy_webhook.go b/internal/webhook/v1alpha/httpproxy_webhook.go index 13db962c..5bfe14dc 100644 --- a/internal/webhook/v1alpha/httpproxy_webhook.go +++ b/internal/webhook/v1alpha/httpproxy_webhook.go @@ -24,7 +24,7 @@ import ( // SetupHTTPProxyWebhookWithManager registers the webhook for HTTPProxy in the manager. func SetupHTTPProxyWebhookWithManager(mgr mcmanager.Manager, gatewayConfig config.GatewayConfig) error { return ctrl.NewWebhookManagedBy(mgr.GetLocalManager(), &networkingv1alpha.HTTPProxy{}). - WithValidator(&HTTPProxyCustomValidator{mgr: mgr, opts: validation.HTTPProxyValidationOptions{Hostnames: validation.CustomHostnameOptions(gatewayConfig)}}). + WithValidator(&HTTPProxyCustomValidator{mgr: mgr, opts: validation.HTTPProxyOptions(gatewayConfig)}). WithDefaulter(&HTTPProxyCustomDefaulter{}). Complete() } diff --git a/test/e2e/httpproxy-response-timeout/chainsaw-test.yaml b/test/e2e/httpproxy-response-timeout/chainsaw-test.yaml new file mode 100644 index 00000000..579b698c --- /dev/null +++ b/test/e2e/httpproxy-response-timeout/chainsaw-test.yaml @@ -0,0 +1,322 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +# +# A response through an HTTPProxy may run longer than Envoy's 15-second default +# route timeout (issue #459). +# +# The generated HTTPRoute carried no timeouts, so Envoy applied its own default +# and cut every response still in flight at 15 seconds: large downloads, +# server-sent events, long polls. The rule now carries a request timeout that +# defaults to one hour and that the user can set. +# +# The origin is go-httpbin on the downstream cluster. Its /drip endpoint sends +# one byte at a time over the requested duration, so a 20-second drip through +# the edge is the exact request the default used to cut. +# +# The scenario proves the field is in the path from both sides: a proxy with +# the default completes the 20-second drip, and a proxy whose rule sets a +# 5-second request timeout has the same drip cut. Without the second case a +# timeout that Envoy ignored would pass the first. +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: httpproxy-response-timeout +spec: + cluster: nso-standard + timeouts: + cleanup: 120s + steps: + - name: Deploy a slow origin on the downstream cluster + try: + - create: + cluster: nso-infra + resource: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: drip-origin + namespace: default + spec: + replicas: 1 + selector: + matchLabels: + app: drip-origin + template: + metadata: + labels: + app: drip-origin + spec: + containers: + - name: go-httpbin + image: ghcr.io/mccutchen/go-httpbin:2.18.1 + command: ["/bin/go-httpbin"] + args: + - -host + - "0.0.0.0" + - -port + - "8080" + - -max-duration + - 60s + ports: + - containerPort: 8080 + resources: + requests: + cpu: 100m + memory: 128Mi + limits: + cpu: 100m + memory: 128Mi + - create: + cluster: nso-infra + resource: + apiVersion: v1 + kind: Service + metadata: + name: drip-origin + namespace: default + spec: + selector: + app: drip-origin + ports: + - port: 8080 + targetPort: 8080 + - assert: + cluster: nso-infra + timeout: 120s + resource: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: drip-origin + namespace: default + status: + availableReplicas: 1 + + - name: Create a proxy with the default timeout and one with a short timeout + try: + - script: + cluster: nso-standard + skipCommandOutput: true + skipLogOutput: true + content: | + kubectl get ns $NAMESPACE -o json + outputs: + - name: downstreamNamespaceName + value: (join('-', ['ns', json_parse($stdout).metadata.uid])) + - create: + cluster: nso-standard + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: default-timeout + spec: + rules: + - backends: + - endpoint: http://drip-origin.default.svc.cluster.local:8080 + - create: + cluster: nso-standard + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: short-timeout + spec: + rules: + - backends: + - endpoint: http://drip-origin.default.svc.cluster.local:8080 + timeouts: + request: 5s + - assert: + cluster: nso-standard + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: default-timeout + spec: + rules: + - timeouts: + request: 1h + - assert: + cluster: nso-standard + timeout: 120s + resource: + apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: default-timeout + spec: + rules: + - timeouts: + request: 1h + - assert: + cluster: nso-infra + timeout: 180s + resource: + apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: default-timeout + namespace: ($downstreamNamespaceName) + spec: + rules: + - timeouts: + request: 1h + - assert: + cluster: nso-infra + timeout: 180s + resource: + apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: short-timeout + namespace: ($downstreamNamespaceName) + spec: + rules: + - timeouts: + request: 5s + - assert: + cluster: nso-standard + timeout: 300s + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: default-timeout + status: + (conditions[?type == 'Programmed']): + - status: "True" + - assert: + cluster: nso-standard + timeout: 300s + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: short-timeout + status: + (conditions[?type == 'Programmed']): + - status: "True" + catch: + - script: + cluster: nso-standard + content: | + kubectl -n $NAMESPACE get httpproxy,httproute,gateway -o yaml + kubectl -n network-services-operator-system logs -l app.kubernetes.io/name=network-services-operator --tail=100 + - script: + cluster: nso-infra + env: + - name: DOWNSTREAM_NAMESPACE + value: ($downstreamNamespaceName) + content: | + kubectl -n $DOWNSTREAM_NAMESPACE get httproute,gateway,backendtrafficpolicy -o yaml + + - name: A 20-second response completes with the default and is cut by a 5-second timeout + description: | + Both proxies front the same origin through the shared downstream Envoy + on the kind hostPort. The request is served once a GET returns 200, and + then one 20-second drip is sent through each proxy. The default proxy + delivers every byte; the short-timeout proxy loses the stream before + the last byte, which shows the timeout on the rule is what Envoy + enforces. + try: + - script: + cluster: nso-standard + skipCommandOutput: true + skipLogOutput: true + content: | + kubectl -n $NAMESPACE get httpproxy default-timeout -o jsonpath='{.status.canonicalHostname}' + outputs: + - name: defaultHostname + value: ($stdout) + - script: + cluster: nso-standard + skipCommandOutput: true + skipLogOutput: true + content: | + kubectl -n $NAMESPACE get httpproxy short-timeout -o jsonpath='{.status.canonicalHostname}' + outputs: + - name: shortHostname + value: ($stdout) + - script: + timeout: 300s + env: + - name: DEFAULT_HOSTNAME + value: ($defaultHostname) + - name: SHORT_HOSTNAME + value: ($shortHostname) + content: | + set -u + for host in "$DEFAULT_HOSTNAME" "$SHORT_HOSTNAME"; do + for i in $(seq 1 60); do + code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 5 \ + -H "Host: ${host}" http://localhost:30080/get) || code=000 + echo "${host} attempt ${i}: HTTP ${code}" + [ "${code}" = "200" ] && break + sleep 3 + done + [ "${code}" = "200" ] || { echo "${host} never answered 200"; exit 1; } + done + + drip() { + curl -s -o /dev/null --max-time 40 \ + -w 'http=%{http_code} bytes=%{size_download} seconds=%{time_total}\n' \ + -H "Host: $1" 'http://localhost:30080/drip?duration=20s&numbytes=20&code=200' + } + + echo "default timeout:" + out=$(drip "$DEFAULT_HOSTNAME"); rc=$? + echo " ${out} exit=${rc}" + case "${out}" in + *"http=200 bytes=20 "*) ;; + *) echo "the 20-second drip did not complete through the default proxy"; exit 1 ;; + esac + + echo "5-second timeout:" + out=$(drip "$SHORT_HOSTNAME"); rc=$? + echo " ${out} exit=${rc}" + case "${out}" in + *"bytes=20 "*) echo "the 20-second drip completed despite a 5-second request timeout"; exit 1 ;; + esac + echo "ok" + catch: + - script: + cluster: nso-infra + content: | + kubectl -n datum-downstream-gateway get pods -o wide + kubectl -n datum-downstream-gateway logs -l gateway.envoyproxy.io/owning-gateway-namespace --all-containers --tail=100 || true + + - name: A timeout above the maximum or of zero is refused at admission + try: + - apply: + cluster: nso-standard + expect: + - check: + ($error != null): true + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: timeout-too-long + spec: + rules: + - backends: + - endpoint: http://drip-origin.default.svc.cluster.local:8080 + timeouts: + request: 2h + - apply: + cluster: nso-standard + expect: + - check: + ($error != null): true + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: timeout-disabled + spec: + rules: + - backends: + - endpoint: http://drip-origin.default.svc.cluster.local:8080 + timeouts: + request: 0s From 3ed692709901f6ee2bdafd94430e82bb53a06c1e Mon Sep 17 00:00:00 2001 From: Michael Costello Date: Tue, 10 Nov 2026 15:09:58 -0500 Subject: [PATCH 2/2] fix: Default the request timeout on partly set rules The CRD default applies only when a rule leaves timeouts unset. A rule that set backendRequest alone left request empty, and Envoy's 15-second default returned for that rule. The mutating webhook now fills request with the default whenever it is missing, and the e2e scenario covers a rule that sets only backendRequest. The default is fixed at one hour while the cap is operator config. A cap below the default would turn every proxy without a timeout of its own Accepted=False on the next reconcile, so operator config now refuses a cap below the default at startup. Key changes: - Fill a missing request timeout in the HTTPProxy defaulter - Refuse an httpMaxRequestTimeout below the HTTPProxy default - Assert the defaulted request timeout on a partly set rule end to end --- api/v1alpha/httpproxy_types.go | 2 ++ internal/config/config.go | 16 +++++++++ internal/config/config_test.go | 25 +++++++++++++ internal/validation/httpproxy_validation.go | 19 ++++++---- .../validation/httpproxy_validation_test.go | 36 +++++++++++++++++++ .../v1alpha/httpproxy_mutating_test.go | 24 +++++++++++++ internal/webhook/v1alpha/httpproxy_webhook.go | 1 + .../chainsaw-test.yaml | 25 +++++++++++++ 8 files changed, 141 insertions(+), 7 deletions(-) diff --git a/api/v1alpha/httpproxy_types.go b/api/v1alpha/httpproxy_types.go index 67581075..5e4c45c0 100644 --- a/api/v1alpha/httpproxy_types.go +++ b/api/v1alpha/httpproxy_types.go @@ -281,6 +281,8 @@ type HTTPProxyRule struct { Timeouts *gatewayv1.HTTPRouteTimeouts `json:"timeouts,omitempty"` } +const HTTPProxyDefaultRequestTimeout = "1h" + // +kubebuilder:validation:XValidation:message="endpoint is required unless instance or networkService is set; instance and networkService are mutually exclusive with each other and with endpoint and connector",rule="has(self.instance) ? (!has(self.endpoint) && !has(self.connector) && !has(self.networkService)) : (has(self.networkService) ? (!has(self.endpoint) && !has(self.connector)) : has(self.endpoint))" // +kubebuilder:validation:XValidation:message="backend TLS is not supported for networkService backends",rule="has(self.networkService) ? !has(self.tls) : true" type HTTPProxyRuleBackend struct { diff --git a/internal/config/config.go b/internal/config/config.go index c761ef17..c0b947be 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2145,9 +2145,25 @@ func (c *GatewayConfig) validate() error { } errs = append(errs, c.validateAuthorityLossGracePeriod()) errs = append(errs, c.DownstreamClientTraffic.validate()) + errs = append(errs, c.validateHTTPMaxRequestTimeout()) return errors.Join(errs...) } +func (c *GatewayConfig) validateHTTPMaxRequestTimeout() error { + max := c.ExtensionAPIValidationOptions.BackendTrafficPolicies.ClusterSettings.HTTPMaxRequestTimeout + if max == nil { + return nil + } + floor, err := time.ParseDuration(networkingv1alpha.HTTPProxyDefaultRequestTimeout) + if err != nil { + return err + } + if max.Duration < floor { + return fmt.Errorf("extensionAPIValidationOptions.backendTrafficPolicies.clusterSettings.httpMaxRequestTimeout %s is below the HTTPProxy request timeout default of %s, which every proxy without a timeout of its own carries", max.Duration, networkingv1alpha.HTTPProxyDefaultRequestTimeout) + } + return nil +} + // validateAuthorityLossGracePeriod refuses a negative period, which would turn the hold off without saying so; // zero is the documented way to turn it off. func (c *GatewayConfig) validateAuthorityLossGracePeriod() error { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e5f1cf51..67ce84e7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -717,6 +717,31 @@ func TestGatewayConfig_ValidateALPNProtocols(t *testing.T) { // An HTTP/2 window outside Envoy's range is refused at start: the edge would // keep its defaults, and nothing the operator reads would say so. +func TestGatewayConfig_ValidateHTTPMaxRequestTimeout(t *testing.T) { + for _, tt := range []struct { + name string + max *metav1.Duration + wantSub string + }{ + {name: "unset"}, + {name: "at the proxy default", max: &metav1.Duration{Duration: time.Hour}}, + {name: "above the proxy default", max: &metav1.Duration{Duration: 2 * time.Hour}}, + {name: "below the proxy default", max: &metav1.Duration{Duration: 10 * time.Minute}, wantSub: "httpMaxRequestTimeout 10m0s is below"}, + } { + t.Run(tt.name, func(t *testing.T) { + cfg := &NetworkServicesOperator{} + cfg.Gateway.ExtensionAPIValidationOptions.BackendTrafficPolicies.ClusterSettings.HTTPMaxRequestTimeout = tt.max + err := cfg.Validate() + if tt.wantSub == "" && err != nil { + t.Fatalf("expected nil, got %v", err) + } + if tt.wantSub != "" && (err == nil || !strings.Contains(err.Error(), tt.wantSub)) { + t.Fatalf("expected an error containing %q, got %v", tt.wantSub, err) + } + }) + } +} + func TestGatewayConfig_ValidateHTTP2Windows(t *testing.T) { tests := []struct { name, stream, connection, wantSub string diff --git a/internal/validation/httpproxy_validation.go b/internal/validation/httpproxy_validation.go index 01baf5aa..db1117fb 100644 --- a/internal/validation/httpproxy_validation.go +++ b/internal/validation/httpproxy_validation.go @@ -33,9 +33,6 @@ type HTTPProxyValidationOptions struct { MaxRequestTimeout time.Duration } -// HTTPProxyOptions derives the HTTPProxy rules from operator settings. The -// timeout cap is the one BackendTrafficPolicy already enforces, so a timeout -// set on the proxy and one set on a policy meet the same limit. func HTTPProxyOptions(gatewayConfig config.GatewayConfig) HTTPProxyValidationOptions { opts := HTTPProxyValidationOptions{Hostnames: CustomHostnameOptions(gatewayConfig)} if max := gatewayConfig.ExtensionAPIValidationOptions.BackendTrafficPolicies.ClusterSettings.HTTPMaxRequestTimeout; max != nil { @@ -44,6 +41,18 @@ func HTTPProxyOptions(gatewayConfig config.GatewayConfig) HTTPProxyValidationOpt return opts } +func DefaultHTTPProxyRuleTimeouts(httpProxy *networkingv1alpha.HTTPProxy) { + for i := range httpProxy.Spec.Rules { + rule := &httpProxy.Spec.Rules[i] + if rule.Timeouts == nil { + rule.Timeouts = &gatewayv1.HTTPRouteTimeouts{} + } + if rule.Timeouts.Request == nil { + rule.Timeouts.Request = ptr.To(gatewayv1.Duration(networkingv1alpha.HTTPProxyDefaultRequestTimeout)) + } + } +} + func ValidateHTTPProxy(httpProxy *networkingv1alpha.HTTPProxy, opts HTTPProxyValidationOptions) field.ErrorList { allErrs := field.ErrorList{} @@ -152,10 +161,6 @@ func validateHTTPProxyRule(rule networkingv1alpha.HTTPProxyRule, fldPath *field. return allErrs } -// validateHTTPProxyRuleTimeouts refuses a timeout above the operator's cap and -// refuses zero, which Gateway API defines as no timeout at all. Envoy Gateway -// drops the route's idle timeout along with a disabled request timeout, so a -// zero would leave an abandoned stream open for good. func validateHTTPProxyRuleTimeouts(timeouts *gatewayv1.HTTPRouteTimeouts, fldPath *field.Path, maxRequestTimeout time.Duration) field.ErrorList { if timeouts == nil { return nil diff --git a/internal/validation/httpproxy_validation_test.go b/internal/validation/httpproxy_validation_test.go index 1d17671c..d2c2a543 100644 --- a/internal/validation/httpproxy_validation_test.go +++ b/internal/validation/httpproxy_validation_test.go @@ -824,6 +824,42 @@ func TestValidateHTTPProxyTimeouts(t *testing.T) { } } +func TestDefaultHTTPProxyRuleTimeouts(t *testing.T) { + scenarios := map[string]struct { + timeouts *gatewayv1.HTTPRouteTimeouts + expected *gatewayv1.HTTPRouteTimeouts + }{ + "unset gets the default": { + expected: &gatewayv1.HTTPRouteTimeouts{Request: ptr.To(gatewayv1.Duration("1h"))}, + }, + "only backendRequest set gets the default request": { + timeouts: &gatewayv1.HTTPRouteTimeouts{BackendRequest: ptr.To(gatewayv1.Duration("5m"))}, + expected: &gatewayv1.HTTPRouteTimeouts{ + Request: ptr.To(gatewayv1.Duration("1h")), + BackendRequest: ptr.To(gatewayv1.Duration("5m")), + }, + }, + "request set is kept": { + timeouts: &gatewayv1.HTTPRouteTimeouts{Request: ptr.To(gatewayv1.Duration("30s"))}, + expected: &gatewayv1.HTTPRouteTimeouts{Request: ptr.To(gatewayv1.Duration("30s"))}, + }, + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { + proxy := &networkingv1alpha.HTTPProxy{ + Spec: networkingv1alpha.HTTPProxySpec{ + Rules: []networkingv1alpha.HTTPProxyRule{{Timeouts: scenario.timeouts}}, + }, + } + DefaultHTTPProxyRuleTimeouts(proxy) + if delta := cmp.Diff(scenario.expected, proxy.Spec.Rules[0].Timeouts); delta != "" { + t.Errorf("unexpected timeouts, diff: %s", delta) + } + }) + } +} + func rulePath() *field.Path { return field.NewPath("spec", "rules").Index(0) } diff --git a/internal/webhook/v1alpha/httpproxy_mutating_test.go b/internal/webhook/v1alpha/httpproxy_mutating_test.go index 0a74e3a9..d48a1e9b 100644 --- a/internal/webhook/v1alpha/httpproxy_mutating_test.go +++ b/internal/webhook/v1alpha/httpproxy_mutating_test.go @@ -45,3 +45,27 @@ func TestHTTPProxyCustomDefaulter_Default(t *testing.T) { require.NoError(t, (&HTTPProxyCustomDefaulter{}).Default(context.Background(), proxy)) assert.Equal(t, "Test ALB", proxy.Annotations[display.AnnotationDisplayName]) } + +func TestHTTPProxyCustomDefaulter_DefaultsRuleRequestTimeout(t *testing.T) { + t.Parallel() + + proxy := &networkingv1alpha.HTTPProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "alb", Namespace: "proj"}, + Spec: networkingv1alpha.HTTPProxySpec{ + Rules: []networkingv1alpha.HTTPProxyRule{ + { + Backends: []networkingv1alpha.HTTPProxyRuleBackend{{Endpoint: "https://origin.example.com"}}, + Timeouts: &gatewayv1.HTTPRouteTimeouts{BackendRequest: ptr.To(gatewayv1.Duration("5m"))}, + }, + { + Backends: []networkingv1alpha.HTTPProxyRuleBackend{{Endpoint: "https://origin.example.com"}}, + }, + }, + }, + } + + require.NoError(t, (&HTTPProxyCustomDefaulter{}).Default(context.Background(), proxy)) + assert.Equal(t, gatewayv1.Duration("1h"), ptr.Deref(proxy.Spec.Rules[0].Timeouts.Request, "")) + assert.Equal(t, gatewayv1.Duration("5m"), ptr.Deref(proxy.Spec.Rules[0].Timeouts.BackendRequest, "")) + assert.Equal(t, gatewayv1.Duration("1h"), ptr.Deref(proxy.Spec.Rules[1].Timeouts.Request, "")) +} diff --git a/internal/webhook/v1alpha/httpproxy_webhook.go b/internal/webhook/v1alpha/httpproxy_webhook.go index 5bfe14dc..12b5e5fc 100644 --- a/internal/webhook/v1alpha/httpproxy_webhook.go +++ b/internal/webhook/v1alpha/httpproxy_webhook.go @@ -37,6 +37,7 @@ var _ admission.Defaulter[*networkingv1alpha.HTTPProxy] = &HTTPProxyCustomDefaul func (d *HTTPProxyCustomDefaulter) Default(ctx context.Context, httpProxy *networkingv1alpha.HTTPProxy) error { _ = display.EnsureHTTPProxyAnnotations(httpProxy, oldHTTPProxy(ctx)) + validation.DefaultHTTPProxyRuleTimeouts(httpProxy) return nil } diff --git a/test/e2e/httpproxy-response-timeout/chainsaw-test.yaml b/test/e2e/httpproxy-response-timeout/chainsaw-test.yaml index 579b698c..a5284bec 100644 --- a/test/e2e/httpproxy-response-timeout/chainsaw-test.yaml +++ b/test/e2e/httpproxy-response-timeout/chainsaw-test.yaml @@ -126,6 +126,19 @@ spec: - endpoint: http://drip-origin.default.svc.cluster.local:8080 timeouts: request: 5s + - create: + cluster: nso-standard + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: partial-timeout + spec: + rules: + - backends: + - endpoint: http://drip-origin.default.svc.cluster.local:8080 + timeouts: + backendRequest: 5m - assert: cluster: nso-standard resource: @@ -137,6 +150,18 @@ spec: rules: - timeouts: request: 1h + - assert: + cluster: nso-standard + resource: + apiVersion: networking.datumapis.com/v1alpha + kind: HTTPProxy + metadata: + name: partial-timeout + spec: + rules: + - timeouts: + request: 1h + backendRequest: 5m - assert: cluster: nso-standard timeout: 120s