Improve / Fix S3 Bucket Usage (#2760)

* add `random_id` to bucket name

* Update documentation

* change default force_destroy_bucket

* make bucket names configurable and prevent_destroy

* remove specifies

* remove `force_destroy` flag

* add prevent_destroy

* add note

* update docs

* update docs

Author: rudransh-shrivastava

infrastructure/README.md

@@ -21,7 +21,8 @@ Follow these steps to set up the infrastructure:
   ```bash
   cd infrastructure/backend/
   ```
-*Note:* Optionally change the region: set `aws_region` in a `.tfvars` file.
+
+**Note:** Optionally change the region: set `aws_region` in a `.tfvars` file.
 
 - Initialize Terraform if needed:
   ```bash
@@ -33,6 +34,10 @@ Follow these steps to set up the infrastructure:
   terraform apply
   ```
 
+**Note:** Copy the state bucket name from the output.
+
+**Note:** It is recommended to not destroy the backend resources unless absolutely necessary.
+
 2. **Setup Main Infrastructure (staging)**:
 
 - Navigate to the main infrastructure directory. If you are in `infrastructure/backend`, you can use:
@@ -50,13 +55,23 @@ Follow these steps to set up the infrastructure:
   cat terraform.tfvars.example > terraform.tfvars
   ```
 
-- *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>`.
+- Create a local backend configuration file:
+  ```bash
+  touch terraform.tfbackend
+  ```
+
+- Copy the contents from the example file:
+  ```bash
+  cat terraform.tfbackend.example > terraform.tfbackend
+  ```
+
+*Note:* Update the state bucket name in `terraform.tfbackend` with the name of the state bucket created in the previous step.
+
+*Note:* Update defaults (e.g. `region`) as needed.
 
 - Initialize Terraform with the backend configuration:
   ```bash
-  terraform init
+  terraform init -backend-config=terraform.tfbackend
   ```
 
 - Apply the changes to create the main infrastructure using the command:
@@ -114,13 +129,15 @@ The Django backend deployment is managed by Zappa. This includes the API Gateway
 
 5. **Deploy**:
 
-    - *Note*: Make sure to populate all `DJANGO_*` secrets that are set as `to-be-set-in-aws-console`
+    - **Note**: Make sure to populate all `DJANGO_*` secrets that are set as `to-be-set-in-aws-console`
       in the Parameter Store. The deployment might fail with no logs if secrets such as
       `DJANGO_SLACK_BOT_TOKEN` are invalid.
 
     ```bash
     zappa deploy staging
     ```
+    - **Note**: If the deployment is successful but returns a `5xx` error, resolve the issues
+      and use `zappa undeploy staging` & `zappa deploy staging`. The command `zappa update staging` may not work.
 
 Once deployed, use the URL provided by Zappa to test the API.
 
@@ -163,7 +180,7 @@ Migrate and load data into the new database.
    - Upload the fixture present in `backend/data` to `nest-fixtures` bucket using the following command:
 
      ```bash
-     aws s3 cp data/nest.json.gz s3://nest-fixtures/
+     aws s3 cp data/nest.json.gz s3://owasp-nest-fixtures-<id>/
      ```
 
 3. **Run ECS Tasks**:
@@ -188,6 +205,9 @@ Migrate and load data into the new database.
   zappa undeploy staging
   ```
 
+- Ensure all buckets and ECR repositories are empty.
+
+**Note:** Some resources have `prevent_destroy` set to `true`. Please set it to `false` before destruction.
 - To destroy Terraform infrastructure:
 
   ```bash

infrastructure/backend/.terraform.lock.hcl

@@ -5,6 +5,10 @@ terraform {
       source  = "hashicorp/aws"
       version = "6.22.0"
     }
+    random = {
+      source  = "hashicorp/random"
+      version = "3.7.2"
+    }
   }
 }
 
@@ -46,6 +50,10 @@ data "aws_iam_policy_document" "state_https_only" {
   }
 }
 
