Suggestion: Improve clarity of some of the examples

[philomory]

There's two areas where I feel like the examples are a bit lacking:

1. There's no examples (in either the README, or in any of the individual example directories) of how to use a customer-managed policy; the lack of examples, combined with the fact that the data type for permission_sets is listed simply as any rather than having an enforced structure, means that the only way to even know that the key is supposed to be customer_managed_policies is to look at the code; and to figure out what the value of the key should be, you need to either reason through the code yourself, or do some trial and error (I mean, it turns out to be the "obvious" option, which is that it's just an array of strings, each of which is a policy name; but it wouldn't be that weird to expect it to be an array of maps with name and path keys, like the customer_managed_policy_reference attribute in the ssoadmin_customer_managed_policy_attachment resource)
2. In the existing-users-and-groups example, there's some (apparent) conflation between the map keys used to reference users and groups, the value of the principal_name key within an account assignment, and the value of the user_name/group_name keys in the existing_sso_users and existing_sso_groups entries. From reading the module, I was able to suss out the following:
   • The map keys within existing_sso_users and existing_sso_groups can be whatever you want them to be, but you'll need to refer to them later.
   • The value of the user_name and group_name keys within the individual records in those maps, needs to match the actual name of the record in Identity Center.
   • The map keys within the account_assignments map are completely arbitrary and don't need to match anything (and good thing, too; if the map key needed to match the principal then you couldn't have multiple records for the same principal).
   • The value of the principal_name key in the account assignment needs to match the key from the existing_sso_users or existing_sso_groups map.

That's all well and good, but, in the examples the same value is used in all four places. It'd be a lot clearer if the example looked like, e.g.

existing_sso_users = {
  testuser : {
    user_name = "testuser@example.com"
  }
}
existing_sso_groups = {
  testgroup : {
    group_name = "Test Group"
  }
}

account_assignments = {
  testgroup_admin : {
      principal_name  = "testgroup"
      principal_type  = "GROUP"
      principal_idp   = "EXTERNAL"
      permission_sets = ["AdministratorAccess", "ViewOnlyAccess", ]
      account_ids = [...]
  }
  testgroup_poweruser : {
      principal_name  = "testgroup"
      principal_type  = "GROUP"
      principal_idp   = "EXTERNAL"
      permission_sets = ["PowerUserAccess", "ViewOnlyAccess", ]
      account_ids = [...]
  }
  testuser_something : {
      principal_name  = "testuser"
      principal_type  = "USER"
      principal_idp   = "EXTERNAL"
      permission_sets = ["ViewOnlyAccess"]
      account_ids = [...]
  }
}

This would make it much more clear which specific keys and values need to match each other (especially since the name of the principal_name key really makes it sound as it's supposed to be the actual record name that AWS sees, e.g. testuser@example.com or Test Group, etc).

[novekm]

Hi @philomory,

Thanks for raising this! We're working to enhance the docs a bit. A few responses to the above:

1. There's no examples (in either the README, or in any of the individual example directories) of how to use a customer-managed policy; the lack of examples, combined with the fact that the data type for permission_sets is listed simply as any rather than having an enforced structure, means that the only way to even know that the key is supposed to be customer_managed_policies is to look at the code; and to figure out what the value of the key should be, you need to either reason through the code yourself, or do some trial and error (I mean, it turns out to be the "obvious" option, which is that it's just an array of strings, each of which is a policy name; but it wouldn't be that weird to expect it to be an array of maps with name and path keys, like the customer_managed_policy_reference attribute in the ssoadmin_customer_managed_policy_attachment resource)

We are working on a basic example of using Customer Managed Policies. The main reason this was omitted initially is because of the potential dependencies. As mentioned in the README.md around potential errors that may be encountered around IAM:

When using Customer Managed Policies with account assignments, you must ensure these policies exist in all target accounts before using the module. Failure to do this will cause deployment errors because IAM Identity Center will attempt to reference policies that do not exist.

So the example of creating and using customer managed policies would need to be a two step apply, with some profile switching:

1. Switch AWS profile to the account you wish to create the Customer Managed Policy in
2. Create customer managed policy (terraform apply)
3. Switch AWS profile to the account that your IAM Identity Center is configured in
4. Reference customer managed policy in the module, and apply (`terraform apply)

Due to these complexities, we've seen many users prefer to leverage Inline Policies to prevent having to do the above, especially when working in large multi-account environments.

For the additional example we will create (existing-customer-managed-policies, we will take the assumption that the Customer Managed Policy one desires to reference already exists in the AWS Account they wish to use in the account assignment and permission set.

2. In the existing-users-and-groups example, there's some (apparent) conflation between the map keys used to reference users and groups, the value of the principal_name key within an account assignment, and the value of the user_name/group_name keys in the existing_sso_users and existing_sso_groups entries. From reading the module, I was able to suss out the following:
   The map keys within existing_sso_users and existing_sso_groups can be whatever you want them to be, but you'll need to refer to them later.
   The value of the user_name and group_name keys within the individual records in those maps, needs to match the actual name of the record in Identity Center.
   The map keys within the account_assignments map are completely arbitrary and don't need to match anything (and good thing, too; if the map key needed to match the principal then you couldn't have multiple records for the same principal).
   The value of the principal_name key in the account assignment needs to match the key from the existing_sso_users or existing_sso_groups map.

You are correct. Per the README.md:

Ensure that the name of your object(s) match the name of your principal(s) (e.g. user name or group name). See the following example with object/principal names 'Admin' and 'nuzumaki'

However, I understand that this may not be as clear as it could be, as it could leave one to believe that the name of the object must be the same name as the actual value in AWS for the user or group you wish to reference. Docs will be updated to say something such as the following:

_"The names of your object(s) (e.g. the groups or users you wish to create or reference) are used to reference them elsewhere within the module, such as referencing groups you wish to add users to, or permission sets you wish to use with your account assignments. While the names of these objects can be anything since they are just local references to the each object, ensure you reference the string exactly (case-sensitive) when using elsewhere in the module.

However, for the actual names of the existing groups, existing users, etc. these must match exactly as they appear in your AWS IAM Identity Center configuration. This is because for these resources, a data source is being used to fetch information about the existing resource using a filter on the name. To simplify this and prevent confusion, we recommend using the same name for the object as the resource itself. "_

We'll call this out in the core README.md as well as all related other examples.

Thank for your feedback! 🙂