chore(infra): parameterize the AWS account id and fix stale docs - #375
Merged
Merged
Conversation
The account id was hardcoded into six workflow role ARNs, so pointing CI at a different AWS account meant editing six files with no single source of truth. Publish it as a Terraform-managed Actions variable instead: `var.aws_account_id` -> `vars.AWS_ACCOUNT_ID`, interpolated by every role ARN. Changing accounts is now one default plus an apply. The state bucket stays literal in the five backend.tf files because Terraform backend blocks cannot interpolate variables. Also pins the infisical provider. It was unconstrained in both roots and .terraform.lock.hcl is gitignored, so a fresh init resolves it fresh -- aws had drifted to 0.17.0 and github to 0.15.39. Both now pinned to 0.17.0. Docs were actively misleading: - root README.md was upstream terraform-docs boilerplate, not about this project - infrastructure/README.md documented a module.aws/module.github root composition that does not exist, and is never regenerated since that dir holds no .tf files - example.env was 0 bytes and referenced by nothing; removed in favour of a real apps/frontend/.env.example, which needed a gitignore exception since NEXT_PUBLIC_API_BASE_URL was undocumented in template form - infrastructure/AGENTS.md said three root modules (five), postgres 17.6 (17.9), and described an RDS in test/ whose main.tf is empty Drops the dead /github Infisical data source in aws/ (declared, never referenced) and corrects the oidc.tf comment claiming branch_rds has no explicit identifier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com>
Chicken-and-egg in the previous commit: every workflow role ARN interpolates vars.AWS_ACCOUNT_ID, so if the variable does not exist the ARN resolves to arn:aws:iam:::role/... and no job can assume a role. CI therefore cannot apply infrastructure/github, which is the thing that would have created the variable. The variable has to be set by hand once, before CI runs. Given that, Terraform must adopt it rather than create it -- a create against an existing variable is a 409. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The terraform-docs release tarball ships its own README.md and LICENSE. Unpacking it in the checkout root overwrote the repo's README.md, and the auto-commit step in the same job then committed the result -- which is why the root README has been upstream terraform-docs boilerplate. Extract only the binary, into RUNNER_TEMP. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverted by the auto-format job before the tarball-extraction fix landed in this branch. Restoring the intended content. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Auto-formatted .tf files with terraform fmt - Updated README.md with terraform-docs Co-authored-by: nourshoreibah <nourshoreibah@users.noreply.github.com>
The job ran with `if: always()` and never looked at what it needed -- it echoed a message and exited 0 regardless. Since terraform-plan-summary is the only terraform context in branch protection, a PR whose plan errored still showed a green required check and could merge. Check the results of terraform-fmt-docs and terraform-plan explicitly. skipped still counts as success, so PRs that touch no terraform are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
S3_BUCKET_NAME is read nowhere -- docker-compose passes REPORTS_BUCKET_NAME, so anyone following this template left the reports service with an empty bucket name and no obvious reason why. Also drops the placeholder values that looked like real settings (`key`, `secret`, `region or us-east-2`) and notes where the credentials come from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Terraform Plan 📖
|
Contributor
Terraform Plan 📖
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The AWS account id was hardcoded into six workflow role ARNs, so pointing CI at a different account meant editing six files with no single source of truth. This makes that a one-value change, and clears out documentation that actively misleads anyone setting the project up.
What changed
Account id is now a Terraform-managed Actions variable.
var.aws_account_id(infrastructure/github/variables.tf) →github_actions_variable.aws_account_id→${{ vars.AWS_ACCOUNT_ID }}, interpolated by every role ARN infrontend-deploy,lambda-deploy(×2),preview-env,terraform-apply, andterraform-plan. Default is the current account, so this is a no-op until the variable is changed.The state bucket in the five
backend.tffiles stays literal — Terraform backend blocks cannot interpolate variables. That's a language limitation, not an oversight.Pinned the
infisicalprovider to0.17.0in both roots. It was unconstrained and.terraform.lock.hclis gitignored, so every freshinitresolves it fresh —aws/had drifted to0.17.0andgithub/to0.15.39. Worth a look at the plan output to confirmgithub/is happy on the newer version.Docs:
README.mdwas upstream terraform-docs boilerplate. Replaced with a real one (stack, quick start, first-admin bootstrap, doc index, deploy model).infrastructure/README.mddocumented amodule.aws/module.githubroot composition that doesn't exist. It's never regenerated by terraform-docs either, since that dir holds no.tffiles. Replaced with a module index.example.envwas 0 bytes and referenced by nothing — removed. Addedapps/frontend/.env.exampleinstead, sinceNEXT_PUBLIC_API_BASE_URLhad no template anywhere and silently falls back to localhost. Needed a.gitignoreexception.infrastructure/AGENTS.md: said three root modules (five), postgres 17.6 (17.9), and described an RDS intest/whosemain.tfis empty. Also notes the account-bootstrap exception to "don't apply by hand".Cleanups: dropped the
/githubInfisical data source inaws/secrets.tf(declared, never referenced in that module) and corrected theoidc.tfcomment claimingbranch_rdshas no explicitidentifier— it does,main.tf:6.Verification
terraform fmt -recursive -checkcleanterraform validatepasses inaws/andgithub/(pre-existinghas_downloadsdeprecation warnings only)grep -rn 489881683177returns only the variable defaultThe plan comment on this PR is the real check — it should show the new
github_actions_variableas the only resource change, and no diff inaws/.vars.AWS_ACCOUNT_IDmust exist before any workflow runs, or every role ARN resolves toarn:aws:iam:::role/.... Applyinginfrastructure/githubcreates it; theterraform-applyjob on this merge does that in the same run, but it's worth watching.🤖 Generated with Claude Code