Skip to content

[INF-178] Fetch Brainstore license and Datadog keys from Secrets Manager - #307

Open
Erik Weathers (erikdw) wants to merge 7 commits into
mainfrom
erikdw/ecs-ec2-secrets-from-sm
Open

[INF-178] Fetch Brainstore license and Datadog keys from Secrets Manager#307
Erik Weathers (erikdw) wants to merge 7 commits into
mainfrom
erikdw/ecs-ec2-secrets-from-sm

Conversation

@erikdw

@erikdw Erik Weathers (erikdw) commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Store Brainstore license key and internal observability API key in Secrets Manager instead of embedding them in Brainstore EC2 user_data / gateway ECS plain env vars.
  • Brainstore boots fetch both secrets (version IDs baked into user_data so rotations still force rolling refresh); gateway injects BRAINSTORE_LICENSE_KEY via ECS task secrets.
  • Mark root brainstore_license_key and internal_observability_api_key as sensitive.
  • CI also validates the external-EKS example with brainstore_license_key unset (wiring/syntax coverage only — validate does not catch eval-time coalesce-on-null).

https://linear.app/braintrustdata/issue/INF-178

Follow-up: plan-mode terraform test for the null license path → #308

Test plan

  • mise run lint / mise run validate (includes external-EKS example with license key unset — wiring only)
  • Plan against a sandbox with Brainstore (+ optional private gateway): confirm new license secret create, user_data no longer contains plaintext keys, gateway task def uses secrets for license
  • Apply and verify Brainstore instances boot with license + (if set) Datadog agent; gateway has BRAINSTORE_LICENSE_KEY from SM
  • Rotate license / observability key via TF var and confirm secret version bump triggers Brainstore rolling refresh / ECS rollout

Keep license and observability API keys out of EC2 user_data and gateway
ECS plain env vars so plans stay readable and credentials are not baked
into launch templates or task definitions.

Co-authored-by: Cursor <cursoragent@cursor.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d5bdbdaac2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

resource "aws_iam_role_policy" "task_execution_observability_secrets" {
count = local.observability_enabled ? 1 : 0
resource "aws_iam_role_policy" "task_execution_secrets" {
count = local.observability_enabled || local.license_key_enabled ? 1 : 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Derive the secrets-policy count from a plan-known boolean

When create_ai_gateway is enabled with the default internal-observability setting, brainstore_license_key_secret_arn is the ARN of a secret being created in this same apply, so it is unknown during planning; consequently local.license_key_enabled and this count are unknown. Terraform requires count to be known while constructing the plan and will reject the normal one-pass plan, requiring a targeted/multi-phase apply. Pass the already-known creation boolean into this module or otherwise make the count independent of the computed ARN.

AGENTS.md reference: AGENTS.md:L56-L62

Useful? React with 👍 / 👎.

Comment thread modules/gateway-ecs/main.tf Outdated
local.license_key_enabled ? [
{
name = "BRAINSTORE_LICENSE_KEY"
valueFrom = var.brainstore_license_key_secret_arn

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Version the gateway secret reference

When brainstore_license_key is rotated after deployment, this task-definition value remains the same secret ARN, so Terraform does not create a new task-definition revision or roll the ECS service. ECS injects secret values only when tasks start, leaving all running gateway tasks on the previous license indefinitely until an unrelated restart or deployment occurs; include the secret version in the task definition or otherwise trigger a service rollout when the version changes.

AGENTS.md reference: AGENTS.md:L56-L62

Useful? React with 👍 / 👎.

Erik Weathers (erikdw) and others added 4 commits July 30, 2026 16:11
Pass a plan-known enable flag into gateway-ecs so the task-exec secrets
policy count is never derived from a computed secret ARN, and pin the
license secret version in the task definition so key rotations roll ECS.

Co-authored-by: Cursor <cursoragent@cursor.com>
Pin DD_API_KEY / FireLens apikey valueFrom to the secret version so
rotating internal_observability_api_key revises gateway and API ECS task
definitions and rolls services. Also pass a plan-known enable flag into
api-ecs instead of deriving it from the computed secret ARN.

Co-authored-by: Cursor <cursoragent@cursor.com>
coalesce(null, "") fails plan when brainstore_license_key is unset; use a
null ternary instead. Also pin ECS secrets by version-id only (empty stage)
so out-of-band AWSCURRENT moves cannot disagree with the pinned version.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cover the null brainstore_license_key path by validating the external-EKS
example without a license key in mise validate / validate-tofu (already
run by Terraform CI).

Co-authored-by: Cursor <cursoragent@cursor.com>
@erikdw

Copy link
Copy Markdown
Contributor Author

Follow-up: plan-mode regression coverage for unset brainstore_license_key is staged separately — validate alone cannot catch the coalesce class of bug.

#308

Erik Weathers (erikdw) and others added 2 commits July 30, 2026 17:32
Prevent gateway/API tasks and Brainstore instance refreshes from starting
before Secrets Manager GetSecretValue is granted on the execution/instance
roles, which can fail secret resolution and trip the ECS circuit breaker.

Co-authored-by: Cursor <cursoragent@cursor.com>
Soften comments that overclaimed CI coverage of the null license path;
validate only checks wiring, while plan-mode terraform test is deferred.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

1 participant