Skip to content

Conversation

@volcanonoodle
Copy link
Contributor

Related to #1705

Adds a rotating token resource for Service Account tokens like done with Cloud Access Policy tokens in #2390.

I think a concern here is that AFAIK we do not have an API in Grafana to fetch specific tokens by ID, but rather we have to list all the tokens that belong to a Service Account (endpoint) and we cannot filter out expired tokens either. This means that in the long term, as this new Terraform resource keeps rotating tokens, the list of expired tokens returned by Grafana on each TF plan will keep growing. I don't think we should worry about it for now, as it seems unlikely that someone will accumulate too many rotated tokens under one Service Account in the near future, but at some point we might need to consider adding a filter for expired tokens to the endpoint mentioned previously.

@volcanonoodle volcanonoodle requested a review from a team as a code owner November 18, 2025 09:51
@github-actions
Copy link
Contributor

In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically.
To do so, a Grafana Labs employee must trigger the cloud acceptance tests workflow manually.

@volcanonoodle volcanonoodle force-pushed the volcanonoodle/sa-rotating-token branch from 4a34c32 to 40df264 Compare November 18, 2025 10:12
@volcanonoodle volcanonoodle requested review from a team, jtroy and mgyongyosi and removed request for a team November 18, 2025 10:22
Copy link
Member

@cinaglia cinaglia left a comment

Choose a reason for hiding this comment

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

LGTM. I've been able to test this locally. One thing I'm not particularly in love with is how we're exposing ServiceAccountRotatingTokenNow, though I don't think there's a much better way to do this.

@volcanonoodle
Copy link
Contributor Author

One thing I'm not particularly in love with is how we're exposing ServiceAccountRotatingTokenNow, though I don't think there's a much better way to do this.

@cinaglia I totally agree. It doesn't look very good but I couldn't come up with something better given that all the resources live under the same cloud package and the tests are in a separate package. Oh well, if anyone comes up with something more elegant we can always change it in the future 😄 Thank you for the review!

@volcanonoodle volcanonoodle requested a review from a team as a code owner November 20, 2025 22:11
@volcanonoodle
Copy link
Contributor Author

volcanonoodle commented Nov 24, 2025

I just realized that a couple of tests were setting the time based on the time when the test started, rather than on the token's expiration time, when we actually want the latter because it's the expiration time that's used to determine if the token needs rotation and there could be a second difference between the 2 depending on how long the initial request to create the token took. I fixed it in d368bb8.

@volcanonoodle
Copy link
Contributor Author

I'll wait to merge this until #2445 has been approved, so that we can release both resources under the same minor version bump of the provider.

@volcanonoodle volcanonoodle merged commit 0c87be7 into main Nov 25, 2025
31 checks passed
@volcanonoodle volcanonoodle deleted the volcanonoodle/sa-rotating-token branch November 25, 2025 19:58
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.

2 participants