From 7189837cde1230ba54c06fe29eadc66667763fd7 Mon Sep 17 00:00:00 2001 From: Abel Luck Date: Tue, 14 Feb 2023 15:47:16 +0000 Subject: [PATCH 1/4] fix: Deprecate `deployment_principal_arns` (#257) This patch refactors the passing of deployment principals, such that it uses static/known map keys. This allows this module to be applied in the same terraform run that the deployment principal (e.g., an iam user) is applied. Hashicorp recommends storing only known values in map keys, and leaving all dynamic/unknown values in the map values ([source0](https://developer.hashicorp.com/terraform/language/meta-arguments/for_each#limitations-on-values-used-in-for_each), [source1](https://github.com/hashicorp/terraform/issues/30838#issuecomment-1096848884)). --- deprecated.tf | 2 ++ main.tf | 6 +++--- variables.tf | 13 ++++++++++--- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/deprecated.tf b/deprecated.tf index 81dc6e85..9d4719a9 100644 --- a/deprecated.tf +++ b/deprecated.tf @@ -7,6 +7,8 @@ locals { cloudfront_access_log_include_cookies = var.log_include_cookies == null ? var.cloudfront_access_log_include_cookies : var.log_include_cookies cloudfront_access_log_prefix = var.log_prefix == null ? var.cloudfront_access_log_prefix : var.log_prefix + deployment_principals = var.deployment_principal_arns == null ? var.deployment_principals : { for arn, path_prefix in var.deployment_principal_arns : arn => { "arn" : arn, "path_prefix" : path_prefix } } + # New variables, but declare them here for consistency cloudfront_access_log_create_bucket = var.cloudfront_access_log_create_bucket } diff --git a/main.tf b/main.tf index 7ba9c1e3..76615a11 100644 --- a/main.tf +++ b/main.tf @@ -183,19 +183,19 @@ data "aws_iam_policy_document" "s3_website_origin" { } data "aws_iam_policy_document" "deployment" { - for_each = local.enabled ? var.deployment_principal_arns : {} + for_each = local.enabled ? local.deployment_principals : {} statement { actions = var.deployment_actions resources = distinct(flatten([ [local.origin_bucket.arn], - formatlist("${local.origin_bucket.arn}/%s*", each.value), + formatlist("${local.origin_bucket.arn}/%s*", each.value.path_prefix), ])) principals { type = "AWS" - identifiers = [each.key] + identifiers = [each.value.arn] } } } diff --git a/variables.tf b/variables.tf index 256a445b..0bbe4dc9 100644 --- a/variables.tf +++ b/variables.tf @@ -480,8 +480,8 @@ variable "versioning_enabled" { description = "When set to 'true' the s3 origin bucket will have versioning enabled" } -variable "deployment_principal_arns" { - type = map(list(string)) +variable "deployment_principals" { + type = map(object({ path_prefix = string, arn = string })) default = {} description = <<-EOT (Optional) Map of IAM Principal ARNs to lists of S3 path prefixes to grant `deployment_actions` permissions. @@ -633,6 +633,13 @@ variable "origin_groups" { # Variables below here are DEPRECATED and should not be used anymore +variable "deployment_principal_arns" { + type = map(list(string)) + default = null + description = "DEPRECATED. Use `deployment_principals` instead." +} + + variable "access_log_bucket_name" { type = string default = null @@ -679,4 +686,4 @@ variable "http_version" { type = string default = "http2" description = "The maximum HTTP version to support on the distribution. Allowed values are http1.1, http2, http2and3 and http3" -} \ No newline at end of file +} From 46cd5226c8403587e9a8c6005f07feacec552219 Mon Sep 17 00:00:00 2001 From: Abel Luck Date: Mon, 7 Aug 2023 12:50:58 +0000 Subject: [PATCH 2/4] Break-apart logic for readability --- deprecated.tf | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/deprecated.tf b/deprecated.tf index 9d4719a9..15eb0cbe 100644 --- a/deprecated.tf +++ b/deprecated.tf @@ -7,7 +7,14 @@ locals { cloudfront_access_log_include_cookies = var.log_include_cookies == null ? var.cloudfront_access_log_include_cookies : var.log_include_cookies cloudfront_access_log_prefix = var.log_prefix == null ? var.cloudfront_access_log_prefix : var.log_prefix - deployment_principals = var.deployment_principal_arns == null ? var.deployment_principals : { for arn, path_prefix in var.deployment_principal_arns : arn => { "arn" : arn, "path_prefix" : path_prefix } } + deployment_principals_from_deprecated_deployment_principal_arns = { + for arn, path_prefix in coalesce(var.deployment_principal_arns, {}) : + arn => { + "arn" : arn, + "path_prefix" : path_prefix + } + } + deployment_principals = var.deployment_principal_arns == null ? var.deployment_principals : local.deployment_principals_from_deprecated_deployment_principal_arns # New variables, but declare them here for consistency cloudfront_access_log_create_bucket = var.cloudfront_access_log_create_bucket From 670243acf7f8af844e0d062a6a0d6397d2334142 Mon Sep 17 00:00:00 2001 From: Abel Luck Date: Fri, 11 Aug 2023 07:54:47 +0000 Subject: [PATCH 3/4] Allow multiple path_prefixes and update documentation --- README.yaml | 12 +++++++++--- deprecated.tf | 3 +-- variables.tf | 7 ++++--- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/README.yaml b/README.yaml index 2ef85ec6..29d982e9 100644 --- a/README.yaml +++ b/README.yaml @@ -79,9 +79,15 @@ usage: |- dns_alias_enabled = true parent_zone_name = "cloudposse.com" - deployment_principal_arns = { - "arn:aws:iam::123456789012:role/principal1" = ["prefix1/", "prefix2/"] - "arn:aws:iam::123456789012:role/principal2" = [""] + deployment_principals = { + "principal1": { + "arn": "arn:aws:iam::123456789012:role/principal1" + "path_prefixes": ["prefix1/", "prefix2/"] + }, + "principal2": { + "arn": "arn:aws:iam::123456789012:role/principal2" + "path_prefixes": [""] + } } } ``` diff --git a/deprecated.tf b/deprecated.tf index 15eb0cbe..25d18f55 100644 --- a/deprecated.tf +++ b/deprecated.tf @@ -18,5 +18,4 @@ locals { # New variables, but declare them here for consistency cloudfront_access_log_create_bucket = var.cloudfront_access_log_create_bucket -} - +} \ No newline at end of file diff --git a/variables.tf b/variables.tf index 0bbe4dc9..dce02878 100644 --- a/variables.tf +++ b/variables.tf @@ -481,10 +481,11 @@ variable "versioning_enabled" { } variable "deployment_principals" { - type = map(object({ path_prefix = string, arn = string })) + type = map(object({ path_prefix = list(string), arn = string })) default = {} description = <<-EOT - (Optional) Map of IAM Principal ARNs to lists of S3 path prefixes to grant `deployment_actions` permissions. + (Optional) Map of objects that define the IAM Principal's to grant `deployment_actions` permissions. Each object in the map should have an IAM Principal ARN and a list of S3 path + prefixes to scope that principal's actions in the bucket. Resource list will include the bucket itself along with all the prefixes. Prefixes should not begin with '/'. EOT } @@ -492,7 +493,7 @@ variable "deployment_principals" { variable "deployment_actions" { type = list(string) default = ["s3:PutObject", "s3:PutObjectAcl", "s3:GetObject", "s3:DeleteObject", "s3:ListBucket", "s3:ListBucketMultipartUploads", "s3:GetBucketLocation", "s3:AbortMultipartUpload"] - description = "List of actions to permit `deployment_principal_arns` to perform on bucket and bucket prefixes (see `deployment_principal_arns`)" + description = "List of actions to permit `deployment_principals` to perform on bucket and bucket prefixes (see `deployment_principals`)" } variable "cloudfront_origin_access_identity_iam_arn" { From f2d3d8f8e24e57c146592ae0ab4e433e631f389a Mon Sep 17 00:00:00 2001 From: Abel Luck Date: Thu, 7 Sep 2023 13:22:49 +0000 Subject: [PATCH 4/4] Update readme --- README.md | 17 ++++++++++++----- docs/terraform.md | 5 +++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index b225651c..cda0e358 100644 --- a/README.md +++ b/README.md @@ -110,9 +110,15 @@ module "cdn" { dns_alias_enabled = true parent_zone_name = "cloudposse.com" - deployment_principal_arns = { - "arn:aws:iam::123456789012:role/principal1" = ["prefix1/", "prefix2/"] - "arn:aws:iam::123456789012:role/principal2" = [""] + deployment_principals = { + "principal1": { + "arn": "arn:aws:iam::123456789012:role/principal1" + "path_prefixes": ["prefix1/", "prefix2/"] + }, + "principal2": { + "arn": "arn:aws:iam::123456789012:role/principal2" + "path_prefixes": [""] + } } } ``` @@ -511,8 +517,9 @@ Available targets: | [default\_root\_object](#input\_default\_root\_object) | Object that CloudFront return when requests the root URL | `string` | `"index.html"` | no | | [default\_ttl](#input\_default\_ttl) | Default amount of time (in seconds) that an object is in a CloudFront cache | `number` | `60` | no | | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | -| [deployment\_actions](#input\_deployment\_actions) | List of actions to permit `deployment_principal_arns` to perform on bucket and bucket prefixes (see `deployment_principal_arns`) | `list(string)` | 
[
  "s3:PutObject",
  "s3:PutObjectAcl",
  "s3:GetObject",
  "s3:DeleteObject",
  "s3:ListBucket",
  "s3:ListBucketMultipartUploads",
  "s3:GetBucketLocation",
  "s3:AbortMultipartUpload"
]
| no | -| [deployment\_principal\_arns](#input\_deployment\_principal\_arns) | (Optional) Map of IAM Principal ARNs to lists of S3 path prefixes to grant `deployment_actions` permissions.
Resource list will include the bucket itself along with all the prefixes. Prefixes should not begin with '/'. | `map(list(string))` | `{}` | no | +| [deployment\_actions](#input\_deployment\_actions) | List of actions to permit `deployment_principals` to perform on bucket and bucket prefixes (see `deployment_principals`) | `list(string)` | 
[
  "s3:PutObject",
  "s3:PutObjectAcl",
  "s3:GetObject",
  "s3:DeleteObject",
  "s3:ListBucket",
  "s3:ListBucketMultipartUploads",
  "s3:GetBucketLocation",
  "s3:AbortMultipartUpload"
]
| no | +| [deployment\_principal\_arns](#input\_deployment\_principal\_arns) | DEPRECATED. Use `deployment_principals` instead. | `map(list(string))` | `null` | no | +| [deployment\_principals](#input\_deployment\_principals) | (Optional) Map of objects that define the IAM Principal's to grant `deployment_actions` permissions. Each object in the map should have an IAM Principal ARN and a list of S3 path
prefixes to scope that principal's actions in the bucket.
Resource list will include the bucket itself along with all the prefixes. Prefixes should not begin with '/'. | `map(object({ path_prefix = list(string), arn = string }))` | `{}` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [distribution\_enabled](#input\_distribution\_enabled) | Set to `false` to create the distribution but still prevent CloudFront from serving requests. | `bool` | `true` | no | | [dns\_alias\_enabled](#input\_dns\_alias\_enabled) | Create a DNS alias for the CDN. Requires `parent_zone_id` or `parent_zone_name` | `bool` | `false` | no | diff --git a/docs/terraform.md b/docs/terraform.md index ff4cf8a0..ca357389 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -83,8 +83,9 @@ | [default\_root\_object](#input\_default\_root\_object) | Object that CloudFront return when requests the root URL | `string` | `"index.html"` | no | | [default\_ttl](#input\_default\_ttl) | Default amount of time (in seconds) that an object is in a CloudFront cache | `number` | `60` | no | | [delimiter](#input\_delimiter) | Delimiter to be used between ID elements.
Defaults to `-` (hyphen). Set to `""` to use no delimiter at all. | `string` | `null` | no | -| [deployment\_actions](#input\_deployment\_actions) | List of actions to permit `deployment_principal_arns` to perform on bucket and bucket prefixes (see `deployment_principal_arns`) | `list(string)` | 
[
  "s3:PutObject",
  "s3:PutObjectAcl",
  "s3:GetObject",
  "s3:DeleteObject",
  "s3:ListBucket",
  "s3:ListBucketMultipartUploads",
  "s3:GetBucketLocation",
  "s3:AbortMultipartUpload"
]
| no | -| [deployment\_principal\_arns](#input\_deployment\_principal\_arns) | (Optional) Map of IAM Principal ARNs to lists of S3 path prefixes to grant `deployment_actions` permissions.
Resource list will include the bucket itself along with all the prefixes. Prefixes should not begin with '/'. | `map(list(string))` | `{}` | no | +| [deployment\_actions](#input\_deployment\_actions) | List of actions to permit `deployment_principals` to perform on bucket and bucket prefixes (see `deployment_principals`) | `list(string)` | 
[
  "s3:PutObject",
  "s3:PutObjectAcl",
  "s3:GetObject",
  "s3:DeleteObject",
  "s3:ListBucket",
  "s3:ListBucketMultipartUploads",
  "s3:GetBucketLocation",
  "s3:AbortMultipartUpload"
]
| no | +| [deployment\_principal\_arns](#input\_deployment\_principal\_arns) | DEPRECATED. Use `deployment_principals` instead. | `map(list(string))` | `null` | no | +| [deployment\_principals](#input\_deployment\_principals) | (Optional) Map of objects that define the IAM Principal's to grant `deployment_actions` permissions. Each object in the map should have an IAM Principal ARN and a list of S3 path
prefixes to scope that principal's actions in the bucket.
Resource list will include the bucket itself along with all the prefixes. Prefixes should not begin with '/'. | `map(object({ path_prefix = list(string), arn = string }))` | `{}` | no | | [descriptor\_formats](#input\_descriptor\_formats) | Describe additional descriptors to be output in the `descriptors` output map.
Map of maps. Keys are names of descriptors. Values are maps of the form
`{
format = string
labels = list(string)
}`
(Type is `any` so the map values can later be enhanced to provide additional options.)
`format` is a Terraform format string to be passed to the `format()` function.
`labels` is a list of labels, in order, to pass to `format()` function.
Label values will be normalized before being passed to `format()` so they will be
identical to how they appear in `id`.
Default is `{}` (`descriptors` output will be empty). | `any` | `{}` | no | | [distribution\_enabled](#input\_distribution\_enabled) | Set to `false` to create the distribution but still prevent CloudFront from serving requests. | `bool` | `true` | no | | [dns\_alias\_enabled](#input\_dns\_alias\_enabled) | Create a DNS alias for the CDN. Requires `parent_zone_id` or `parent_zone_name` | `bool` | `false` | no |