-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix #16298: Allow customization of headers in Route #16355
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
base: main
Are you sure you want to change the base?
Conversation
- Add AppendHeaders to TrafficTarget
- Update Route validation
- Update Ingress generation to propagate AppendHeaders
- Add unit tests
- Manually update DeepCopy for AppendHeaders
|
Hi @aviralgarg05. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aviralgarg05 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16355 +/- ##
==========================================
- Coverage 80.23% 80.20% -0.03%
==========================================
Files 216 216
Lines 13440 13450 +10
==========================================
+ Hits 10784 10788 +4
- Misses 2293 2298 +5
- Partials 363 364 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @aviralgarg05 , I'm not experienced enough to know extending the spec would work especially considering that adding headers might not be supported by all implementations, I guess @dprotaso has more background on what could be done? |
Pull Request Description
Fixes #16298
Proposed Changes
AppendHeadersfield (typemap[string]string) to theTrafficTargetstruct inpkg/apis/serving/v1/route_types.go. This allows users to specify custom HTTP headers for specific traffic targets in a Knative Route.validateAppendHeadersinpkg/apis/serving/v1/route_validation.goto ensure that all keys provided inAppendHeadersare valid HTTP header names according to RFC 7230 and that the field is not present in the Status of the Route.pkg/reconciler/route/resources/ingress.go. TheAppendHeadersfrom theRoute'sTrafficTargetare now merged with system-level headers (like activator headers) usingkmeta.UnionMapsand propagated to theIngressBackendSplit.pkg/apis/serving/v1/zz_generated.deepcopy.goto include deep copy support for the newAppendHeadersmap, ensuring the field survives reconciliation cycles.pkg/apis/serving/v1/route_validation_test.goto verify valid and invalid header configurations.TestMakeIngressSpecWithAppendHeaderstopkg/reconciler/route/resources/ingress_test.goto confirm that headers are correctly passed through to the networking layer.Release Note