aws-ia / terraform-aws-iam-identity-center

Apache License 2.0
16 stars 6 forks source link

Confusion using existing groups and users #36

Closed josesolisrosales closed 2 weeks ago

josesolisrosales commented 3 weeks ago

I've been trying to use this module for a while. Our use case is to manage user and group assignments for users and groups created using SCIM. Looking at the examples for existing groups/users it makes it seem like defining assignments is enough for the module to dynamically fetch the groups and users from AWS and use those (without defining any sso_users or sso_groups). I tried doing that but it is failing with:

╷
│ Error: Invalid index
│
│   on .terraform/modules/aws-iam-identity-center/main.tf line 230, in resource "aws_ssoadmin_account_assignment" "account_assignment":
│  230:   principal_id   = each.value.principal_type == "GROUP" ? (contains(local.this_groups, each.value.principal_name) ? aws_identitystore_group.sso_groups[each.value.principal_name].group_id : data.aws_identitystore_group.existing_sso_groups[each.value.principal_name].id) : (contains(local.this_users, each.value.principal_name) ? aws_identitystore_user.sso_users[each.value.principal_name].user_id : data.aws_identitystore_user.existing_sso_users[each.value.principal_name].id)
│     ├────────────────
│     │ data.aws_identitystore_group.existing_sso_groups is object with no attributes
│     │ each.value.principal_name is "DevOps"
│
│ The given key does not identify an element in this collection value.

DevOps is a SCIM group, present in the identity center and that is the correct display name for it. Looking at the code however I find that existing_groups is computed from sso_groups. Is this correct? Do external groups also have to be defined in sso_groups for the module to fetch them from AWS? The documentation reads as if groups in sso_groups will be created by the module but in the code I don't see a way to fetch them without defining them in that variable. What is the right way to manage existing groups and users? Thanks!

samsuse commented 3 weeks ago

The same issue has occurred in our project.

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: Invalid index
│ 
│   on .terraform/modules/iam_identity_center/main.tf line 141, in resource "aws_identitystore_group_membership" "sso_group_membership":
│  141:   member_id = (contains(local.this_users, each.value.user_name) ? aws_identitystore_user.sso_users[each.value.user_name].user_id : data.aws_identitystore_user.existing_sso_users[each.value.user_name].id)
│     ├────────────────
│     │ aws_identitystore_user.sso_users is object with 1 attribute "ro_user"
│     │ each.value.user_name is "ro_user@samsuse.com"
│ 
│ The given key does not identify an element in this collection value.
╵

Exited with code exit status 1

We previously used version 0.0.2 of the aws-ia/iam-identity-center/aws module to create users, groups, and permission sets in the IAM Identity Center. After upgrading the module version to 0.0.3, the apply step failed and reported the error mentioned above. This error is primarily associated with this submission. And found the solution here.

novekm commented 2 weeks ago

Hi @josesolisrosales,

Sorry for the confusion. As @samsuse mentioned, v0.0.3 changed the way some of the references work in the module. They are correct that the solution to the error in the code snippet they posted is in the current version of the README.

As for your specific issue, I have just submitted a PR that should resolve this (along with updates to that example), and include a few other enhancements, such as easier use for those using Google workspace as the external IdP (mentioned in #32).

Will follow up here after pipeline tests finish and it is merged. New release should be live by the end of the week 🙂

novekm commented 2 weeks ago

Hi @josesolisrosales, this has been resolved in v0.0.4 that was merged today. It may take a bit longer for it to appear in the docs in the Terraform Registry, but you should be able to use the new version now. Thanks for bringing this to our attention and let us know if you need anything else 🙂

rfum commented 1 week ago

Hi @novekm I'm also having the same issue. when will these changes going to be pushed into terraform registry?