-
Notifications
You must be signed in to change notification settings - Fork 585
Add support for CORS configuration in TrafficPolicy and in HTTPRoute #11252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Yossi Mesika <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds end-to-end CORS support for the TrafficPolicy
API by extending the API, control-plane translation, CRDs, and integration tests.
- Introduce
CorsPolicy
toTrafficPolicySpec
, deep-copy generation, and apply-configuration support - Implement
toCorsFilterConfig
, IR wiring (CorsIR
), and Envoy filter/plugin changes - Add E2E fixtures and Go test suite covering route-level, gateway-level, and override scenarios
- Update Helm CRD template and Go client machinery for CORS fields
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
api/v1alpha1/traffic_policy_types.go | Add CorsPolicy to TrafficPolicySpec |
api/v1alpha1/zz_generated.deepcopy.go | Generate deep-copy for CorsPolicy |
install/helm/kgateway-crds/templates/..._trafficpolicies.yaml | Extend CRD schema with CORS properties |
api/applyconfiguration/** | Add CorsPolicyApplyConfiguration and integrate into builder |
internal/**/cors_policy.go | Translate CorsPolicy into Envoy CorsPolicy |
internal/**/traffic_policy_plugin.go | Wire up CorsIR in plugin, filter chain, merge logic |
pkg/utils/requestutils/curl/option.go | Add WithHeaders helper for bulk header injection in tests |
test/kubernetes/e2e/features/cors/testdata/*.yaml | E2E YAML fixtures for service, routes, policies |
test/kubernetes/e2e/features/cors/suite.go | New Go test suite covering CORS behavior |
internal/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Yossi Mesika <[email protected]>
Signed-off-by: Yossi Mesika <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds CORS configuration support to the TrafficPolicy
API, translating the spec into Envoy CORS filters and adding end-to-end tests.
- Introduces
CorsPolicy
field in TrafficPolicy CRD, API types, deepcopy, and applyconfig - Implements translation logic (
cors_policy.go
) and integration into the HTTP filter chain (traffic_policy_plugin.go
) - Adds e2e test manifests and suite enhancements to validate route- and gateway-level CORS, including override behavior
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
test/kubernetes/e2e/features/cors/testdata/httproutes.yaml | Adds two HTTPRoute test cases for CORS/non-CORS match paths |
test/kubernetes/e2e/features/cors/testdata/gw-cors.yaml | Defines a TrafficPolicy with gateway-level CORS settings |
test/kubernetes/e2e/features/cors/testdata/common.yaml | Common Gateway manifest used by CORS tests |
test/kubernetes/e2e/features/cors/suite.go | E2E test suite for route- and gateway-level CORS behavior |
pkg/utils/requestutils/curl/option.go | Adds WithHeaders helper to set multiple request headers |
internal/kgateway/setup/testdata/standard/cors-route.yaml | Standard test manifest for route-level CORS Policy |
internal/kgateway/setup/testdata/standard/cors-route-out.yaml | Expected Envoy bootstrap output for route-level CORS |
internal/kgateway/setup/testdata/standard/cors-gw.yaml | Standard test manifest for gateway-level CORS Policy |
internal/kgateway/setup/testdata/standard/cors-gw-out.yaml | Expected Envoy bootstrap output for gateway-level CORS |
internal/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go | Integrates CORS IR into the HTTP Connection Manager pipeline |
internal/kgateway/extensions2/plugins/trafficpolicy/cors_policy.go | Translates CorsPolicy spec into Envoy CorsPolicy IR |
install/helm/kgateway-crds/templates/gateway.kgateway.dev_trafficpolicies.yaml | Adds CORS schema to the TrafficPolicy CRD |
hack/utils/oss_compliance/osa_provided.md | Updates license list for github.com/golang/protobuf |
go.mod | Adds direct dependency on github.com/golang/protobuf |
api/v1alpha1/zz_generated.deepcopy.go | Generates deep-copy functions for CorsPolicy |
api/v1alpha1/traffic_policy_types.go | Introduces CorsPolicy in the TrafficPolicySpec API |
api/applyconfiguration/utils.go | Registers CorsPolicyApplyConfiguration |
api/applyconfiguration/internal/internal.go | Adds schema entries for CorsPolicy apply-configuration |
api/applyconfiguration/api/v1alpha1/trafficpolicyspec.go | Hooks WithCors into TrafficPolicySpec apply-configuration |
api/applyconfiguration/api/v1alpha1/corspolicy.go | Implements declarative builder for CorsPolicyApplyConfiguration |
Comments suppressed due to low confidence (1)
test/kubernetes/e2e/features/cors/suite.go:112
- Tests cover allowed origins, methods, and headers but do not assert
Access-Control-Expose-Headers
orAccess-Control-Max-Age
; consider adding those to validate full policy output.
"Access-Control-Allow-Headers": "x-custom-header",
internal/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go
Outdated
Show resolved
Hide resolved
internal/kgateway/extensions2/plugins/trafficpolicy/cors_policy.go
Outdated
Show resolved
Hide resolved
internal/kgateway/extensions2/plugins/trafficpolicy/cors_policy.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Yossi Mesika <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for specifying a CORS policy in the TrafficPolicy
API, translating it into an Envoy CORS filter, and validating behavior with new end-to-end tests.
- Extend API types, deepcopy logic, CRDs, and applyconfiguration for
CorsPolicy
- Implement translation from
CorsPolicy
to EnvoyCorsPolicy
and integrate into the HTTP filter chain - Add a
WithHeaders
helper in the curl request utility and new e2e tests/testdata for CORS
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
pkg/utils/requestutils/curl/option.go | Added WithHeaders option for setting multiple headers on a curl request |
internal/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go | Integrated handling of CORS in virtual host, route, and filter chain |
internal/kgateway/extensions2/plugins/trafficpolicy/cors_policy.go | Converted TrafficPolicy CORS spec into Envoy filter config |
api/v1alpha1/traffic_policy_types.go | Added CorsPolicy to TrafficPolicySpec |
install/helm/kgateway-crds/templates/gateway.kgateway.dev_trafficpolicies.yaml | Updated CRD schema to include CORS fields |
test/kubernetes/e2e/features/cors/suite.go | New e2e suite for exercising CORS behaviors |
Comments suppressed due to low confidence (1)
test/kubernetes/e2e/features/cors/suite.go:109
- [nitpick] Tests cover allowOrigins, allowMethods, and allowHeaders but omit scenarios for exposeHeaders, maxAge, and allowCredentials. Consider adding cases for these fields to ensure full CORS support is validated.
expectedHeaders := map[string]any{
internal/kgateway/extensions2/plugins/trafficpolicy/traffic_policy_plugin.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Yossi Mesika <[email protected]>
gateway api also has CORS - https://gateway-api.sigs.k8s.io/reference/spec/#httpcorsfilter |
@yuval-k thanks for raising this. |
Signed-off-by: Yossi Mesika <[email protected]>
Signed-off-by: Yossi Mesika <[email protected]>
Signed-off-by: Yossi Mesika <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good! left some feedback
api/v1alpha1/traffic_policy_types.go
Outdated
|
||
// Cors specifies the CORS configuration for the policy. | ||
// +optional | ||
Cors *gwv1.HTTPCORSFilter `json:"cors,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like the api re-use here; lets make sure this meets all our use-vases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a slight change to have a CorsPolicy which inlines the fields from HTTPCORSFilter
.
In the future if we will want to add some fields which aren't part of the gw api (such as merge policy) we can do it in the CorsPolicy.
Signed-off-by: Yossi Mesika <[email protected]>
Signed-off-by: Yossi Mesika <[email protected]>
Signed-off-by: Yossi Mesika <[email protected]>
removed from MQ due to known extauth flake; re-queueing |
…gateway-dev#11252) Signed-off-by: Yossi Mesika <[email protected]>
Description
Support for configuring CORS policy in various levels.
This configuration will add an http cors filter and set the envoy cors configuration.
This main changes in this PR are:
HTTPRoute
's cors filter that can be configured for a route rule or for a specificbackendRef
in the rule.cors
policy to theTrafficPolicy
. This allows users to set CORS on a route or gateway. Policy for a gateway target ref translates into vhost cors which can't be done with HTTPRoute cors.Fixes #11185
Change Type
/kind new_feature
Changelog
Additional Notes
HTTPRoute
support is being handled by thebuiltin
plugin which handles all other HTTPRoute filters.TrafficPolicy
support is being handled by theTrafficPolicy
plugin.