Introduce interface to ease forward/access HTTP headers #562
Conversation
This file contains 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
marefr
changed the title
marefr
commented
Nov 15, 2022
backend/data.go
Outdated
| req.Headers = map[string]string{} | ||
| } | ||
|
|
||
| req.Headers[fmt.Sprintf("http_%s", key)] = value |
There was a problem hiding this comment.
Reason to this would be to allow the environment info to hold other things than HTTP headers.
Main goal being to not always forward all environment info in outgoing HTTP headers.
Not sure it make sense 🤷
xnyo
requested changes
Dec 2, 2022
refactor a bit, adds tests, adds proper documentation of methods and fields.
marefr
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In grafana/grafana#58132 we're type switching on different backend request types, see code.
Introduce a new interface
ForwardHTTPHeaderswould allow us to typeswitch/typecast to that interface instead and simplify the code.From plugin authors perspective we have documentation for Forward OAuth identity for the logged-in user, Forward cookies for the logged-in user and soon for Forward user header and we provides instruction for accessing these forwarded headers by access the
request.Headersfield. This field is different for different request types:QueryDataRequestandCheckHealthRequestthere's amap[string]string(they're suppose to hold environment context/metadataCallResourceRequestthere's amap[string][]string(suppose to hold HTTP headers since it's HTTP proxy over gRPC basically.With these changes, rather than accessing
request.Headersdirectly to access HTTP headers it's suggested that they userequest.GetHTTPHeader(<key>)instead, e.g.request.GetHTTPHeader(backend.OAuthIdentityTokenHeaderName)orrequest.GetHTTPHeader(backend.OAuthIdentityIDTokenHeaderName), to access Forwarded OAuth Identity headers.Using
request.GetHTTPHeader(<key>)or ``request.GetHTTPHeaders()` would also automatically handle canonical/non-canonical form problems, i.e. grafana/grafana#58598A potential alternative could be to send forwarded HTTP headers via gRPC metadata, but not sure if that can affect the gRPC connection since it uses HTTP2.
WDYT?`