Forward headers starting with Kn-#572
Forward headers starting with Kn-#572knative-prow[bot] merged 1 commit intoknative-extensions:mainfrom
Conversation
The eventshub forwarder should forward headers such as Kn-Namespace. The sending client automatically adds some headers such as User-Agent and Content-Length. We also don't want to forward tracing-related headers starting with X- and Traceparent. So this feature only forwards specific headers related to Knative.
|
@mgencur: The label(s) DetailsIn response to this:
Instructions 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/test-infra repository. |
knative-prow
bot
added
the
do-not-merge/work-in-progress
knative-prow
bot
added
the
size/S
| headers[k] = v | ||
| receivedHeaders[k] = v | ||
| } | ||
| if strings.HasPrefix(k, "Kn-") { |
There was a problem hiding this comment.
The other option here would be something like this:
if !strings.HasPrefix(k, "Ce-")&&
!strings.HasPrefix(k, "X-") &&
k != "Traceparent" &&
k != "User-Agent" &&
k != "Accept-Encoding" &&
k != "Content-Length" &&
k != "Host" {
headersToBeSent[k] = v
}
This would forward everything except for the unwanted ones. WDYT?
There was a problem hiding this comment.
It looks ok, to forward those, since they have a meaning for us/eventing
There was a problem hiding this comment.
your forward everything except seems also reasonable.
I'd perhaps make that some better code, but yeah.
If this current code works, let's do that, and backport
and than think about future, sounds good?
There was a problem hiding this comment.
Ok. Testing the current solution with forwarding "Kn-*". We can leave any improvements for the future.
There was a problem hiding this comment.
It works! This is logged at my forwarder:
{"level":"info","ts":"2023-08-09T11:33:00.349Z","logger":"eventshub.event logger","caller":"logger_vent/logger.go:26","msg":"Event: \n-- EventInfo --\n--- Kind: Received ---\n--- Event ---\nContext Attributes,\n specversion: 1.0\n type: dev.knative.sources.ping\n source: /apis/v1/namespaces/test-cktbvfkv/pingsources/pingsource-oqvyeqoj\n id: b2c43312-8767-4d17-af90-55008697dca1\n time: 2023-08-09T11:33:00.318564821Z\n\n--- HTTP headers ---\n Accept-Encoding: gzip\n Forwarded: for=10.128.2.49;proto=http\n Traceparent: 00-a4aaf20f277d96c1a067e3d478235fed-91af74b9b2d6ad70-00\n X-Forwarded-For: 10.128.2.49, 10.129.2.53\n X-Forwarded-Proto: http\n Content-Length: 0\n Kn-Namespace: test-cktbvfkv\n User-Agent: Go-http-client/1.1\n X-Request-Id: 6d4020dd-fa21-43ee-ae2c-d52751bfacf2\n K-Proxy-Request: activator\n Host: sink-krcxpelq.test-cktbvfkv.svc.cluster.local\n\n--- Origin: '127.0.0.1:55982' ---\n--- Observer: 'sink-krcxpelq' ---\n--- Time: 2023-08-09 11:33:00.34908641 +0000 UTC m=+358.441989515 ---\n--- Sequence: 0 ---\n--- Sent Id: ' ---\n--------------------\n"}
{"level":"info","ts":"2023-08-09T11:33:00.391Z","logger":"eventshub.event logger","caller":"logger_vent/logger.go:26","msg":"Event: \n-- EventInfo --\n--- Kind: Sent ---\n--- Event ---\nContext Attributes,\n specversion: 1.0\n type: dev.knative.sources.ping\n source: /apis/v1/namespaces/test-cktbvfkv/pingsources/pingsource-oqvyeqoj\n id: b2c43312-8767-4d17-af90-55008697dca1\n time: 2023-08-09T11:33:00.318564821Z\n\n--- HTTP headers ---\n Ce-Source: /apis/v1/namespaces/test-cktbvfkv/pingsources/pingsource-oqvyeqoj\n Ce-Specversion: 1.0\n Ce-Type: dev.knative.sources.ping\n Ce-Time: 2023-08-09T11:33:00.318564821Z\n Kn-Namespace: test-cktbvfkv\n Ce-Id: b2c43312-8767-4d17-af90-55008697dca1\n\n--- Origin: 'sink-krcxpelq' ---\n--- Observer: 'sink-krcxpelq' ---\n--- Time: 2023-08-09 11:33:00.391297182 +0000 UTC m=+358.484200268 ---\n--- Sequence: 0 ---\n--- Sent Id: 'b2c43312-8767-4d17-af90-55008697dca1 ---\n--------------------\n"}
knative-prow
bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matzew, mgencur The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow
bot
added
the
approved
|
/cherry-pick release-1.10 |
|
@mgencur: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you. DetailsIn response to this:
Instructions 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/test-infra repository. |
mgencur
changed the title
knative-prow
bot
removed
the
do-not-merge/work-in-progress
|
/cherry-pick release-1.11 |
|
@mgencur: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you. DetailsIn response to this:
Instructions 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/test-infra repository. |
|
/unhold |
knative-prow
bot
removed
the
do-not-merge/hold
|
@mgencur: new pull request created: #573 DetailsIn response to this:
Instructions 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/test-infra repository. |
|
@mgencur: new pull request created: #574 DetailsIn response to this:
Instructions 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/test-infra repository. |
4 participants
The eventshub forwarder should forward headers such as Kn-Namespace.
The sending client automatically adds some headers such as User-Agent and Content-Length. We also don't want to forward tracing-related headers starting with X- and Traceparent. So this feature only forwards specific headers related to Knative.
Changes
/kind
Fixes #
Release Note
Docs