length of privileged_principal_arns not determinable before apply

[ghost]

Describe the Bug

If no resources are created yet and the var.privileged_principal_arns is a variable, this will lead in to this error:

Error: Invalid count argument

  on .terraform/modules/service.s3_bucket.s3_bucket/main.tf line 367, in resource "aws_s3_bucket_policy" "default":
 367:   count      = local.enabled && (var.allow_ssl_requests_only || var.allow_encrypted_uploads_only || length(var.s3_replication_source_roles) > 0 || length(var.privileged_principal_arns) > 0 || var.policy != "") ? 1 : 0

The "count" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the count depends on.

ERRO[0043] Hit multiple errors:
Hit multiple errors:
exit status 1 

Expected Behavior

Only checking if the variable is set or not.

Additional Context

Previous PR #101

[nitrocode]

What are the full list of inputs you're using to receive this error?

[ghost]

This is the entire call. Logging and encryption are enabled by a boolean and depending on other resources/modules.

locals {
  enable_bucket_policy = var.privileged_principal_arn != ""
}

module "s3_bucket" {
  source  = "../../../vendor/github.com/cloudposse/terraform-aws-s3-bucket"

  bucket_name = var.bucket_name

  acl                = "private"
  sse_algorithm      = var.enable_kms_sse ? "aws:kms" : "AES256"
  kms_master_key_arn = var.enable_kms_sse ? aws_kms_key.bucket_encryption_key[0].arn : ""

  versioning_enabled = true
  force_destroy      = false
  
  privileged_principal_arns = local.enable_bucket_policy ? {
    tostring(var.privileged_principal_arn) = ["*"]
  } : null
  
  privileged_principal_actions = local.enable_bucket_policy ? [
    "s3:PutObject",
    "s3:GetObject",
    "s3:DeleteObject",
    "s3:ListBucket",
    "s3:ListBucketMultipartUploads",
    "s3:GetBucketLocation",
    "s3:AbortMultipartUpload"
  ] : null
  

  logging = var.enable_access_logging ? {
    bucket_name = module.s3_bucket_log_storage.bucket_id
    prefix      = module.s3_bucket_log_storage.prefix
  } : null

  depends_on = [module.s3_bucket_log_storage, aws_kms_key.bucket_encryption_key]
}

var.privileged_principal_arn is the task execution role given by our ECS-module so it's just a string

module "s3_bucket" {
  source   = "../../../technology/aws/s3-bucket"
 ...
  privileged_principal_arn = module.ecs_service.ecs_task_exec_role_arn
 ...
  
  depends_on = [module.ecs_service]
}

[ghost]

I think I spotted the main problem: The output of the ECS-Module is not determinable. That a bit wired because, the module has the task execution role arn defined as an output (https://github.com/cloudposse/terraform-aws-ecs-alb-service-task/tree/0.58.0#output_task_exec_role_arn)

I've added a boolean all the way from the top to the s3-bucket module. If I set that variable (enable_privileged_principal_bucket_policy) explicitly to true it is working.

module "s3_bucket" {
  source   = "../../../technology/aws/s3-bucket"
  ...
  enable_privileged_principal_bucket_policy = true
  privileged_principal_arn = module.ecs_service.ecs_task_exec_role_arn
  ...
  depends_on = [module.ecs_service]
}

S3-bucket content:

dynamic "statement" {
    for_each = var.privileged_principal_enabled ? keys(var.privileged_principal_arns) : []

    content {
      ...
    }
  }
}

resource "aws_s3_bucket_policy" "default" {
  count      = local.enabled && (var.allow_ssl_requests_only || var.allow_encrypted_uploads_only || length(var.s3_replication_source_roles) > 0 || var.privileged_principal_enabled || var.policy != "") ? 1 : 0
  bucket     = join("", aws_s3_bucket.default.*.id)
  policy     = join("", data.aws_iam_policy_document.aggregated_policy.*.json)
  depends_on = [aws_s3_bucket_public_access_block.default]
}

if I leave it variable depending on the output of the ECS-module it is again failing.

module "s3_bucket" {
  source   = "../../../technology/aws/s3-bucket"
  ...
  enable_privileged_principal_bucket_policy = module.ecs_service.ecs_task_exec_role_arn != null
  privileged_principal_arn = module.ecs_service.ecs_task_exec_role_arn
  ...
  depends_on = [module.ecs_service]
}

or if I try to replace it with in the S3-bucket-module with a similar check :

locals {
  privileged_principal_enabled = var.privileged_principal_arns != null
}

dynamic "statement" {
    for_each = local.privileged_principal_enabled ? keys(var.privileged_principal_arns) : []

    content {
      ...
    }
  }
}

resource "aws_s3_bucket_policy" "default" {
  count      = local.enabled && (var.allow_ssl_requests_only || var.allow_encrypted_uploads_only || length(var.s3_replication_source_roles) > 0 || local.privileged_principal_enabled || var.policy != "") ? 1 : 0
  bucket     = join("", aws_s3_bucket.default.*.id)
  policy     = join("", data.aws_iam_policy_document.aggregated_policy.*.json)
  depends_on = [aws_s3_bucket_public_access_block.default]
}

