Replace PG_URL env var with Secrets Manager-based credential injection#217
Replace PG_URL env var with Secrets Manager-based credential injection#217
PG_URL env var with Secrets Manager-based credential injection#217Conversation
- Add AWS Parameters and Secrets Lambda Extension layers (arm64/x86_64) to all Lambda functions - Create secrets-wrapper exec wrapper script that fetches DB credentials from Secrets Manager and constructs PG_URL at runtime - Replace hardcoded PG_URL environment variable with PG_HOST, PG_PORT, and DATABASE_SECRETS_ARN across all Lambda functions - Grant IAM permissions for secretsmanager:GetSecretValue and KMS Decrypt to Lambda execution roles
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04b63e9aa4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
|
|
||
| layers = local.observability_enabled ? [local.datadog_python_layer_arn, local.datadog_extension_layer_arn] : [] | ||
| layers = concat( | ||
| local.observability_enabled ? [local.datadog_node_layer_arn, local.datadog_extension_arm_layer_arn] : [], |
There was a problem hiding this comment.
Use Python/x86 Datadog layers for migration Lambda
When internal_observability_api_key is set, this Lambda now attaches Datadog-Node22-x plus the ARM extension even though migrate_database runs python3.13 on the default x86_64 architecture. In that mode the update can fail due to incompatible layer architecture, and even if it deploys, the Python Datadog handler path (datadog_lambda.handler.handler) is missing the expected Python layer, so migration invocations break in observability-enabled environments.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| JQ=$(resolve_jq) | ||
| [[ -x "$JQ" ]] || die "jq binary not found or not executable: $JQ" |
There was a problem hiding this comment.
Package jq in the secrets wrapper layer
This wrapper hard-fails startup unless /opt/bin/jq-linux-amd64 or /opt/bin/jq-linux-arm64 exists, but the newly added secrets-wrapper layer source only contains this shell script, so clean deployments do not provide those binaries. As soon as AWS_LAMBDA_EXEC_WRAPPER is set, cold starts terminate with jq binary not found, preventing PG_URL injection and stopping the Lambda handler from running.
Useful? React with 👍 / 👎.
|
thejohnny please dont tag reviewers on your PR until you're ready. No CI failures, no codex errors, etc |
| locals { | ||
| secrets_ext_arns_arm64 = { | ||
| us-east-1 = { | ||
| arn = "arn:aws:lambda:us-east-1:177933569100:layer:AWS-Parameters-and-Secrets-Lambda-Extension-Arm64" | ||
| version = 61 | ||
| } | ||
| us-east-2 = { | ||
| arn = "arn:aws:lambda:us-east-2:590474943231:layer:AWS-Parameters-and-Secrets-Lambda-Extension-Arm64" | ||
| version = 67 | ||
| } | ||
| us-west-2 = { | ||
| arn = "arn:aws:lambda:us-west-2:345057560386:layer:AWS-Parameters-and-Secrets-Lambda-Extension-Arm64" | ||
| version = 61 | ||
| } |
There was a problem hiding this comment.
Can these be looked up? This isn't maintainable.
Mike Deeks (mdeeks)
left a comment
There was a problem hiding this comment.
There are a lot of TODOs and commented out things.
Fetch existing TF-generated postgresql password from AWS Secrets Manager and inject to all Lambda functions at cold start.
PG_URLat runtimePG_URLenvironment variable withPG_HOST,PG_PORT, andDATABASE_SECRETS_ARNacross all Lambda functions