Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions devops-aws
Submodule devops-aws added at 1b9ae8
36 changes: 19 additions & 17 deletions terraform/bootstrap/main.tf
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
###############################################################################
# bootstrap/main.tf
# Run ONCE before the main Terraform to create the S3 backend + DynamoDB lock
# Usage: cd terraform/bootstrap && terraform init && terraform apply
###############################################################################

terraform {
required_version = ">= 1.6.0"
required_providers {
aws = {
source = "hashicorp/aws"
version = "~> 5.0"
version = "~> 5.100.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Provider version constraint is inconsistent with root module.

The bootstrap module specifies ~> 5.100.0 while terraform/main.tf uses ~> 5.0. This could cause version drift or unexpected behavior if different provider versions are used between bootstrap and main infrastructure.

Consider aligning both constraints, either by using ~> 5.100.0 in both files or relaxing this to ~> 5.0 here as well.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/bootstrap/main.tf` at line 5, The AWS provider version constraint
in the bootstrap module specifies `~> 5.100.0` while the root module
terraform/main.tf uses `~> 5.0`, creating a version mismatch. Update the version
constraint in the bootstrap module to match the root module by changing the
version from `~> 5.100.0` to `~> 5.0` to ensure consistent provider versions are
used across both modules.

}
}
}
Expand All @@ -21,22 +14,25 @@ provider "aws" {
variable "aws_region" { default = "us-east-1" }
variable "project_name" { default = "serene-stay" }

# ── S3 bucket for Terraform state ────────────────────────────────────────────

# --- 1. S3 Bucket for Terraform state (Unique Name) ---
resource "aws_s3_bucket" "tfstate" {
bucket = "${var.project_name}-tfstate"
bucket = "serene-stay-tfstate-ramesh-98"

tags = {
Name = "${var.project_name}-tfstate"
Name = "serene-stay-tfstate-ramesh-98"
ManagedBy = "Terraform Bootstrap"
}
}

# --- 2. Bucket Versioning ---
resource "aws_s3_bucket_versioning" "tfstate" {
bucket = aws_s3_bucket.tfstate.id
versioning_configuration { status = "Enabled" }
versioning_configuration {
status = "Enabled"
}
}

# --- 3. Bucket Server Side Encryption ---
resource "aws_s3_bucket_server_side_encryption_configuration" "tfstate" {
bucket = aws_s3_bucket.tfstate.id
rule {
Expand All @@ -46,6 +42,7 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "tfstate" {
}
}

# --- 4. Public Access Block ---
resource "aws_s3_bucket_public_access_block" "tfstate" {
bucket = aws_s3_bucket.tfstate.id
block_public_acls = true
Expand All @@ -54,8 +51,7 @@ resource "aws_s3_bucket_public_access_block" "tfstate" {
restrict_public_buckets = true
}

# ── DynamoDB table for state locking ─────────────────────────────────────────

# --- 5. DynamoDB Table for State Locking ---
resource "aws_dynamodb_table" "tfstate_lock" {
name = "${var.project_name}-tfstate-lock"
billing_mode = "PAY_PER_REQUEST"
Expand All @@ -72,5 +68,11 @@ resource "aws_dynamodb_table" "tfstate_lock" {
}
}

output "tfstate_bucket" { value = aws_s3_bucket.tfstate.bucket }
output "tfstate_lock_table" { value = aws_dynamodb_table.tfstate_lock.name }
# --- Outputs ---
output "tfstate_bucket" {
value = aws_s3_bucket.tfstate.bucket
}

output "tfstate_lock_table" {
value = aws_dynamodb_table.tfstate_lock.name
}
5 changes: 2 additions & 3 deletions terraform/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ terraform {

# Remote state — S3 backend (run bootstrap first)
backend "s3" {
bucket = "serene-stay-tfstate"
key = "demo/terraform.tfstate"
bucket = "serene-stay-tfstate-ramesh-98" # <- මෙන්න මේ නම විතරක් වෙනස් කරන්න
key = "dev/terraform.tfstate"
region = "us-east-1"
encrypt = true
dynamodb_table = "serene-stay-tfstate-lock"
}
Comment on lines 22 to 27

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Removed encrypt attribute from S3 backend configuration.

The encrypt = true attribute was removed from the backend block. While the bucket has server-side encryption configured, explicitly setting encrypt = true in the backend provides defense-in-depth by ensuring Terraform encrypts state data during upload operations.

🔒 Recommended fix
   backend "s3" {
     bucket         = "serene-stay-tfstate-ramesh-98" # <- මෙන්න මේ නම විතරක් වෙනස් කරන්න
     key            = "dev/terraform.tfstate"
     region         = "us-east-1"
+    encrypt        = true
     dynamodb_table = "serene-stay-tfstate-lock"
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
backend "s3" {
bucket = "serene-stay-tfstate"
key = "demo/terraform.tfstate"
bucket = "serene-stay-tfstate-ramesh-98" # <- මෙන්න මේ නම විතරක් වෙනස් කරන්න
key = "dev/terraform.tfstate"
region = "us-east-1"
encrypt = true
dynamodb_table = "serene-stay-tfstate-lock"
}
backend "s3" {
bucket = "serene-stay-tfstate-ramesh-98" # <- මෙන්න මේ නම විතරක් වෙනස් කරන්න
key = "dev/terraform.tfstate"
region = "us-east-1"
encrypt = true
dynamodb_table = "serene-stay-tfstate-lock"
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/main.tf` around lines 22 - 27, The backend S3 configuration block
is missing the encrypt attribute which ensures Terraform encrypts state data
during upload operations. Add the encrypt = true attribute to the backend "s3"
block alongside the existing bucket, key, region, and dynamodb_table attributes
to restore this security layer.

}
Expand Down
2 changes: 1 addition & 1 deletion terraform/modules/database/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ resource "aws_db_instance" "main" {
multi_az = false

# Backups
backup_retention_period = 3
backup_retention_period = 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Backup retention drop to 1 day weakens recovery guarantees and breaks the documented reliability contract.

At Line 78, changing retention to 1 reduces PITR/restore window and now conflicts with README.md Line 466 (3-day retention). If this module is shared beyond ephemeral demo stacks, keep 3 or make retention environment-driven (e.g., var.backup_retention_days) so non-demo environments preserve the longer window.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/modules/database/main.tf` at line 78, The backup_retention_period
value at line 78 has been set to 1 day, which conflicts with the documented
3-day retention standard in README.md Line 466 and weakens recovery
capabilities. To fix this, either revert backup_retention_period back to 3, or
better yet, create a new variable called var.backup_retention_days (with a
default value of 3) in the variables file and replace the hardcoded 1 with a
reference to this variable. This allows non-demo environments to maintain longer
retention periods while still allowing ephemeral stacks to override it if
needed. Update the README.md documentation accordingly to reflect the new
variable-driven approach if implemented.

backup_window = "03:00-04:00"
maintenance_window = "Mon:04:00-Mon:05:00"
copy_tags_to_snapshot = true
Expand Down
13 changes: 6 additions & 7 deletions terraform/modules/storage/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@

locals {
name_prefix = "${var.project_name}-${var.environment}"
bucket_name = "${var.project_name}-uploads-${var.environment}"
}
Comment on lines 7 to 9

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Unused locals block and hardcoded bucket names reduce module reusability.

The name_prefix local is defined but no longer used since bucket names are now hardcoded. This:

  1. Leaves dead code (locals.name_prefix)
  2. Makes the module non-reusable across environments
  3. Embeds what appear to be developer-specific identifiers (demo, ramesh-98) in the module

Consider either removing the module's parameterization entirely (if single-use is intentional) or restoring variable-based naming for proper module reusability.

♻️ Option: Restore variable-based naming
 resource "aws_s3_bucket" "uploads" {
-  bucket = "serene-stay-uploads-demo-ramesh-98"
+  bucket = "${local.name_prefix}-uploads"

   tags = {
-    Name = "serene-stay-uploads-demo-ramesh-98"
+    Name = "${local.name_prefix}-uploads"
   }
 }

Similar change for access_logs bucket.

Also applies to: 13-18

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/modules/storage/main.tf` around lines 7 - 9, Remove the unused
`locals` block containing `name_prefix` and replace the hardcoded bucket names
(such as "demo" and "ramesh-98" and the access_logs bucket) with dynamic naming
that uses the `name_prefix` variable derived from var.project_name and
var.environment. This will restore proper module reusability across different
environments and projects by using the intended variable-based naming scheme
instead of hardcoded developer-specific identifiers.


# ── S3 Bucket ─────────────────────────────────────────────────────────────────
# ── S3 Bucket (Uploads) ───────────────────────────────────────────────────────

resource "aws_s3_bucket" "uploads" {
bucket = local.bucket_name
bucket = "serene-stay-uploads-demo-ramesh-98"

tags = {
Name = local.bucket_name
Name = "serene-stay-uploads-demo-ramesh-98"
}
}

Expand Down Expand Up @@ -98,10 +97,10 @@ resource "aws_s3_bucket_cors_configuration" "uploads" {
# ── Access Logging Bucket ─────────────────────────────────────────────────────

resource "aws_s3_bucket" "access_logs" {
bucket = "${local.bucket_name}-access-logs"
bucket = "serene-stay-uploads-demo-access-logs-ramesh-98"

tags = {
Name = "${local.bucket_name}-access-logs"
Name = "serene-stay-uploads-demo-access-logs-ramesh-98"
}
}

Expand Down Expand Up @@ -143,4 +142,4 @@ resource "aws_s3_bucket_logging" "uploads" {
bucket = aws_s3_bucket.uploads.id
target_bucket = aws_s3_bucket.access_logs.id
target_prefix = "s3-access-logs/"
}
}