Skip to content

Support X-Access-Token for Grafana Cloud #107

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

Merged
merged 2 commits into from
May 6, 2025
Merged

Conversation

csmarchbanks
Copy link
Contributor

In Grafana Cloud it is possible to have a plugin submit requests on behalf of the calling user by setting the X-Access-Token header instead of the standard Authorization header. This PR will preferentially look for the presence of an access token and use that before falling back to an API key like normal.

@csmarchbanks csmarchbanks requested a review from a team as a code owner April 24, 2025 22:17
@@ -67,6 +70,7 @@ func oncallClientFromContext(ctx context.Context) (*aapi.Client, error) {

grafanaOnCallURL = strings.TrimRight(grafanaOnCallURL, "/")

// TODO: Allow access to OnCall using an access token instead of an API key.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TODO will wait for another PR as the OnCall client needs to be upgraded to take a transport. In the meantime we may just need to disable these tools when we use an access token.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sd2k mentioned there are additional complications apart from just the OnCall client upgrade (something about the way routing is set up in cloud IIRC) but yeah that's non-blocking for this PR.

@csmarchbanks csmarchbanks force-pushed the support-x-access-token branch from fbfe711 to d8ed43d Compare April 24, 2025 22:19
annanay25
annanay25 previously approved these changes Apr 24, 2025
Copy link
Contributor

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, nice!

@@ -67,6 +70,7 @@ func oncallClientFromContext(ctx context.Context) (*aapi.Client, error) {

grafanaOnCallURL = strings.TrimRight(grafanaOnCallURL, "/")

// TODO: Allow access to OnCall using an access token instead of an API key.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sd2k mentioned there are additional complications apart from just the OnCall client upgrade (something about the way routing is set up in cloud IIRC) but yeah that's non-blocking for this PR.

@@ -97,11 +97,12 @@ type siftClient struct {
url string
}

func newSiftClient(url, apiKey string) *siftClient {
func newSiftClient(url, accessToken, apiKey string) *siftClient {
client := &http.Client{
Transport: &authRoundTripper{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should move this authRoundTripper out into a common location in the future

Copy link
Collaborator

@sd2k sd2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will work but IMO we shouldn't mention OBO auth until the complementary ID token is supported too.

@csmarchbanks csmarchbanks dismissed annanay25’s stale review April 29, 2025 20:20

Dismissing as I have changed the signature

In Grafana Cloud it is possible to have a plugin submit requests on
behalf of the calling user by setting the X-Access-Token header instead
of the standard Authorization header. This PR will preferentially look
for the presence of an access token and use that before falling back to
an API key like normal.
@csmarchbanks csmarchbanks force-pushed the support-x-access-token branch from 897c9bf to 4b5c527 Compare May 2, 2025 19:58
Copy link
Collaborator

@sd2k sd2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@csmarchbanks csmarchbanks merged commit ab3d4ba into main May 6, 2025
9 checks passed
@csmarchbanks csmarchbanks deleted the support-x-access-token branch May 6, 2025 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants