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 for Error: Invalid for_each argument #13

Conversation

innominatus
Copy link
Contributor

@innominatus innominatus commented May 17, 2021

what

  • a.permission_set_arn is providing a unique value to the account_assignment name.
  • However the permission_set_arn can not be determined until after the apply of the permission sets.
  • Using a.permission_set_arn in account_assignments before the permission sets have been applied will result in the following error.
Error: Invalid for_each argument

  on .terraform/modules/sso_account_assignments/modules/account-assignments/main.tf line 30, in resource "aws_ssoadmin_account_assignment" "this":
  30:   for_each = local.assignment_map

The "for_each" 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 for_each depends on.

why

  • Attempting to provide a unique (enough) value that is already predetermined before apply by adding a.permission_set_name instead of arn.
  account_assignments = [
    {
        account = "111111111111",
        permission_set_name = "administratoraccess",
        permission_set_arn = "arn:aws:sso:::permissionSet/ssoins-0000000000000000/ps-31d20e5987f0ce66",
        principal_type = "GROUP",
        principal_name = "Administrators"
    },
  • Yes, its less unique but there should be only one assignment using the same account, principal, principal type, and permission set name.
  • Yes, you are carrying 2 values over from the permission set (name and arn) instead of just one (arn). But it gets around the uniqueness problem. I couldnt think of any other good choice at the moment. Maybe someone else will come up with a better value. Maybe use a random_string.name.result?

references

…nment name.

However the permission_set_arn can not be determined until after the apply of the permission sets.

Using a.permission_set_arn in account_assignments before the permission sets have been applied will result in the following error.

```
Error: Invalid for_each argument

  on .terraform/modules/sso_account_assignments/modules/account-assignments/main.tf line 30, in resource "aws_ssoadmin_account_assignment" "this":
  30:   for_each = local.assignment_map

The "for_each" 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 for_each depends on.
```

Attempting to provide a unique (enough) value that is already predetermined before apply by adding a.permission_set_name instead of arn.

Yes, its less unique but there should be only one assignment using the same account, principal, principal type, and permission set name.

Yes, you are carrying 2 values over from the permission set (name and arn) instead of just one (arn). But it gets around the uniqueness problem. I couldnt think of any other good choice at the moment. Maybe someone else will come up with a better value. Maybe use a random_string.name.result?
@innominatus innominatus requested review from a team as code owners May 17, 2021 16:30
@nicolaevladescu
Copy link

nicolaevladescu commented May 19, 2021

I have been successful with a workaround that does not involve another arbitrary key, i have opened a PR, can you guys check it out before we make the compromise? #14

@jgrumboe
Copy link
Contributor

I'm happy with this solution. Although it's a new arbitrary key, you can see it as a description or comment of the assignment and it helps to have stable resource names which avoid unnecessary recreation of resources when the list is altered.
That's a big benefit for me.

@aknysh
Copy link
Member

aknysh commented Jun 4, 2021

/terraform-fmt

Co-authored-by: Johannes Grumböck <johannes.grumboeck@redbull.com>
@aknysh
Copy link
Member

aknysh commented Jun 4, 2021

/test all

@nitrocode nitrocode changed the title issue 12 possible fix Fix for Error: Invalid for_each argument Jun 7, 2021
@nitrocode nitrocode added terraform/0.13 Module requires Terraform 0.13 or later and removed terraform/0.13 Module requires Terraform 0.13 or later labels Jun 7, 2021
@nitrocode
Copy link
Member

/test all

@osterman osterman requested review from mcalhoun and Nuru June 7, 2021 21:42
@jgrumboe
Copy link
Contributor

jgrumboe commented Jun 7, 2021

I think test all is simply not applicable in this repo.

Nuru
Nuru previously requested changes Jun 10, 2021
modules/account-assignments/main.tf Outdated Show resolved Hide resolved
@nitrocode nitrocode requested a review from Nuru June 10, 2021 22:17
@mergify mergify bot dismissed Nuru’s stale review June 10, 2021 22:17

This Pull Request has been updated, so we're dismissing all reviews.

@nitrocode nitrocode merged commit 109f86c into cloudposse:master Jun 11, 2021
@jgrumboe
Copy link
Contributor

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants