chore(aws): remove Grafana logs secrets from logs collector#2341
Conversation
Logs collector Grafana secrets is something we don't want to maintain for aws iac setup.
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit 8ef3f63. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
LGTM — straightforward cleanup removing Grafana logs secrets from the variable/output chain.
Extended reasoning...
Overview
The PR removes three Grafana logs-related secrets (, , ) from the AWS IaC module chain: the block in , the variable pass-through in , the module invocation in , and the three variable declarations in . All four changes are consistent and complete for the stated purpose.
Security risks
None. The removed items are placeholder secrets (initialized to ) that were never consumed downstream. No credentials are exposed or left dangling in an insecure way.
Level of scrutiny
Low. This is mechanical IaC configuration cleanup with a clear, narrow scope — removing an unsupported feature. The only finding (stale keys remaining in the initializer) is a nit with no security or runtime impact, already covered by the inline comment.
Other factors
The guard means existing deployments are completely unaffected. For new deployments, the orphaned keys are inert placeholder values. No CODEOWNER-sensitive paths are touched.
|
|
||
| output "grafana" { | ||
| value = { | ||
| api_key = local.grafana_raw["API_KEY"] | ||
| otlp_url = local.grafana_raw["OTLP_URL"] | ||
| otel_collector_token = local.grafana_raw["OTEL_COLLECTOR_TOKEN"] | ||
| username = local.grafana_raw["USERNAME"] | ||
| logs_user = local.grafana_raw["LOGS_USER"] | ||
| logs_url = local.grafana_raw["LOGS_URL"] | ||
| logs_collector_api_token = local.grafana_raw["LOGS_COLLECTOR_API_TOKEN"] | ||
| api_key = local.grafana_raw["API_KEY"] | ||
| otlp_url = local.grafana_raw["OTLP_URL"] | ||
| otel_collector_token = local.grafana_raw["OTEL_COLLECTOR_TOKEN"] | ||
| username = local.grafana_raw["USERNAME"] | ||
| } | ||
| sensitive = true | ||
| } |
There was a problem hiding this comment.
🟡 The aws_secretsmanager_secret_version.grafana resource still initializes the secret with LOGS_USER, LOGS_URL, and LOGS_COLLECTOR_API_TOKEN keys even though the PR removes these from the output block. New deployments will have an AWS Secrets Manager secret containing three orphaned placeholder keys that are never consumed — incomplete cleanup inconsistent with the PR's stated intent.
Extended reasoning...
What the bug is and how it manifests
The PR removes Grafana logs secrets from the AWS IaC setup. It correctly removes logs_user, logs_url, and logs_collector_api_token from the output "grafana" block (now lines 82–91) and removes all downstream consumers in main.tf and nomad/. However, the initial secret template in aws_secretsmanager_secret_version.grafana (lines 56–71 of iac/provider-aws/init/secrets.tf) was not updated and still includes all three keys.
The specific code path that triggers it
When a fresh deployment runs terraform apply, the aws_secretsmanager_secret_version.grafana resource creates the AWS Secrets Manager secret with this JSON body:
{
"API_KEY": " ",
"OTLP_URL": " ",
"OTEL_COLLECTOR_TOKEN": " ",
"USERNAME": " ",
"LOGS_USER": " ",
"LOGS_URL": " ",
"LOGS_COLLECTOR_API_TOKEN": " "
}The output block only surfaces api_key, otlp_url, otel_collector_token, and username — the three LOGS_* keys are initialized but never referenced again.
Why existing code doesn't prevent it
The lifecycle { ignore_changes = [secret_string] } block protects existing deployments from being mutated — Terraform won't try to update the real secret on existing infrastructure. But for new deployments this lifecycle clause has no effect on the initial creation: the resource is created fresh with the stale template that still includes the three orphaned keys.
Impact
The practical impact is minimal. The orphaned keys are set to the harmless placeholder value " " (a single space), and no code reads them after creation. There is no credential leak, no runtime failure, and no security concern. The only effect is that new deployments carry three dead keys in their Grafana secret, which can confuse operators inspecting the secret manually or auditing what keys the secret is expected to hold.
How to fix it
Remove the three stale entries from the secret_string initializer in aws_secretsmanager_secret_version.grafana:
secret_string = jsonencode({
"API_KEY" = " ",
"OTLP_URL" = " ",
"OTEL_COLLECTOR_TOKEN" = " ",
"USERNAME" = " ",
})Step-by-step proof
- A new environment runs
terraform applyagainst the post-PR code. - Terraform creates
aws_secretsmanager_secret_version.grafanawith thesecret_stringcontaining all seven keys includingLOGS_USER,LOGS_URL, andLOGS_COLLECTOR_API_TOKEN. - Terraform reads back the secret via
data.aws_secretsmanager_secret_version.grafanaand decodes it intolocal.grafana_raw. - The
output "grafana"block only readslocal.grafana_raw["API_KEY"],["OTLP_URL"],["OTEL_COLLECTOR_TOKEN"], and["USERNAME"]— the three LOGS keys are present in the secret but never surfaced. - Subsequent
terraform applyruns leave the secret untouched due toignore_changes, so the orphaned keys persist in the AWS secret indefinitely.
Remove Grafana logs secrets (logs_user, logs_url, logs_collector_api_token)
from the AWS IaC setup — this is a case we don't want to support.