Add Remote State Management#2699
Conversation
github-actions
bot
added
docs
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a Terraform remote state backend (S3 + DynamoDB) and related lockfiles, updates staging to use that backend and newer Terraform/AWS provider versions, restructures infrastructure setup docs, updates .gitignore and cspell, and adjusts Terraform variables and module paths for staging. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Tip ✨ Issue Enrichment is now available!CodeRabbit can now help you manage issues more effectively:
Enable issue planningTo enable issue planning, add the following to your issue_enrichment:
planning:
enabled: trueYou can then request a plan by commenting Disable automatic issue enrichmentTo disable automatic issue enrichment, add the following to your issue_enrichment:
auto_enrich:
enabled: falseThanks 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 |
rudransh-shrivastava
changed the base branch from
main
to
feature/nest-zappa-migration
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
backend/wsgi.py (1)
21-21: Preferos.path.basenamefor extracting parameter names.Using
pathlib.Pathto extract the basename from SSM parameter paths works but is semantically odd since SSM parameter names are not filesystem paths. Usingos.path.basename()or simple string operations (e.g.,param["Name"].rsplit("/", 1)[-1]) would be more idiomatic for this use case.- os.environ[Path(param["Name"]).name] = param["Value"] + os.environ[os.path.basename(param["Name"])] = param["Value"]infrastructure/modules/parameters/variables.tf (1)
45-48:environmentvariable should include validation or default for consistency.This variable is required (no default) and lacks validation. Consider adding a validation block matching the root module (line 78-81 in
infrastructure/variables.tf) to enforce["staging", "production"], ensuring consistency across the infrastructure stack. Alternatively, if the intent is to make it optional with a default, add one for easier module reuse.variable "environment" { description = "The environment (e.g., staging, production)." type = string + validation { + condition = contains(["staging", "production"], var.environment) + error_message = "Environment must be either 'staging' or 'production'." + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
.gitignore(1 hunks)backend/settings/staging.py(0 hunks)backend/wsgi.py(1 hunks)backend/zappa_settings.example.json(1 hunks)cspell/custom-dict.txt(1 hunks)infrastructure/README.md(3 hunks)infrastructure/backend/.terraform.lock.hcl(1 hunks)infrastructure/backend/main.tf(1 hunks)infrastructure/backend/outputs.tf(1 hunks)infrastructure/backend/variables.tf(1 hunks)infrastructure/main.tf(5 hunks)infrastructure/modules/cache/main.tf(2 hunks)infrastructure/modules/cache/variables.tf(0 hunks)infrastructure/modules/database/main.tf(4 hunks)infrastructure/modules/database/outputs.tf(1 hunks)infrastructure/modules/database/variables.tf(2 hunks)infrastructure/modules/ecs/main.tf(10 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)infrastructure/modules/parameters/main.tf(1 hunks)infrastructure/modules/parameters/outputs.tf(1 hunks)infrastructure/modules/parameters/variables.tf(1 hunks)infrastructure/modules/security/main.tf(3 hunks)infrastructure/modules/security/outputs.tf(1 hunks)infrastructure/modules/security/variables.tf(1 hunks)infrastructure/outputs.tf(0 hunks)infrastructure/staging.s3.tfbackend.example(1 hunks)infrastructure/terraform.tfvars.example(1 hunks)infrastructure/variables.tf(2 hunks)
💤 Files with no reviewable changes (3)
- backend/settings/staging.py
- infrastructure/modules/cache/variables.tf
- infrastructure/outputs.tf
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:1-191
Timestamp: 2025-11-08T11:16:25.725Z
Learning: The parameters module in infrastructure/modules/parameters/ is currently configured for staging environment only. The `configuration` and `settings_module` variables default to "Staging" and "settings.staging" respectively, and users can update parameter values via the AWS Parameter Store console. The lifecycle.ignore_changes blocks on these parameters support manual console updates without Terraform reverting them.
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:16-26
Timestamp: 2025-11-08T11:43:19.276Z
Learning: KMS CMK encryption for SSM SecureString parameters in infrastructure/modules/parameters/ is planned to be implemented after S3 state management is completed. Currently using AWS-managed keys for the testing infrastructure.
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2431
File: infrastructure/main.tf:0-0
Timestamp: 2025-10-23T19:22:23.811Z
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-11-08T11:16:25.725Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:1-191
Timestamp: 2025-11-08T11:16:25.725Z
Learning: The parameters module in infrastructure/modules/parameters/ is currently configured for staging environment only. The `configuration` and `settings_module` variables default to "Staging" and "settings.staging" respectively, and users can update parameter values via the AWS Parameter Store console. The lifecycle.ignore_changes blocks on these parameters support manual console updates without Terraform reverting them.
Applied to files:
infrastructure/backend/variables.tfbackend/zappa_settings.example.jsoninfrastructure/modules/parameters/outputs.tfinfrastructure/terraform.tfvars.exampleinfrastructure/modules/parameters/main.tfinfrastructure/modules/ecs/modules/task/variables.tfinfrastructure/modules/parameters/variables.tfinfrastructure/main.tf
📚 Learning: 2025-10-17T15:25:34.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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/backend/variables.tfinfrastructure/staging.s3.tfbackend.example.gitignoreinfrastructure/README.md
📚 Learning: 2025-10-23T19:22:23.811Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2431
File: infrastructure/main.tf:0-0
Timestamp: 2025-10-23T19:22:23.811Z
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.
Applied to files:
backend/zappa_settings.example.jsoninfrastructure/terraform.tfvars.exampleinfrastructure/README.md
📚 Learning: 2025-10-17T15:25:53.713Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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/terraform.tfvars.example
📚 Learning: 2025-11-08T11:43:19.276Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:16-26
Timestamp: 2025-11-08T11:43:19.276Z
Learning: KMS CMK encryption for SSM SecureString parameters in infrastructure/modules/parameters/ is planned to be implemented after S3 state management is completed. Currently using AWS-managed keys for the testing infrastructure.
Applied to files:
infrastructure/modules/parameters/main.tf
📚 Learning: 2025-10-17T15:25:55.624Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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:
.gitignoreinfrastructure/README.md
🪛 Checkov (3.2.334)
infrastructure/modules/parameters/main.tf
[high] 16-26: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 28-38: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 80-86: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 104-114: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 124-130: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 132-138: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 152-162: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 164-174: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
[high] 176-186: Ensure SSM parameters are using KMS CMK
(CKV_AWS_337)
infrastructure/backend/main.tf
[high] 15-27: Ensure DynamoDB point in time recovery (backup) is enabled
(CKV_AWS_28)
[medium] 29-35: Ensure that an S3 bucket has a lifecycle configuration
(CKV2_AWS_61)
🪛 markdownlint-cli2 (0.18.1)
infrastructure/README.md
75-75: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
76-76: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
⏰ 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). (3)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
🔇 Additional comments (33)
cspell/custom-dict.txt (1)
124-124: Spell check dictionary entry placement looks good.The
tfbackendentry is correctly positioned in alphabetical order and supports the Terraform backend terminology introduced in this PR..gitignore (1)
27-28: Gitignore patterns follow Terraform conventions.The addition of backend state file and Terraform variable file patterns—with exceptions for
.examplevariants—follows standard practice. This supports the PR's shift to remote state management and configuration-as-code.Also applies to: 31-32
infrastructure/modules/ecs/modules/task/variables.tf (1)
17-21: Variable rename correctly reflects SSM parameter ARN usage.The shift from
container_environmenttocontainer_parameters_arnsaligns with the PR's migration from inline environment values to SSM-based secret management. The description accurately documents the new contract.Verify that all consumers of this variable (e.g., task definition templates) have been updated to reference SSM parameter ARNs via
valueFrominstead ofvalue.infrastructure/modules/ecs/variables.tf (1)
12-16: ECS module variable rename is consistent with task module changes.The
container_parameters_arnsvariable correctly propagates SSM parameter ARNs through the ECS module hierarchy, enabling the shift from inline environment values to external secret management.infrastructure/backend/main.tf (1)
45-52: S3 bucket uses AWS-managed encryption instead of customer-managed KMS.Per prior learnings, KMS CMK encryption for state is planned after this PR. Confirm the encryption roadmap is tracked and document the timeline.
Verify that KMS encryption for the S3 backend is tracked in a follow-up issue or ADR, and that a timeline exists for this migration.
infrastructure/backend/outputs.tf (1)
1-9: Backend outputs are correctly structured.The DynamoDB table and S3 bucket names are appropriately exported for root-level Terraform configuration to reference during backend initialization.
infrastructure/modules/database/main.tf (2)
79-98: RDS proxy conditional creation is correctly implemented with count.All count-based resources (
aws_iam_role,aws_iam_role_policy,aws_db_proxy,aws_db_proxy_default_target_group,aws_db_proxy_target) consistently usecount = var.create_rds_proxy ? 1 : 0, and all downstream references are properly indexed with[0]. The non-proxied RDS instance and secrets manager remain unconditional, which is correct.Also applies to: 100-115, 117-139, 141-151, 153-158
58-58: Variable renamedb_username→db_useris complete and correct.The verification confirms no remaining references to
db_usernameexist in the Terraform files. Lines 58 and 74 correctly use the renamedvar.db_uservariable, and the change is fully migrated across the codebase.infrastructure/modules/database/variables.tf (2)
7-11: Newcreate_rds_proxyvariable enables optional proxy provisioning.The boolean flag with a conservative default (
false) allows teams to opt into RDS proxy without forcing mandatory upgrades. This is well-suited for testing/staging deployments.
87-90: Variable rename fromdb_usernametodb_useris concise and clear.The shortened name improves readability without sacrificing semantics. This aligns with the parallel rename in
infrastructure/modules/database/main.tf.infrastructure/modules/security/variables.tf (1)
7-11: LGTM!The
create_rds_proxyvariable follows Terraform conventions and provides a clean way to conditionally enable RDS Proxy resources. The default value offalseis appropriate for an optional feature.infrastructure/staging.s3.tfbackend.example (1)
1-5: LGTM! Good security practices.The S3 backend configuration correctly enables encryption (
encrypt = true) and uses DynamoDB for state locking, which prevents concurrent modifications and ensures state integrity.infrastructure/modules/security/outputs.tf (1)
6-9: LGTM!The conditional output correctly handles the optional RDS proxy security group, returning the security group ID when
create_rds_proxyis true andnullotherwise.infrastructure/backend/.terraform.lock.hcl (1)
1-25: LGTM!This is an auto-generated Terraform lockfile that ensures consistent AWS provider versions across environments. The AWS provider version 6.22.0 satisfies the
~> 6.0constraint.infrastructure/terraform.tfvars.example (1)
1-10: LGTM!The simplified configuration appropriately focuses on infrastructure variables. The settings are suitable for a testing environment:
db_backup_retention_period = 0(no automated backups)force_destroy_bucket = true(allows easy cleanup)create_rds_proxy = false(optional feature disabled by default)Based on learnings
backend/zappa_settings.example.json (1)
4-6: Good security improvement!Migrating from inline environment variables to AWS Systems Manager Parameter Store centralizes secret management and follows security best practices.
infrastructure/backend/variables.tf (1)
1-17: LGTM!The backend infrastructure variables are properly defined with sensible defaults for the staging testing environment. These will be used to configure the S3 state bucket and DynamoDB locking table.
infrastructure/modules/ecs/modules/task/main.tf (1)
46-49: Based on the verification through script execution and web search, I can now generate the rewritten review comment:
Verify IAM permissions for SSM Parameter Store access.
The ECS task execution role has been correctly configured with ssm:GetParameters permission required if referencing a Systems Manager Parameter Store parameter in a task definition. The custom IAM policy and AWS managed policy attachment (AmazonECSTaskExecutionRolePolicy) are in place.
However, consider the following for comprehensive coverage:
- AWS best practices recommend including both ssm:GetParameter and ssm:GetParameters actions in the policy, though official ECS documentation specifies only ssm:GetParameters is required. You may add ssm:GetParameter for additional flexibility.
- kms:Decrypt is required only if your secret uses a customer managed key and not the default key. Per the infrastructure design, this is planned for future implementation.
infrastructure/modules/parameters/outputs.tf (1)
1-22: LGTM!The output structure correctly maps environment variable names to their corresponding SSM parameter ARNs, enabling downstream modules (particularly ECS) to reference these parameters as secrets via
valueFrom.infrastructure/modules/parameters/main.tf (1)
1-191: LGTM with planned KMS CMK encryption.The parameters module structure correctly defines SSM parameters for Django configuration with appropriate types (SecureString for secrets, String for non-sensitive values) and lifecycle rules to support manual console updates. The static analysis warnings about missing KMS CMK encryption are noted and align with your planned implementation timeline.
Based on learnings: KMS CMK encryption is planned after S3 state management is completed.
infrastructure/modules/cache/main.tf (1)
30-36: LGTM!The simplification to always generate a Redis auth token with
count = 1improves code clarity and ensures Redis is always secured with authentication. The explicitspecial = truecombined withoverride_specialcorrectly configures the password requirements for Redis.infrastructure/modules/ecs/main.tf (1)
119-257: LGTM on parameter ARN refactoring.The migration from inline
container_environmenttocontainer_parameters_arnscorrectly shifts sensitive configuration to AWS Systems Manager Parameter Store. This improves security by preventing secrets from being visible in Terraform state and task definitions.infrastructure/README.md (1)
1-216: LGTM! Clear documentation for multi-stage deployment.The updated documentation clearly explains the new multi-stage deployment workflow with backend setup, main infrastructure provisioning, and secrets population via Parameter Store. The addition of the
zappa tailcommand and emphasis on populating secrets before deployment improves the developer experience.infrastructure/main.tf (2)
4-4: LGTM! Empty backend block for external configuration.The empty
backend "s3" {}block is the correct pattern for using backend configuration files (e.g.,staging.s3.tfbackend) as documented in the README. This allows different environments to use different backend configurations while keeping the main code clean.
26-122: LGTM! Clean module integration with parameters.The refactoring successfully:
- Introduces the parameters module to centralize SSM parameter management
- Wires parameter ARNs to ECS for secrets management
- Adds RDS proxy conditional provisioning via
create_rds_proxy- Simplifies cache module by removing explicit auth token input
The module dependencies and data flow are correctly structured.
infrastructure/modules/database/outputs.tf (1)
9-9: No action required—the code is correct.The aws_db_instance resource is unconditionally created (lines 37–60 contain no
countparameter). The conditional logic in outputs.tf correctly uses indexed access[0]only for the conditionally-createdaws_db_proxy.main, and direct access for the always-availableaws_db_instance.main.address. The concern in the original review comment is unfounded.Likely an incorrect or invalid review comment.
infrastructure/variables.tf (3)
13-17: ✓ Newcreate_rds_proxyvariable is well-defined.The variable correctly gates conditional RDS proxy infrastructure. No issues flagged.
74-82: ✓ Environment validation is well-implemented.The validation block correctly constrains environment to staging or production. This aligns with the PR objectives for environment-specific configuration and improves type safety.
68-72: No action required—the rename is complete and consistent across all modules.The
db_username→db_uservariable rename has been successfully propagated. Verification found zero remaining references todb_usernameacross the entire codebase. All module references—including the database module (infrastructure/modules/database/main.tf), parameters module (infrastructure/modules/parameters/main.tf), root module calls (infrastructure/main.tf), and outputs—correctly use the newdb_uservariable name.infrastructure/modules/security/main.tf (3)
50-66: ✓ Conditional RDS proxy security group is correctly structured.The count-based approach correctly gates RDS proxy SG creation on
var.create_rds_proxy. Naming and tags are consistent with the Lambda SG pattern.
93-124: ✓ Conditional ingress rules correctly handle both proxy and direct connection paths.The three rules implement the correct security posture:
- When
create_rds_proxy = false:rds_from_lambda(count=1) allows direct Lambda→RDS access.- When
create_rds_proxy = true:rds_from_proxy+rds_proxy_from_lambda(count=1 each) route traffic through the proxy.The conditional logic is sound and aligns with the architecture.
33-48: ****The review comment mischaracterizes the changes. The code contains two separate security group resources, not a renamed resource:
aws_security_group.rds(lines 33-48): Always created main RDS security groupaws_security_group.rds_proxy(lines 50-66): Conditionally created proxy security groupThese coexist with complementary conditional logic. The outputs correctly export both
rds_sg_idandrds_proxy_sg_idfor their respective resources. No resource rename occurred, and there is no breaking change to Terraform state or downstream references.Likely an incorrect or invalid review comment.
infrastructure/modules/parameters/variables.tf (1)
1-70: The parameters module invocation is correct—all required variables are provided.The root
infrastructure/main.tfsuccessfully passes all 9 required variables to the parameters module:
- Database parameters (db_host, db_name, db_password, db_port, db_user) sourced from root variables and the database module
- Redis parameters (redis_host, redis_password) sourced from the cache module
- Environment and project_name sourced from root variables
- common_tags sourced from local values
Optional variables with defaults (allowed_hosts, configuration, settings_module) are correctly omitted from the invocation.
|
will update with fixes and lifecycle changes. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
infrastructure/backend/main.tf (2)
15-27: DynamoDB state-lock table is missing point-in-time recovery (PITR).This issue was flagged in previous reviews and remains unresolved. PITR is essential for protecting against accidental or malicious data loss on critical state infrastructure.
Enable PITR on the state-lock table:
resource "aws_dynamodb_table" "state_lock" { name = "${var.project_name}-${var.environment}-terraform-state-lock" billing_mode = "PAY_PER_REQUEST" + point_in_time_recovery_specification { + point_in_time_recovery_enabled = true + } hash_key = "LockID"
29-35: S3 state bucket is missing a lifecycle configuration to clean up old versions.This issue was flagged in previous reviews and remains unresolved. Without lifecycle policies, S3 versions accumulate indefinitely and increase storage costs. The NOSONAR comment suggests this resource may trigger additional quality gate checks—ensure they are addressed.
Add a lifecycle configuration to expire old object versions:
resource "aws_s3_bucket" "state" { # NOSONAR bucket = "${var.project_name}-${var.environment}-terraform-state" tags = { Name = "${var.project_name}-${var.environment}-terraform-state" ManagedBy = "Terraform" } } + +resource "aws_s3_bucket_lifecycle_configuration" "state" { + bucket = aws_s3_bucket.state.id + rule { + id = "delete-old-versions" + status = "Enabled" + noncurrent_version_expiration { + noncurrent_days = 30 + } + } +}
🧹 Nitpick comments (1)
infrastructure/backend/main.tf (1)
29-59: Consider adding S3 access logging and explicit security policies for defense-in-depth.While the current configuration provides strong encryption and public access controls, additional hardening can improve auditability and security posture:
- S3 access logging — Enables audit trail of all state bucket access, useful for compliance and troubleshooting.
- S3 bucket policy — Explicitly enforce TLS/HTTPS to block unencrypted state transmission.
These are optional enhancements for a testing environment, but recommended for production-grade infrastructure.
If you add these, apply them after the lifecycle configuration is in place.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/backend/main.tf(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2431
File: infrastructure/main.tf:0-0
Timestamp: 2025-10-23T19:22:23.811Z
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.
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:1-191
Timestamp: 2025-11-08T11:16:25.725Z
Learning: The parameters module in infrastructure/modules/parameters/ is currently configured for staging environment only. The `configuration` and `settings_module` variables default to "Staging" and "settings.staging" respectively, and users can update parameter values via the AWS Parameter Store console. The lifecycle.ignore_changes blocks on these parameters support manual console updates without Terraform reverting them.
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:16-26
Timestamp: 2025-11-08T11:43:19.276Z
Learning: KMS CMK encryption for SSM SecureString parameters in infrastructure/modules/parameters/ is planned to be implemented after S3 state management is completed. Currently using AWS-managed keys for the testing infrastructure.
🪛 Checkov (3.2.334)
infrastructure/backend/main.tf
[high] 15-27: Ensure DynamoDB point in time recovery (backup) is enabled
(CKV_AWS_28)
[medium] 29-35: Ensure that an S3 bucket has a lifecycle configuration
(CKV2_AWS_61)
⏰ 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). (3)
- GitHub Check: Run CI Denendencies Scan
- GitHub Check: Run Code Scan
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
infrastructure/backend/main.tf (5)
1-9: Terraform and provider versions are well configured.The required_version and AWS provider ~> 6.0 constraints are appropriate. Ensure the codebase is tested with AWS provider 6.x (the PR mentions
make check-testwas run locally).
11-13: AWS provider region configuration is correct.Using
var.aws_regionallows region to be parameterized per environment. Ensure the variable is defined ininfrastructure/backend/variables.tf.
37-43: S3 public access block is properly hardened.All four public access restrictions are correctly enabled. This is the appropriate security posture for a Terraform state bucket.
45-52: S3 encryption using AES256 is functional; KMS CMK planned as upgrade.The encryption configuration uses AWS-managed keys (AES256). Per prior learnings, KMS CMK encryption is planned for later phases of the infrastructure refactor. For now, this is acceptable for the testing environment.
54-59: S3 versioning is correctly enabled.Versioning is essential for Terraform state recovery and auditability. The configuration is correct.
arkid15r
force-pushed
the
feature/nest-zappa-migration
branch
from
1020b29 to
9ba7313
Compare
rudransh-shrivastava
changed the title
rudransh-shrivastava
force-pushed
the
feature/nest-zappa-migration-s3-state-management
branch
from
207d748 to
13ff3f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
infrastructure/backend/main.tf (2)
11-22: DynamoDB state table is missing point-in-time recovery (PITR).This is a duplicate of a prior review comment. Point-in-time recovery protects against accidental or malicious data loss in the state-lock table, a critical resource for Terraform coordination.
Consider enabling PITR on the DynamoDB state-lock table:
resource "aws_dynamodb_table" "state_lock" { name = "${var.project_name}-terraform-state-lock" billing_mode = "PAY_PER_REQUEST" + point_in_time_recovery_specification { + point_in_time_recovery_enabled = true + } hash_key = "LockID"
24-29: S3 state bucket lacks a lifecycle configuration.This is a duplicate of a prior review comment. Without a lifecycle policy, S3 object versions accumulate indefinitely, increasing storage costs. A lifecycle rule can automatically delete old versions after a retention period.
Add a lifecycle policy to clean up old object versions:
resource "aws_s3_bucket" "state" { bucket = "${var.project_name}-terraform-state" tags = { Name = "${var.project_name}-terraform-state" } } + +resource "aws_s3_bucket_lifecycle_configuration" "state" { + bucket = aws_s3_bucket.state.id + rule { + id = "delete-old-versions" + status = "Enabled" + noncurrent_version_expiration { + noncurrent_days = 30 + } + } +}infrastructure/README.md (1)
65-66: Fix list indentation for consistency.This is a duplicate of a prior review comment. The nested list items have 3 spaces of indentation instead of 0, which violates markdown linting rules (MD007).
Apply this diff:
- - Visit the AWS Console > Systems Manager > Parameter Store. - - Populate all `DJANGO_*` secrets that have `to-be-set-in-aws-console` value. +- Visit the AWS Console > Systems Manager > Parameter Store. +- Populate all `DJANGO_*` secrets that have `to-be-set-in-aws-console` value.
🧹 Nitpick comments (1)
infrastructure/staging/main.tf (1)
2-2: Consider using a flexible Terraform version constraint.Line 2 specifies
required_version = "1.14.0", which enforces an exact version. This is unusually restrictive and may limit adoption across teams with different Terraform installations.Consider relaxing the constraint to allow compatible versions:
- required_version = "1.14.0" + required_version = ">= 1.14.0"Or, if you want to lock within a minor version:
- required_version = "1.14.0" + required_version = "~> 1.14.0"Verify whether the exact version constraint is intentional before making this change.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
infrastructure/README.md(2 hunks)infrastructure/backend/.terraform.lock.hcl(1 hunks)infrastructure/backend/main.tf(1 hunks)infrastructure/backend/variables.tf(1 hunks)infrastructure/staging/.terraform.lock.hcl(1 hunks)infrastructure/staging/backend.tf(1 hunks)infrastructure/staging/main.tf(8 hunks)infrastructure/staging/providers.tf(1 hunks)infrastructure/staging/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/backend/.terraform.lock.hcl
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-08T11:16:25.725Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:1-191
Timestamp: 2025-11-08T11:16:25.725Z
Learning: The parameters module in infrastructure/modules/parameters/ is currently configured for staging environment only. The `configuration` and `settings_module` variables default to "Staging" and "settings.staging" respectively, and users can update parameter values via the AWS Parameter Store console. The lifecycle.ignore_changes blocks on these parameters support manual console updates without Terraform reverting them.
Applied to files:
infrastructure/staging/variables.tfinfrastructure/staging/.terraform.lock.hclinfrastructure/staging/backend.tfinfrastructure/staging/main.tf
📚 Learning: 2025-11-23T11:52:15.463Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2699
File: backend/wsgi.py:13-13
Timestamp: 2025-11-23T11:52:15.463Z
Learning: In the OWASP Nest project, the SSM parameter store setup in backend/wsgi.py (using boto3 to fetch parameters from AWS Systems Manager) is designed for staging and production environments, not just for testing purposes.
Applied to files:
infrastructure/staging/backend.tfinfrastructure/backend/variables.tf
📚 Learning: 2025-10-17T15:25:34.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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/backend/variables.tf
📚 Learning: 2025-10-23T19:22:23.811Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2431
File: infrastructure/main.tf:0-0
Timestamp: 2025-10-23T19:22:23.811Z
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.
Applied to files:
infrastructure/README.md
📚 Learning: 2025-10-17T15:25:53.713Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 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/README.md
🪛 Checkov (3.2.334)
infrastructure/backend/main.tf
[high] 11-22: Ensure DynamoDB point in time recovery (backup) is enabled
(CKV_AWS_28)
[medium] 24-29: Ensure that an S3 bucket has a lifecycle configuration
(CKV2_AWS_61)
🔇 Additional comments (8)
infrastructure/staging/.terraform.lock.hcl (1)
1-44: Terraform lock file updates are consistent with provider upgrades across the PR.The AWS provider upgrade to 6.22.0 and random provider constraints align with the backend and staging infrastructure changes. Lock files are auto-maintained by
terraform initand require no manual review.infrastructure/backend/main.tf (1)
1-9: Verify Terraform version constraint.Line 2 specifies
required_version = "1.14.0", which enforces an exact version. This is unusually strict for infrastructure code.Most projects use a constraint like
">= 1.14.0"or"~> 1.14.0"to allow minor updates and patches. Verify whether the exact version constraint is intentional or should be relaxed for broader compatibility.infrastructure/backend/variables.tf (1)
1-11: Backend variables are well-defined.The
aws_regionandproject_namevariables provide clear interfaces for backend customization. Defaults align with the project setup.infrastructure/staging/providers.tf (1)
1-3: AWS provider configuration is clean.The region references
var.aws_region, enabling flexible region management.infrastructure/staging/variables.tf (1)
1-10: Region and availability zone updates are consistent.The defaults for
aws_regionandavailability_zonesnow align with the us-east-2 backend setup. Changes are coherent across the infrastructure.infrastructure/staging/main.tf (1)
20-108: Module source paths are correctly updated.All module source paths have been updated from
"./modules/<name>"to"../modules/<name>", which is correct given the staging directory structure now sits alongside the backend directory.infrastructure/README.md (1)
18-61: Documentation clearly reflects the new backend-centric workflow.The reorganized setup steps (backend, staging, secrets) provide a logical flow for infrastructure provisioning. Instructions are clear and properly sequenced.
infrastructure/staging/backend.tf (1)
1-9: Review comment is incorrect and should be disregarded.The backend module (
infrastructure/backend/) and staging configuration are not coupled at the variable level as the review assumes. The staging configuration does not call the backend module—it references pre-existing backend infrastructure by hardcoded name.Architecture:
infrastructure/backend/main.tfcreates backend resources withproject_name="owasp-nest"(default), producing"owasp-nest-terraform-state"and"owasp-nest-terraform-state-lock"infrastructure/staging/backend.tfreferences these same pre-existing resources via hardcoded names- Staging's
project_name="nest"(default) is used only for staging-specific resources (VPC, RDS, cache, etc.), not the backendThe hardcoded backend names in
infrastructure/staging/backend.tfare intentional and correct. There is no risk of misconfiguration or mismatch.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
infrastructure/backend/main.tf (2)
54-83: ✅ DynamoDB and S3 bucket resources initialized correctly.PITR is enabled on the state-lock table (addressing prior feedback). Basic bucket configs are clean and lean; concerns like encryption, versioning, and policies are handled via separate resources.
Optional enhancement: Consider adding a TTL configuration to the DynamoDB state-lock table to automatically clean up abandoned locks:
resource "aws_dynamodb_table" "state_lock" { name = "${var.project_name}-terraform-state-lock" billing_mode = "PAY_PER_REQUEST" hash_key = "LockID" attribute { name = "LockID" type = "S" } point_in_time_recovery { enabled = true } + ttl { + attribute_name = "LockExpiry" + enabled = true + } tags = { Name = "${var.project_name}-terraform-state-lock" } }This prevents stale locks from indefinitely blocking operations if a process crashes.
147-163: Consider KMS encryption instead of AES256 for enhanced protection of sensitive state.Terraform state files can contain sensitive data (database passwords, API keys, secrets). While AES256 is acceptable for a test backend, AWS-managed KMS encryption provides additional security, key rotation, and audit logging benefits.
This is a best-practice recommendation for production-grade backends storing sensitive infrastructure state:
resource "aws_s3_bucket_server_side_encryption_configuration" "state" { bucket = aws_s3_bucket.state.id rule { apply_server_side_encryption_by_default { - sse_algorithm = "AES256" + sse_algorithm = "aws:kms" + kms_master_key_id = aws_kms_key.state.arn } } }If retained as AES256 for simplicity, document the decision and consider a future migration to KMS.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/backend/main.tf(1 hunks)infrastructure/backend/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/backend/variables.tf
🧰 Additional context used
🪛 Checkov (3.2.334)
infrastructure/backend/main.tf
[medium] 101-112: Ensure S3 lifecycle configuration sets period for aborting failed uploads
(CKV_AWS_300)
🔇 Additional comments (2)
infrastructure/backend/main.tf (2)
1-52: ✅ Terraform versioning and IAM policies look solid.Explicit provider version pinning ensures reproducibility. The HTTPS-only and logging policies are properly scoped with least-privilege principle.
131-145: ✅ Public access blocks and versioning provide strong security posture.All four public access block settings are enabled on both buckets, and versioning is active. This prevents accidental public exposure and enables recovery from accidental deletions or modifications. Architecture is hardened appropriately.
rudransh-shrivastava
force-pushed
the
feature/nest-zappa-migration-s3-state-management
branch
from
c6a1491 to
b65e89c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
infrastructure/README.md (1)
36-65: Add documentation for backend configuration in staging setup.Step 2 directs users to
infrastructure/staging/but does not explain how to configure that Terraform module to use the backend created in Step 1. Users need to know:
- Whether staging requires a
.tfbackendfile to reference the backend state bucket and DynamoDB table- How to create or populate that file
- Whether to use
terraform init -backend-config=or another methodCurrently, the instructions jump from "backend created" to "staging initialized," but the connection between them is missing. This could leave users with disconnected state files.
Consider adding a subsection or clarifying note (e.g., after line 41) that explains the backend-config setup, such as:
- Create or use a `.tfbackend` file to point to the S3 bucket and DynamoDB table created in Step 1: ```bash terraform init -backend-config=backend.tfbackend(or include backend configuration details in the .tfbackend template)
This ensures users understand the backend handoff between steps. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between c6a14910c948e70e2f73a1a119715ceed26503e0 and b65e89c2f472d430a10d403b0360a4fa0486e137. </details> <details> <summary>📒 Files selected for processing (12)</summary> * `.gitignore` (1 hunks) * `cspell/custom-dict.txt` (1 hunks) * `infrastructure/README.md` (2 hunks) * `infrastructure/backend/.terraform.lock.hcl` (1 hunks) * `infrastructure/backend/main.tf` (1 hunks) * `infrastructure/backend/outputs.tf` (1 hunks) * `infrastructure/backend/variables.tf` (1 hunks) * `infrastructure/staging/.terraform.lock.hcl` (1 hunks) * `infrastructure/staging/backend.tf` (1 hunks) * `infrastructure/staging/main.tf` (8 hunks) * `infrastructure/staging/providers.tf` (1 hunks) * `infrastructure/staging/variables.tf` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (9)</summary> * cspell/custom-dict.txt * infrastructure/staging/backend.tf * infrastructure/backend/outputs.tf * infrastructure/staging/.terraform.lock.hcl * infrastructure/staging/providers.tf * infrastructure/staging/variables.tf * infrastructure/backend/variables.tf * infrastructure/staging/main.tf * .gitignore </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (2)</summary> <details> <summary>📚 Learning: 2025-10-23T19:22:23.811Z</summary>Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2431
File: infrastructure/main.tf:0-0
Timestamp: 2025-10-23T19:22:23.811Z
Learning: In Zappa-based serverless deployments, Lambda functions and IAM execution roles are managed by Zappa at application deployment time (viazappa 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.**Applied to files:** - `infrastructure/README.md` </details> <details> <summary>📚 Learning: 2025-10-17T15:25:53.713Z</summary>Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2431
File: infrastructure/modules/database/main.tf:22-60
Timestamp: 2025-10-17T15:25:53.713Z
Learning: The infrastructure code in theinfrastructure/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/README.md` </details> </details><details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>infrastructure/README.md</summary> 54-54: Unordered list indentation Expected: 0; Actual: 2 (MD007, ul-indent) --- 55-55: Unordered list indentation Expected: 0; Actual: 2 (MD007, ul-indent) --- 57-57: Inconsistent indentation for list items at the same level Expected: 2; Actual: 0 (MD005, list-indent) --- 62-62: Inconsistent indentation for list items at the same level Expected: 2; Actual: 0 (MD005, list-indent) </details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: Run frontend unit tests * GitHub Check: Run frontend e2e tests * GitHub Check: Run backend tests * GitHub Check: CodeQL (javascript-typescript) </details> <details> <summary>🔇 Additional comments (5)</summary><blockquote> <details> <summary>infrastructure/backend/.terraform.lock.hcl (1)</summary><blockquote> `1-25`: **Lockfile format and content look good.** The provider lockfile correctly pins AWS provider 6.22.0 and includes the required hashes for reproducible infrastructure deployments. Standard auto-generated header comments are in place. </blockquote></details> <details> <summary>infrastructure/backend/main.tf (3)</summary><blockquote> `54-69`: **DynamoDB state-lock table configuration looks solid.** PITR is enabled (lines 63–65) as a best practice, and the resource is properly configured with PAY_PER_REQUEST billing for a state-lock table. --- `85-115`: **S3 lifecycle configurations align with best practices.** Both buckets include `abort_incomplete_multipart_upload` to prevent storage leakage, and the state bucket correctly manages old versions via `noncurrent_version_expiration`. The logs bucket expires entries after 90 days, which is reasonable for operational logs. --- `54-83`: **Verify resource naming for multi-environment isolation.** The DynamoDB table (line 55) and S3 buckets (lines 72, 79) do not include an environment variable in their names. If this backend infrastructure is meant to be: - **Shared centrally across all environments**: this naming is correct. - **Deployed per-environment for isolation**: resource names should include `var.environment` to prevent conflicts. Please confirm the intended deployment model and update names accordingly if multiple independent backend instances will coexist. </blockquote></details> <details> <summary>infrastructure/README.md (1)</summary><blockquote> `53-62`: **Fix markdown list indentation inconsistencies.** Lines 54–55 and 57–62 have inconsistent indentation that violates markdown linting rules (MD005, MD007). The *Note:* on line 53 creates a non-list parent, but lines 54–55 are indented 2 spaces as children, while lines 57–62 are top-level list items at 0 spaces. This structure should be flattened for consistency. **Suggested fix:** ```diff -*Note:* Optionally change the region: - - set `aws_region` in a `.tfvars` file. - - set `region` in a `.tfbackend` file and provide it using `terraform init -backend-config=<file>`. +*Note:* Optionally change the region: set `aws_region` in a `.tfvars` file or set `region` in a `.tfbackend` file. - Initialize Terraform with the backend configuration:Alternatively, if the two options should remain as separate list items, make the Note: line itself a list item for proper nesting:
-- **Optionally change the region:** +- Optionally change the region: - set `aws_region` in a `.tfvars` file. - set `region` in a `.tfbackend` file and provide it using `terraform init -backend-config=<file>`.Which approach do you prefer?
| @@ -1,14 +1,10 @@ | |||
| terraform { | |||
| required_version = ">= 1.0" | |||
| required_version = "1.14.0" | |||
There was a problem hiding this comment.
I haven't pinned versions in other modules yet, will handle in a different issue #2715
rudransh-shrivastava
changed the title
rudransh-shrivastava
requested review from
arkid15r and
kasya
as code owners
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
infrastructure/backend/main.tf (2)
49-64: Consider adding deletion protection to the DynamoDB state-lock table.While accidental deletion is unlikely with proper AWS IAM policies, the state-lock table holds critical infrastructure state. Standard Terraform backends typically apply additional safeguards (e.g., AWS account-level policies or table-level protections) to prevent accidental deletion.
Note: DynamoDB doesn't have a native
deletion_protectionattribute like RDS. Consider implementing deletion protection via AWS account policies, MFA delete requirements, or resource-based access controls at the AWS level.
66-78: Consider adding deletion protection and Object Lock to S3 buckets.The state bucket holds critical Terraform state. While versioning (lines 162–174) provides recovery, deletion protection or Object Lock (write-once-read-many) would prevent accidental or malicious deletion of all versions.
For the state bucket specifically, consider enabling Object Lock with governance mode to enforce a minimum retention period:
resource "aws_s3_bucket" "state" { # NOSONAR bucket = "${var.project_name}-terraform-state" + object_lock_enabled = true tags = { Name = "${var.project_name}-terraform-state" } } + +resource "aws_s3_bucket_object_lock_configuration" "state" { + bucket = aws_s3_bucket.state.id + rule { + default_retention { + mode = "GOVERNANCE" + days = 30 + } + } +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrastructure/backend/main.tf(1 hunks)infrastructure/backend/variables.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- infrastructure/backend/variables.tf
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2551
File: infrastructure/modules/parameters/main.tf:1-191
Timestamp: 2025-11-08T11:16:25.725Z
Learning: The parameters module in infrastructure/modules/parameters/ is currently configured for staging environment only. The `configuration` and `settings_module` variables default to "Staging" and "settings.staging" respectively, and users can update parameter values via the AWS Parameter Store console. The lifecycle.ignore_changes blocks on these parameters support manual console updates without Terraform reverting them.
⏰ 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). (5)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run backend tests
- GitHub Check: Run frontend unit tests
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (5)
infrastructure/backend/main.tf (5)
50-50: Verify environment-specific naming if this backend serves multiple Terraform environments.The resource names omit an environment variable (e.g.,
"${var.project_name}-${var.environment}-terraform-state"). If this backend infrastructure is deployed per-environment (dev, staging, prod), separate buckets and tables ensure state isolation and prevent accidental cross-environment operations.However, if this backend is a single shared infrastructure where each environment stores state in separate S3 keys (e.g.,
staging/terraform.tfstate,prod/terraform.tfstate) managed by individual Terraform configurations, the current naming is correct.Please confirm the deployment model:
- Per-environment deployment: Each environment should have its own backend infrastructure (add
var.environmentto resource names).- Shared backend with key-based isolation: Current naming is appropriate; environments use different S3 keys and locks via DynamoDB.
If per-environment deployment is intended, apply this pattern:
resource "aws_dynamodb_table" "state_lock" { - name = "${var.project_name}-terraform-state-lock" + name = "${var.project_name}-${var.environment}-terraform-state-lock"and similarly for S3 buckets.
Also applies to: 67-67, 74-74
1-9: Version pinning is well-configured.Terraform 1.14.0 and AWS provider 6.22.0 are explicitly pinned. Confirm these versions align with your deployment pipeline and CI/CD tools.
11-47: IAM policies are correctly scoped and secure.
- The logs policy correctly allows only the S3 logging service to write to the logs bucket.
- The state HTTPS-only policy correctly denies all S3 operations when
aws:SecureTransportis false, protecting the bucket from unencrypted access.
80-110: Lifecycle and versioning configurations are well-implemented.Both lifecycle rules properly handle:
- Noncurrent version expiration (30 days) to control costs.
- Incomplete multipart upload cleanup (7 days by default) to prevent orphaned uploads.
- Log expiration (90 days) to limit storage.
These are well-parameterized for flexibility across environments.
128-142: Public access blocks are correctly configured.All four flags (block_public_acls, block_public_policy, ignore_public_acls, restrict_public_buckets) are enabled on both buckets, ensuring no accidental public exposure.
arkid15r
merged commit 932c4a4
into
OWASP:feature/nest-zappa-migration
rudransh-shrivastava
deleted the
feature/nest-zappa-migration-s3-state-management
branch
* add remote state management * refactor staging into its own directory * change steps to account for staging * bot suggestions * sort alphabetically * Update code * Update documentation * update formatting * update code
* add remote state management * refactor staging into its own directory * change steps to account for staging * bot suggestions * sort alphabetically * Update code * Update documentation * update formatting * update code
2 participants
Resolves #2691
Proposed change
Checklist
make check-testlocally; all checks and tests passed.