chore: Add token rotation for grafana-operator service account#3745
chore: Add token rotation for grafana-operator service account#3745bengtfredh wants to merge 1 commit into
Conversation
Add automatic rotation of the grafana-operator service account token before it expires. Uses the time provider to trigger token regeneration every 180 days by default, preventing service disruption. Also upgrades the default Grafana version from 11 to 12.
📝 WalkthroughWalkthroughThe Grafana Terraform module gains automatic rotation for the operator service account token. Two new variables ( ChangesGrafana Operator Token Auto-Rotation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infrastructure/modules/grafana/grafana.tf`:
- Around line 59-61: The lifecycle block in the Grafana token rotation
configuration currently uses only replace_triggered_by, which enforces
destroy-before-create semantics that creates an authentication gap when the old
token is revoked before the new one is ready. Add create_before_destroy = true
to the lifecycle block alongside the existing replace_triggered_by configuration
to ensure the new token is fully created and propagated before the old token is
destroyed, eliminating the authentication window.
In `@infrastructure/modules/grafana/variables.tf`:
- Around line 87-103: The validation block for
grafana_operator_token_rotation_days only enforces ordering but does not
validate that both token expiration and rotation day values are positive whole
numbers. Add validation conditions to both
grafana_operator_token_expiration_days and grafana_operator_token_rotation_days
variables to ensure they are greater than zero and are not decimal values.
Update the existing validation block for grafana_operator_token_rotation_days to
include these additional conditions alongside the existing ordering check, and
add a new validation block to grafana_operator_token_expiration_days to enforce
the same positive whole-number constraints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f3c346e4-0410-44a4-98e3-dc7e8510871a
📒 Files selected for processing (3)
infrastructure/modules/grafana/README.mdinfrastructure/modules/grafana/grafana.tfinfrastructure/modules/grafana/variables.tf
| lifecycle { | ||
| replace_triggered_by = [time_rotating.grafana_operator_token.id] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Terraform lifecycle behavior, when a resource is replaced due to replace_triggered_by, is the default destroy-before-create unless create_before_destroy = true` is set?
For the Grafana provider resource grafana_service_account_token, can multiple tokens with the same name exist simultaneously for one service account, or must names be unique?`
💡 Result:
In Terraform, when a resource is replaced due to replace_triggered_by, the default behavior is indeed to destroy the existing resource before creating the new one [1][2][3]. The replace_triggered_by attribute does not automatically enable create_before_destroy; if you require the replacement resource to be created before the old one is destroyed, you must explicitly set create_before_destroy = true within the resource's lifecycle block [2][3]. Note that create_before_destroy propagation can sometimes lead to dependency cycles, and in certain complex graph scenarios, Terraform may enforce or ignore specific lifecycle settings to maintain graph validity [4][1]. Regarding the Grafana provider resource grafana_service_account_token, multiple tokens with the same name can exist simultaneously for a single service account [5][6][7]. While the service account's own display name must be unique [5][7], there is no requirement for individual service account tokens associated with that account to have unique names; they are managed by their own unique internal IDs [5][7][8].
Citations:
- 1: https://github.com/hashicorp/terraform/blob/main/docs/destroying.md
- 2: https://docs.hashicorp.com/terraform/language/block/resource
- 3: https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle
- 4: When
replace_triggered_byon resource referring to another resourcecreate_before_destroyis ignored on that referred resource hashicorp/terraform#35519 - 5: https://grafana.com/docs/grafana/latest/administration/service-accounts/?pg=blog&plcmt=body-txt
- 6: https://grafana.com/docs/grafana/latest/administration/service-accounts/migrate-api-keys/?pg=grafana-update-service-account-tokens-are-replacing-api-keys&plcmt=in-text
- 7: https://grafana.com/docs/grafana/v11.5/administration/service-accounts/
- 8: https://grafana.com/docs/grafana-cloud/developer-resources/api-reference/http-api/serviceaccount/?camp=timeshift_82&pg=grafana-the-open-and-composable-observability-platform&plcmt=related-content-1&src=blog
🏁 Script executed:
cd infrastructure/modules/grafana && head -80 grafana.tf | tail -30Repository: Altinn/altinn-platform
Length of output: 1183
🏁 Script executed:
cd infrastructure/modules/grafana && rg -A 5 -B 5 "grafana_operator" .Repository: Altinn/altinn-platform
Length of output: 7108
Add create_before_destroy = true to prevent auth gaps during token rotation.
The current lifecycle using only replace_triggered_by enforces destroy-before-create semantics, which revokes the old token before the new token is created. This leaves a brief window where external consumers (like grafana-operator) using the rotated token would fail authentication. Since Grafana allows multiple tokens with the same name for a single service account, create_before_destroy = true can be safely added to ensure the new token is fully created and propagated to downstream consumers before the old token is revoked.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infrastructure/modules/grafana/grafana.tf` around lines 59 - 61, The
lifecycle block in the Grafana token rotation configuration currently uses only
replace_triggered_by, which enforces destroy-before-create semantics that
creates an authentication gap when the old token is revoked before the new one
is ready. Add create_before_destroy = true to the lifecycle block alongside the
existing replace_triggered_by configuration to ensure the new token is fully
created and propagated before the old token is destroyed, eliminating the
authentication window.
| variable "grafana_operator_token_expiration_days" { | ||
| type = number | ||
| default = 360 | ||
| description = "Lifetime in days for the grafana-operator service account token. Must be less than or equal to the Grafana instance's service_accounts.token_expiration_day_limit." | ||
| } | ||
|
|
||
| variable "grafana_operator_token_rotation_days" { | ||
| type = number | ||
| default = 180 | ||
| description = "Number of days after which the grafana-operator service account token is rotated. Must be less than grafana_operator_token_expiration_days so the token is recreated before it expires." | ||
|
|
||
| validation { | ||
| condition = var.grafana_operator_token_rotation_days < var.grafana_operator_token_expiration_days | ||
| error_message = "grafana_operator_token_rotation_days must be less than grafana_operator_token_expiration_days so the token is rotated before it expires." | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Add strict validation for positive whole-day values.
Line 99 only enforces ordering. Inputs like 0, negative numbers, or decimals still pass and can break rotation_days/seconds_to_live usage downstream.
Suggested patch
variable "grafana_operator_token_expiration_days" {
type = number
default = 360
description = "Lifetime in days for the grafana-operator service account token. Must be less than or equal to the Grafana instance's service_accounts.token_expiration_day_limit."
+
+ validation {
+ condition = var.grafana_operator_token_expiration_days > 0 && floor(var.grafana_operator_token_expiration_days) == var.grafana_operator_token_expiration_days
+ error_message = "grafana_operator_token_expiration_days must be a positive whole number of days."
+ }
}
variable "grafana_operator_token_rotation_days" {
type = number
default = 180
description = "Number of days after which the grafana-operator service account token is rotated. Must be less than grafana_operator_token_expiration_days so the token is recreated before it expires."
validation {
- condition = var.grafana_operator_token_rotation_days < var.grafana_operator_token_expiration_days
- error_message = "grafana_operator_token_rotation_days must be less than grafana_operator_token_expiration_days so the token is rotated before it expires."
+ condition = var.grafana_operator_token_rotation_days > 0 && floor(var.grafana_operator_token_rotation_days) == var.grafana_operator_token_rotation_days && var.grafana_operator_token_rotation_days < var.grafana_operator_token_expiration_days
+ error_message = "grafana_operator_token_rotation_days must be a positive whole number less than grafana_operator_token_expiration_days."
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| variable "grafana_operator_token_expiration_days" { | |
| type = number | |
| default = 360 | |
| description = "Lifetime in days for the grafana-operator service account token. Must be less than or equal to the Grafana instance's service_accounts.token_expiration_day_limit." | |
| } | |
| variable "grafana_operator_token_rotation_days" { | |
| type = number | |
| default = 180 | |
| description = "Number of days after which the grafana-operator service account token is rotated. Must be less than grafana_operator_token_expiration_days so the token is recreated before it expires." | |
| validation { | |
| condition = var.grafana_operator_token_rotation_days < var.grafana_operator_token_expiration_days | |
| error_message = "grafana_operator_token_rotation_days must be less than grafana_operator_token_expiration_days so the token is rotated before it expires." | |
| } | |
| } | |
| variable "grafana_operator_token_expiration_days" { | |
| type = number | |
| default = 360 | |
| description = "Lifetime in days for the grafana-operator service account token. Must be less than or equal to the Grafana instance's service_accounts.token_expiration_day_limit." | |
| validation { | |
| condition = var.grafana_operator_token_expiration_days > 0 && floor(var.grafana_operator_token_expiration_days) == var.grafana_operator_token_expiration_days | |
| error_message = "grafana_operator_token_expiration_days must be a positive whole number of days." | |
| } | |
| } | |
| variable "grafana_operator_token_rotation_days" { | |
| type = number | |
| default = 180 | |
| description = "Number of days after which the grafana-operator service account token is rotated. Must be less than grafana_operator_token_expiration_days so the token is recreated before it expires." | |
| validation { | |
| condition = var.grafana_operator_token_rotation_days > 0 && floor(var.grafana_operator_token_rotation_days) == var.grafana_operator_token_rotation_days && var.grafana_operator_token_rotation_days < var.grafana_operator_token_expiration_days | |
| error_message = "grafana_operator_token_rotation_days must be a positive whole number less than grafana_operator_token_expiration_days." | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infrastructure/modules/grafana/variables.tf` around lines 87 - 103, The
validation block for grafana_operator_token_rotation_days only enforces ordering
but does not validate that both token expiration and rotation day values are
positive whole numbers. Add validation conditions to both
grafana_operator_token_expiration_days and grafana_operator_token_rotation_days
variables to ensure they are greater than zero and are not decimal values.
Update the existing validation block for grafana_operator_token_rotation_days to
include these additional conditions alongside the existing ordering check, and
add a new validation block to grafana_operator_token_expiration_days to enforce
the same positive whole-number constraints.
2 participants
Add automatic rotation of the grafana-operator service account token before it expires. Uses the time provider to trigger token regeneration every 180 days by default, preventing service disruption. Also upgrades the default Grafana version from 11 to 12.
Description
Related Issue(s)
Verification
Documentation
Summary by CodeRabbit
Release Notes
New Features
Chores