pbs / terraform-aws-s3-module

Standard PBS TF S3 Module
MIT License
0 stars 0 forks source link

count in bucket_policy_doc forces targeted apply #2

Closed socketbox closed 2 years ago

socketbox commented 2 years ago

When using the S3 module in otter, I have come across this problem:

│ Error: Invalid count argument
│ 
│   on .terraform/modules/s3/security.tf line 53, in data "aws_iam_policy_document" "bucket_policy_doc":
│   53:   count = local.create_bucket_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.

In locals.tf there is a chain of logic whose factors include user-supplied variables:

  create_bucket_policy = var.bucket_policy != null || local.generate_bucket_policy
  generate_bucket_policy = var.bucket_policy == null && (var.allow_anonymous_vpce_access || local.create_replication_target_policy)
  create_replication_target_policy = var.replication_source != null

I'm not sure how to avoid this while still allowing for the generation of a bucket policy, as well as giving the user the option of bringing their own.

yhakbar commented 2 years ago

Unfortunately, this is just a limitation of Terraform, as far as I know. There's no good solution to address it.

I've been able to reduce the frequency of this kind of error in the past by utilizing Terragrunt, as Terraform will only throw this error if it's trying to make an update to a single piece of state and it can't work out the dependency graph properly. Splitting up state reduces the likelihood that one piece of state has resources that are dynamically being provisioned based on other factors in the same piece of state.

We've provided warnings elsewhere to ensure that users know how to handle this when it's the default behavior of the module to have this error. https://github.com/pbs/terraform-aws-static-website-module#warning-warning

You probably only got this error after supplying the bucket policy, right? If it's the default behavior we should definitely update the docs. We might want to update the docs anyways.

socketbox commented 2 years ago

as Terraform will only throw this error if it's trying to make an update to a single piece of state and it can't work out the dependency graph properly.

I just ran plan again and the error has gone away, without me having applied anything. It seems that the difficulty terraform has producing a dependency graph is transitory. Maybe this is the result of a failed API call? I've predicated the creation of resources on the number of developers returned from GitHub. The number of principals in the policy doc that I'm supplying depends, indirectly, on the number of developers in the repo:

data "aws_iam_policy_document" "policy_doc" {
  statement {
    actions   = ["s3:GetObject"]
    resources = ["${module.s3.arn}/*"]
    principals {
      type        = "AWS"
      identifiers = [for cf in module.cf_distribution_ga_collaborators : cf.oia_arn]
    }
  }
  statement {
    actions   = ["s3:ListBucket"]
    resources = [module.s3.arn]
    principals {
      type        = "AWS"
      identifiers = [for cf in module.cf_distribution_ga_collaborators : cf.oia_arn]
    }
  }
}

I wonder if this would have happened had I not introduced the GitHub provider/dependency.