r/aws_sso_permission_set: New Resource

[lawdhavmercy]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #15108
Relates #15540

Release note for CHANGELOG:

data-source/aws_ssoadmin_instances: new data-source
data-source/aws_ssoadmin_permission_set: new data-source
resource/aws_ssoadmin_permission_set: new resource
resource/aws_ssoadmin_permission_set_inline_policy: new resource
resource/aws_ssoadmin_managed_policy_attachment: new resource

Output from acceptance testing:

Commerical Organizations-Only

--- PASS: TestAccAWSProvider_AssumeRole_Empty (57.99s)
--- PASS: TestAccAWSProvider_Endpoints (54.60s)
--- PASS: TestAccAWSProvider_IgnoreTags_EmptyConfigurationBlock (53.59s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_Multiple (51.08s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_None (52.84s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_One (53.78s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_Multiple (52.68s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_None (51.76s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_One (51.93s)
--- PASS: TestAccAWSProvider_Region_AwsChina (39.31s)
--- PASS: TestAccAWSProvider_Region_AwsCommercial (39.64s)
--- PASS: TestAccAWSProvider_Region_AwsGovCloudUs (39.68s)
--- PASS: TestAccAWSSSOAdminManagedPolicyAttachment_basic (68.53s)
--- PASS: TestAccAWSSSOAdminManagedPolicyAttachment_disappears (63.25s)
--- PASS: TestAccAWSSSOAdminManagedPolicyAttachment_disappears_permissionSet (62.04s)
--- PASS: TestAccAWSSSOAdminManagedPolicyAttachment_forceNew (121.07s)
--- PASS: TestAccAWSSSOAdminManagedPolicyAttachment_multipleManagedPolicies (115.87s)
--- PASS: TestAccAWSSSOAdminPermissionSetInlinePolicy_basic (59.10s)
--- PASS: TestAccAWSSSOAdminPermissionSetInlinePolicy_disappears (51.81s)
--- PASS: TestAccAWSSSOAdminPermissionSetInlinePolicy_disappears_permissionSet (53.72s)
--- PASS: TestAccAWSSSOAdminPermissionSetInlinePolicy_update (98.95s)
--- PASS: TestAccAWSSSOAdminPermissionSet_basic (46.09s)
--- PASS: TestAccAWSSSOAdminPermissionSet_mixedPolicyAttachments (85.43s)
--- PASS: TestAccAWSSSOAdminPermissionSet_tags (105.08s)
--- PASS: TestAccAWSSSOAdminPermissionSet_updateDescription (85.98s)
--- PASS: TestAccAWSSSOAdminPermissionSet_updateRelayState (85.58s)
--- PASS: TestAccAWSSSOAdminPermissionSet_updateSessionDuration (85.20s)
--- PASS: TestAccDataSourceAWSSSOAdminInstances_basic (55.83s)
--- PASS: TestAccDataSourceAWSSSOAdminPermissionSet_arn (58.78s)
--- PASS: TestAccDataSourceAWSSSOAdminPermissionSet_name (58.74s)
--- PASS: TestAccDataSourceAWSSSOAdminPermissionSet_nonExistent (11.14s)
--- PASS: TestProvider (1.04s)
--- PASS: TestProvider_impl (0.31s)

Commerical:

--- PASS: TestAccAWSProvider_AssumeRole_Empty (32.46s)
--- PASS: TestAccAWSProvider_Endpoints (31.21s)
--- PASS: TestAccAWSProvider_IgnoreTags_EmptyConfigurationBlock (30.26s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_Multiple (31.47s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_None (31.60s)
--- PASS: TestAccAWSProvider_IgnoreTags_KeyPrefixes_One (30.97s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_Multiple (29.94s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_None (31.66s)
--- PASS: TestAccAWSProvider_IgnoreTags_Keys_One (31.14s)
--- PASS: TestAccAWSProvider_Region_AwsChina (25.88s)
--- PASS: TestAccAWSProvider_Region_AwsCommercial (26.63s)
--- PASS: TestAccAWSProvider_Region_AwsGovCloudUs (25.92s)
--- PASS: TestProvider (0.53s)
--- PASS: TestProvider_impl (0.27s)
--- SKIP: TestAccAWSSSOAdminManagedPolicyAttachment_basic (0.89s)
--- SKIP: TestAccAWSSSOAdminManagedPolicyAttachment_disappears (0.76s)
--- SKIP: TestAccAWSSSOAdminManagedPolicyAttachment_disappears_permissionSet (0.77s)
--- SKIP: TestAccAWSSSOAdminManagedPolicyAttachment_forceNew (1.11s)
--- SKIP: TestAccAWSSSOAdminManagedPolicyAttachment_multipleManagedPolicies (1.23s)
--- SKIP: TestAccAWSSSOAdminPermissionSetInlinePolicy_basic (1.04s)
--- SKIP: TestAccAWSSSOAdminPermissionSetInlinePolicy_disappears (1.08s)
--- SKIP: TestAccAWSSSOAdminPermissionSetInlinePolicy_disappears_permissionSet (0.88s)
--- SKIP: TestAccAWSSSOAdminPermissionSetInlinePolicy_update (1.14s)
--- SKIP: TestAccAWSSSOAdminPermissionSet_basic (1.10s)
--- SKIP: TestAccAWSSSOAdminPermissionSet_mixedPolicyAttachments (1.05s)
--- SKIP: TestAccAWSSSOAdminPermissionSet_tags (1.25s)
--- SKIP: TestAccAWSSSOAdminPermissionSet_updateDescription (1.06s)
--- SKIP: TestAccAWSSSOAdminPermissionSet_updateRelayState (0.88s)
--- SKIP: TestAccAWSSSOAdminPermissionSet_updateSessionDuration (0.96s)
--- SKIP: TestAccDataSourceAWSSSOAdminInstances_basic (1.12s)
--- SKIP: TestAccDataSourceAWSSSOAdminPermissionSet_arn (1.17s)
--- SKIP: TestAccDataSourceAWSSSOAdminPermissionSet_name (1.08s)
--- SKIP: TestAccDataSourceAWSSSOAdminPermissionSet_nonExistent (0.95s)

[anGie44]

Hi @lawdhavmercy @burck1, thank you for this PR and your patience! Are you still interested and available to work on this? We have been unable to get to the SSO service until now but we aim to get this work in before the end of the year.

To be upfront about the process, here is what to expect: A design for this service, essentially covering the resources/data-sources we feel are essential for a practitioner's enablement, has been created internally and is currently in review by our team. Once the design is approved, implementation will need to follow the design. Hopefully, we can use some or all of your work to support this core service. We won't know until we have consensus on the design, however. Once we have approval, I'll circle back here and provide an update/initial code review.

Currently the list of data-sources/resources we are considering (per RoadMap and some of which are covered in this PR as well as in #15322) include:

Resources:

• aws_ssoadmin_permission_set
• aws_ssoadmin_permission_set_inline_policy
• aws_ssoadmin_managed_policy_attachment
• aws_ssoadmin_account_assignment

DataSources:

• aws_ssoadmin_instance(s)
• aws_identitystore_user(s)
• aws_identitystore_group(s)

**Note: to keep inline with the service package -> resource/data-source naming, we'll be using the ssoadmin prefix and identitystore where applicable

[burck1]

Hi @anGie44! Great to hear. Once the design is finalized, we'd be happy to help migrate our code to the updated design. Feel free to reach out as soon as the design is ready.

One nuance of the ssoadmin apis that may help inform your design is the ProvisionPermissionSet request. This API must be called after any updates to the permission set resource or else the change will not actually be propagated to the assigned accounts. This is in fact the reason why we decided not to create separate aws_ssoadmin_permission_set_inline_policy and aws_ssoadmin_managed_policy_attachment resources and instead made those properties of the permission set resource. I.E. Whenever those policies are modified, the ProvisionPermissionSet API must be called. Separating out those resources means you would need to call the ProvisionPermissionSet API within the update function for each of those resources which means you would run into conflicts whenever more than one of those resources has changed. The API prevents multiple provisionings to the same assigned account from happening at once. The solution we ended up coming up with was simply to incorporate the inline and managed policies as properties of the permission set resource so that we would only need to call the ProvisionPermissionSet API one time within the permission set resource's update function. I discussed this a bit in this comment: #15322 (comment)

Let us know if you have any questions.

Note: We have been running our custom version of the aws provider with the changes from #15322 in production for over a month now without any problems.

[burck1]

The code for calling the ProvisionPermissionSet API can be found here:

terraform-provider-aws/aws/resource_aws_sso_permission_set.go

Lines 483 to 506 in 1b750b9

	// Reprovision if anything has changed
	if d.HasChanges("description", "relay_state", "session_duration", "inline_policy", "managed_policy_arns", "tags") {
	// Auto provision all accounts
	targetType := ssoadmin.ProvisionTargetTypeAllProvisionedAccounts
	provisionInput := &ssoadmin.ProvisionPermissionSetInput{
	InstanceArn: aws.String(instanceArn),
	PermissionSetArn: aws.String(permissionSetArn),
	TargetType: aws.String(targetType),
	}
	log.Printf("[INFO] Provisioning AWS SSO Permission Set")
	provisionResponse, err := ssoadminconn.ProvisionPermissionSet(provisionInput)
	if err != nil {
	return fmt.Errorf("Error provisioning AWS SSO Permission Set (%s): %w", d.Id(), err)
	}
	status := provisionResponse.PermissionSetProvisioningStatus
	_, waitErr := waitForPermissionSetProvisioning(ssoadminconn, instanceArn, aws.StringValue(status.RequestId), d.Timeout(schema.TimeoutUpdate))
	if waitErr != nil {
	return waitErr
	}
	}

[anGie44]

Thanks for clarifying @burck1 -- definitely to our advantage that these changes have been tested in production 👍 My only hesitation in not moving out the policy related attributes to their own resources is the need to make various API calls to gather these attributes on import/read which strays a bit from convention. You mentioned in your testing you came across the need to call ProvisionPermissionSet on all types of resource updates, though in the API docs, AWS only explicitly notes to use ProvisionPermissionSet after use of AttachManagedPolicyToPermissionSet and PutInlinePolicyToPermissionSet and interestingly not on UpdatePermissionSet 🤔 -- just wanted to clarify if you confirmed it's indeed necessary to call ProvisionPersimissionSet on all sort of updates? (definitely could be a miss though in the AWS documentation)

[burck1]

Hi @anGie44. Some great questions!

My only hesitation in not moving out the policy related attributes to their own resources is the need to make various API calls to gather these attributes on import/read which strays a bit from convention.

True that is strays from convention. Unfortunately we couldn't find any other AWS resources in this repo that we could model our changes after that also had to follow this pattern of calling a different API (like ProvisionPermissionSet) after any changes.

You mentioned in your testing you came across the need to call ProvisionPermissionSet on all types of resource updates, though in the API docs, AWS only explicitly notes to use ProvisionPermissionSet after use of AttachManagedPolicyToPermissionSet and PutInlinePolicyToPermissionSet and interestingly not on UpdatePermissionSet 🤔 -- just wanted to clarify if you confirmed it's indeed necessary to call ProvisionPersimissionSet on all sort of updates? (definitely could be a miss though in the AWS documentation)

I think this is a miss in the AWS documentation. I just tested this out with the AWS CLI (IDs are obfuscated). Currently the only way to tell if you need to call the ProvisionPersimissionSet API for a given account assignment is to call the ListAccountsForProvisionedPermissionSet API with the ProvisioningStatus parameter set to LATEST_PERMISSION_SET_NOT_PROVISIONED. This tells you that the permission set has been updated, but the change has not been pushed to assigned account(s). I've included this call in the CLI output.

# This is the current permission set settings
$ aws sso-admin describe-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111"
{
    "PermissionSet": {
        "Name": "alex-test",
        "PermissionSetArn": "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111",
        "Description": "before update",
        "CreatedDate": "2020-12-11T17:18:37.654000-06:00",
        "SessionDuration": "PT1H"
    }
}

# There is currently one account assigned to the permission set that has been provisioned with the latest changes
$ aws sso-admin list-accounts-for-provisioned-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --provisioning-status LATEST_PERMISSION_SET_PROVISIONED
{
    "AccountIds": [
        "012345678910"
    ]
}

# There is currently no accounts assigned to the permission set that have not been provisioned with the latest changes
$ aws sso-admin list-accounts-for-provisioned-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --provisioning-status LATEST_PERMISSION_SET_NOT_PROVISIONED
{
    "AccountIds": []
}

# Let's update the description of the permission set
$ aws sso-admin update-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --description "after update"

# We can see the updated description
$ aws sso-admin describe-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111"
{
    "PermissionSet": {
        "Name": "alex-test",
        "PermissionSetArn": "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111",
        "Description": "after update",
        "CreatedDate": "2020-12-11T17:18:37.654000-06:00",
        "SessionDuration": "PT1H"
    }
}

# but now there are currently no accounts assigned to the permission set that have been provisioned with the latest changes
$ aws sso-admin list-accounts-for-provisioned-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --provisioning-status LATEST_PERMISSION_SET_PROVISIONED    
{
    "AccountIds": []
}

# and our account now shows up as not being provisioned with the latest changes
$ aws sso-admin list-accounts-for-provisioned-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --provisioning-status LATEST_PERMISSION_SET_NOT_PROVISIONED
{
    "AccountIds": [
        "012345678910"
    ]
}

# so we can now call provision-permission-set
$ aws sso-admin provision-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --target-type ALL_PROVISIONED_ACCOUNTS
{
    "PermissionSetProvisioningStatus": {
        "Status": "IN_PROGRESS",
        "RequestId": "47ac8a85-2212-41d6-8bde-c69a7a81c834",
        "PermissionSetArn": "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111"
    }
}

# and await for status to be SUCCEEDED
$ aws sso-admin describe-permission-set-provisioning-status --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --provision-permission-set-request-id "47ac8a85-2212-41d6-8bde-c69a7a81c834"
{
    "PermissionSetProvisioningStatus": {
        "Status": "SUCCEEDED",
        "RequestId": "47ac8a85-2212-41d6-8bde-c69a7a81c834",
        "PermissionSetArn": "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111",
        "CreatedDate": "2020-12-11T17:50:26.049000-06:00"
    }
}

# and now our account is back to having the latest permission set changes provisioned
$ aws sso-admin list-accounts-for-provisioned-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --provisioning-status LATEST_PERMISSION_SET_PROVISIONED
{
    "AccountIds": [
        "012345678910"
    ]
}

# and none that are not at the latest
$ aws sso-admin list-accounts-for-provisioned-permission-set --instance-arn "arn:aws:sso:::instance/ssoins-9999999999999999" --permission-set-arn "arn:aws:sso:::permissionSet/ssoins-9999999999999999/ps-1111111111111111" --provisioning-status LATEST_PERMISSION_SET_NOT_PROVISIONED
{
    "AccountIds": []
}

Note: We also need to call ProvisionPersimissionSet after any calls to TagResource or UntagResource, even though that is not documented either.

I can also provide some screenshots of the AWS console as well if that helps.

[burck1]

Note: to keep inline with the service package -> resource/data-source naming, we'll be using the ssoadmin prefix and identitystore where applicable

Regarding this note, does it make sense for @lawdhavmercy and I to go ahead and update this PR and #15322 as follows?

Resources

• aws_sso_permission_set → aws_ssoadmin_permission_set
• aws_sso_assignment → aws_ssoadmin_assignment

Data Sources

• aws_sso_instance → aws_ssoadmin_instance
• aws_sso_permission_set → aws_ssoadmin_permission_set
• aws_identity_store_group → aws_identitystore_group
• aws_identity_store_user → aws_identitystore_user

[burck1]

Also, regarding the aws_ssoadmin_instance data source, although the ListInstances API responds with a list, based on our current understanding of the AWS SSO service that API can only ever return zero or one instances. Therefore, for this pull request, we have coded that data source to only ever return the first instance in the response from that API and we return an error if more than one instances are returned.

terraform-provider-aws/aws/data_source_aws_sso_instance.go

Lines 53 to 55 in 1b750b9

	if len(instances) > 1 {
	return fmt.Errorf("Found multiple AWS SSO Instances. Not sure which one to use. %s", instances)
	}

We could not find a way to create multiple AWS SSO Instances from the AWS console, so we think that AWS just decided to design the ListInstances API like the ListAccountAliases API in that they can only ever return zero or one instances, but our assumption could be flawed here. It may be a good idea to get in contact with AWS to see if this assumption is correct. It would be unfortunate if it was possible for there to be more than one sso instance because that would mean it would require users of the aws_ssoadmin_instance data source to hardcode their instance arn and identity store id in their terraform as sso instances don't have any other non-id properties (like a name) that we can filter on.

[anGie44]

It would be unfortunate if it was possible for there to be more than one sso instance because that would mean it would require users of the aws_ssoadmin_instance data source to hardcode their instance arn and identity store id in their terraform as sso instances don't have any other non-id properties (like a name) that we can filter on.

Hmm given the API method seems to imply more than 1, we could proactively account for this by making the data source plural i.e. have a field for instance_arns and one for identity_store_ids . the data_source_aws_efs_access_points comes to mind as a reference in keeping track of multiple plural fields.

[anGie44]

Thanks again for your all your contributions and time @burck1 @lawdhavmercy 🚀 🚀

[ghost]

This has been released in version 3.23.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!