Improve database Module

[rudransh-shrivastava]

Resolves #2778

Proposed change

• Set db_skip_final_snapshot to false by default.
• Add deletion protection.
• Set secret_recovery_window_in_days to 7 by default.

Checklist

• I've read and followed the contributing guidelines.
• I've run make check-test locally; all checks and tests passed.

[coderabbitai]

Summary by CodeRabbit

• Improvements
  • Enabled deletion protection for database instances to prevent accidental removal
  • Database snapshots now retained by default on deletion
  • Secrets configured with a 7-day recovery period before deletion
  • RDS proxy enabled by default for improved connection handling

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

Enables RDS deletion protection via a new variable, changes db_skip_final_snapshot and secret_recovery_window_in_days defaults, wires the new inputs into the staging database module, and flips create_rds_proxy to true in staging.

Changes

Cohort / File(s)	Summary
Database module infrastructure/modules/database/main.tf, infrastructure/modules/database/variables.tf	Added deletion_protection = var.db_deletion_protection to the RDS resource. Added variable db_deletion_protection (bool, default true). Changed db_skip_final_snapshot default true → false. Changed secret_recovery_window_in_days default 0 → 7.
Staging configuration infrastructure/staging/main.tf, infrastructure/staging/variables.tf, infrastructure/staging/terraform.tfvars.example	Passed db_deletion_protection and secret_recovery_window_in_days into the database module. Added variables db_deletion_protection (bool, default true) and secret_recovery_window_in_days (number, default 7). Changed create_rds_proxy default false → true. Updated example tfvars (enabled RDS proxy, removed db_backup_retention_period entry).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Verify the new deletion_protection wiring matches expectations and won't block intended teardown flows.
• Confirm db_skip_final_snapshot default flip and secret_recovery_window_in_days change are propagated consistently across environments and examples.
• Review CI/CD or destroy automation for assumptions about snapshot/skipping behavior and deletion protection.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Improve database Module' clearly summarizes the main change—enhancing the database module with new features and configuration defaults.
Description check	✅ Passed	The description directly addresses the pull request changes, listing the three specific improvements: db_skip_final_snapshot default change, deletion protection addition, and secret_recovery_window_in_days default change.
Linked Issues check	✅ Passed	All three coding objectives from issue #2778 are met: db_skip_final_snapshot default changed to false, deletion protection added via new variable, and secret_recovery_window_in_days default changed to 7.
Out of Scope Changes check	✅ Passed	All changes directly support the linked issue objectives. The modification to create_rds_proxy default in staging aligns with the database module improvements and does not introduce unrelated functionality.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between caba7c4 and 4562dad.

📒 Files selected for processing (4)

• infrastructure/modules/database/main.tf (1 hunks)
• infrastructure/modules/database/variables.tf (3 hunks)
• infrastructure/staging/main.tf (1 hunks)
• infrastructure/staging/variables.tf (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• infrastructure/modules/database/main.tf
• infrastructure/staging/main.tf

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.

🔇 Additional comments (6)

infrastructure/staging/variables.tf (3)

16-16: Approve default change for RDS proxy enablement.

The change correctly enables the RDS proxy in the staging environment.

---

31-35: Approve new deletion protection variable.

The new db_deletion_protection variable is properly defined with a safe default of true, protecting the database from accidental deletion.

---

138-142: Approve new secrets recovery window variable.

The new secret_recovery_window_in_days variable with a default of 7 days provides safer handling of secrets compared to the previous immediate deletion.

infrastructure/modules/database/variables.tf (3)

36-40: Approve new deletion protection variable.

The new db_deletion_protection variable is properly defined with a safe default of true. This provides a configurable option to prevent accidental database deletion at the module level.

---

76-80: Approve safer snapshot default.

Changing db_skip_final_snapshot default from true to false ensures a final snapshot is created before the database instance is deleted, providing data recovery protection.

---

113-117: Approve safer secrets recovery default.

Changing secret_recovery_window_in_days default from 0 to 7 provides a safer default that prevents accidental immediate deletion of database secrets, allowing a recovery window.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

infrastructure/modules/database/main.tf (1)

44-44: Consider making deletion_protection configurable via variable.

While hardcoding deletion_protection = true enhances safety, it may hinder testing workflows where rapid teardown/rebuild is needed. Combined with db_skip_final_snapshot = false, destroying the RDS instance requires manual intervention (disabling deletion protection via console/CLI).

Based on learnings, this infrastructure is used for quick testing purposes. Consider exposing this as a variable with a sensible default:

+variable "db_deletion_protection" {
+  description = "Enables deletion protection for the RDS instance."
+  type        = bool
+  default     = true
+}

-  deletion_protection             = true
+  deletion_protection             = var.db_deletion_protection

This allows testing environments to optionally disable protection while keeping the safe default.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 358a811 and 83e5184.

📒 Files selected for processing (5)

• infrastructure/modules/database/main.tf (1 hunks)
• infrastructure/modules/database/variables.tf (2 hunks)
• infrastructure/staging/main.tf (1 hunks)
• infrastructure/staging/terraform.tfvars.example (1 hunks)
• infrastructure/staging/variables.tf (2 hunks)

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.

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.

📚 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/terraform.tfvars.example

📚 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/terraform.tfvars.example

📚 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/modules/database/main.tf

⏰ 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). (1)

• GitHub Check: Run frontend e2e tests

🔇 Additional comments (5)

infrastructure/modules/database/variables.tf (2)

70-74: LGTM! Default changed to create final snapshots.

The change from true to false ensures that a final snapshot is created before database deletion, aligning with the PR objectives and providing better data protection.

---

107-111: LGTM! Recovery window added for safer secret deletion.

The default of 7 days provides a recovery window before permanent secret deletion, improving safety over immediate deletion (0 days). This aligns with the PR objectives.

infrastructure/staging/main.tf (1)

34-53: LGTM! Variable properly wired to module.

The secret_recovery_window_in_days variable is correctly passed from the staging configuration to the database module, completing the integration of the new parameter.

infrastructure/staging/variables.tf (2)

13-17: LGTM! RDS proxy enabled by default in staging.

The default change from false to true enables RDS proxy for staging deployments. While not explicitly mentioned in the PR objectives, this aligns with the staging infrastructure improvements and RDS proxy configuration seen throughout the PR.

---

132-136: LGTM! Variable properly defined with safe default.

The new variable correctly implements the PR objective to default the recovery window to 7 days, providing a safety net before permanent secret deletion.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud