Adds authorization for sidecar-promclient #4612
Conversation
Signed-off-by: someshkoli <[email protected]>
Signed-off-by: someshkoli <[email protected]>
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
Signed-off-by: someshkoli <[email protected]>
Signed-off-by: someshkoli <[email protected]>
Signed-off-by: someshkoli <[email protected]>
|
Let me know if you need any help @someshkoli. Would be great to have this feature merged next release. |
will update this soon |
|
Hi @someshkoli, please let me know whether you are willing to continue this pr or not. It would be good to have this feature in. |
Signed-off-by: someshkoli <[email protected]>
someshkoli
force-pushed
the
feature/promclient-authorization
branch
2 times, most recently
from
8e5f70f
to
d2f80dc
Compare
|
@yeya24 had been a bit occupied last month, updated the PR with all the review comments, having a bit trouble rebasing it 🤔 , also any idea why ci is not running for e2e ? |
I guess it is because of the conflicts you have. If you don't mind I can help you rebase it. WDYT? |
yeah that would be great |
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
|
Changelog added and the E2E test should be fixed as well. This is ready for review now |
Great, waiting for all greens. I guess almost all the review comments were fixed in previous PR and this one, @yeya24 one final review ? |
I added the new commits so it is already LGTM from me. Maybe let's hear from @GiedriusS @bwplotka? |
Signed-off-by: Ben Ye <[email protected]>
| pc.httpClient = extflag.RegisterPathOrContent( | ||
| cmd, | ||
| "prometheus.http-client", | ||
| "YAML file or string with http client configs. see Format details : ...", |
There was a problem hiding this comment.
@someshkoli One small nit. I think we can remove see Format details....
There was a problem hiding this comment.
Yep, its redundant
There was a problem hiding this comment.
Should I make another PR for this ? 😂😂😂
There was a problem hiding this comment.
It would be good to include it in a pr for auth documentation. Would you like to take it?
There was a problem hiding this comment.
Yeah, I'll make one 🙌
There was a problem hiding this comment.
It would be great if we can have document for this feature.
awd
Changes
Re: #4104
Added new flag prometheus.http-client to sidecar to pass http client config via file or string(similar to alertmanager). Using this flag an instance of thanoshttp.Client is created and is passed to promclient.NewClient method which uses this client as base client instead of whole new client which then makes authorized api calls.
The same client is passed to reloader config which then makes authorized api calls to prom api.
There are some code duplication which has been duplicated from prometheus which can be easily removed by exposing one of the functions in prometheus http_config
raised_hands
Close: #3975
Verification
Added a e2e test:
Checks if query is able to query when prom uses auth