Add CI/CD for staging deployment#3466
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughReplaces SSH/Ansible staging deployment with Terraform-driven CI/CD, adds AWS/ECR login and ECR tagging in workflows, surfaces Terraform plans/artifacts, orchestrates ECS task/migrations in staging, and renames project/SSM/S3 identifiers from "owasp-nest" to "nest". Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/run-ci-cd.yaml (1)
586-656: Guard against emptyDOMAIN_NAMEvalues interraform.tfvars.When
DOMAIN_NAMEis unset, the current code writesdomain_name="", which Terraform treats as a non-null empty string. This causesvar.domain_name != nullchecks to evaluate totrue, enabling HTTPS with an empty domain and producing invalid URLs likehttps://with no host. Emitdomain_name=nullinstead when unset.🐛 Suggested guard
- name: Prepare terraform variables env: AVAILABILITY_ZONES: ${{ vars.AWS_AVAILABILITY_ZONES }} AWS_REGION: ${{ vars.AWS_REGION }} CREATE_RDS_PROXY: false DOMAIN_NAME: ${{ vars.DOMAIN_NAME }} @@ run: | umask 377 + if [ -z "$DOMAIN_NAME" ]; then + DOMAIN_NAME_TF=null + else + DOMAIN_NAME_TF="\"$DOMAIN_NAME\"" + fi cat > infrastructure/staging/terraform.tfvars <<-EOF @@ - domain_name="$DOMAIN_NAME" + domain_name=$DOMAIN_NAME_TF
🧹 Nitpick comments (2)
.github/workflows/run-ci-cd.yaml (2)
450-549: Prefer OIDC-based AWS auth over long‑lived access keys.Static
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEYincreases credential exposure. Consider switching to OIDC withrole-to-assume. Also verify the ECR repo URL secrets exist for staging so the new tags resolve. The timeout bump looks reasonable for multi-registry pushes.♻️ Example OIDC switch
- - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@61815dcd50bd041e203e49132bacad1fd04d2708 - with: - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-region: ${{ vars.AWS_REGION }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@61815dcd50bd041e203e49132bacad1fd04d2708 + with: + aws-region: ${{ vars.AWS_REGION }} + role-to-assume: ${{ secrets.AWS_DEPLOY_ROLE_ARN }} + role-session-name: github-actions
687-851: Avoid drift by sourcing ECS network inputs from Terraform outputs.The migrate/index tasks rely on subnet and SG IDs from static secrets. With Terraform now managing infra, those IDs can drift after
terraform apply, causing task failures or cross‑VPC runs. Consider pulling these values fromterraform output(or SSM) in the job; if index-data success is required, optionally wait for completion and check the exit code.
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)
.github/workflows/run-ci-cd.yaml (1)
586-685: ValidateAVAILABILITY_ZONESis a valid JSON array before writing tfvars.If
AWS_AVAILABILITY_ZONESis set in GitHub variables as a comma-separated string (e.g.,us-east-2a,us-east-2b) instead of a JSON array, the lineavailability_zones=$AVAILABILITY_ZONESwill write invalid HCL syntax and breakterraform validateandterraform plan. Add validation to catch this early.✅ Add a quick format check
- name: Prepare terraform variables env: AVAILABILITY_ZONES: ${{ vars.AWS_AVAILABILITY_ZONES }} @@ run: | umask 377 + echo "$AVAILABILITY_ZONES" | jq -e 'type=="array"' >/dev/null || { + echo "::error::AWS_AVAILABILITY_ZONES must be a JSON array, e.g. [\"us-east-2a\",\"us-east-2b\"]" + exit 1 + } cat > infrastructure/staging/terraform.tfvars <<-EOF availability_zones=$AVAILABILITY_ZONES
🤖 Fix all issues with AI agents
In @.github/workflows/run-ci-cd.yaml:
- Around line 687-862: The Run ECS migrate task step can yield an empty tasks
array so capture the full run-task response and guard against missing task ARN:
after calling aws ecs run-task in the step with id migrate-task, check the
TASK_ARN (the variable written to GITHUB_OUTPUT and referenced as
steps.migrate-task.outputs.task_arn) and if it's empty or equals "None" fail
fast with a clear ::error:: message (include the run-task response or reason)
and exit 1 so the workflow doesn’t proceed to aws ecs wait/describe with an
invalid task ARN.
🧹 Nitpick comments (1)
.github/workflows/run-ci-cd.yaml (1)
450-459: Consider using GitHub OIDC instead of long‑lived AWS keys for ECR pushes.Using
role-to-assumewith GitHub's OIDC token reduces secret exposure and eliminates rotation burden. This requires:
- Adding
id-token: writeto job permissions- Creating an IAM role with an OIDC trust policy scoped to your repository
- Replacing
aws-access-key-idandaws-secret-access-keywithrole-to-assume: ${{ secrets.AWS_ROLE_ARN }}This pattern appears in multiple jobs throughout the workflow. The 10-minute timeout is appropriate for the current dual-registry push setup.
♻️ Example adjustment (OIDC)
permissions: contents: read + id-token: write - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@61815dcd50bd041e203e49132bacad1fd04d2708 with: - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} aws-region: ${{ vars.AWS_REGION }} + role-to-assume: ${{ secrets.AWS_ROLE_ARN }}
bd306a7 to
d5863c5
Compare
|
| "s3_bucket": "${ZAPPA_S3_BUCKET}", | ||
| "slim_handler": true, | ||
| "snap_start": "PublishedVersions", | ||
| "snap_start": "None", |
There was a problem hiding this comment.
currently has no effect.
8fd17a5
into
OWASP:feature/nest-zappa-migration



Proposed change
Resolves #2694
Required GitHub environment secrets:
Required GitHub environment variables:
["ap-south-1a", "ap-south-1b", "ap-south-1c"])ap-south-1)nest.rudransh.app)Checklist
make check-testlocally: all warnings addressed, tests passed