feat: Refactor to use resources directly and add malware scanning. (#6)

Author: jamesiarmes

.github/workflows/tflint.yaml

@@ -2,50 +2,13 @@ name: TFLint Checks
 
 on:
   push:
+    branches: [main]
   pull_request:
-    branches:
-      - main
 
 permissions:
   contents: read
   security-events: write
 
 jobs:
-  lint:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout source code
-        uses: actions/checkout@v6
-      - name: Cache plugin directory
-        uses: actions/cache@v5
-        with:
-          path: ~/.tflint.d/plugins
-          key: tflint-${{ hashFiles('.tflint.hcl') }}
-      - uses: terraform-linters/setup-tflint@v6
-        name: Setup TFLint
-      - name: Show version
-        run: tflint --version
-      - name: Init TFLint
-        run: tflint --init
-      - name: Run TFLint
-        # Run TFLint, outputting the results to a SARIF file. We use `tee` so
-        # that we can still see the output in the logs, and capture the exit
-        # code properly with `pipefail`.
-        run: |
-          set -o pipefail
-          tflint --format sarif --recursive \
-            --config "$GITHUB_WORKSPACE/.tflint.hcl" \
-            | tee tflint-results.sarif
-          exit "${PIPESTATUS[0]}"
-      - name: Parse SARIF file for annotations
-        if: always()
-        uses: jontyms/sarif-annotations@v0.0.3
-        with:
-          annotation-level: notice
-          sarif-file: tflint-results.sarif
-      # When run on main, upload the SARIF file to GitHub.
-      - name: Upload SARIF result
-        if: always() && github.ref == 'refs/heads/main'
-        uses: github/codeql-action/upload-sarif@v4
-        with:
-          sarif_file: tflint-results.sarif
+  tflint:
+    uses: codeforamerica/github-actions/.github/workflows/tflint.yaml@main

.github/workflows/trivy.yaml

@@ -2,36 +2,13 @@ name: Trivy Analysis
 
 on:
   push:
+    branches: [main]
+  pull_request:
 
 permissions:
   contents: read
   security-events: write
 
 jobs:
   trivy:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout source code
-        uses: actions/checkout@v6
-      - name: Run Trivy vulnerability scanner
-        # v0.35.0
-        uses: aquasecurity/trivy-action@57a97c7e7821a5776cebc9bb87c984fa69cba8f1
-        with:
-          scan-type: config
-          ignore-unfixed: true
-          skip-dirs: "**/*/.terraform"
-          exit-code: 1
-          format: sarif
-          output: trivy-results.sarif
-      - name: Parse SARIF file for annotations
-        if: always()
-        uses: jontyms/sarif-annotations@v0.0.3
-        with:
-          annotation-level: notice
-          sarif-file: trivy-results.sarif
-      # When run on main, upload the SARIF file to GitHub.
-      - name: Upload SARIF result
-        if: always() && github.ref == 'refs/heads/main'
-        uses: github/codeql-action/upload-sarif@v4
-        with:
-          sarif_file: trivy-results.sarif
+    uses: codeforamerica/github-actions/.github/workflows/trivy.yaml@main

.tflint.hcl

@@ -1,20 +1,11 @@
-# Uncomment if your module uses the aws provider.
-# plugin "aws" {
-#   enabled = true
-#   version = "0.37.0"
-#   source  = "github.com/terraform-linters/tflint-ruleset-aws"
-# }
+plugin "aws" {
+  enabled = true
+  version = "0.48.0"
+  signature = "pgp"
+  source  = "github.com/terraform-linters/tflint-ruleset-aws"
+}
 
 plugin "terraform" {
   preset = "all"
   enabled = true
 }
-
-# TFLint doesn't understand the provider for_each syntax introduced with
-# OpenTofu 1.9, so we need to disable these rules so it doesn't error out.
-rule "terraform_required_providers" {
-  enabled = false
-}
-rule "terraform_unused_required_providers" {
-  enabled = false
-}

README.md

@@ -1,3 +1,5 @@
 data "aws_caller_identity" "identity" {}
 
 data "aws_partition" "current" {}
+
+data "aws_region" "current" {}

data.tf

@@ -0,0 +1,22 @@
+resource "aws_kms_key" "bucket" {
+  for_each = var.kms.create ? toset(["this"]) : toset([])
+
+  description             = "Encryption key for bucket ${local.bucket_name}"
+  deletion_window_in_days = var.kms.recovery_period
+  enable_key_rotation     = true
+  policy = jsonencode(yamldecode(templatefile("${path.module}/templates/key-policy.yaml.tftpl", {
+    account : data.aws_caller_identity.identity.account_id
+    bucket : local.bucket_name
+    partition : data.aws_partition.current.partition
+    principals : var.kms.allowed_principals
+  })))
+
+  tags = var.tags
+}
+
+resource "aws_kms_alias" "bucket" {
+  for_each = var.kms.create ? toset(["this"]) : toset([])
+
+  name          = "alias/${local.bucket_name}"
+  target_key_id = aws_kms_key.bucket["this"].arn
+}

kms.tf

@@ -1,6 +1,29 @@
 locals {
-  bucket_name = join("-", [var.project, var.environment, var.name])
-  kms_key_arn = var.encryption_key_arn != null ? var.encryption_key_arn : aws_kms_key.bucket["this"].arn
+  # Number of random characters in the suffix appended when add_suffix is
+  # enabled. The suffix also includes a leading hyphen.
+  suffix_length = 8
+
+  base_name = join("-", [var.project, var.environment, var.name])
+
+  # When a suffix is added, truncate the base name so the base plus the suffix
+  # stays within the 63-character bucket name limit, stripping any trailing
+  # hyphen left at the truncation boundary.
+  max_base_length = 63 - local.suffix_length - 1
+  truncated_base  = replace(substr(local.base_name, 0, local.max_base_length), "/-+$/", "")
+
+  bucket_name = var.add_suffix ? "${local.truncated_base}-${one(random_string.suffix[*].result)}" : local.base_name
+
+  # Base bucket policy rendered from the template, before merging in any
+  # additional statements provided by the caller.
+  base_bucket_policy = yamldecode(templatefile("${path.module}/templates/bucket-policy.yaml.tftpl", {
+    account          = data.aws_caller_identity.identity.account_id
+    bucket           = local.bucket_name
+    partition        = data.aws_partition.current.partition
+    restrict_malware = var.malware_scanning.restrict_access
+    scan_role_arn    = var.malware_scanning.restrict_access ? aws_iam_role.malware_scanning["this"].arn : ""
+  }))
+
+  kms_key_arn = var.kms.create ? aws_kms_key.bucket["this"].arn : var.kms.arn
   logs_path   = "/AWSLogs/${data.aws_caller_identity.identity.account_id}"
-  tags = merge({ use : "file-uploads" }, var.tags)
+  tags        = merge({ sensitivity = var.sensitivity }, var.tags)
 }

locals.tf

@@ -1,65 +1,143 @@
-resource "aws_kms_key" "bucket" {
-  for_each = var.encryption_key_arn != null ? toset([]) : toset(["this"])
-
-  description             = "Encryption key for bucket ${local.bucket_name}"
-  deletion_window_in_days = var.key_recovery_period
-  enable_key_rotation     = true
-  policy = jsonencode(yamldecode(templatefile("${path.module}/templates/key-policy.yaml.tftpl", {
-    account : data.aws_caller_identity.identity.account_id
-    bucket : local.bucket_name
-    partition : data.aws_partition.current.partition
-    principals : var.allowed_principals
-  })))
+resource "random_string" "suffix" {
+  count = var.add_suffix ? 1 : 0
+
+  length  = local.suffix_length
+  lower   = true
+  numeric = true
+  special = false
+  upper   = false
+}
+
+resource "aws_s3_bucket" "this" {
+  bucket        = local.bucket_name
+  force_destroy = var.force_delete
 
   tags = local.tags
+
+  lifecycle {
+    precondition {
+      condition     = length(local.bucket_name) >= 3 && length(local.bucket_name) <= 63
+      error_message = <<-EOT
+        The bucket name must be between 3 and 63 characters. Shorten project,
+        environment, or name, or set add_suffix to true to truncate the name
+        automatically.
+        EOT
+    }
+  }
 }
 
-resource "aws_kms_alias" "bucket" {
-  for_each = var.encryption_key_arn != null ? toset([]) : toset(["this"])
+resource "aws_s3_bucket_public_access_block" "this" {
+  bucket = aws_s3_bucket.this.id
 
-  name          = "alias/${local.bucket_name}"
-  target_key_id = aws_kms_key.bucket["this"].arn
+  block_public_acls       = true
+  block_public_policy     = true
+  ignore_public_acls      = true
+  restrict_public_buckets = true
 }
 
-module "this" {
-  source  = "boldlink/s3/aws"
-  version = "2.6.0"
+resource "aws_s3_bucket_ownership_controls" "this" {
+  bucket = aws_s3_bucket.this.id
 
-  bucket        = local.bucket_name
-  force_destroy = var.force_delete
+  rule {
+    object_ownership = "BucketOwnerEnforced"
+  }
+}
 
-  bucket_policy = jsonencode(yamldecode(templatefile("${path.module}/templates/bucket-policy.yaml.tftpl", {
-    partition : data.aws_partition.current.partition
-    bucket : local.bucket_name
-  })))
+resource "aws_s3_bucket_versioning" "this" {
+  bucket = aws_s3_bucket.this.id
 
-  lifecycle_configuration = [{
+  versioning_configuration {
+    status = "Enabled"
+  }
+}
+
+resource "aws_s3_bucket_object_lock_configuration" "this" {
+  for_each = var.object_lock.enabled ? toset(["this"]) : toset([])
+
+  # Object lock requires versioning to be enabled on the bucket first.
+  depends_on = [aws_s3_bucket_versioning.this]
+
+  bucket = aws_s3_bucket.this.id
+
+  dynamic "rule" {
+    for_each = var.object_lock.days != null ? toset(["this"]) : toset([])
+
+    content {
+      default_retention {
+        mode = var.object_lock.mode
+        days = var.object_lock.days
+      }
+    }
+  }
+}
+
+resource "aws_s3_bucket_server_side_encryption_configuration" "this" {
+  bucket = aws_s3_bucket.this.id
+
+  rule {
+    apply_server_side_encryption_by_default {
+      kms_master_key_id = local.kms_key_arn
+      sse_algorithm     = "aws:kms"
+    }
+
+    bucket_key_enabled = true
+  }
+}
+
+resource "aws_s3_bucket_logging" "this" {
+  bucket = aws_s3_bucket.this.id
+
+  target_bucket = var.logging_bucket
+  target_prefix = "${local.logs_path}/s3accesslogs/${local.bucket_name}"
+}
+
+resource "aws_s3_bucket_lifecycle_configuration" "this" {
+  # Referencing noncurrent-version behavior requires versioning to be enabled
+  # first.
+  depends_on = [aws_s3_bucket_versioning.this]
+
+  bucket = aws_s3_bucket.this.id
+
+  rule {
     id     = "state"
     status = "Enabled"
 
-    filter = {
+    filter {
       prefix = ""
     }
 
-    abort_incomplete_multipart_upload_days = var.abort_incomplete_multipart_upload_days
+    abort_incomplete_multipart_upload {
+      days_after_initiation = var.abort_incomplete_multipart_upload_days
+    }
 
-    noncurrent_version_expiration = [{
+    noncurrent_version_expiration {
       noncurrent_days = var.noncurrent_version_expiration_days
-    }]
-
-    transition = var.storage_class_transitions
-  }]
-
-  sse_bucket_key_enabled = true
-  sse_kms_master_key_arn = local.kms_key_arn
-  sse_sse_algorithm      = "aws:kms"
+    }
 
-  versioning_status = "Enabled"
+    dynamic "expiration" {
+      for_each = var.expiration != null ? toset(["this"]) : toset([])
+      content {
+        days = var.expiration
+      }
+    }
 
-  s3_logging = {
-    target_bucket = var.logging_bucket
-    target_prefix = "${local.logs_path}/s3accesslogs/${local.bucket_name}"
+    dynamic "transition" {
+      for_each = var.storage_class_transitions
+      content {
+        days          = transition.value.days
+        storage_class = transition.value.storage_class
+      }
+    }
   }
+}
 
-  tags = local.tags
+resource "aws_s3_bucket_policy" "this" {
+  # The public access block must be in place before a bucket policy can be
+  # applied.
+  depends_on = [aws_s3_bucket_public_access_block.this]
+
+  bucket = aws_s3_bucket.this.id
+  policy = jsonencode(merge(local.base_bucket_policy, {
+    Statement = concat(local.base_bucket_policy.Statement, var.additional_policy_statements)
+  }))
 }