-
Notifications
You must be signed in to change notification settings - Fork 397
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
feat(client): add support for custom HTTP headers #1756
base: master
Are you sure you want to change the base?
feat(client): add support for custom HTTP headers #1756
Conversation
Signed-off-by: christophe.boucharlat <[email protected]>
…trumented Transport object and reuse it in subsequent requests Signed-off-by: christophe.boucharlat <[email protected]>
…e to simplify syntax Signed-off-by: christophe.boucharlat <[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.
After changing the data type, the code became much easier, nice job :)
I've added a few minor suggestions.
…, removed useless pointers & follow coding style Signed-off-by: christophe.boucharlat <[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.
I've pushed two commits to your branch (regenerated code after the Headers data type was changed + fixed nil pointer dereference). With those changes in place, I think we should be good to merge the PR, but it would still be handy to have a second look at the code by one of my fellow maintainers.
Thank's a lot for those fixes and your time ! 🙏 |
Hi @weisdd, theres currently an issue with trivy's ghcr where if you run at an unfortunate time you can get rate limited with an error similar to below.
|
@MaoMaoCake No worries, it's something that we've been seeing in every PR for a while. It's not a blocking issue. |
Adds support for Generic headers. Addresses #1726
Unfortunately here the following example does not work out of the box:
Even tho the
TransportConfig.HTTPHeaders
exist it is not reused afterwards and this instrumented client is (i did not understood exactly why, if anyone has hints on that i would love to know ! 😃).I feel this PR does something weird so do not hesitate to point me a cleaner way to achieve the same result.
What does this PR do:
headers
object under Grafana.spec.clientinstrumentedRoundTripper
TypeaddHeaders
oninstrumentedRoundTripper
to set headers key / valuesRoundTrip
method to loop over the content of theinstrumentedRoundTripper
Type and Add the present headers to the request.Here is a Grafana object example using custom Headers to display different allowed ways to set custom headers: