Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(main.tf): change count check for privileged_principal_arns #103

Closed
wants to merge 4 commits into from
Closed

fix(main.tf): change count check for privileged_principal_arns #103

wants to merge 4 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Aug 16, 2021

what

  • The length(var.privileged_principal_arns) is not determinable before apply if the input itself is dependent on other resources.

why

  • Terraform cannot know the length of the variable before it is created e.g. by another module.

references

@ghost ghost requested review from a team as code owners August 16, 2021 06:40
@ghost ghost requested review from adamcrews and max-lobur August 16, 2021 06:40
@nitrocode
Copy link
Member

/test all

@nitrocode nitrocode added bug 🐛 An issue with the system patch A minor, backward compatible change labels Aug 16, 2021
nitrocode
nitrocode previously approved these changes Aug 16, 2021
@nitrocode
Copy link
Member

Hmmm

Error: Error putting S3 policy: MalformedPolicy: Could not parse the policy: Statement is not well formatted.

@nitrocode nitrocode self-requested a review August 16, 2021 13:37
@nitrocode
Copy link
Member

With this change, I run this locally

cd examples/complete
terraform init
terraform plan -var-file=object-lock.us-east-2.tfvars

And it tries to create this empty resource

  # module.s3_bucket.aws_s3_bucket_policy.default[0] will be created
  + resource "aws_s3_bucket_policy" "default" {
      + bucket = (known after apply)
      + id     = (known after apply)
      + policy = jsonencode(
            {
              + Statement = null
              + Version   = "2012-10-17"
            }
        )
    }

If we can change var.privileged_principal_arns to default to null instead of {}, it may be easier to verify if the map is empty. Unsure if the same issue you were getting will come up.

FYI the error you're probably getting (source)

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.

@nitrocode nitrocode dismissed their stale review August 16, 2021 13:56

terratest error

@ghost
Copy link
Author

ghost commented Aug 17, 2021

In order to test, I added some more scenarios like a plain one and one creating an S3 bucket with a policy attached.

The change to null as default is not solving the initial issue :(

@nitrocode nitrocode mentioned this pull request Aug 17, 2021
@nitrocode
Copy link
Member

Is there a way to see if the statement is null?

@ghost
Copy link
Author

ghost commented Aug 18, 2021

Dont think so. As soon as TF doesn't know what is stored inside the variable it won't work.
I explained it a bit more in detail in the issue-ticket.

I'd consider closing the ticket. If you'd like you can still take over the extended examples. What do you think?

@nitrocode
Copy link
Member

Let's close for now and reopen if there's a better solution

@nitrocode nitrocode closed this Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 An issue with the system patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants