policy: implement per-route disable for extAuth,extProc,buffer,cors #11893
Conversation
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
github-actions
bot
added
kind/breaking_change
kind/feature
There was a problem hiding this comment.
Pull Request Overview
This PR implements per-route disable functionality for extAuth, extProc, buffer, and cors policies, allowing routes to opt-out of policies attached at the Gateway/Listener level. The implementation adds a disable field to these policies and includes a breaking change that removes the extAuth.enablement field in favor of extAuth.disable.
- Added
disablefield to extAuth, extProc, buffer, and cors policies - Implemented per-route opt-out mechanism using various Envoy filter strategies
- Updated API validation to require exactly one of
extensionRefordisablefields
Reviewed Changes
Copilot reviewed 57 out of 58 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/kubernetes/testutils/assertions/curl.go | Refactored assertion methods from public to private and improved variable declaration |
| test/kubernetes/e2e/features/rate_limit/suite.go | Updated test to use eventual assertions instead of immediate ones |
| test/kubernetes/e2e/features/local_rate_limit/suite.go | Updated test to use eventual assertions instead of immediate ones |
| test/kubernetes/e2e/features/extproc/testdata/gateway-targetref.yaml | Added test route with extproc disable policy |
| test/kubernetes/e2e/features/extproc/suite.go | Enhanced test suite with disable functionality testing |
| test/kubernetes/e2e/features/extauth/testdata/ | Updated test files to use new disable field instead of enablement |
| test/kubernetes/e2e/features/cors/testdata/ | Added CORS disable test configuration |
| test/kubernetes/e2e/features/cors/suite.go | Added test assertions for CORS disable functionality |
| pkg/pluginsdk/ir/iface.go | Cleaned up interface method ordering |
| pkg/plugins/gwextbase/base.go | Refactored disable filter utility function |
| internal/kgateway/utils/cors.go | Updated CORS policy creation to support disable functionality |
| internal/kgateway/utils/any.go | Added MustMessageToAny utility function |
| internal/kgateway/translator/gateway/testutils/ | Updated test outputs and inputs for new disable functionality |
| internal/kgateway/krtcollections/builtin.go | Updated CORS IR conversion for new disable parameter |
| internal/kgateway/extensions2/plugins/trafficpolicy/ | Implemented disable functionality across all policy types |
| install/helm/kgateway-crds/ | Updated CRD with new disable fields and validation |
| design/10683.md | Updated design document to reflect API changes |
| api/v1alpha1/ | Updated API types with disable fields and removed enablement field |
internal/kgateway/extensions2/plugins/trafficpolicy/extproc_policy.go
Outdated
Show resolved
Hide resolved
shashankram
force-pushed
the
filter-disable
branch
from
ff587b8 to
9f619d6
Compare
howardjohn
reviewed
Aug 6, 2025
|
|
||
| // Disable the CORS filter. | ||
| // +optional | ||
| Disable *PolicyDisable `json:"disable,omitempty"` |
There was a problem hiding this comment.
should we make it so you cannot set any other fields if you use disable?
There was a problem hiding this comment.
We can, just annoying to do so because every field in the inlined type is optional. Since we inline the upstream API, we need to keep the CEL rules in sync with API changes to the upstream type. I don't feel strongly to make them mutually exclusive even though that is the desired behavior
There was a problem hiding this comment.
how about this: has(self.disabled) ? self.size() == 1 : true
There was a problem hiding this comment.
Can't apply size() to objects, only list, map, string, bytes are supported
There was a problem hiding this comment.
There was a problem hiding this comment.
Maybe not and the playground doesn't account for that
There was a problem hiding this comment.
Just tested:
The CustomResourceDefinition "trafficpolicies.gateway.kgateway.dev" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[cors].x-kubernetes-validations[0].rule: Invalid value: apiextensions.ValidationRule{Rule:"has(self.disable) ? self.size() == 1 : true", Message:"policy fields cannot be set when disabled", MessageExpression:"", Reason:(*apiextensions.FieldValueErrorReason)(nil), FieldPath:"", OptionalOldSelf:(*bool)(nil)}: compilation failed: ERROR: <input>:1:30: found no matching overload for 'size' applied to 'selfType185128119.()'
| has(self.disable) ? self.size() == 1 : true
puertomontt
reviewed
Aug 6, 2025
shashankram
force-pushed
the
filter-disable
branch
3 times, most recently
from
930c9e4 to
4fd5171
Compare
timflannagan
approved these changes
Aug 7, 2025
internal/kgateway/extensions2/plugins/trafficpolicy/extproc_policy.go
Outdated
Show resolved
Hide resolved
internal/kgateway/extensions2/plugins/trafficpolicy/gateway_extension.go
Outdated
Show resolved
Hide resolved
internal/kgateway/extensions2/plugins/trafficpolicy/gateway_extension.go
Show resolved
Hide resolved
Description Adds a `disable` field to the policies to allow disabling policies attached to Gateway/Listener at the route level as a form of opt-out mechanism. - ExtAuth uses the existing global_disable/ext_auth filter, though this change updates the metadata key and value for consistency. - ExtProc is configured using a composite filter that conditionally enables the actual ExtProc filter based on the absence of the global_disable/ext_proc filter. This allows the route plugin to simply add the global disable filter to the filter config instead of complex post-processing of routes that would be otherwise required (vhost plugin runs after route plugin). - CORS implements disable capability using using the fractional filter_enabled setting to disable it for all requests on the route. - Buffer is disabled using BufferPerRoute.Disabled. Change Type ``` /kind breaking_change /kind new_feature ``` Changelog ```release-note Adds disable field to extAuth, extProc, cors, buffer policies to allow disabling the policies per-route. Breaking change: extAuth.enablement has been removed in favor of extAuth.disable. ``` Additional Notes Resolves 11892 Signed-off-by: Shashank Ram <[email protected]>
Signed-off-by: Shashank Ram <[email protected]>
shashankram
force-pushed
the
filter-disable
branch
from
4fd5171 to
39a176d
Compare
timflannagan
approved these changes
Aug 7, 2025
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.
Description
Adds a
disablefield to the policies to allow disabling policies attached to Gateway/Listener at the route level as a form of opt-out mechanism.ExtAuth uses the existing global_disable/ext_auth filter, though this change updates the metadata key and value for consistency.
ExtProc is configured using a composite filter that conditionally enables the actual ExtProc filter based on the absence of the global_disable/ext_proc filter. This allows the route plugin to simply add the global disable filter to the filter config instead of complex post-processing of routes that would be otherwise required (because vhost plugin runs after route plugin).
CORS implements disable capability using using the fractional filter_enabled setting to disable it for all requests on the route.
Buffer is disabled using BufferPerRoute.Disabled.
Change Type
Changelog
Additional Notes
Resolves #11892
Also fixes e2e test flakes that were not using the correct assertion API.