feat: add LocalStack-based local infrastructure automation and documentation - #5126
feat: add LocalStack-based local infrastructure automation and documentation#5126devnchill wants to merge 52 commits into
Conversation
github-actions
Bot
added
docs
e8f5a03 to
2549aff
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds LocalStack startup and provisioning automation, updates infrastructure make targets and path resolution, expands local prerequisites, and allows the environment validation to accept ChangesLocalStack infrastructure automation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 3
🤖 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/scripts/start-localstack.sh`:
- Around line 18-21: The LOCALSTACK_AUTH_TOKEN check in start-localstack.sh
should use the same bash-style conditional and stderr handling as the other
script checks. Update the token validation block to use [[ for the emptiness
test, and send the error message to stderr instead of stdout before exiting.
Keep the logic in the existing startup guard around LOCALSTACK_AUTH_TOKEN so it
matches the script’s other validation patterns.
- Around line 23-24: The start-localstack.sh entrypoint assumes the localstack
CLI exists, so it can fail with a generic command-not-found error instead of a
clear prerequisite message. Add a pre-check before the exec localstack start
call in the startup flow (near the existing echo "Starting LocalStack..." block)
to verify localstack is available, and exit with a helpful message if it is
missing.
- Around line 8-12: The startup check in start-localstack.sh should use the
safer bash conditional syntax and send failures to stderr. Update the ENV_FILE
existence guard to use the script’s existing conditional block with [[ ... ]]
instead of [ ... ], and change the two ERROR echo statements in that block to
write to standard error so failures are not mixed with normal output.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 41c4b436-ca9c-4396-a58f-eeab26d32481
📒 Files selected for processing (5)
Makefileinfrastructure/.env.exampleinfrastructure/Makefileinfrastructure/README.mdinfrastructure/scripts/start-localstack.sh
|
@rudransh-shrivastava should i modify this to include |
Yes, sounds good. |
|
for ╷
│ Error: listing tags for S3 (Simple Storage) Bucket (nest-local-alb-logs-0da353d3): operation error S3 Control: ListTagsForResource, https response error StatusCode: 0, RequestID: , HostID: , request send failed, Get "http://000000000000.localhost:4566/v20180820/tags/arn%3Aaws%3As3%3A%3A%3Anest-local-alb-logs-0da353d3": dial tcp: lookup 000000000000.localhost on 127.0.0.1:53: no such host
│
│ with module.alb.aws_s3_bucket.alb_logs,
│ on ../modules/alb/main.tf line 195, in resource "aws_s3_bucket" "alb_logs":
│ 195: resource "aws_s3_bucket" "alb_logs" { # NOSONAR
│
╵This happens during |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/live/variables.tf (1)
238-245: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError message doesn't mention the newly-allowed
localvalue.The validation condition now accepts
"local", buterror_messagestill only references'staging'or'production', which will confuse anyone hitting the validation failure.🔧 Proposed fix
validation { condition = contains(["local", "staging", "production"], var.environment) - error_message = "Environment must be either 'staging' or 'production'." + error_message = "Environment must be one of 'local', 'staging', or 'production'." }🤖 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/live/variables.tf` around lines 238 - 245, The `environment` variable validation in `variables.tf` now allows `local`, but the `error_message` still only mentions `staging` and `production`. Update the validation message in the `variable "environment"` block so it explicitly lists `local` alongside the other allowed values, keeping it consistent with the `contains(...)` condition.
🤖 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/scripts/provision-infra.sh`:
- Around line 38-42: The provision-infra.sh setup is using a hardcoded fallback
for DB_PASSWORD, which should be removed from the default env assignments.
Update the script to either require DB_PASSWORD explicitly with a clear error or
generate a non-static local default, and align the behavior with the explicit
validation approach used in start-localstack.sh for LOCALSTACK_AUTH_TOKEN.
Reference the DB_PASSWORD initialization block and the related environment
variable handling near the top of the script so the fix stays localized and easy
to find.
---
Outside diff comments:
In `@infrastructure/live/variables.tf`:
- Around line 238-245: The `environment` variable validation in `variables.tf`
now allows `local`, but the `error_message` still only mentions `staging` and
`production`. Update the validation message in the `variable "environment"`
block so it explicitly lists `local` alongside the other allowed values, keeping
it consistent with the `contains(...)` condition.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0ca74ae7-9d9a-4f1d-b739-edd17ab61821
📒 Files selected for processing (5)
Makefileinfrastructure/Makefileinfrastructure/live/variables.tfinfrastructure/scripts/provision-infra.shinfrastructure/scripts/start-localstack.sh
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
infrastructure/README.md (1)
9-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
jqprerequisite.
provision-infra.sh(same cohort) generatestfvars.jsonviajqbut this list doesn't mention it. Without it, users hit a raw "command not found" failure instead of a documented prerequisite.📝 Proposed addition
- Curl: [Curl](https://github.com/curl/curl) +- jq: [jq](https://jqlang.github.io/jq/download/) - Terraform: [Terraform Documentation](https://developer.hashicorp.com/terraform/docs)🤖 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/README.md` around lines 9 - 17, The prerequisites list in infrastructure/README.md is missing jq, even though provision-infra.sh relies on it to generate tfvars.json. Update the setup/prerequisites section to include jq alongside the existing tools so users can install it before running the infrastructure provisioning flow. Use the existing checklist items near the infrastructure setup references to keep the documentation consistent.infrastructure/scripts/provision-infra.sh (1)
14-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPrerequisite check runs after
curlis already used.Line 14 calls
curlfor the LocalStack health check before the prerequisite loop (lines 20-25) verifiescurlexists. Ifcurlis missing, theif ! curl ...test fails for the wrong reason and the user sees a misleading "LocalStack is not running or not ready" message instead of the actual "curl not found" error. Move the prerequisite check above the health-check block.🐛 Proposed fix
+for cmd in curl tflocal awslocal docker; do + if ! command -v "$cmd" >/dev/null 2>&1; then + echo "ERROR: '$cmd' not found. See infrastructure/README.md prerequisites." >&2 + exit 1 + fi +done + if ! curl -sf "http://localhost.localstack.cloud:4566/_localstack/health" >/dev/null 2>&1; then echo "ERROR: LocalStack is not running or not ready." >&2 echo "Start it in another terminal with: make start-localstack" >&2 exit 1 fi - -for cmd in curl tflocal awslocal docker; do - if ! command -v "$cmd" >/dev/null 2>&1; then - echo "ERROR: '$cmd' not found. See infrastructure/README.md prerequisites." >&2 - exit 1 - fi -done🤖 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/scripts/provision-infra.sh` around lines 14 - 25, Move the prerequisite validation in provision-infra.sh so the command-existence loop runs before the LocalStack health check. The issue is that curl is invoked in the initial health probe before command -v checks verify it exists, which can produce a misleading LocalStack failure message. Update the script around the curl health-check and the for cmd in curl tflocal awslocal docker block so missing curl is reported by the prerequisite check first.
🤖 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/scripts/provision-infra.sh`:
- Around line 47-48: The jq argument for environment in the infra provisioning
script is hardcoded to a fixed value, so it cannot be overridden like the other
configuration fields. Update the jq invocation in the provisioning flow to
source environment from an environment variable with a local/development
default, matching the pattern used by DJANGO_CONFIGURATION, DOMAIN_NAME,
ENABLE_CRON_TASKS, and DB_PASSWORD, so the script supports override-friendly
deployment configuration.
- Line 54: The `provision-infra.sh` argument passed to `jq -n --argjson` for
`ENABLE_CRON_TASKS` can fail on non-boolean input, so normalize or validate the
environment value before building the JSON args. Update the script around the
`--argjson enable_cron_tasks "$ENABLE_CRON_TASKS"` usage to accept only valid
boolean-style values (or convert common variants) and emit a clear error message
if the value is invalid, instead of letting `jq` fail with a parse error.
---
Outside diff comments:
In `@infrastructure/README.md`:
- Around line 9-17: The prerequisites list in infrastructure/README.md is
missing jq, even though provision-infra.sh relies on it to generate tfvars.json.
Update the setup/prerequisites section to include jq alongside the existing
tools so users can install it before running the infrastructure provisioning
flow. Use the existing checklist items near the infrastructure setup references
to keep the documentation consistent.
In `@infrastructure/scripts/provision-infra.sh`:
- Around line 14-25: Move the prerequisite validation in provision-infra.sh so
the command-existence loop runs before the LocalStack health check. The issue is
that curl is invoked in the initial health probe before command -v checks verify
it exists, which can produce a misleading LocalStack failure message. Update the
script around the curl health-check and the for cmd in curl tflocal awslocal
docker block so missing curl is reported by the prerequisite check first.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0a620622-1192-492a-af2b-f31a36f447e8
📒 Files selected for processing (2)
infrastructure/README.mdinfrastructure/scripts/provision-infra.sh
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
infrastructure/scripts/provision-infra.sh (1)
38-38: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDefault
ENVIRONMENTtolocal.
infrastructure/live/variables.tfonly acceptslocal,staging, andproduction, but this script defaultsENVIRONMENTtoLocaland passes it straight to Terraform. Fresh runs will fail validation unless the caller overrides it.Suggested change
-ENVIRONMENT="${ENVIRONMENT:-Local}" +ENVIRONMENT="${ENVIRONMENT:-local}"🤖 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/scripts/provision-infra.sh` at line 38, The default value assigned in provision-infra.sh for ENVIRONMENT uses an invalid capitalized value, which will fail Terraform validation in scripts that do not override it. Update the ENVIRONMENT default in the provisioning script to the lowercase supported value so it matches the allowed inputs from the Terraform variables, and keep the existing flow that passes ENVIRONMENT through to Terraform unchanged.
🤖 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/scripts/provision-infra.sh`:
- Line 43: The DB_PASSWORD assignment in provision-infra.sh is malformed because
it still includes a diff marker. Remove the leading plus so the shell treats it
as a normal variable assignment, not a command invocation. Check the top-level
DB_PASSWORD initialization in the script and keep the strict-mode provisioning
flow intact.
- Around line 57-64: The jq invocation in the infrastructure provisioning script
has a malformed option where the `enable_cron_tasks` argument is passed with
four leading dashes, causing `TFVARS_JSON` generation to fail. Fix the `jq`
command in `provision-infra.sh` by correcting the flag to the proper `--argjson`
form, keeping the existing `jq -n` argument pattern and the `enable_cron_tasks`
variable name intact.
---
Duplicate comments:
In `@infrastructure/scripts/provision-infra.sh`:
- Line 38: The default value assigned in provision-infra.sh for ENVIRONMENT uses
an invalid capitalized value, which will fail Terraform validation in scripts
that do not override it. Update the ENVIRONMENT default in the provisioning
script to the lowercase supported value so it matches the allowed inputs from
the Terraform variables, and keep the existing flow that passes ENVIRONMENT
through to Terraform unchanged.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 26745d9e-5d13-423b-9178-192445c4b976
📒 Files selected for processing (2)
infrastructure/README.mdinfrastructure/scripts/provision-infra.sh
There was a problem hiding this comment.
3 issues found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
00cb5f5 to
633b334
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/live/variables.tf (1)
238-245: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError message doesn't mention the newly accepted
localvalue.The validation now accepts
local, but the error message still only listsstaging/production, which will confuse users who hit this validation with an invalid value.🔧 Proposed fix
validation { condition = contains(["local", "staging", "production"], var.environment) - error_message = "Environment must be either 'staging' or 'production'." + error_message = "Environment must be one of 'local', 'staging', or 'production'." }Also worth checking other
variableblocks ininfrastructure/for similarcontains([...])validations that may need the samelocaladdition, per the review thread's note that "there may be other similar validations that also need updating."#!/bin/bash # Find other environment-style contains() validations across infrastructure/ rg -nP 'contains\(\[.*staging.*production.*\]' infrastructure --type=hcl -C2 rg -nP 'contains\(\[.*production.*staging.*\]' infrastructure --type=hcl -C2🤖 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/live/variables.tf` around lines 238 - 245, The environment variable validation in variable "environment" accepts local, but the error_message in the same block still only mentions staging and production; update that message to list local as well. While you’re in infrastructure/live/variables.tf, scan other variable blocks that use contains([...]) for environment-style validations and make the same local update wherever applicable, using the variable block names and their validation/error_message fields to locate them.
🤖 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/scripts/provision-infra.sh`:
- Line 43: The DB_PASSWORD fallback in provision-infra.sh currently generates a
new random value on every run, which breaks rerunnability. Update the password
handling near the DB_PASSWORD assignment so it either persists the generated
value back into .env on first run or fails fast with a clear error when
DB_PASSWORD is unset, similar to the LOCALSTACK_AUTH_TOKEN check. Keep the fix
localized to the provisioning flow and the DB_PASSWORD/DB_PASSWORD_FILE handling
so repeated runs reuse the same secret.
- Around line 57-69: The jq invocation in provision-infra.sh is broken because
the option chain is terminated early by the missing line-continuation after
enable_cron_tasks. Update the jq -n argument list so the db_password,
db_deletion_protection, db_skip_final_snapshot, and enable_nat_gateway flags
remain part of the same command, using the existing jq setup and
_normalize_bool/ENABLE_CRON_TASKS flow.
---
Outside diff comments:
In `@infrastructure/live/variables.tf`:
- Around line 238-245: The environment variable validation in variable
"environment" accepts local, but the error_message in the same block still only
mentions staging and production; update that message to list local as well.
While you’re in infrastructure/live/variables.tf, scan other variable blocks
that use contains([...]) for environment-style validations and make the same
local update wherever applicable, using the variable block names and their
validation/error_message fields to locate them.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 879377aa-ce05-4195-a4c9-8f49c031ca0e
📒 Files selected for processing (7)
Makefileinfrastructure/.env.exampleinfrastructure/Makefileinfrastructure/README.mdinfrastructure/live/variables.tfinfrastructure/scripts/provision-infra.shinfrastructure/scripts/start-localstack.sh
654f58c
Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
…ocalstack Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
…bot auth and environ import Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
Signed-off-by: Viena <169875752+devnchill@users.noreply.github.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
…ev environment Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
Signed-off-by: Viena <theviena96@gmail.com>
…ration Signed-off-by: Viena <theviena96@gmail.com>
4e7cfe1 to
a311138
Compare
|
|
Hi @devnchill |
|
@rudransh-shrivastava yes , i've ported scripts properly . everything seems to work fine. you can takeover from here on,i'm burnt out from working on this pr for so long |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| self.env_path.parent.mkdir(parents=True, exist_ok=True) | ||
| self.env_path.touch(exist_ok=True) | ||
| with self.env_path.open("a", encoding="utf-8") as handle: | ||
| handle.write(header) |
4 participants
Proposed change
Resolves #5115
Checklist