Migrate OWASP Nest to Zappa for serverless deployment#2431
Conversation
Summary by CodeRabbit
WalkthroughAdds a new Terraform-based AWS infrastructure (modules for networking, security, storage, database, cache, ECS and task submodules), per-module provider lockfiles, Zappa support for the backend including an example settings file and dependency, pre-commit and CI TFLint setup, cspell Terraform dictionary additions, and an infrastructure README plus example tfvars. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Points to review closely:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.gitignore(1 hunks)backend/pyproject.toml(1 hunks)backend/zappa_settings.example.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
backend/pyproject.toml (1)
51-51: ...
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
backend/zappa_settings.example.json (1)
1-36: Add Lambda timeout and memory configuration; document secrets management strategy.The example Zappa config is missing critical Lambda settings and lacks clarity on environment variable population:
Missing Lambda timeout: Default is 30 seconds, which may be insufficient for Django app initialization or long-running requests. Zappa should explicitly configure this (e.g.,
"lambda_handler_timeout").No memory/concurrency settings: Production deployments should define
memory_sizeandreserved_concurrencyto avoid cold starts and manage costs.Secrets management unclear: The 23 environment variables are populated via template substitution, but the PR objectives mention "AWS Parameter Store or Secrets Manager." Consider adding an example of how sensitive values are managed (e.g., DJANGO_SECRET_KEY, DB password).
VPC security groups: The config references
VPC_SECURITY_GROUPand assumes RDS/Redis SGs are correctly configured, but there's no validation or documentation linking to the security module outputs.Enhance the example with timeout and memory settings:
{ "staging": { ... "lambda_handler_timeout": 60, "memory_size": 1024, "reserved_concurrency": 5, ... } }Additionally, create or update documentation in the repository root or deployment guide that explains:
- How to populate each environment variable (e.g., from AWS Secrets Manager, Parameter Store, or
.env)- Which variables should never be committed to version control
- How to link VPC security groups and subnet IDs from the Terraform infrastructure outputs
Would you like me to generate a template deployment guide or a companion
.env.examplefile to clarify the environment variable requirements?infrastructure/modules/networking/variables.tf (2)
1-4: Consider adding default availability zones or validation guidance.The
availability_zonesvariable has no default value, requiring callers to explicitly provide zones. While this is flexible, it can be error-prone if not documented. Consider either providing sensible defaults (e.g.,["ap-south-1a", "ap-south-1b", "ap-south-1c"]for the chosen region) or adding an inline comment clarifying the expected format.If defaults are desired, update the variable:
variable "availability_zones" { description = "A list of availability zones for the VPC (e.g., [\"ap-south-1a\", \"ap-south-1b\"])" type = list(string) default = ["ap-south-1a", "ap-south-1b", "ap-south-1c"] }Alternatively, add validation to ensure at least one zone is provided:
variable "availability_zones" { description = "A list of availability zones for the VPC" type = list(string) validation { condition = length(var.availability_zones) > 0 error_message = "At least one availability zone must be provided." } }
21-29: Add validation for CIDR block variables.Public and private subnet CIDR blocks should be validated to prevent invalid IP ranges from being deployed. This catches configuration errors early.
Add validation blocks to both CIDR variables:
variable "vpc_cidr" { description = "The CIDR block for the VPC" type = string validation { condition = can(cidrhost(var.vpc_cidr, 0)) error_message = "VPC CIDR must be a valid IPv4 CIDR block." } } variable "public_subnet_cidrs" { description = "A list of CIDR blocks for the public subnets" type = list(string) validation { condition = alltrue([for cidr in var.public_subnet_cidrs : can(cidrhost(cidr, 0))]) error_message = "All public subnet CIDRs must be valid IPv4 CIDR blocks." } } variable "private_subnet_cidrs" { description = "A list of CIDR blocks for the private subnets" type = list(string) validation { condition = alltrue([for cidr in var.private_subnet_cidrs : can(cidrhost(cidr, 0))]) error_message = "All private subnet CIDRs must be valid IPv4 CIDR blocks." } }infrastructure/modules/storage/outputs.tf (1)
1-4: Consider exposing the S3 bucket name as an additional output for operational convenience.While the ARN is correct, exposing the actual bucket name makes it easier for operators to reference the bucket in AWS CLI commands or manual interventions without parsing the ARN.
Add a companion output:
output "zappa_s3_bucket_arn" { description = "The ARN of the S3 bucket for Zappa deployments" value = aws_s3_bucket.zappa.arn } output "zappa_s3_bucket_name" { description = "The name of the S3 bucket for Zappa deployments" value = aws_s3_bucket.zappa.id }infrastructure/modules/iam/variables.tf (1)
11-14: Consider adding validation for the S3 bucket ARN format.The
s3_bucket_arnvariable should validate that the provided value is a valid ARN to catch configuration errors early.Add validation to ensure the input is a properly formatted ARN:
variable "s3_bucket_arn" { description = "The ARN of the S3 bucket for Zappa deployments" type = string validation { condition = can(regex("^arn:aws:s3:::[a-z0-9][a-z0-9.-]*[a-z0-9]$", var.s3_bucket_arn)) error_message = "S3 bucket ARN must be a valid format, e.g., 'arn:aws:s3:::my-bucket'." } }infrastructure/modules/database/main.tf (1)
12-19: Password special character override is reasonable but verify connection handling.Line 18 overrides special characters to avoid those that might cause shell escaping or encoding issues in connections strings. The chosen set
"!#$%&*()-_=+[]{}<>:?"is well-curated and excludes problematic characters like backticks, quotes, and backslashes.Verify that connection pooling (RDS Proxy) and application configuration properly handle these characters in connection strings, especially in URL-encoded forms or shell contexts.
infrastructure/modules/storage/main.tf (1)
1-56: LGTM. S3 bucket configuration implements solid security practices: public access blocked, versioning enabled, server-side encryption with AES256, and lifecycle rules to clean up old versions and incomplete uploads. Appropriate for Zappa deployment artifacts.For enhanced durability and compliance, consider these optional additions in future iterations:
- MFA Delete:
mfa_delete = "Enabled"requires MFA to permanently delete objects (prevents ransomware/accidental deletion).- Server Access Logging: Configure S3 server access logs to CloudWatch for audit trails.
- Bucket Policy: Restrict bucket access to Zappa Lambda execution role only (principle of least privilege).
- S3 Object Lock: Enable for immutability guarantees if compliance/archival is a requirement.
infrastructure/modules/iam/main.tf (1)
1-26: Verify if all principals are necessary for Zappa execution.The assume role policy includes
apigateway.amazonaws.comandevents.amazonaws.comservices. For a typical Zappa deployment, onlylambda.amazonaws.comis needed for Lambda function execution. Confirm whether API Gateway or EventBridge integration requires these principals, or remove them to follow least-privilege principle.infrastructure/modules/networking/main.tf (1)
50-71: LGTM!NAT Gateway and EIP are properly configured with explicit dependencies. One note: the current setup places the NAT Gateway in a single availability zone, which creates a potential single point of failure for private subnet egress traffic. For production environments, consider deploying NAT Gateways in each AZ for high availability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.gitignore(2 hunks)backend/zappa_settings.example.json(1 hunks)infrastructure/.terraform.lock.hcl(1 hunks)infrastructure/main.tf(1 hunks)infrastructure/modules/cache/main.tf(1 hunks)infrastructure/modules/cache/outputs.tf(1 hunks)infrastructure/modules/cache/variables.tf(1 hunks)infrastructure/modules/database/main.tf(1 hunks)infrastructure/modules/database/outputs.tf(1 hunks)infrastructure/modules/database/variables.tf(1 hunks)infrastructure/modules/iam/main.tf(1 hunks)infrastructure/modules/iam/variables.tf(1 hunks)infrastructure/modules/networking/main.tf(1 hunks)infrastructure/modules/networking/outputs.tf(1 hunks)infrastructure/modules/networking/variables.tf(1 hunks)infrastructure/modules/security/main.tf(1 hunks)infrastructure/modules/security/outputs.tf(1 hunks)infrastructure/modules/security/variables.tf(1 hunks)infrastructure/modules/storage/main.tf(1 hunks)infrastructure/modules/storage/outputs.tf(1 hunks)infrastructure/modules/storage/variables.tf(1 hunks)infrastructure/outputs.tf(1 hunks)infrastructure/providers.tf(1 hunks)infrastructure/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
🧰 Additional context used
🪛 Checkov (3.2.334)
infrastructure/modules/database/main.tf
[medium] 22-60: Ensure RDS database has IAM authentication enabled
(CKV_AWS_161)
[medium] 22-60: Ensure that AWS database instances have deletion protection enabled
(CKV_AWS_293)
[high] 22-60: Ensure RDS Performance Insights are encrypted using KMS CMKs
(CKV_AWS_354)
🔇 Additional comments (19)
infrastructure/modules/networking/outputs.tf (1)
1-14: Module outputs are well-structured and properly expose networking infrastructure.The outputs correctly expose VPC and subnet identifiers for consumption by dependent modules (security, database, cache). Descriptions are clear, and the use of splat syntax for subnet lists is appropriate.
infrastructure/modules/security/outputs.tf (1)
1-14: Security group outputs correctly expose infrastructure identifiers.The outputs provide access to Lambda, RDS, and Redis security group IDs as expected. Ensure that the corresponding
main.tffile defines appropriate ingress/egress rules for each security group (e.g., Lambda SG allows outbound to RDS/Redis on the correct ports, RDS SG allows inbound from Lambda SG on port 5432).infrastructure/.terraform.lock.hcl (1)
1-42: Terraform lockfile is properly configured and ensures reproducible deployments.The lockfile pins AWS provider to 6.17.0 and random to 3.7.2 with full hash verification. This is a best practice that should be committed to version control. Ensure the lockfile is checked in and team members run
terraform initto respect these versions.infrastructure/modules/storage/variables.tf (1)
1-14: LGTM. Variable declarations are clear and well-described. No concerns.infrastructure/modules/database/outputs.tf (1)
1-10: LGTM. Outputs properly expose RDS endpoint and password with appropriate sensitive marking to prevent credential leakage.infrastructure/modules/cache/outputs.tf (1)
1-10: LGTM. Outputs correctly expose the Redis endpoint and auth token with sensitive marking for credentials.infrastructure/modules/security/main.tf (1)
1-72: LGTM. Security groups implement proper least-privilege design: Lambda has unrestricted egress to reach RDS and Redis via cross-referenced ingress rules, while RDS and Redis restrict inbound to Lambda only. Tagging is consistent across resources.infrastructure/modules/security/variables.tf (1)
1-24: LGTM. Variable declarations are clear, properly typed, and align with usage in security group resources.infrastructure/outputs.tf (1)
1-21: LGTM. Root outputs correctly surface module-level infrastructure endpoints and credentials with proper sensitive marking to prevent accidental logging of secrets.infrastructure/modules/database/variables.tf (1)
1-51: LGTM!The variable definitions are clean and follow Terraform best practices. The
db_passwordis correctly marked as sensitive with a null default, allowing it to be optionally provided by the caller.infrastructure/modules/cache/main.tf (1)
22-22: Encryption and high-availability settings are well-configured.The use of conditional
auth_tokenlogic, automatic failover based on node count, and encryption at rest/in-transit all follow security best practices.Also applies to: 26-53
infrastructure/modules/cache/variables.tf (1)
1-46: LGTM!The cache variables are well-defined with appropriate types, descriptions, and sensible defaults. The
redis_auth_tokencorrectly defaults to null to enable automatic token generation.infrastructure/modules/iam/main.tf (3)
29-38: LGTM!The managed policy attachments for VPC access and basic Lambda execution are appropriate for this serverless deployment.
40-63: LGTM!S3 permissions are appropriately scoped to the Zappa deployment bucket and necessary actions only (read/write/delete/list).
65-85: LGTM!CloudWatch logging permissions are standard and necessary for Lambda execution observability.
infrastructure/main.tf (1)
1-64: LGTM!Module composition is well-organized with proper dependency wiring. All inter-module outputs are correctly referenced as inputs to downstream modules, establishing a clear infrastructure dependency graph.
infrastructure/modules/networking/main.tf (1)
73-115: LGTM!Route table configuration and associations are properly designed. Public subnets route to the Internet Gateway, private subnets route through the NAT Gateway, and all associations use count to match the subnet configuration.
infrastructure/variables.tf (2)
1-115: LGTM with verification requests.The top-level variables are well-structured with sensible defaults for staging environment. The defaults align well with the module implementations. Two items need verification:
- Confirm
db_passwordnull handling in the database module (potential deployment blocker)- Address the
redis_engine_version/parameter_group_namecoupling (related to critical issue in cache/main.tf)
31-36: Database module correctly handles null password—review comment is incorrect.The database module (
infrastructure/modules/database/main.tf) implements proper null handling for the password. Arandom_passwordresource is conditionally created whendb_passwordis null or empty (line 13), and the RDS instance uses either the provided password or the generated one (line 33). No deployment failure will occur, and no changes are needed.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
infrastructure/main.tf (1)
30-31: Remove unnecessary blank lines.Lines 30–31 introduce extra whitespace between the
storageanddatabasemodules with no clear purpose. Remove these for cleaner formatting.} - - module "database" {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/zappa_settings.example.json(1 hunks)infrastructure/main.tf(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- backend/zappa_settings.example.json
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
infrastructure/README.md (1)
67-67: Use the modern Poetry activation syntax.The command at line 67 uses the older
eval $(poetry env info ...)pattern. Modern Poetry recommends usingpoetry shellorpoetry runto execute commands within the virtual environment context.Apply this diff to use the current best practice:
- eval $(poetry env activate) + poetry shellOr, if you prefer to run individual commands without a persistent shell session, use
poetry run <command>for subsequent steps.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
infrastructure/README.md
39-39: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
87-87: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
.pre-commit-config.yaml (1)
12-18: Add a.tflint.hclconfiguration file to define terraform_tflint linting rules.Verification confirms no
.tflint.hclfile exists in the repository. Theterraform_tflinthook is configured to run on all infrastructure Terraform files but will use tflint's default linting rules without explicit configuration. For consistency with other hooks in this file and to enforce project-specific standards, add a.tflint.hclfile at the repository root to define desired linting rules, or add explicitargsto the hook configuration.infrastructure/modules/storage/main.tf (1)
16-72: LGTM—S3 bucket is properly secured with encryption, versioning, and lifecycle policies.Public access is blocked, encryption is enabled, and lifecycle rules manage old versions and incomplete uploads. The configuration is appropriate for Zappa deployment storage.
Note: Line 23
force_destroy = trueis acceptable for testing but should be set tofalseor removed for production to prevent accidental data loss.Would you like me to add an inline comment warning about
force_destroyfor future production use?infrastructure/modules/networking/main.tf (2)
1-14: Unused provider import.The
randomprovider is declared (lines 9-12) but no resources from it are instantiated in this module. While harmless, this adds unnecessary overhead duringterraform initand future maintenance.Remove the unused
randomprovider block:required_providers { aws = { source = "hashicorp/aws" version = "~> 6.0" } - random = { - source = "hashicorp/random" - version = "~> 3.0" - } }
65-86: Single NAT Gateway creates egress bottleneck.Currently, all private subnets route egress through a single NAT Gateway in the first public subnet (line 79). This is acceptable for quick testing but becomes a single point of failure for any production-like workloads.
For future production migration, consider deploying one NAT Gateway per AZ for higher availability:
- subnet_id = aws_subnet.public[0].id + count = length(var.availability_zones) + subnet_id = aws_subnet.public[count.index].idThen update the private route table to reference the corresponding NAT Gateway per AZ.
infrastructure/modules/database/main.tf (1)
70-85: Secrets Manager recovery window is set to immediate deletion.Line 76 sets
recovery_window_in_days = 0, meaning deleted secrets cannot be recovered. This is acceptable for quick testing but note that accidental deletions are permanent. For longer-running testing or pre-production environments, consider setting a recovery window (e.g.,recovery_window_in_days = 7).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.pre-commit-config.yaml(1 hunks)backend/zappa_settings.example.json(1 hunks)infrastructure/main.tf(1 hunks)infrastructure/modules/cache/main.tf(1 hunks)infrastructure/modules/database/main.tf(1 hunks)infrastructure/modules/database/outputs.tf(1 hunks)infrastructure/modules/database/variables.tf(1 hunks)infrastructure/modules/networking/main.tf(1 hunks)infrastructure/modules/security/main.tf(1 hunks)infrastructure/modules/security/outputs.tf(1 hunks)infrastructure/modules/storage/main.tf(1 hunks)infrastructure/outputs.tf(1 hunks)infrastructure/providers.tf(1 hunks)infrastructure/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infrastructure/modules/database/outputs.tf
- infrastructure/providers.tf
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-17T15:25:53.713Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/modules/database/main.tf:22-60
Timestamp: 2025-10-17T15:25:53.713Z
Learning: The infrastructure code in the `infrastructure/` directory is intended for quick testing purposes only, not production-grade deployment. Production-grade security hardening controls (such as IAM database authentication, deletion protection, Performance Insights KMS encryption) are not required for this testing infrastructure.
Applied to files:
infrastructure/modules/cache/main.tfinfrastructure/modules/database/main.tf
📚 Learning: 2025-10-17T15:25:55.624Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/providers.tf:1-3
Timestamp: 2025-10-17T15:25:55.624Z
Learning: The infrastructure code in the OWASP/Nest repository (infrastructure/ directory) is intended for quick testing purposes only, not for production deployment.
Applied to files:
infrastructure/modules/cache/main.tfinfrastructure/modules/database/main.tf
📚 Learning: 2025-10-17T15:25:34.963Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/modules/cache/main.tf:30-30
Timestamp: 2025-10-17T15:25:34.963Z
Learning: The infrastructure/Terraform code in the OWASP Nest repository under the `infrastructure/` directory is intended for quick testing purposes only, not for production deployment.
Applied to files:
infrastructure/modules/cache/main.tf
🪛 Checkov (3.2.334)
infrastructure/modules/database/main.tf
[medium] 37-68: Ensure RDS database has IAM authentication enabled
(CKV_AWS_161)
[medium] 37-68: Ensure that AWS database instances have deletion protection enabled
(CKV_AWS_293)
[high] 37-68: Ensure RDS Performance Insights are encrypted using KMS CMKs
(CKV_AWS_354)
🔇 Additional comments (21)
.pre-commit-config.yaml (2)
12-18: LGTM: Terraform pre-commit hooks appropriately added.The addition of Terraform formatting and linting hooks is well-aligned with the PR objectives of managing Terraform-based AWS infrastructure. The version pinning (v1.92.0) ensures reproducibility, and the hook placement within the config is logical and consistent with the file's structure.
12-18: Pattern verification complete — no issues found.The file pattern
^infrastructure/.*\.tf$correctly captures all Terraform files in the repository. All 20.tffiles are located under theinfrastructure/directory (including root-level configs and modular subdirectories), with no Terraform files elsewhere that need separate handling. The pre-commit configuration is appropriately scoped.infrastructure/outputs.tf (1)
1-31: LGTM—outputs properly expose required infrastructure for Lambda deployment.The outputs correctly surface RDS Proxy endpoint (not direct RDS), private subnets, and security group ID needed for Zappa Lambda configuration. Sensitive flags appropriately protect credentials.
infrastructure/modules/security/outputs.tf (1)
1-19: LGTM—security group outputs are properly exposed for module composition.All four security groups (Lambda, RDS, RDS Proxy, Redis) are correctly exported for downstream wiring in the root module.
infrastructure/modules/security/main.tf (1)
16-114: LGTM—security groups follow defense-in-depth principles with proper ingress/egress rules.Design correctly prevents direct Lambda-to-RDS access by routing through RDS Proxy. All ingress rules are appropriately restricted by source (Lambda and RDS Proxy SGs), and egress permits outbound connectivity needed for Lambda. Naming, descriptions, and tags are clear.
infrastructure/modules/cache/main.tf (1)
26-73: LGTM—Redis configuration includes proper encryption, dynamic parameter group naming, and flexible auth token handling.The dynamic parameter group name (line 45) correctly adapts to the engine version, and encryption is enabled for both at-rest and in-transit. Auth token logic properly respects user-provided values while generating one when needed.
infrastructure/main.tf (1)
1-77: LGTM—root module properly orchestrates networking, security, storage, database, and cache with correct dependency ordering.Module interdependencies are well-defined: networking provides VPC foundation, security consumes VPC resources, and storage/database/cache all wire to appropriate dependencies. All security groups are properly passed to database and cache modules.
Note: Lambda and IAM orchestration are not included in this root module. These can be added as subsequent modules (e.g.,
module "lambda"andmodule "iam") once the foundational infrastructure is proven in testing. The current setup (security groups pre-defined, outputs exported) is well-positioned for that expansion.infrastructure/modules/database/variables.tf (1)
1-68: LGTM—database module variables are well-defined with sensible defaults and proper sensitivity handling.All required inputs are present and clearly described. The
db_passwordvariable is properly marked sensitive, defaults are reasonable (gp3 storage, 7-day backups), and the presence ofproxy_security_group_idsconfirms RDS Proxy integration.infrastructure/modules/networking/main.tf (3)
16-34: Good VPC and IGW configuration.DNS support and hostnames are properly enabled for Lambda/ECS integration. Tagging is consistent.
36-63: Solid multi-AZ subnet design.Public subnets correctly enable auto-assigned public IPs, private subnets disable them. AZ-aware naming and tagging improves operational visibility.
88-130: Route table design is standard and correct.Public/private route separation is properly implemented with NAT Gateway for private egress. Associations scale with subnet counts.
infrastructure/modules/database/main.tf (5)
1-14: Provider configuration is appropriate.The
randomprovider is properly declared and used for password generation (line 27-34).
16-24: DB Subnet Group configuration is correct.Properly references provided subnet IDs and includes descriptive tagging.
26-34: Well-reasoned password generation logic.Conditional creation, sufficient length, and careful special character filtering reduce operational friction.
36-68: RDS configuration is appropriate for testing infrastructure.Storage encryption enabled, non-public access enforced, and backup retention configurable. CloudWatch logging exports are useful for debugging. Configuration aligns with testing-only deployment goals. Based on learnings.
87-163: RDS Proxy and IAM configuration follows security best practices.The IAM role follows least-privilege (grants only secretsmanager:GetSecretValue on the specific secret). RDS Proxy enforces TLS (line 130) and uses Secrets Manager for credentials (Secrets-based auth, not IAM). Connection pool settings are reasonable for testing. Proxy target wiring is complete.
infrastructure/variables.tf (5)
1-119: Networking and AWS region variables are well-structured.Defaults provide a working out-of-the-box configuration. CIDR ranges don't overlap, and availability zones scale with infrastructure needs. Environment validation prevents misconfiguration.
13-54: Database variables provide sensible testing defaults.Engine version 16.10 is current, instance class is appropriate for testing costs, and credentials are handled with null defaults and sensitivity marking.
84-125: Cache and storage variables follow consistent patterns.Redis auth token properly marked sensitive with null default. Bucket naming is descriptive and follows common conventions.
127-137: Additional database configuration is well-chosen.GP3 storage provides good price/performance for testing, and 7-day retention balances testing needs with cost.
1-137: Variables file is comprehensive and well-organized.All variables have proper types, descriptions, and sensible defaults. Sensitive values (
db_password,redis_auth_token) are marked appropriately. Environment validation prevents misconfiguration. Defaults support immediate deployment to testing environments.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
infrastructure/main.tf (1)
86-112: [DUPLICATE] Clarify module scope: is Lambda orchestration in this PR, or is ECS for background workers only?Past review flagged that Lambda, IAM, RDS Proxy, and Secrets Manager module orchestration are missing. This file defines an
ecsmodule that receiveslambda_sg_idfrom security, which creates ambiguity:
- Is
ecsmodule for background workers (separate from Lambda backend)?- Is Lambda module orchestration missing or deferred?
- Where are IAM roles, RDS Proxy, and Secrets Manager integration?
The PR objectives (issue #2214) require: "Deploy the Django + Ninja backend to AWS Lambda using Zappa." This file does not show Lambda function deployment orchestration.
Before approval, clarify:
- Add a comment explaining the purpose of the
ecsmodule (background workers? backend? both?).- Confirm whether Lambda module orchestration is in scope for this PR or managed separately.
- If Lambda/IAM/RDS Proxy/Secrets are out of scope, update the PR description and add a follow-up issue.
🧹 Nitpick comments (5)
infrastructure/modules/ecs/main.tf (1)
272-345: Consolidate duplicate environment variable definitions.The same 16 environment variables (Django config, Algolia, OpenAI, etc.) are repeated identically in the migrate (lines 272–345), load_data (lines 378–451), and index_data (lines 492–565) task definitions. This creates 78 lines of duplicate code.
Extract these variables to a local value at the top of the file to reduce duplication and simplify maintenance:
locals { # ... existing locals ... django_environment_variables = [ { name = "DJANGO_ALGOLIA_APPLICATION_ID" value = var.django_algolia_application_id }, # ... 15 more variables ... ] }Then reference in each task:
container_definitions = jsonencode([ { name = "backend" image = aws_ecr_repository.main.repository_url command = [...] essential = true logConfiguration = { ... } - environment = [ - { name = "DJANGO_ALGOLIA_APPLICATION_ID", value = var.django_algolia_application_id }, - # ... 15 more ... - ] + environment = local.django_environment_variables } ])Also applies to: 378-451, 492-565
infrastructure/modules/database/variables.tf (3)
57-62: Clarifydb_passwordnull default behavior.Marking
db_passwordassensitivewithdefault = nullsuggests the password may be optionally provided. If the database module generates a password when null, document this. If the module requires a password, this should be a required variable (remove default).Verify how the database module (infrastructure/modules/database/main.tf) handles
db_password = null. If password generation is intended, consider using arandom_passwordresource in the database module and returning it as an output, rather than pushing password management to the root module.
64-68: Changedb_skip_final_snapshotdefault to false for safer production deployments.Defaulting
db_skip_final_snapshottotruemeans Terraform destroy will not create a final snapshot, risking data loss in production. The safer default isfalse, or split by environment:-variable "db_skip_final_snapshot" { - description = "Determines whether a final DB snapshot is created before the DB instance is deleted." - type = bool - default = true -} +variable "db_skip_final_snapshot" { + description = "Determines whether a final DB snapshot is created before the DB instance is deleted." + type = bool + default = false +}Alternatively, add environment-specific logic in infrastructure/variables.tf or infrastructure/terraform.prod.tfvars to set
db_skip_final_snapshot = trueonly for dev/staging andfalsefor production.
101-105: Increasesecret_recovery_window_in_daysdefault to prevent accidental secret deletion.Defaulting to
0causes AWS Secrets Manager to immediately delete the secret on removal, risking loss of critical credentials (db_password,redis_auth_token). AWS best practice is 7–30 days recovery window.-variable "secret_recovery_window_in_days" { - description = "The number of days that Secrets Manager waits before it can delete the secret. Set to 0 to delete immediately." - type = number - default = 0 -} +variable "secret_recovery_window_in_days" { + description = "The number of days that Secrets Manager waits before it can delete the secret. Set to 0 to delete immediately." + type = number + default = 7 +}infrastructure/main.tf (1)
53-69: Reordersourceattribute to first position per Terraform conventions.In Terraform module blocks,
sourceshould be the first attribute for readability and consistency.module "database" { + source = "./modules/database" + common_tags = local.common_tags db_allocated_storage = var.db_allocated_storage db_backup_retention_period = var.db_backup_retention_period db_engine_version = var.db_engine_version db_instance_class = var.db_instance_class db_name = var.db_name db_password = var.db_password db_storage_type = var.db_storage_type db_subnet_ids = module.networking.private_subnet_ids db_username = var.db_username environment = var.environment project_name = var.project_name proxy_security_group_ids = [module.security.rds_proxy_sg_id] security_group_ids = [module.security.rds_sg_id] - source = "./modules/database"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignore(2 hunks)infrastructure/main.tf(1 hunks)infrastructure/modules/cache/main.tf(1 hunks)infrastructure/modules/cache/outputs.tf(1 hunks)infrastructure/modules/cache/variables.tf(1 hunks)infrastructure/modules/database/main.tf(1 hunks)infrastructure/modules/database/outputs.tf(1 hunks)infrastructure/modules/database/variables.tf(1 hunks)infrastructure/modules/ecs/main.tf(1 hunks)infrastructure/modules/ecs/outputs.tf(1 hunks)infrastructure/modules/ecs/variables.tf(1 hunks)infrastructure/outputs.tf(1 hunks)infrastructure/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infrastructure/modules/cache/variables.tf
- infrastructure/modules/database/outputs.tf
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-17T15:25:53.713Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/modules/database/main.tf:22-60
Timestamp: 2025-10-17T15:25:53.713Z
Learning: The infrastructure code in the `infrastructure/` directory is intended for quick testing purposes only, not production-grade deployment. Production-grade security hardening controls (such as IAM database authentication, deletion protection, Performance Insights KMS encryption) are not required for this testing infrastructure.
Applied to files:
infrastructure/modules/cache/main.tfinfrastructure/modules/database/main.tf
📚 Learning: 2025-10-17T15:25:55.624Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/providers.tf:1-3
Timestamp: 2025-10-17T15:25:55.624Z
Learning: The infrastructure code in the OWASP/Nest repository (infrastructure/ directory) is intended for quick testing purposes only, not for production deployment.
Applied to files:
infrastructure/modules/cache/main.tfinfrastructure/modules/database/main.tf
📚 Learning: 2025-10-17T15:25:34.963Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/modules/cache/main.tf:30-30
Timestamp: 2025-10-17T15:25:34.963Z
Learning: The infrastructure/Terraform code in the OWASP Nest repository under the `infrastructure/` directory is intended for quick testing purposes only, not for production deployment.
Applied to files:
infrastructure/modules/cache/main.tf
🪛 Checkov (3.2.334)
infrastructure/modules/database/main.tf
[medium] 37-60: Ensure RDS database has IAM authentication enabled
(CKV_AWS_161)
[medium] 37-60: Ensure that AWS database instances have deletion protection enabled
(CKV_AWS_293)
[high] 37-60: Ensure RDS Performance Insights are encrypted using KMS CMKs
(CKV_AWS_354)
infrastructure/modules/ecs/main.tf
[high] 25-28: Ensure ECR image scanning on push is enabled
(CKV_AWS_163)
🔇 Additional comments (3)
.gitignore (2)
20-22: Security-aware ignores align with serverless infrastructure migration.The Terraform, Zappa deployment, and sensitive configuration ignores (lines 20–22, 30–33, 36, 47) follow security best practices and prevent local state files, deployment artifacts, and secrets from being inadvertently committed.
However, there is a discrepancy: the AI summary claims lines 36 and 47 were already present in the file, but they are marked as new additions (
~) in the provided code. Please confirm whether these lines are new additions or pre-existing, as this affects whether the changes are correct.Also applies to: 30-33, 36-36, 47-47
36-36: Verify sensitive files are kept local and documented.Lines 36 and 47 ignore files that typically contain deployment-specific secrets (AWS credentials, database passwords, etc.). Ensure:
- Your deployment process provisions these files locally (via CI/CD, manual setup, or config injection) on each environment.
- If these files are not committed, the repository includes clear documentation (e.g., in
infrastructure/README.mdorbackend/README.md) explaining which local files are required and how to generate them for each environment (dev, staging, production).Also applies to: 47-47
infrastructure/modules/database/main.tf (1)
40-41: All referenced variables are properly defined ininfrastructure/modules/database/variables.tf. No action required.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
infrastructure/modules/ecs/main.tf (2)
68-194: Standardize container image tagging strategy across all tasks.Scheduled tasks (lines 80, 101, 122) reference the bare repository URL without an explicit tag, while on-demand tasks (lines 142, 161, 188) explicitly use
:latest. This inconsistency can lead to unpredictable image versions. Choose one approach consistently:
- Option A (recommended): All tasks explicitly use
:latestfor clarity- Option B: All tasks use the bare repository URL, documented as a standard practice
Option A (recommended): Make all image tags explicit
module "sync_data_task" { source = "./modules/task" ... - image_url = aws_ecr_repository.main.repository_url + image_url = "${aws_ecr_repository.main.repository_url}:latest" ... } module "owasp_update_project_health_metrics_task" { source = "./modules/task" ... - image_url = aws_ecr_repository.main.repository_url + image_url = "${aws_ecr_repository.main.repository_url}:latest" ... } module "owasp_update_project_health_scores_task" { source = "./modules/task" ... - image_url = aws_ecr_repository.main.repository_url + image_url = "${aws_ecr_repository.main.repository_url}:latest" ... }
85-85: Clarify security group naming for ECS tasks.All tasks reference
var.lambda_sg_id, but this variable is being used for ECS task networking. Consider renaming to a more generic name (e.g.,service_sg_id,backend_sg_id, orapp_sg_id) to better reflect its use across both Lambda and ECS resources, or add a separate variable with clearer intent if the architectures require different security groups.This is a minor clarification that could prevent confusion for future maintainers, but acceptable as-is if the shared security group is intentional by design.
Also applies to: 106-106, 127-127, 146-146, 165-165, 192-192
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
infrastructure/main.tf(1 hunks)infrastructure/modules/ecs/main.tf(1 hunks)infrastructure/modules/ecs/modules/task/main.tf(1 hunks)infrastructure/modules/ecs/modules/task/variables.tf(1 hunks)infrastructure/modules/ecs/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/main.tf
🧰 Additional context used
🪛 Checkov (3.2.334)
infrastructure/modules/ecs/main.tf
[high] 17-20: Ensure ECR image scanning on push is enabled
(CKV_AWS_163)
🔇 Additional comments (2)
infrastructure/modules/ecs/modules/task/variables.tf (1)
1-83: Variable definitions are well-structured and properly typed.All required and optional variables are clearly defined with appropriate defaults and type constraints. The use of
nulldefaults for optional scheduling (event_bridge_role_arn,schedule_expression) and empty map defaults for environment variables follows Terraform best practices.infrastructure/modules/ecs/modules/task/main.tf (1)
1-80: Task module refactoring resolves past log group reference issues.The modular structure properly isolates each task with its own CloudWatch log group (line 13), eliminating the previous issue where multiple tasks incorrectly shared log groups. Each task definition correctly references only its own log group (line 39), and EventBridge integration is properly conditional on schedule presence. The FARGATE configuration with awsvpc networking and private IP assignment follows AWS best practices for serverless containers.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
infrastructure/modules/storage/main.tf (1)
9-12: Remove unusedrandomprovider declaration.The
randomprovider is imported but never used in this module. Remove it to reduce unnecessary dependencies.required_providers { aws = { source = "hashicorp/aws" version = "~> 6.0" } - random = { - source = "hashicorp/random" - version = "~> 3.0" - } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/modules/ecs/modules/task/main.tf(1 hunks)infrastructure/modules/ecs/modules/task/variables.tf(1 hunks)infrastructure/modules/storage/main.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infrastructure/modules/ecs/modules/task/variables.tf
- infrastructure/modules/ecs/modules/task/main.tf
🧰 Additional context used
🪛 Checkov (3.2.334)
infrastructure/modules/storage/main.tf
[medium] 27-33: Ensure that an S3 bucket has a lifecycle configuration
(CKV2_AWS_61)
🔇 Additional comments (1)
infrastructure/modules/storage/main.tf (1)
105-130: HTTPS-only access policy is well-implemented.The IAM policy correctly enforces TLS-only access by denying all S3 actions when
aws:SecureTransportis false. This is a strong security control that prevents unencrypted data transit.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
infrastructure/modules/security/variables.tf (2)
7-10: Add validation constraints for port number variables.The
db_portandredis_portvariables lack validation to ensure they fall within valid port ranges (1–65535). Adding avalidationblock will catch configuration errors early.Apply this diff to add port range validation:
variable "db_port" { description = "The port for the RDS database" type = number + validation { + condition = var.db_port > 0 && var.db_port <= 65535 + error_message = "Port must be between 1 and 65535." + } }Repeat for
redis_portas well.Also applies to: 28-31
18-21: Consider adding validation for naming parameters.The
environmentandproject_namevariables lack constraints on allowed values or naming conventions. Adding optional validation (e.g., restricting environment to known values likestaging,production, or applying naming rules toproject_name) can prevent misconfiguration and improve infrastructure consistency.For
environment, consider:variable "environment" { description = "The environment (e.g., staging, production)" type = string + validation { + condition = contains(["staging", "production"], var.environment) + error_message = "Environment must be 'staging' or 'production'." + } }For
project_name, consider constraining to alphanumeric and hyphens (common for AWS resource naming):variable "project_name" { description = "The name of the project" type = string + validation { + condition = can(regex("^[a-z0-9\\-]+$", var.project_name)) + error_message = "Project name must contain only lowercase letters, numbers, and hyphens." + } }Also applies to: 23-26
.pre-commit-config.yaml (1)
12-20: Consider excluding Terraform state and lock files from hook processing.The file pattern
^infrastructure/.*\.tf$will match all.tffiles, but.terraform.lock.hclshould typically be excluded from formatting hooks to preserve provider version locks and avoid unnecessary changes during hook runs. Additionally, the.terraform/directory (if it exists) shouldn't be processed.Consider refining the pattern or adding exclusions to avoid processing these files:
- id: terraform_fmt - files: ^infrastructure/.*\.tf$ + files: ^infrastructure/.*\.tf$ + exclude: ^infrastructure/.terraform/ - id: terraform_validate - files: ^infrastructure/.*\.tf$ + files: ^infrastructure/.*\.tf$ + exclude: ^infrastructure/.terraform/ - id: terraform_tflint - files: ^infrastructure/.*\.tf$ + files: ^infrastructure/.*\.tf$ + exclude: ^infrastructure/.terraform/Note:
.terraform.lock.hclexclusion may not be needed if the hooks respect.gitignore, but explicit exclusion is safer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/run-ci-cd.yaml(1 hunks).pre-commit-config.yaml(1 hunks)infrastructure/main.tf(1 hunks)infrastructure/modules/cache/.terraform.lock.hcl(1 hunks)infrastructure/modules/database/.terraform.lock.hcl(1 hunks)infrastructure/modules/ecs/.terraform.lock.hcl(1 hunks)infrastructure/modules/ecs/modules/task/.terraform.lock.hcl(1 hunks)infrastructure/modules/networking/.terraform.lock.hcl(1 hunks)infrastructure/modules/security/.terraform.lock.hcl(1 hunks)infrastructure/modules/security/main.tf(1 hunks)infrastructure/modules/security/outputs.tf(1 hunks)infrastructure/modules/security/variables.tf(1 hunks)infrastructure/modules/storage/.terraform.lock.hcl(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- infrastructure/modules/security/.terraform.lock.hcl
- infrastructure/modules/storage/.terraform.lock.hcl
- infrastructure/modules/cache/.terraform.lock.hcl
- infrastructure/modules/networking/.terraform.lock.hcl
🚧 Files skipped from review as they are similar to previous changes (3)
- infrastructure/main.tf
- infrastructure/modules/security/main.tf
- infrastructure/modules/security/outputs.tf
🔇 Additional comments (8)
infrastructure/modules/security/variables.tf (2)
1-36: Module variable integration looks good.The set of variables is well-scoped for a security module: they capture VPC context, port configurations, naming/environment, and default tagging conventions. Descriptions are clear, and the use of
common_tagsanddefaultvalues follows Terraform best practices. Once the concerns above (egress CIDR permissiveness and port validation) are addressed, this will be solid infrastructure code.
12-16: Verify whether the permissive egress default aligns with security requirements.The code confirms that
default_egress_cidr_blocksdefaults to["0.0.0.0/0"]and is applied identically across all four security groups (Lambda, RDS Proxy, RDS, and Redis), allowing unrestricted egress on all protocols. The module is called without overriding this default, so the permissive behavior is live.No documentation or business rationale is present in the codebase. You should determine whether this aligns with your application's actual egress needs and security posture. If unconstrained outbound access is intentional for external API calls or similar requirements, document this tradeoff. If not, either remove the default to make it explicit, constrain it to specific CIDRs (e.g., VPC CIDR plus necessary external services), or add inline comments explaining the design decision.
.pre-commit-config.yaml (1)
12-20: Consider updating pre-commit-terraform to a current version.The latest version of antonbabenko/pre-commit-terraform is v1.103.0 (released 2025-10-17), whereas your configuration pins v1.92.0. There are no security advisories for v1.92.0, so it's safe to use as-is, but you may want to review the changelog to determine if the 11 point releases since v1.92.0 include relevant fixes or feature improvements for your Terraform hooks. If you do update, test that the three configured hooks (terraform_fmt, terraform_validate, terraform_tflint) continue to work as expected with the newer version.
.github/workflows/run-ci-cd.yaml (1)
58-61: No issues found. TFLint v0.59.1 is the latest version and secure.The version v0.59.1 is the latest released version of TFLint, with no public security advisories or CVE entries reported. Vulnerability scanners show no reported issues for the package. The step configuration is correct and uses a current, secure version.
infrastructure/modules/ecs/modules/task/.terraform.lock.hcl (2)
1-24: Lockfile is properly committed and tracked.The
.terraform.lock.hclfile is already committed to version control, is not excluded by.gitignore, and git will continue tracking it. No action needed.
4-6: No issues identified—AWS provider version is current and compatible.Verification confirms v6.17.0 (released Oct 16, 2025) is actively maintained with no flagged CVEs, meets your Terraform requirement of ">= 1.0," and is consistent with the 7 other lockfiles across infrastructure modules. The version constraint "~> 6.0" is appropriate and properly committed to version control.
infrastructure/modules/database/.terraform.lock.hcl (1)
1-44: No security advisories found; provider versions are current.AWS provider 6.17.0 (released Oct 16, 2025) and random provider 3.7.2 have no public security advisories as of October 2025. Both versions are current and match the ECS module, ensuring consistent infrastructure reproducibility across modules.
infrastructure/modules/ecs/.terraform.lock.hcl (1)
1-24: No issues found—provider versions are secure and current.AWS Terraform provider v6.17.0 (released Oct 16, 2025) has no CVEs or provider-specific security advisories as of Oct 23, 2025. The Terraform random provider v3.7.2 (released Apr 22, 2025) is secure for the official HashiCorp build and has no critical vulnerabilities. The lockfile correctly specifies the official HashiCorp provider registry paths, so the related CVE-2025-4673 affecting the Chainguard FIPS variant does not apply.
The lockfile structure, constraints, and hashes are correct. No changes are required.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
infrastructure/README.md (3)
58-102: Clarify the two-step terraform apply process and its temporary nature.Lines 101–102 note that steps 4–5 (outputting and re-applying) will be removed when Secrets Manager is integrated. This suggests the process is incomplete/temporary. While the note is present, users may not understand why this two-step apply is necessary or when they should expect it to be simplified.
Consider adding a brief explanation immediately before step 4 (or in a callout box) to clarify that this manual step is temporary and will be automated:
3. **Apply Changes**: ... terraform apply ``` -4. **Copy Outputs**: +4. **Copy Outputs** *(temporary step)*: + + > **Note**: This step extracts and manually populates sensitive outputs into `terraform.tfvars`, then re-applies Terraform. This is a temporary workaround until AWS Secrets Manager integration is complete.Do you want me to generate a helper script that automates the output extraction and variable population to reduce manual error?
142-150: Validate that zappa_settings.json population guidance is sufficient.Line 144 instructs users to "Replace all
${...}variables inzappa_settings.jsonwith appropriate output variables," but it does not explicitly map which Terraform outputs correspond to which JSON keys. Users may struggle to identify the correct mappings.Consider adding an example or reference that shows the mapping. For instance:
4. **Populate Settings File**: -- Replace all `${...}` variables in `zappa_settings.json` with appropriate output variables. +- Replace all `${...}` variables in `zappa_settings.json` with appropriate output variables. + - `${database_endpoint}` → Terraform output: `database_endpoint` + - `${db_password}` → Terraform output: `db_password` + - `${redis_auth_token}` → Terraform output: `redis_auth_token` + - `${redis_endpoint}` → Terraform output: `redis_endpoint`Can you verify that
zappa_settings.example.jsonincludes all${...}placeholders and that this mapping covers them all?
196-209: Add guidance for ECS task troubleshooting and validation.Lines 196–209 provide step-by-step instructions for running ECS tasks (migrate, load-data, index-data) via the AWS Console. However, there is no guidance on how to validate success or troubleshoot failures (e.g., what to look for in the logs, common failure modes, how long tasks typically take).
Consider adding a troubleshooting section after the ECS task steps:
- Follow the same steps for `owasp-nest-staging-load-data` and `owasp-nest-staging-index-data`. + + **Validation**: + - Check task logs in the Console for errors or successful completion messages. + - Verify that all three tasks complete successfully before proceeding to API testing. + - If a task fails, review the logs and check that all Terraform outputs were correctly populated in environment variables.Would you like me to add a section with common error messages and their resolutions for the ECS task setup?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(2 hunks)infrastructure/README.md(1 hunks)infrastructure/terraform.tfvars.example(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infrastructure/terraform.tfvars.example
- .gitignore
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/main.tf:0-0
Timestamp: 2025-10-23T19:22:23.769Z
Learning: In Zappa-based serverless deployments, Lambda functions and IAM execution roles are managed by Zappa at application deployment time (via `zappa deploy`/`zappa update`), not via Terraform. Terraform provisions the supporting infrastructure (VPC, RDS, S3, security groups, RDS Proxy, Secrets Manager), while Zappa handles the Lambda orchestration layer.
📚 Learning: 2025-10-17T15:25:34.963Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/modules/cache/main.tf:30-30
Timestamp: 2025-10-17T15:25:34.963Z
Learning: The infrastructure/Terraform code in the OWASP Nest repository under the `infrastructure/` directory is intended for quick testing purposes only, not for production deployment.
Applied to files:
infrastructure/README.md
📚 Learning: 2025-10-17T15:25:55.624Z
Learnt from: rudransh-shrivastava
PR: OWASP/Nest#2431
File: infrastructure/providers.tf:1-3
Timestamp: 2025-10-17T15:25:55.624Z
Learning: The infrastructure code in the OWASP/Nest repository (infrastructure/ directory) is intended for quick testing purposes only, not for production deployment.
Applied to files:
infrastructure/README.md
| # Infrastructure | ||
|
|
||
| This document provides instructions on how to setup the infrastructure for this project. | ||
|
|
||
| ## Prerequisites | ||
|
|
||
| Ensure you have the following setup/installed: | ||
|
|
||
| - Setup Project: [CONTRIBUTING.md](https://github.com/OWASP/Nest/blob/main/CONTRIBUTING.md) | ||
| - Terraform: [Terraform Documentation](https://developer.hashicorp.com/terraform/docs) | ||
| - AWS CLI: [AWS CLI](https://docs.aws.amazon.com/cli/latest/userguide/getting-started-install.html) | ||
| - An AWS account with credentials configured locally. | ||
|
|
||
| ## Setting up the infrastructure |
There was a problem hiding this comment.
🧩 Analysis chain
Add upfront disclaimer that infrastructure is for testing/development only.
Based on the learnings, this infrastructure code is intended for quick testing purposes only, not for production deployment. However, the README does not prominently state this upfront. Users could mistakenly attempt to deploy this setup in production.
Add a clear warning banner at the top of the README (immediately after the first heading) to ensure users understand the scope and intended use:
# Infrastructure
+> ⚠️ **Warning**: This infrastructure code is intended for **development and testing purposes only**. It is not configured for production deployment. Before using in a production environment, review security groups, encryption settings, backup policies, disaster recovery, and compliance requirements.
+
This document provides instructions on how to setup the infrastructure for this project.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the full content of infrastructure/README.md
cat infrastructure/README.md
Length of output: 6474
🏁 Script executed:
#!/bin/bash
# Search for any testing, development, or production-related disclaimers
rg -n -i "testing|development|production|warning|disclaimer" infrastructure/README.mdLength of output: 94
Add upfront disclaimer that infrastructure is for testing/development only.
Verified: The infrastructure/README.md file currently has no disclaimer indicating this code is for testing purposes only. This poses a risk of users mistakenly deploying to production. The learnings confirm this infrastructure is "intended for quick testing purposes only, not production-grade deployment."
Add a clear warning banner immediately after the first heading to ensure users understand the intended scope:
# Infrastructure
+> ⚠️ **Warning**: This infrastructure code is intended for **development and testing purposes only**. It is not configured for production deployment. Before using in a production environment, review security groups, encryption settings, backup policies, disaster recovery, and compliance requirements.
+
This document provides instructions on how to setup the infrastructure for this project.🤖 Prompt for AI Agents
in infrastructure/README.md around lines 1 to 14, add a prominent disclaimer
banner immediately after the top-level heading indicating the infra is for
testing/development only and not production-ready; insert one short, clear
sentence such as "WARNING: This infrastructure is intended for quick testing and
development only — do NOT use for production deployments." styled as a visible
callout (e.g., uppercase/WARNING or a markdown blockquote) and optionally
include one brief note recommending users review production hardening and use
managed/secure tooling if deploying to production.
|
arkid15r
merged commit 2175602
into
OWASP:feature/nest-zappa-migration
* Install zappa and initial example configuration * add temporary IaC code for testing * Update code * Remove IAM and let Zappa manage it * add README * add RDS proxy, pre-commit hooks, clean-up code * feat: add ecs tasks * refactor/clean cache module * refactor/clean database module * refactor/clean ecs module * refactor/clean networking module * address Sonar Qube bot suggestions * keep some Sonar Qube bot suggestions but add #NOSONAR * add terraform dictionary * refactor/clean security module * fix pre-commit hooks and add terraform_validate * add SHA hash and remove terraform_validate * refactor/clean storage module * Update docs * add S3 support for load-data task * Update README and necessary examples * coderabbit suggestions * Update code: install awscli before invoking aws in task * update README * NOSONAR * update README * fix spell check * Update README * fix: typo * remove immutable tagged images * fix: load-data task fail due to no permissions * fix spell check: add PYTHONPATH * add AWS CLI * add set -e * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
* Install zappa and initial example configuration * add temporary IaC code for testing * Update code * Remove IAM and let Zappa manage it * add README * add RDS proxy, pre-commit hooks, clean-up code * feat: add ecs tasks * refactor/clean cache module * refactor/clean database module * refactor/clean ecs module * refactor/clean networking module * address Sonar Qube bot suggestions * keep some Sonar Qube bot suggestions but add #NOSONAR * add terraform dictionary * refactor/clean security module * fix pre-commit hooks and add terraform_validate * add SHA hash and remove terraform_validate * refactor/clean storage module * Update docs * add S3 support for load-data task * Update README and necessary examples * coderabbit suggestions * Update code: install awscli before invoking aws in task * update README * NOSONAR * update README * fix spell check * Update README * fix: typo * remove immutable tagged images * fix: load-data task fail due to no permissions * fix spell check: add PYTHONPATH * add AWS CLI * add set -e * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
* Install zappa and initial example configuration * add temporary IaC code for testing * Update code * Remove IAM and let Zappa manage it * add README * add RDS proxy, pre-commit hooks, clean-up code * feat: add ecs tasks * refactor/clean cache module * refactor/clean database module * refactor/clean ecs module * refactor/clean networking module * address Sonar Qube bot suggestions * keep some Sonar Qube bot suggestions but add #NOSONAR * add terraform dictionary * refactor/clean security module * fix pre-commit hooks and add terraform_validate * add SHA hash and remove terraform_validate * refactor/clean storage module * Update docs * add S3 support for load-data task * Update README and necessary examples * coderabbit suggestions * Update code: install awscli before invoking aws in task * update README * NOSONAR * update README * fix spell check * Update README * fix: typo * remove immutable tagged images * fix: load-data task fail due to no permissions * fix spell check: add PYTHONPATH * add AWS CLI * add set -e * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
2 participants
Resolves #2214
Proposed change
README.mdwith setup documentation.Checklist
make check-testlocally; all checks and tests passed.