diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 7eaa782b..59cd0a89 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -7,30 +7,30 @@ on: - master jobs: -# Min Terraform version(s) + # Min Terraform version(s) getDirectories: - name: Get root directories - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Install Python - uses: actions/setup-python@v2 - - name: Build matrix - id: matrix - run: | - DIRS=$(python -c "import json; import glob; print(json.dumps([x.replace('/versions.tf', '') for x in glob.glob('./**/versions.tf', recursive=True)]))") - echo "::set-output name=directories::$DIRS" - outputs: - directories: ${{ steps.matrix.outputs.directories }} + name: Get root directories + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Install Python + uses: actions/setup-python@v2 + - name: Build matrix + id: matrix + run: | + DIRS=$(python -c "import json; import glob; print(json.dumps([x.replace('/versions.tf', '') for x in glob.glob('./**/versions.tf', recursive=True)]))") + echo "::set-output name=directories::$DIRS" + outputs: + directories: ${{ steps.matrix.outputs.directories }} preCommitMinVersions: name: Min TF validate needs: getDirectories runs-on: ubuntu-latest strategy: - matrix: - directory: ${{ fromJson(needs.getDirectories.outputs.directories) }} + matrix: + directory: ${{ fromJson(needs.getDirectories.outputs.directories) }} steps: - name: Checkout uses: actions/checkout@v2 @@ -59,7 +59,7 @@ jobs: pre-commit run terraform_validate --color=always --show-diff-on-failure --files $(ls *.tf) -# Max Terraform version + # Max Terraform version getBaseVersion: name: Module max TF version runs-on: ubuntu-latest @@ -94,7 +94,7 @@ jobs: - name: Install pre-commit dependencies run: | pip install pre-commit - curl -L "$(curl -s https://api.github.com/repos/terraform-docs/terraform-docs/releases/latest | grep -o -E "https://.+?-v0.12.0-linux-amd64" | head -n1)" > terraform-docs && chmod +x terraform-docs && sudo mv terraform-docs /usr/bin/ + curl -L "$(curl -s https://api.github.com/repos/terraform-docs/terraform-docs/releases/latest | grep -o -E "https://.+?-v0.12\..+?-linux-amd64" | head -n1)" > terraform-docs && chmod +x terraform-docs && sudo mv terraform-docs /usr/bin/ curl -L "$(curl -s https://api.github.com/repos/terraform-linters/tflint/releases/latest | grep -o -E "https://.+?_linux_amd64.zip")" > tflint.zip && unzip tflint.zip && rm tflint.zip && sudo mv tflint /usr/bin/ - name: Execute pre-commit # Run all pre-commit checks on max version supported diff --git a/README.md b/README.md index b684853a..f6266ecc 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,8 @@ No modules. | [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.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_public_access_block) | resource | | [aws_elb_service_account.this](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/elb_service_account) | data source | +| [aws_iam_policy_document.combined](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | +| [aws_iam_policy_document.deny_insecure_transport](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | | [aws_iam_policy_document.elb_log_delivery](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | ## Inputs @@ -111,6 +113,7 @@ No modules. |------|-------------|------|---------|:--------:| | [acceleration\_status](#input\_acceleration\_status) | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | `string` | `null` | no | | [acl](#input\_acl) | (Optional) The canned ACL to apply. Defaults to 'private'. Conflicts with `grant` | `string` | `"private"` | no | +| [attach\_deny\_insecure\_transport\_policy](#input\_attach\_deny\_insecure\_transport\_policy) | Controls if S3 bucket should have deny non-SSL transport policy attached | `bool` | `false` | no | | [attach\_elb\_log\_delivery\_policy](#input\_attach\_elb\_log\_delivery\_policy) | Controls if S3 bucket should have ELB log delivery policy attached | `bool` | `false` | no | | [attach\_policy](#input\_attach\_policy) | Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy) | `bool` | `false` | no | | [attach\_public\_policy](#input\_attach\_public\_policy) | Controls if a user defined public bucket policy will be attached (set to `false` to allow upstream to apply defaults to the bucket) | `bool` | `true` | no | diff --git a/examples/complete/main.tf b/examples/complete/main.tf index 90bd6f9b..587a19c2 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -51,10 +51,11 @@ data "aws_iam_policy_document" "bucket_policy" { module "log_bucket" { source = "../../" - bucket = "logs-${random_pet.this.id}" - acl = "log-delivery-write" - force_destroy = true - attach_elb_log_delivery_policy = true + bucket = "logs-${random_pet.this.id}" + acl = "log-delivery-write" + force_destroy = true + attach_elb_log_delivery_policy = true + attach_deny_insecure_transport_policy = true } module "cloudfront_log_bucket" { @@ -86,6 +87,8 @@ module "s3_bucket" { attach_policy = true policy = data.aws_iam_policy_document.bucket_policy.json + attach_deny_insecure_transport_policy = true + tags = { Owner = "Anton" } diff --git a/main.tf b/main.tf index 98d288af..650ff419 100644 --- a/main.tf +++ b/main.tf @@ -1,3 +1,7 @@ +locals { + attach_policy = var.attach_elb_log_delivery_policy || var.attach_deny_insecure_transport_policy || var.attach_policy +} + resource "aws_s3_bucket" "this" { count = var.create_bucket ? 1 : 0 @@ -231,10 +235,20 @@ resource "aws_s3_bucket" "this" { } resource "aws_s3_bucket_policy" "this" { - count = var.create_bucket && (var.attach_elb_log_delivery_policy || var.attach_policy) ? 1 : 0 + count = var.create_bucket && local.attach_policy ? 1 : 0 bucket = aws_s3_bucket.this[0].id - policy = var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json : var.policy + policy = data.aws_iam_policy_document.combined[0].json +} + +data "aws_iam_policy_document" "combined" { + count = var.create_bucket && local.attach_policy ? 1 : 0 + + source_policy_documents = compact([ + var.attach_elb_log_delivery_policy ? data.aws_iam_policy_document.elb_log_delivery[0].json : "", + var.attach_deny_insecure_transport_policy ? data.aws_iam_policy_document.deny_insecure_transport[0].json : "", + var.attach_policy ? var.policy : "" + ]) } # AWS Load Balancer access log delivery policy @@ -265,12 +279,43 @@ data "aws_iam_policy_document" "elb_log_delivery" { } } +data "aws_iam_policy_document" "deny_insecure_transport" { + count = var.create_bucket && var.attach_deny_insecure_transport_policy ? 1 : 0 + + statement { + sid = "denyInsecureTransport" + effect = "Deny" + + actions = [ + "s3:*", + ] + + resources = [ + aws_s3_bucket.this[0].arn, + "${aws_s3_bucket.this[0].arn}/*", + ] + + principals { + type = "*" + identifiers = ["*"] + } + + condition { + test = "Bool" + variable = "aws:SecureTransport" + values = [ + "false" + ] + } + } +} + resource "aws_s3_bucket_public_access_block" "this" { count = var.create_bucket && var.attach_public_policy ? 1 : 0 # Chain resources (s3_bucket -> s3_bucket_policy -> s3_bucket_public_access_block) # to prevent "A conflicting conditional operation is currently in progress against this resource." - bucket = (var.attach_elb_log_delivery_policy || var.attach_policy) ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id + bucket = local.attach_policy ? aws_s3_bucket_policy.this[0].id : aws_s3_bucket.this[0].id block_public_acls = var.block_public_acls block_public_policy = var.block_public_policy diff --git a/variables.tf b/variables.tf index 9e2b7b1d..f07d4169 100644 --- a/variables.tf +++ b/variables.tf @@ -10,6 +10,12 @@ variable "attach_elb_log_delivery_policy" { default = false } +variable "attach_deny_insecure_transport_policy" { + description = "Controls if S3 bucket should have deny non-SSL transport policy attached" + type = bool + default = false +} + variable "attach_policy" { description = "Controls if S3 bucket should have bucket policy attached (set to `true` to use value of `policy` as bucket policy)" type = bool