Skip to content

Commit

Permalink
feat: Add bucket acl policy grants (#44)
Browse files Browse the repository at this point in the history
  • Loading branch information
bryantbiggs authored Oct 6, 2020
1 parent 828cf67 commit e74d150
Show file tree
Hide file tree
Showing 19 changed files with 131 additions and 28 deletions.
19 changes: 17 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down
10 changes: 7 additions & 3 deletions examples/complete/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,18 @@ Note that this example may create resources which cost money. Run `terraform des
<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
## 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

Expand Down
23 changes: 22 additions & 1 deletion examples/complete/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 = "../../"

Expand Down Expand Up @@ -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
Expand Down
Empty file added examples/complete/variables.tf
Empty file.
8 changes: 8 additions & 0 deletions examples/complete/versions.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
terraform {
required_version = ">= 0.12.6, < 0.14"

required_providers {
aws = ">= 3.0, < 4.0"
random = "~> 2"
}
}
13 changes: 9 additions & 4 deletions examples/notification/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,20 @@ Note that this example may create resources which cost money. Run `terraform des
<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
## 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

Expand Down
4 changes: 2 additions & 2 deletions examples/notification/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand Down
Empty file.
9 changes: 9 additions & 0 deletions examples/notification/versions.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
terraform {
required_version = ">= 0.12.6, < 0.14"

required_providers {
aws = ">= 3.0, < 4.0"
random = "~> 2"
null = "~> 2"
}
}
12 changes: 8 additions & 4 deletions examples/s3-replication/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,19 @@ Note that this example may create resources which cost money. Run `terraform des
<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
## 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

Expand Down
Empty file.
8 changes: 8 additions & 0 deletions examples/s3-replication/versions.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
terraform {
required_version = ">= 0.12.6, < 0.14"

required_providers {
aws = ">= 3.0, < 4.0"
random = "~> 2"
}
}
15 changes: 13 additions & 2 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions modules/notification/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ Creates S3 bucket notification resource with all supported types of deliveries:
<!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
## 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

Expand Down
10 changes: 5 additions & 5 deletions modules/notification/main.tf
Original file line number Diff line number Diff line change
@@ -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", "") == "" }
}

Expand Down Expand Up @@ -54,7 +54,7 @@ resource "aws_s3_bucket_notification" "this" {
]
}

// Lambda
# Lambda
resource "aws_lambda_permission" "allow" {
for_each = var.lambda_notifications

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
8 changes: 8 additions & 0 deletions modules/notification/versions.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
terraform {
required_version = ">= 0.12.6, < 0.14"

required_providers {
aws = ">= 3.0, < 4.0"
random = "~> 2"
}
}
1 change: 0 additions & 1 deletion outputs.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 7 additions & 1 deletion variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e74d150

Please sign in to comment.