fix(api): increase RateLimitSelectCondition.headers MaxItems from 16 to 64#8906
Merged
Merged
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
wucm667
force-pushed
the
fix/ratelimit-headers-maxitems
branch
from
9f458ba to
8b96984
Compare
![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: 8b96984a3b
ℹ️ 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".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8906 +/- ##
==========================================
- Coverage 74.67% 74.65% -0.02%
==========================================
Files 251 251
Lines 40257 40257
==========================================
- Hits 30062 30055 -7
- Misses 8127 8133 +6
- Partials 2068 2069 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rudrakhp
requested changes
May 3, 2026
Comment on lines
+232
to
+233
| // Increase the maxItems from 16 to 64, aligning with HTTPHeaderFilter. | ||
| // Revert this change once the upstream Gateway API supports items more than 64. |
Member
There was a problem hiding this comment.
Do we need this in the API docs? Maybe just create a follow up issue to track this if required.
Contributor
Author
|
Removed the implementation note from the source field. Ran make generate and it cleaned up the generated CRDs and docs as well. |
arkodg
approved these changes
May 3, 2026
zirain
approved these changes
May 4, 2026
…to 64 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
… MaxItems change Running make generate updated the pre-generated CRD YAML files and API docs to reflect the MaxItems increase from 16 to 64. Without these changes, clusters installed from the charts would still reject headers lists longer than 16. Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
…ers MaxItems change These generated test fixtures reflect the API change from the previous commit that increased RateLimitSelectCondition.headers MaxItems from 16 to 64. Run `make generate` to reproduce. Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
…aders field The comment about reverting once upstream Gateway API supports >64 items was appearing in generated CRD descriptions and API docs. Per reviewer feedback, remove it from the source file. Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
zirain
force-pushed
the
fix/ratelimit-headers-maxitems
branch
from
346e75e to
41ba83e
Compare
jukie
added a commit
that referenced
this pull request
May 13, 2026
* fix(api): increase RateLimitSelectCondition.headers MaxItems from 16 to 64 (#8906) * fix(api): increase RateLimitSelectCondition.headers MaxItems from 16 to 64 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: wucm667 <stevenwucongmin@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * feat: policy field owner (#8538) * feat: policy field owner Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * skip invalid listener first in IR (#8577) * skip invalid listener Signed-off-by: zirain <zirain2009@gmail.com> * fix specValid Signed-off-by: zirain <zirain2009@gmail.com> * nit Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> * MUST NOT pick one conflicting Listener as the winner Signed-off-by: zirain <zirain2009@gmail.com> * update Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> Co-authored-by: Isaac Wilson <isaac.wilson514@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: remove cross ns policy attachment status (#8901) * Revert "add warning for partially accepted targets" This reverts commit 5d88fbb. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * remove warning condition for cross-ns policy attachments without referenceGrants Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * feat: add runner event metrics (#8802) * add metrics for runner Signed-off-by: zirain <zirain2009@gmail.com> * rename Signed-off-by: zirain <zirain2009@gmail.com> * rename Signed-off-by: zirain <zirain2009@gmail.com> * reuse Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: respect backend endpoint hostname for health checks (#8929) * fix: respect backend endpoint hostname for health checks ### Summary - Keep BackendTrafficPolicy HTTP health check hostnames as explicit cluster-level hosts, and leave route-derived host fallback to xDS cluster translation. - Preserve Backend endpoint hostnames as per-endpoint overrides via Endpoint.HealthCheckConfig.hostname, ahead of the route fallback. - Update gatewayapi/xDS fixtures, release notes, and generated API docs/CRDs for the host selection order. ### Test plan - go test ./internal/ir - go test ./internal/xds/translator - go test ./internal/gatewayapi -run TestTranslate/backendtrafficpolicy - go test ./internal/gatewayapi -run TestTranslate/(clienttrafficpolicy-http-health-check|envoyextensionpolicy-with-extproc-with-retries|envoyextensionpolicy-with-extproc-with-traffic-features|envoyproxy-accesslog-with-traffic|envoyproxy-tracing-backend-uds|envoyproxy-tracing-backend|securitypolicy-with-jwt-backendcluster|securitypolicy-with-jwt-backendsettings) - make generate - make manifests - git diff --check Signed-off-by: Arko Dasgupta <arkodg@gmail.com> Co-authored-by: Codex <noreply@openai.com> * fix gen Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: Arko Dasgupta <arkodg@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> Co-authored-by: Codex <noreply@openai.com> Co-authored-by: zirain <zirain2009@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix(helm): propagate commonLabels to RBAC resources (#8818) * feat(helm): propagate commonLabels to RBAC resources Issue #8817 reported that 'helm template ... --set commonLabels.custom-label=custom-value' left ClusterRole, ClusterRoleBinding, Role, and RoleBinding resources unlabelled. The other resources in the chart already include 'eg.labels' in their metadata - which picks up 'commonLabels' via the helper at _helpers.tpl:43 - but envoy-gateway-rbac.yaml didn't set any labels block. Add 'labels: {{- include "eg.labels" . | nindent 4 }}' on every Role / RoleBinding / ClusterRole / ClusterRoleBinding declared in envoy-gateway-rbac.yaml. Matches the existing labels pattern used in certgen-rbac.yaml and envoy-gateway-deployment.yaml. Scopes are '$' inside the watched-namespaces 'range' and '.' at the template root, same rule the helper block inside the file already used. Verified locally with: helm dependency update charts/gateway-helm envsubst < charts/gateway-helm/values.tmpl.yaml > \ charts/gateway-helm/values.yaml helm template eg charts/gateway-helm \ --set commonLabels.custom-label=custom-value | yq ... All four RBAC resources now emit 'custom-label: custom-value' in their metadata.labels, matching the issue's repro steps. Cert-gen RBAC resources already carried it; this PR brings the core envoy-gateway RBAC set into parity. Fixes #8817 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> * chore: regenerate helm-template snapshots for RBAC labels Run 'make helm-template.gateway-helm' to regenerate the snapshot fixtures after the envoy-gateway-rbac.yaml labels change. Adds the 'labels:' block to the RBAC resources in all 27 test cases. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> * fix gen Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> * add release note Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> --------- Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef (#8871) * fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef When a ListenerSet listener has an unresolvable TLS certificate reference (InvalidCertificateRef or RefNotPermitted), Accepted: False was incorrectly set on both the listener and ListenerSet object. The Gateway API spec places InvalidCertificateRef exclusively under ResolvedRefs, not Accepted — a missing certificate is a reference resolution concern, not a structural one. Fixes #8870 Signed-off-by: apkatsikas <apkatsikas@gmail.com> * chore: fix gofumpt formatting in validateListenerConditions Signed-off-by: apkatsikas <apkatsikas@gmail.com> * fix(translator): separate RefNotPermitted from InvalidCertificateRef handling Unlike InvalidCertificateRef, RefNotPermitted should not set Accepted:True. Update unit test fixtures to match. Signed-off-by: apkatsikas <apkatsikas@gmail.com> * fix gen Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: apkatsikas <apkatsikas@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: do not downgrade ALPN for only hostnames-overlapping listeners (#8934) fix: do not downgrade ALPN for overlapping hostnames withoug SANs overlapping Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * feat: enableDeferredCreationStats by default (#8937) * feat: enableDeferredCreationStats by default Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix: restore last transition time in merge status conditions (#8962) * fix: restore last transition time in merge status conditions Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * add release note Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> --------- Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * [release/v1.8] v1.8.0 release notes Cherry-picked release-notes/v1.8.0.yaml and VERSION bump from #8942. Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> --------- Signed-off-by: wucm667 <stevenwucongmin@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com> Signed-off-by: Arko Dasgupta <arkodg@gmail.com> Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Signed-off-by: apkatsikas <apkatsikas@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Co-authored-by: wucm667 <109257021+wucm667@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Kota Kimura <86363983+kkk777-7@users.noreply.github.com> Co-authored-by: zirain <zirain2009@gmail.com> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Matt Van Horn <mvanhorn@users.noreply.github.com> Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Andrew Katsikas <apkatsikas@gmail.com> Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8895
What this PR does
Increases the
MaxItemslimit forRateLimitSelectCondition.Headersfrom 16 to 64, allowing users to specify more header matching rules in rate limit configurations.Why 64?
This aligns with the existing
HTTPHeaderFilterpattern already defined in this codebase (shared_types.go:1031), which also increased the limit from 16 to 64 for the same reason.Changes
api/v1alpha1/ratelimit_types.go: Changed+kubebuilder:validation:MaxItems=16to+kubebuilder:validation:MaxItems=64on theHeadersfield ofRateLimitSelectCondition