Skip to content

Commit

Permalink
bugfix deprecate deployment_principal_arns (cloudposse#257)
Browse files Browse the repository at this point in the history
This patch refactors the passing of deployment principals, such that it
uses static/known map keys. This allows this module to be applied at the
same time as the deployment principal (e.g., an iam user) is is
deployed.

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](hashicorp/terraform#30838 (comment))).
  • Loading branch information
abeluck committed Aug 11, 2023
1 parent f9b5c1c commit ba0f445
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
2 changes: 2 additions & 0 deletions deprecated.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 6 additions & 6 deletions main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ locals {
# Encapsulate logic here so that it is not lost/scattered among the configuration
website_enabled = local.enabled && var.website_enabled
website_password_enabled = local.website_enabled && var.s3_website_password_enabled
s3_origin_enabled = local.enabled && ! var.website_enabled
s3_origin_enabled = local.enabled && !var.website_enabled
create_s3_origin_bucket = local.enabled && var.origin_bucket == null
s3_access_logging_enabled = local.enabled && (var.s3_access_logging_enabled == null ? length(var.s3_access_log_bucket_name) > 0 : var.s3_access_logging_enabled)
create_cf_log_bucket = local.cloudfront_access_logging_enabled && local.cloudfront_access_log_create_bucket
Expand Down Expand Up @@ -47,7 +47,7 @@ locals {

override_origin_bucket_policy = local.enabled && var.override_origin_bucket_policy

lookup_cf_log_bucket = local.cloudfront_access_logging_enabled && ! local.cloudfront_access_log_create_bucket
lookup_cf_log_bucket = local.cloudfront_access_logging_enabled && !local.cloudfront_access_log_create_bucket
cf_log_bucket_domain = local.cloudfront_access_logging_enabled ? (
local.lookup_cf_log_bucket ? data.aws_s3_bucket.cf_logs[0].bucket_domain_name : module.logs.bucket_domain_name
) : ""
Expand Down Expand Up @@ -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]
}
}
}
Expand Down Expand Up @@ -441,7 +441,7 @@ resource "aws_cloudfront_distribution" "default" {
origin_path = var.origin_path

dynamic "s3_origin_config" {
for_each = ! var.website_enabled ? [1] : []
for_each = !var.website_enabled ? [1] : []
content {
origin_access_identity = local.cf_access.path
}
Expand Down
13 changes: 10 additions & 3 deletions variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
}
}

0 comments on commit ba0f445

Please sign in to comment.