api: add more timeout fields to ClientTimeout#9315
Conversation
Signed-off-by: kabbohus <hussein.kabbout@united-security-providers.ch>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
As instructed, I have created a PR to propose the new API changes before implementing anything. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7417ab4412
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Default: 10 seconds. | ||
| // | ||
| // +optional | ||
| RequestHeadersTimeout *gwapiv1.Duration `json:"requestHeadersTimeout,omitempty"` |
There was a problem hiding this comment.
These newly exposed ClientTrafficPolicy fields are never read by the controller: buildClientTimeout still only copies TCP idleTimeout and HTTP requestReceivedTimeout/idleTimeout/streamIdleTimeout into IR (internal/gatewayapi/clienttrafficpolicy.go:850-887), and the IR/translator only has those existing fields. In any cluster where a user sets requestHeadersTimeout, listenerFiltersTimeout, or transportSocketConnectTimeout, the accepted value produces unchanged Envoy config, so the API silently has no effect.
Useful? React with 👍 / 👎.
| // Default: 5 seconds. | ||
| // | ||
| // +optional | ||
| TransportSocketConnectTimeout *gwapiv1.Duration `json:"transportSocketConnectTimeout,omitempty"` |
There was a problem hiding this comment.
The generated CRDs were not updated for these API additions: the shipped ClientTrafficPolicy schema still lists only the old timeout properties under charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml:1298-1333, and repo-wide search finds the new JSON names only in this Go file. With those manifests installed, Kubernetes will drop or reject listenerFiltersTimeout, transportSocketConnectTimeout, and requestHeadersTimeout, so users cannot actually configure the new API fields.
Useful? React with 👍 / 👎.
|
@HusseinKabbout can you run |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9315 +/- ##
==========================================
- Coverage 75.20% 75.18% -0.02%
==========================================
Files 252 252
Lines 41049 41049
==========================================
- Hits 30871 30864 -7
- Misses 8085 8090 +5
- Partials 2093 2095 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: kabbohus <hussein.kabbout@united-security-providers.ch>
Sure thing! |
2 participants
What type of PR is this?
Feature + API (non-breaking)
What this PR does / why we need it:
This PR exposes new Envoy proxy timeout settings in
ClientTrafficPolicywhen dealing with untrusted downstreams. By tweaking these new exposed settings, DoS protection can be improved depending on the use case.The settings are:
listener_filters_timeoutrequest_headers_timeouttransport_socket_connect_timeoutWhich issue(s) this PR fixes:
Fixes #9254
Release Notes: Yes/No