Fix athena (#15)

cdaniluk · web-flow · commit 6866fbf7c916 · 2024-03-22T11:52:25.000-04:00
* allow create_athena_query to be false
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,70 +1,34 @@
+# excluding a few checks because it isn't coping well with the hacky way we do the rds monitoring lambda
 exclude: ".terraform"
 repos:
   - repo: https://github.com/antonbabenko/pre-commit-terraform
-    rev: v1.77.1
+    rev: v1.88.0
     hooks:
       - id: terraform_docs
         always_run: true
       - id: terraform_fmt
+      - id: terraform_validate
+        args:
+        - --hook-config=--retry-once-with-cleanup=true
       - id: terraform_tflint
         alias: terraform_tflint_nocreds
         name: terraform_tflint_nocreds
-      - id: terraform_tfsec
-  - repo: local
-    hooks:
-      - id: terraform_validate
-        name: terraform_validate
-        entry: |
-          bash -c '
-            AWS_DEFAULT_REGION=us-east-1
-            declare -a DIRS
-            for FILE in "$@"
-            do
-              DIRS+=($(dirname "$FILE"))
-            done
-            for DIR in $(printf "%s\n" "${DIRS[@]}" | sort -u)
-            do
-              cd $(dirname "$FILE")
-              terraform init --backend=false
-              terraform validate .
-              cd ..
-            done
-          '
-        language: system
-        verbose: true
-        files: \.tf(vars)?$
-        exclude: examples
-      - id: tflock
-        name: provider_locks
-        entry: |
-          bash -c '
-            AWS_DEFAULT_REGION=us-east-1
-            declare -a DIRS
-            for FILE in "$@"
-            do
-              DIRS+=($(dirname "$FILE"))
-            done
-            for DIR in $(printf "%s\n" "${DIRS[@]}" | sort -u)
-            do
-              cd $(dirname "$FILE")
-              terraform providers lock -platform=windows_amd64 -platform=darwin_amd64 -platform=linux_amd64
-              cd ..
-            done
-          '
-        language: system
-        verbose: true
-        files: \.tf(vars)?$
-        exclude: examples
+      - id: terraform_trivy
+        args:
+        - --args=--skip-dirs="**/.terraform,examples/*"
+      - id: terraform_providers_lock
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v4.5.0
     hooks:
+      - id: check-added-large-files
       - id: check-case-conflict
       - id: check-json
       - id: check-merge-conflict
       - id: check-symlinks
       - id: check-yaml
         args:
           - --unsafe
+      - id: detect-private-key
       - id: end-of-file-fixer
       - id: mixed-line-ending
         args:
diff --git a/.terraform-version b/.terraform-version
@@ -1 +1 @@
-latest:^1.1
+latest:^1.6
diff --git a/.terraform.lock.hcl b/.terraform.lock.hcl
diff --git a/.tflint.hcl b/.tflint.hcl
@@ -2,6 +2,12 @@ config {
   module     = true
 }
 
+plugin "aws" {
+    enabled = true
+    version = "0.12.0"
+    source  = "github.com/terraform-linters/tflint-ruleset-aws"
+}
+
 rule "terraform_deprecated_interpolation" {
   enabled = true
 }
diff --git a/LICENSE b/LICENSE
@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2021 Rhythmic Technologies, Inc.
+Copyright (c) 2024 Rhythmic Technologies, Inc.
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal
diff --git a/README.md b/README.md
@@ -53,13 +53,14 @@ No modules.
 | [aws_iam_policy.athena](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_policy) | resource |
 | [aws_s3_bucket.athena_results](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket) | resource |
 | [aws_s3_bucket.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket) | resource |
-| [aws_s3_bucket_acl.athena_results](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_acl) | resource |
-| [aws_s3_bucket_ownership_controls.athena_results](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_ownership_controls) | resource |
+| [aws_s3_bucket_logging.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_logging) | resource |
 | [aws_s3_bucket_policy.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_policy) | resource |
 | [aws_s3_bucket_public_access_block.athena_results](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource |
 | [aws_s3_bucket_public_access_block.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource |
 | [aws_s3_bucket_server_side_encryption_configuration.athena_results](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration) | resource |
+| [aws_s3_bucket_server_side_encryption_configuration.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_server_side_encryption_configuration) | resource |
 | [aws_s3_bucket_versioning.athena_results](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_versioning) | resource |
+| [aws_s3_bucket_versioning.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_versioning) | resource |
 | [null_resource.create_table](https://registry.terraform.io/providers/hashicorp/null/latest/docs/resources/resource) | resource |
 | [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_elb_service_account.principal](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/elb_service_account) | data source |
@@ -74,7 +75,7 @@ No modules.
 |------|-------------|------|---------|:--------:|
 | <a name="input_bucket_name"></a> [bucket\_name](#input\_bucket\_name) | Name to apply to bucket (use `bucket_name` or `bucket_suffix`) | `string` | `null` | no |
 | <a name="input_bucket_suffix"></a> [bucket\_suffix](#input\_bucket\_suffix) | Suffix to apply to the bucket (use `bucket_name` or `bucket_suffix`). When using `bucket_suffix`, the bucket name will be `[ACCOUNT_ID]-[REGION]-s3logging-[BUCKET_SUFFIX].` | `string` | `"elblogging"` | no |
-| <a name="input_create_athena_query"></a> [create\_athena\_query](#input\_create\_athena\_query) | Create an Athena table for querying ALB logs. Uses the aws cli | `bool` | `true` | no |
+| <a name="input_create_athena_query"></a> [create\_athena\_query](#input\_create\_athena\_query) | Create an Athena table for querying ALB logs. Uses the aws cli | `bool` | `false` | no |
 | <a name="input_kms_key_id"></a> [kms\_key\_id](#input\_kms\_key\_id) | KMS key to encrypt bucket with. | `string` | `null` | no |
 | <a name="input_lifecycle_rules"></a> [lifecycle\_rules](#input\_lifecycle\_rules) | lifecycle rules to apply to the bucket | <pre>list(object(<br>    {<br>      id                            = string<br>      enabled                       = bool<br>      prefix                        = string<br>      expiration                    = number<br>      noncurrent_version_expiration = number<br>  }))</pre> | `[]` | no |
 | <a name="input_s3_access_logging_bucket"></a> [s3\_access\_logging\_bucket](#input\_s3\_access\_logging\_bucket) | Optional target for S3 access logging | `string` | `null` | no |
diff --git a/athena_bucket.tf b/athena_bucket.tf
@@ -3,7 +3,7 @@
 # stores results from athena queries
 ##########################################
 
-#tfsec:ignore:aws-s3-enable-bucket-logging
+#trivy:ignore:avd-aws-0089
 resource "aws_s3_bucket" "athena_results" {
   count = var.create_athena_query ? 1 : 0
 
@@ -25,16 +25,6 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "athena_results" {
   }
 }
 
-resource "aws_s3_bucket_ownership_controls" "athena_results" {
-  count = var.create_athena_query ? 1 : 0
-
-  bucket = aws_s3_bucket.athena_results[0].id
-
-  rule {
-    object_ownership = "BucketOwnerEnforced"
-  }
-}
-
 resource "aws_s3_bucket_public_access_block" "athena_results" {
   count = var.create_athena_query ? 1 : 0
 
diff --git a/athena_iam.tf b/athena_iam.tf
@@ -3,6 +3,10 @@
 # minimal IAM policy to allow querying of elb logs with athena
 ##########################################
 
+locals {
+  athena_bucket_arn    = try(aws_s3_bucket.athena_results[0].arn, "arn:${local.partition}:s3:::bucket")
+  athena_workgroup_arn = try(aws_athena_workgroup.this[0].arn, "arn:${local.partition}:athena:${local.region}:${local.account_id}:workgroup/workgroup")
+}
 #tfsec:ignore:aws-iam-no-policy-wildcards
 data "aws_iam_policy_document" "athena" {
   statement {
@@ -24,7 +28,7 @@ data "aws_iam_policy_document" "athena" {
 
   statement {
     sid       = "AllowRunWorkgroup"
-    resources = [aws_athena_workgroup.this[0].arn]
+    resources = [local.athena_workgroup_arn]
     actions = [
       "athena:UpdatePreparedStatement",
       "athena:StopQueryExecution",
@@ -55,7 +59,7 @@ data "aws_iam_policy_document" "athena" {
       "s3:ListBucket"
     ]
     resources = [
-      aws_s3_bucket.athena_results[0].arn,
+      local.athena_bucket_arn,
       aws_s3_bucket.this.arn,
     ]
     condition {
@@ -82,7 +86,7 @@ data "aws_iam_policy_document" "athena" {
 
   statement {
     sid       = "AllowWriteResults"
-    resources = ["${aws_s3_bucket.athena_results[0].arn}/*"]
+    resources = ["${local.athena_bucket_arn}/*"]
     actions = [
       "s3:GetObject",
       "s3:PutObject"
@@ -98,7 +102,7 @@ data "aws_iam_policy_document" "athena" {
 resource "aws_iam_policy" "athena" {
   count = var.create_athena_query ? 1 : 0
 
-  name        = "athena_query_elb_logs"
+  name_prefix = "athena_query_elb_logs"
   path        = "/"
   description = "Allows the user to query ELB logs with Athena"
   policy      = data.aws_iam_policy_document.athena.json
diff --git a/bin/install-macos.sh b/bin/install-macos.sh
@@ -3,8 +3,8 @@
 echo 'installing brew packages'
 brew update
 brew tap liamg/tfsec
-brew install tfenv tflint terraform-docs pre-commit liamg/tfsec/tfsec coreutils
-brew upgrade tfenv tflint terraform-docs pre-commit liamg/tfsec/tfsec coreutils
+brew install tfenv tflint terraform-docs trivy pre-commit liamg/tfsec/tfsec coreutils
+brew upgrade tfenv tflint terraform-docs trivy pre-commit liamg/tfsec/tfsec coreutils
 
 echo 'installing pre-commit hooks'
 pre-commit install
diff --git a/bin/install-ubuntu.sh b/bin/install-ubuntu.sh
@@ -30,3 +30,6 @@ pre-commit init-templatedir ~/.git-template
 
 echo 'installing terraform with tfenv'
 tfenv install
+
+wget https://github.com/aquasecurity/trivy/releases/download/v0.49.1/trivy_0.49.1_Linux-64bit.deb
+sudo dpkg -i trivy_0.49.1_Linux-64bit.deb
diff --git a/main.tf b/main.tf
@@ -15,17 +15,11 @@ locals {
   bucket_name = var.bucket_name == null ? "${local.account_id}-${local.region}-${var.bucket_suffix}" : var.bucket_name
   partition   = data.aws_partition.current.partition
   region      = data.aws_region.current.name
-
-  logging = var.s3_access_logging_bucket == null ? [] : [{
-    bucket = var.s3_access_logging_bucket
-    prefix = var.s3_access_logging_prefix
-  }]
 }
 
-#tfsec:ignore:AWS002
+#trivy:ignore:avd-aws-0089
 resource "aws_s3_bucket" "this" {
   bucket = local.bucket_name
-  acl    = "log-delivery-write"
   tags   = var.tags
 
   dynamic "lifecycle_rule" {
@@ -46,34 +40,27 @@ resource "aws_s3_bucket" "this" {
       }
     }
   }
+}
 
-  dynamic "logging" {
-    iterator = log
-    for_each = local.logging
-
-    content {
-      target_bucket = log.value.bucket
-      target_prefix = lookup(log.value, "prefix", null)
-    }
+resource "aws_s3_bucket_versioning" "this" {
+  bucket = aws_s3_bucket.this.id
+  versioning_configuration {
+    status = var.versioning_enabled ? "Enabled" : "Suspended"
   }
 
-  server_side_encryption_configuration {
-    rule {
-      apply_server_side_encryption_by_default {
-        kms_master_key_id = var.kms_key_id
-        sse_algorithm     = var.kms_key_id != null ? "aws:kms" : "AES256"
-      }
-    }
+  lifecycle {
+    ignore_changes = [versioning_configuration[0].mfa_delete]
   }
+}
 
-  versioning {
-    enabled = var.versioning_enabled
-  }
+resource "aws_s3_bucket_server_side_encryption_configuration" "this" {
+  bucket = aws_s3_bucket.this.id
 
-  # this cannot be configured programatically via TF, so just ignore it if someone
-  # turned it on administratively.
-  lifecycle {
-    ignore_changes = [versioning[0].mfa_delete]
+  rule {
+    apply_server_side_encryption_by_default {
+      kms_master_key_id = var.kms_key_id
+      sse_algorithm     = var.kms_key_id != null ? "aws:kms" : "AES256"
+    }
   }
 }
 
@@ -133,7 +120,13 @@ resource "aws_s3_bucket_policy" "this" {
   bucket = aws_s3_bucket.this.id
   policy = data.aws_iam_policy_document.this.json
 
-  # this isn't really dependent on the public access block but there can be
-  # race conditions when creating bucket policies simultaneously
   depends_on = [aws_s3_bucket_public_access_block.this]
 }
+
+resource "aws_s3_bucket_logging" "this" {
+  count = var.s3_access_logging_bucket == null ? 0 : 1
+
+  bucket        = aws_s3_bucket.this.id
+  target_bucket = var.s3_access_logging_bucket
+  target_prefix = var.s3_access_logging_prefix
+}
diff --git a/variables.tf b/variables.tf
@@ -12,7 +12,7 @@ variable "bucket_suffix" {
 }
 
 variable "create_athena_query" {
-  default     = true
+  default     = false
   description = "Create an Athena table for querying ALB logs. Uses the aws cli"
   type        = bool
 }

Original file line number	Diff line number	Diff line change
`@@ -2,6 +2,12 @@ config {`
`2`	`2`	`module = true`
`3`	`3`	`}`
`4`	`4`
	`5`	`+plugin "aws" {`
	`6`	`+ enabled = true`
	`7`	`+ version = "0.12.0"`
	`8`	`+ source = "github.com/terraform-linters/tflint-ruleset-aws"`
	`9`	`+}`
	`10`	`+`
`5`	`11`	`rule "terraform_deprecated_interpolation" {`
`6`	`12`	`enabled = true`
`7`	`13`	`}`