+resource "random_id" "suffix" {
+  byte_length = 4
+}
+
 resource "aws_dynamodb_table" "state_lock" {
   name         = "${var.project_name}-terraform-state-lock"
   billing_mode = "PAY_PER_REQUEST"
@@ -58,20 +66,32 @@ resource "aws_dynamodb_table" "state_lock" {
     name = "LockID"
     type = "S"
   }
+  lifecycle {
+    prevent_destroy = true
+  }
   point_in_time_recovery {
     enabled = true
   }
 }
 
 resource "aws_s3_bucket" "logs" { # NOSONAR
-  bucket = "${var.project_name}-terraform-state-logs"
+  bucket = "${var.project_name}-terraform-state-logs-${random_id.suffix.hex}"
+
+  lifecycle {
+    prevent_destroy = true
+  }
   tags = {
     Name = "${var.project_name}-terraform-state-logs"
   }
 }
 
 resource "aws_s3_bucket" "state" { # NOSONAR
-  bucket = "${var.project_name}-terraform-state"
+  bucket              = "${var.project_name}-terraform-state-${random_id.suffix.hex}"
+  object_lock_enabled = true
+
+  lifecycle {
+    prevent_destroy = true
+  }
   tags = {
     Name = "${var.project_name}-terraform-state"
   }
@@ -115,6 +135,17 @@ resource "aws_s3_bucket_logging" "state" {
   target_prefix = "s3/"
 }
 
+resource "aws_s3_bucket_object_lock_configuration" "state" {
+  bucket = aws_s3_bucket.state.id
+
+  rule {
+    default_retention {
+      days = 30
+      mode = "GOVERNANCE"
+    }
+  }
+}
+
 resource "aws_s3_bucket_policy" "logs" {
   bucket = aws_s3_bucket.logs.id
   policy = data.aws_iam_policy_document.logs.json

infrastructure/backend/main.tf

@@ -217,7 +217,7 @@ module "load_data_task" {
     set -e
     pip install --target=/tmp/awscli-packages awscli
     export PYTHONPATH="/tmp/awscli-packages:$PYTHONPATH"
-    python /tmp/awscli-packages/bin/aws s3 cp s3://${var.fixtures_s3_bucket}/nest.json.gz /tmp/nest.json.gz
+    python /tmp/awscli-packages/bin/aws s3 cp s3://${var.fixtures_bucket_name}/nest.json.gz /tmp/nest.json.gz
     python manage.py load_data --fixture-path /tmp/nest.json.gz
     EOT
   ]

infrastructure/modules/ecs/main.tf

@@ -30,7 +30,7 @@ variable "fixtures_read_only_policy_arn" {
   type        = string
 }
 
-variable "fixtures_s3_bucket" {
+variable "fixtures_bucket_name" {
   description = "The name of the S3 bucket for fixtures"
   type        = string
 }

infrastructure/modules/ecs/variables.tf

@@ -5,6 +5,10 @@ terraform {
       source  = "hashicorp/aws"
       version = "6.22.0"
     }
+    random = {
+      source  = "hashicorp/random"
+      version = "3.7.2"
+    }
   }
 }
 
@@ -15,16 +19,19 @@ data "aws_iam_policy_document" "fixtures_read_only" {
     ]
     effect = "Allow"
     resources = [
-      "arn:aws:s3:::${var.fixtures_s3_bucket}/*"
+      "arn:aws:s3:::${var.fixtures_bucket_name}-${random_id.suffix.hex}/*"
     ]
   }
 }
 
+resource "random_id" "suffix" {
+  byte_length = 4
+}
+
 module "fixtures_bucket" {
   source = "./modules/s3-bucket"
 
-  bucket_name   = var.fixtures_s3_bucket
-  force_destroy = var.force_destroy_bucket
+  bucket_name = "${var.fixtures_bucket_name}-${random_id.suffix.hex}"
   tags = merge(var.common_tags, {
     Name = "${var.project_name}-${var.environment}-fixtures"
   })
@@ -33,8 +40,7 @@ module "fixtures_bucket" {
 module "zappa_bucket" {
   source = "./modules/s3-bucket"
 
-  bucket_name   = var.zappa_s3_bucket
-  force_destroy = var.force_destroy_bucket
+  bucket_name = "${var.zappa_bucket_name}-${random_id.suffix.hex}"
   tags = merge(var.common_tags, {
     Name = "${var.project_name}-${var.environment}-zappa-deployments"
   })

infrastructure/modules/storage/main.tf

@@ -35,10 +35,13 @@ data "aws_iam_policy_document" "this" {
   }
 }
 
-resource "aws_s3_bucket" "this" { #NOSONAR
-  bucket        = var.bucket_name
-  force_destroy = var.force_destroy
-  tags          = var.tags
+resource "aws_s3_bucket" "this" { # NOSONAR
+  bucket = var.bucket_name
+  tags   = var.tags
+
+  lifecycle {
+    prevent_destroy = true
+  }
 }
 
 resource "aws_s3_bucket_policy" "this" {

infrastructure/modules/storage/modules/s3-bucket/main.tf

@@ -1,5 +1,5 @@
 variable "abort_incomplete_multipart_upload_days" {
-  description = "Specifies the number of days after which an incomplete multipart upload is aborted."
+  description = "The number of days after which an incomplete multipart upload is aborted."
   type        = number
   default     = 7
 }
@@ -9,14 +9,8 @@ variable "bucket_name" {
   type        = string
 }
 
-variable "force_destroy" {
-  description = "If true, deletes all objects from the bucket when the bucket is destroyed."
-  type        = bool
-  default     = false
-}
-
 variable "noncurrent_version_expiration_days" {
-  description = "Specifies the number of days an object is noncurrent before it is expired."
+  description = "The number of days an object is noncurrent before it is expired."
   type        = number
   default     = 30
 }

infrastructure/modules/storage/modules/s3-bucket/variables.tf

@@ -9,23 +9,17 @@ variable "environment" {
   type        = string
 }
 
-variable "fixtures_s3_bucket" {
+variable "fixtures_bucket_name" {
   description = "The name of the S3 bucket for fixtures"
   type        = string
 }
 
-variable "force_destroy_bucket" {
-  description = "If true, deletes all objects from the bucket when the bucket is destroyed."
-  type        = bool
-  default     = false
-}
-
 variable "project_name" {
   description = "The name of the project"
   type        = string
 }
 
-variable "zappa_s3_bucket" {
+variable "zappa_bucket_name" {
   description = "The name of the S3 bucket for Zappa deployments"
   type        = string
 }

infrastructure/modules/storage/variables.tf

@@ -1,9 +1,6 @@
 terraform {
   backend "s3" {
-    bucket         = "owasp-nest-terraform-state"
-    dynamodb_table = "owasp-nest-terraform-state-lock"
-    encrypt        = true
-    key            = "staging/terraform.tfstate"
-    region         = "us-east-2"
+    encrypt = true
+    key     = "staging/terraform.tfstate"
   }
 }