From e74d1509474152fee8bba400a12db2e49a778fda Mon Sep 17 00:00:00 2001 From: Bryant Biggs Date: Tue, 6 Oct 2020 07:33:45 -0400 Subject: [PATCH] feat: Add bucket acl policy grants (#44) --- .pre-commit-config.yaml | 19 +++++++++++++++++-- README.md | 3 ++- examples/complete/README.md | 10 +++++++--- examples/complete/main.tf | 23 ++++++++++++++++++++++- examples/complete/variables.tf | 0 examples/complete/versions.tf | 8 ++++++++ examples/notification/README.md | 13 +++++++++---- examples/notification/main.tf | 4 ++-- examples/notification/variables.tf | 0 examples/notification/versions.tf | 9 +++++++++ examples/s3-replication/README.md | 12 ++++++++---- examples/s3-replication/variables.tf | 0 examples/s3-replication/versions.tf | 8 ++++++++ main.tf | 15 +++++++++++++-- modules/notification/README.md | 8 ++++++-- modules/notification/main.tf | 10 +++++----- modules/notification/versions.tf | 8 ++++++++ outputs.tf | 1 - variables.tf | 8 +++++++- 19 files changed, 131 insertions(+), 28 deletions(-) create mode 100644 examples/complete/variables.tf create mode 100644 examples/complete/versions.tf create mode 100644 examples/notification/variables.tf create mode 100644 examples/notification/versions.tf create mode 100644 examples/s3-replication/variables.tf create mode 100644 examples/s3-replication/versions.tf create mode 100644 modules/notification/versions.tf diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2308d6fe..0c40609f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,10 +1,25 @@ repos: - repo: git://github.com/antonbabenko/pre-commit-terraform - rev: v1.31.0 + rev: v1.43.0 hooks: - id: terraform_fmt - id: terraform_docs + - id: terraform_tflint + args: + - '--args=--only=terraform_deprecated_interpolation' + - '--args=--only=terraform_deprecated_index' + - '--args=--only=terraform_unused_declarations' + - '--args=--only=terraform_comment_syntax' + - '--args=--only=terraform_documented_outputs' + - '--args=--only=terraform_documented_variables' + - '--args=--only=terraform_typed_variables' + - '--args=--only=terraform_module_pinned_source' + - '--args=--only=terraform_naming_convention' + - '--args=--only=terraform_required_version' + - '--args=--only=terraform_required_providers' + - '--args=--only=terraform_standard_module_structure' + - '--args=--only=terraform_workspace_remote' - repo: git://github.com/pre-commit/pre-commit-hooks - rev: v3.1.0 + rev: v3.2.0 hooks: - id: check-merge-conflict diff --git a/README.md b/README.md index 392d9fc1..dab0c7fd 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ module "s3_bucket" { | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| | acceleration\_status | (Optional) Sets the accelerate configuration of an existing bucket. Can be Enabled or Suspended. | `string` | `null` | no | -| acl | (Optional) The canned ACL to apply. Defaults to 'private'. | `string` | `"private"` | no | +| acl | (Optional) The canned ACL to apply. Defaults to 'private'. Conflicts with `grant` | `string` | `"private"` | no | | attach\_elb\_log\_delivery\_policy | Controls if S3 bucket should have ELB log delivery policy attached | `bool` | `false` | no | | 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 | 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 | @@ -109,6 +109,7 @@ module "s3_bucket" { | cors\_rule | List of maps containing rules for Cross-Origin Resource Sharing. | `list(any)` | `[]` | no | | create\_bucket | Controls if S3 bucket should be created | `bool` | `true` | no | | force\_destroy | (Optional, Default:false ) A boolean that indicates all objects should be deleted from the bucket so that the bucket can be destroyed without error. These objects are not recoverable. | `bool` | `false` | no | +| grant | An ACL policy grant. Conflicts with `acl` | `list(any)` | `[]` | no | | ignore\_public\_acls | Whether Amazon S3 should ignore public ACLs for this bucket. | `bool` | `false` | no | | lifecycle\_rule | List of maps containing configuration of object lifecycle management. | `any` | `[]` | no | | logging | Map containing access bucket logging configuration. | `map(string)` | `{}` | no | diff --git a/examples/complete/README.md b/examples/complete/README.md index 6102d9b0..627602c1 100644 --- a/examples/complete/README.md +++ b/examples/complete/README.md @@ -27,14 +27,18 @@ Note that this example may create resources which cost money. Run `terraform des ## Requirements -No requirements. +| Name | Version | +|------|---------| +| terraform | >= 0.12.6, < 0.14 | +| aws | >= 3.0, < 4.0 | +| random | ~> 2 | ## Providers | Name | Version | |------|---------| -| aws | n/a | -| random | n/a | +| aws | >= 3.0, < 4.0 | +| random | ~> 2 | ## Inputs diff --git a/examples/complete/main.tf b/examples/complete/main.tf index 70e14976..de873172 100644 --- a/examples/complete/main.tf +++ b/examples/complete/main.tf @@ -2,6 +2,8 @@ locals { bucket_name = "s3-bucket-${random_pet.this.id}" } +data "aws_canonical_user_id" "current" {} + resource "random_pet" "this" { length = 2 } @@ -55,6 +57,25 @@ module "log_bucket" { attach_elb_log_delivery_policy = true } +module "cloudfront_log_bucket" { + source = "../../" + + bucket = "cloudfront-logs-${random_pet.this.id}" + acl = null # conflicts with default of `acl = "private"` so set to null to use grants + grant = [{ + type = "CanonicalUser" + permissions = ["FULL_CONTROL"] + id = data.aws_canonical_user_id.current.id + }, { + type = "CanonicalUser" + permissions = ["FULL_CONTROL"] + id = "c4c1ede66af53448b93c283ce9448c4ba468c9432aa01d700d3878632f77d2d0" + # Ref. https://github.com/terraform-providers/terraform-provider-aws/issues/12512 + # Ref. https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/AccessLogs.html + }] + force_destroy = true +} + module "s3_bucket" { source = "../../" @@ -183,7 +204,7 @@ module "s3_bucket" { } } - // S3 bucket-level Public Access Block configuration + # S3 bucket-level Public Access Block configuration block_public_acls = true block_public_policy = true ignore_public_acls = true diff --git a/examples/complete/variables.tf b/examples/complete/variables.tf new file mode 100644 index 00000000..e69de29b diff --git a/examples/complete/versions.tf b/examples/complete/versions.tf new file mode 100644 index 00000000..06cd171a --- /dev/null +++ b/examples/complete/versions.tf @@ -0,0 +1,8 @@ +terraform { + required_version = ">= 0.12.6, < 0.14" + + required_providers { + aws = ">= 3.0, < 4.0" + random = "~> 2" + } +} diff --git a/examples/notification/README.md b/examples/notification/README.md index e8c3f1a3..76020d54 100644 --- a/examples/notification/README.md +++ b/examples/notification/README.md @@ -17,15 +17,20 @@ Note that this example may create resources which cost money. Run `terraform des ## Requirements -No requirements. +| Name | Version | +|------|---------| +| terraform | >= 0.12.6, < 0.14 | +| aws | >= 3.0, < 4.0 | +| null | ~> 2 | +| random | ~> 2 | ## Providers | Name | Version | |------|---------| -| aws | n/a | -| null | n/a | -| random | n/a | +| aws | >= 3.0, < 4.0 | +| null | ~> 2 | +| random | ~> 2 | ## Inputs diff --git a/examples/notification/main.tf b/examples/notification/main.tf index fe0a71cf..7696033f 100644 --- a/examples/notification/main.tf +++ b/examples/notification/main.tf @@ -81,7 +81,7 @@ module "all_notifications" { bucket = module.s3_bucket.this_s3_bucket_id - // Common error - Error putting S3 notification configuration: InvalidArgument: Configuration is ambiguously defined. Cannot have overlapping suffixes in two rules if the prefixes are overlapping for the same event type. + # Common error - Error putting S3 notification configuration: InvalidArgument: Configuration is ambiguously defined. Cannot have overlapping suffixes in two rules if the prefixes are overlapping for the same event type. lambda_notifications = { lambda1 = { @@ -106,7 +106,7 @@ module "all_notifications" { filter_prefix = "prefix2/" filter_suffix = ".txt" - // queue_id = aws_sqs_queue.this[0].id // optional + # queue_id = aws_sqs_queue.this[0].id // optional } sqs2 = { diff --git a/examples/notification/variables.tf b/examples/notification/variables.tf new file mode 100644 index 00000000..e69de29b diff --git a/examples/notification/versions.tf b/examples/notification/versions.tf new file mode 100644 index 00000000..02ff150d --- /dev/null +++ b/examples/notification/versions.tf @@ -0,0 +1,9 @@ +terraform { + required_version = ">= 0.12.6, < 0.14" + + required_providers { + aws = ">= 3.0, < 4.0" + random = "~> 2" + null = "~> 2" + } +} diff --git a/examples/s3-replication/README.md b/examples/s3-replication/README.md index 5eefe108..69223f6a 100644 --- a/examples/s3-replication/README.md +++ b/examples/s3-replication/README.md @@ -19,15 +19,19 @@ Note that this example may create resources which cost money. Run `terraform des ## Requirements -No requirements. +| Name | Version | +|------|---------| +| terraform | >= 0.12.6, < 0.14 | +| aws | >= 3.0, < 4.0 | +| random | ~> 2 | ## Providers | Name | Version | |------|---------| -| aws | n/a | -| aws.replica | n/a | -| random | n/a | +| aws | >= 3.0, < 4.0 | +| aws.replica | >= 3.0, < 4.0 | +| random | ~> 2 | ## Inputs diff --git a/examples/s3-replication/variables.tf b/examples/s3-replication/variables.tf new file mode 100644 index 00000000..e69de29b diff --git a/examples/s3-replication/versions.tf b/examples/s3-replication/versions.tf new file mode 100644 index 00000000..06cd171a --- /dev/null +++ b/examples/s3-replication/versions.tf @@ -0,0 +1,8 @@ +terraform { + required_version = ">= 0.12.6, < 0.14" + + required_providers { + aws = ">= 3.0, < 4.0" + random = "~> 2" + } +} diff --git a/main.tf b/main.tf index 5faf7f1b..a34850de 100644 --- a/main.tf +++ b/main.tf @@ -50,6 +50,17 @@ resource "aws_s3_bucket" "this" { } } + dynamic "grant" { + for_each = var.grant + + content { + id = lookup(grant.value, "id", null) + type = grant.value.type + permissions = grant.value.permissions + uri = lookup(grant.value, "uri", null) + } + } + dynamic "lifecycle_rule" { for_each = var.lifecycle_rule @@ -254,8 +265,8 @@ data "aws_iam_policy_document" "elb_log_delivery" { 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." + # 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 block_public_acls = var.block_public_acls diff --git a/modules/notification/README.md b/modules/notification/README.md index cdc6f82e..87c97943 100644 --- a/modules/notification/README.md +++ b/modules/notification/README.md @@ -5,13 +5,17 @@ Creates S3 bucket notification resource with all supported types of deliveries: ## Requirements -No requirements. +| Name | Version | +|------|---------| +| terraform | >= 0.12.6, < 0.14 | +| aws | >= 3.0, < 4.0 | +| random | ~> 2 | ## Providers | Name | Version | |------|---------| -| aws | n/a | +| aws | >= 3.0, < 4.0 | ## Inputs diff --git a/modules/notification/main.tf b/modules/notification/main.tf index 8af092b8..1c97b2e5 100644 --- a/modules/notification/main.tf +++ b/modules/notification/main.tf @@ -1,8 +1,8 @@ locals { bucket_arn = coalesce(var.bucket_arn, "arn:aws:s3:::${var.bucket}") - // Convert from "arn:aws:sqs:eu-west-1:835367859851:bold-starling-0" into "https://sqs.eu-west-1.amazonaws.com/835367859851/bold-starling-0" if queue_id was not specified - // queue_url used in aws_sqs_queue_policy is not the same as arn which is used in all other places + # Convert from "arn:aws:sqs:eu-west-1:835367859851:bold-starling-0" into "https://sqs.eu-west-1.amazonaws.com/835367859851/bold-starling-0" if queue_id was not specified + # queue_url used in aws_sqs_queue_policy is not the same as arn which is used in all other places queue_ids = { for k, v in var.sqs_notifications : k => format("https://%s.%s.amazonaws.com/%s/%s", data.aws_arn.queue[k].service, data.aws_arn.queue[k].region, data.aws_arn.queue[k].account, data.aws_arn.queue[k].resource) if lookup(v, "queue_id", "") == "" } } @@ -54,7 +54,7 @@ resource "aws_s3_bucket_notification" "this" { ] } -// Lambda +# Lambda resource "aws_lambda_permission" "allow" { for_each = var.lambda_notifications @@ -66,7 +66,7 @@ resource "aws_lambda_permission" "allow" { source_arn = local.bucket_arn } -// SQS Queue +# SQS Queue data "aws_arn" "queue" { for_each = var.sqs_notifications @@ -107,7 +107,7 @@ resource "aws_sqs_queue_policy" "allow" { policy = data.aws_iam_policy_document.sqs[each.key].json } -// SNS Topic +# SNS Topic data "aws_iam_policy_document" "sns" { for_each = var.sns_notifications diff --git a/modules/notification/versions.tf b/modules/notification/versions.tf new file mode 100644 index 00000000..06cd171a --- /dev/null +++ b/modules/notification/versions.tf @@ -0,0 +1,8 @@ +terraform { + required_version = ">= 0.12.6, < 0.14" + + required_providers { + aws = ">= 3.0, < 4.0" + random = "~> 2" + } +} diff --git a/outputs.tf b/outputs.tf index 8b0d5129..27ee7e86 100644 --- a/outputs.tf +++ b/outputs.tf @@ -8,7 +8,6 @@ output "this_s3_bucket_arn" { value = element(concat(aws_s3_bucket.this.*.arn, list("")), 0) } - output "this_s3_bucket_bucket_domain_name" { description = "The bucket domain name. Will be of format bucketname.s3.amazonaws.com." value = element(concat(aws_s3_bucket.this.*.bucket_domain_name, list("")), 0) diff --git a/variables.tf b/variables.tf index 3c130481..f911f0ab 100644 --- a/variables.tf +++ b/variables.tf @@ -35,7 +35,7 @@ variable "bucket_prefix" { } variable "acl" { - description = "(Optional) The canned ACL to apply. Defaults to 'private'." + description = "(Optional) The canned ACL to apply. Defaults to 'private'. Conflicts with `grant`" type = string default = "private" } @@ -94,6 +94,12 @@ variable "logging" { default = {} } +variable "grant" { + description = "An ACL policy grant. Conflicts with `acl`" + type = list(any) + default = [] +} + variable "lifecycle_rule" { description = "List of maps containing configuration of object lifecycle management." type = any