[ghost]

I had a chat with a few other people and it is not a bug in this module. It is Terraform by itself that is not capable of building those module-output to variable dependency straight.
I will do a workaround by creating the bucket_policy by myself to "fix" that.

@nitrocode feel free to close the issue and PR. Consider merging the PR to add my additional examples if you like or close it if you don't.

[nitrocode]

@avendretter can we try this one more time and see if you still get the count error ?

module "s3_bucket" {
  source = "https://github.com/cloudposse/terraform-aws-s3-bucket.git?ref=count-fix"

  # other inputs
}

Edit: Perhaps this won't work. See my below comment.

[nitrocode]

Also, could you try building your list of principal arns using this example?

terraform-aws-s3-bucket/examples/complete/privileged-principals.tf

Lines 2 to 10 in 54d3caf

	account_id = data.aws_caller_identity.current.account_id
	principal_names = [
	"arn:aws:iam::${local.account_id}:role/${join("", module.deployment_principal_label.*.id)}",
	"arn:aws:iam::${local.account_id}:role/${join("", module.additional_deployment_principal_label.*.id)}"
	]
	privileged_principal_arns = var.privileged_principal_enabled ? {
	(local.principal_names[0]) = [""]
	(local.principal_names[1]) = ["prefix1/", "prefix2/"]
	} : {}

Perhaps this

locals {
  enable_bucket_policy = var.privileged_principal_arn != ""

  account_id = data.aws_caller_identity.current.account_id 
  principal_names = [ 
    "arn:aws:iam::${local.account_id}:role/${join("", module.deployment_principal_label.*.id)}", 
    "arn:aws:iam::${local.account_id}:role/${join("", module.additional_deployment_principal_label.*.id)}" 
  ] 
  privileged_principal_arns = local.enable_bucket_policy ? { 
    (local.principal_names[0]) = [""] 
    (local.principal_names[1]) = ["prefix1/", "prefix2/"] 
  } : {}

  privileged_principal_actions = local.enable_bucket_policy ? [
    "s3:PutObject",
    "s3:GetObject",
    "s3:DeleteObject",
    "s3:ListBucket",
    "s3:ListBucketMultipartUploads",
    "s3:GetBucketLocation",
    "s3:AbortMultipartUpload"
  ] : []

  var.enable_access_logging ? {
    bucket_name = module.s3_bucket_log_storage.bucket_id
    prefix      = module.s3_bucket_log_storage.prefix
  } : null
}

module "s3_bucket" {
  source  = "../../../vendor/github.com/cloudposse/terraform-aws-s3-bucket"

  bucket_name = var.bucket_name

  acl                = "private"
  sse_algorithm      = var.enable_kms_sse ? "aws:kms" : "AES256"
  kms_master_key_arn = var.enable_kms_sse ? aws_kms_key.bucket_encryption_key[0].arn : ""
  versioning_enabled = true
  force_destroy      = false

  privileged_principal_arns    = local.privileged_principal_arns
  privileged_principal_actions = local.privileged_principal_actions

  logging = local.logging

  depends_on = [module.s3_bucket_log_storage, aws_kms_key.bucket_encryption_key]
}

[ghost]

yeah, Building those ARNs manually would work of cause, but I think this is a bad or not so good practice. It would require hardcoded values that might change in the future, no?

[nitrocode]

Probably not best practice, however, this is how we're doing it in production to get around the same count issue you're experiencing and so far we haven't run into major problems as long as we're passing in the same principal names as we're expecting will be created.

[RaJiska]

Having the same issue here with Cloudfront's OAI:

resource "aws_cloudfront_origin_access_identity" "to_s3" {
  comment = "OAI to S3 ${local.cloudfront_instance_name}"
}

module "s3" {
  source  = "cloudposse/s3-bucket/aws"
  version = "0.43.0"

  bucket_name         = local.cloudfront_instance_name
  acl                 = "private"
  enabled             = true
  versioning_enabled  = false
  force_destroy       = var.env_name.short == "prod" ? false : true

   privileged_principal_arns = {
    "${aws_cloudfront_origin_access_identity.to_s3.iam_arn}" = [""]
  }

  privileged_principal_actions = [
    "s3:ListBucket",
    "s3:GetObject"
  ]
}

We can probably craft the ARN ourself as mentioned above, but this isn't really our preferable way of doing either.

EDIT: Actually not even sure how one would be able to craft the ARN in this specific case as we use Cloudfront's OAI which generates a random ID used in the IAM ARN.

[nitrocode]

If you have an alternative method to do it, we'd love to see it solved. :)

Otherwise you'll have to copy and paste any attribute that triggers this issue.

[MonolithProjects]

I just found out that this issue appeared with release 0.33.0. Release 0.32.0 still works fine (tested with tf 1.0.9)