Migrate the Identity Store service to AWS Go SDK v2

[roberth-k]

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 #26598

AWS Go SDK v2 is required to update user and group attributes. (https://docs.aws.amazon.com/singlesignon/latest/IdentityStoreAPIReference/API_AttributeOperation.html)

The Filter attribute of the ListUsers and ListGroups API-s has been deprecated in favour of GetUserId and GetGroupId, which would also lead to the deprecation of the filters argument to the data sources (where only one type of query is supported anyway). However, I think it'd be simpler to introduce this deprecation after the resources have been implemented, as there's an overlap of finders.

Output from acceptance testing:

$ make testacc TESTS=TestAccIdentityStore PKG=identitystore
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/identitystore/... -v -count 1 -parallel 20 -run='TestAccIdentityStore'  -timeout 180m
=== RUN   TestAccIdentityStoreGroupDataSource_displayName
=== PAUSE TestAccIdentityStoreGroupDataSource_displayName
=== RUN   TestAccIdentityStoreGroupDataSource_groupID
=== PAUSE TestAccIdentityStoreGroupDataSource_groupID
=== RUN   TestAccIdentityStoreGroupDataSource_nonExistent
=== PAUSE TestAccIdentityStoreGroupDataSource_nonExistent
=== RUN   TestAccIdentityStoreUserDataSource_userName
=== PAUSE TestAccIdentityStoreUserDataSource_userName
=== RUN   TestAccIdentityStoreUserDataSource_userID
=== PAUSE TestAccIdentityStoreUserDataSource_userID
=== RUN   TestAccIdentityStoreUserDataSource_nonExistent
=== PAUSE TestAccIdentityStoreUserDataSource_nonExistent
=== CONT  TestAccIdentityStoreGroupDataSource_displayName
=== CONT  TestAccIdentityStoreUserDataSource_userName
=== CONT  TestAccIdentityStoreGroupDataSource_nonExistent
=== CONT  TestAccIdentityStoreGroupDataSource_groupID
=== CONT  TestAccIdentityStoreUserDataSource_nonExistent
=== CONT  TestAccIdentityStoreUserDataSource_userID
--- PASS: TestAccIdentityStoreGroupDataSource_nonExistent (2.57s)
--- PASS: TestAccIdentityStoreUserDataSource_nonExistent (2.59s)
--- PASS: TestAccIdentityStoreUserDataSource_userName (8.24s)
--- PASS: TestAccIdentityStoreGroupDataSource_displayName (8.33s)
--- PASS: TestAccIdentityStoreGroupDataSource_groupID (8.37s)
--- PASS: TestAccIdentityStoreUserDataSource_userID (8.43s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/identitystore   10.129s

[github-actions]

Thank you for your contribution! 🚀

Please note that typically Go dependency changes are handled in this repository by dependabot or the maintainers. This is to prevent pull request merge conflicts and further delay reviews of contributions. Remove any changes to the go.mod or go.sum files and commit them into this pull request.

Additional details:

• Check open pull requests with the dependencies label to view other dependency updates.
• If this pull request includes an update the AWS Go SDK (or any other dependency) version, only updates submitted via dependabot will be merged. This pull request will need to remove these changes and will need to be rebased after the existing dependency update via dependabot has been merged for this pull request to be reviewed.
• If this pull request is for supporting a new AWS service:
  • Ensure the new AWS service changes are following the Contributing Guide section on new services, in particular that the dependency addition and initial provider support are in a separate pull request from other changes (e.g. new resources). Contributions not following this item will not be reviewed until the changes are split.
  • If this pull request is already a separate pull request from the above item, you can ignore this message.

[bschaatsbergen]

LGTM 🚀

[ewbankkit]

LGTM 🚀.

% AWS_IDENTITY_STORE_GROUP_NAME=testgroup1 AWS_IDENTITY_STORE_GROUP_ID=926776f3c6-71733eb8-b181-488e-8ed6-17b07e79380e AWS_IDENTITY_STORE_USER_NAME=testuser1 AWS_IDENTITY_STORE_USER_ID=926776f3c6-53ea24ec-f8f8-4e7c-a015-d238f4ec12fb make testacc TESTARGS='-run=TestAccIdentityStore' PKG=identitystore ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/identitystore/... -v -count 1 -parallel 3  -run=TestAccIdentityStore -timeout 180m
=== RUN   TestAccIdentityStoreGroupDataSource_displayName
=== PAUSE TestAccIdentityStoreGroupDataSource_displayName
=== RUN   TestAccIdentityStoreGroupDataSource_groupID
=== PAUSE TestAccIdentityStoreGroupDataSource_groupID
=== RUN   TestAccIdentityStoreGroupDataSource_nonExistent
=== PAUSE TestAccIdentityStoreGroupDataSource_nonExistent
=== RUN   TestAccIdentityStoreUserDataSource_userName
=== PAUSE TestAccIdentityStoreUserDataSource_userName
=== RUN   TestAccIdentityStoreUserDataSource_userID
=== PAUSE TestAccIdentityStoreUserDataSource_userID
=== RUN   TestAccIdentityStoreUserDataSource_nonExistent
=== PAUSE TestAccIdentityStoreUserDataSource_nonExistent
=== CONT  TestAccIdentityStoreGroupDataSource_displayName
=== CONT  TestAccIdentityStoreUserDataSource_userName
=== CONT  TestAccIdentityStoreUserDataSource_nonExistent
--- PASS: TestAccIdentityStoreUserDataSource_nonExistent (4.56s)
=== CONT  TestAccIdentityStoreGroupDataSource_nonExistent
--- PASS: TestAccIdentityStoreGroupDataSource_nonExistent (2.97s)
=== CONT  TestAccIdentityStoreGroupDataSource_groupID
--- PASS: TestAccIdentityStoreUserDataSource_userName (18.11s)
=== CONT  TestAccIdentityStoreUserDataSource_userID
--- PASS: TestAccIdentityStoreGroupDataSource_displayName (18.29s)
--- PASS: TestAccIdentityStoreGroupDataSource_groupID (16.52s)
--- PASS: TestAccIdentityStoreUserDataSource_userID (13.91s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/identitystore	35.954s

[ewbankkit]

@roberth-k Thanks for the contribution 🎉 👏.

[github-actions]

This functionality has been released in v4.30.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. Thank you!

[jhebert-coursera]

Hi @roberth-k, thanks for adding this! I've been reading through your diff to try to understand exactly what changed. It seems that these changes may have resulted in code taking a different call path with different retry behavior.

With AWS Provider >=4.30.0, I'm no longer able to query users from our IDP and instead we see hundreds of errors like these:

Error: reading AWS SSO Identity Store User Data Source (...): operation error identitystore: ListUsers, failed to get rate limit token, retry quota exceeded, 0 available, 5 requested

  on permission-assignment.tf line 9, in data "aws_identitystore_user" "user_ids":
   9: data "aws_identitystore_user" "user_ids" {

I've dug around to see if there's any sort of config, environment or otherwise, that we may be able to use to work around this to no avail. Do you have any ideas?

One odd thing is that I don't see this behavior from local plans, only within ECS in our VPC. These folks saw a similar issue with DynamoDB but it didn't happen when using the HTTPClient, which makes me wonder if the SDK is using a different API connection type depending on the environment.

Semi-related, given that ListUsers is deprecated now, maybe it would make sense to break here rather than paginating all users just to get that "more than one match" error?

I'd love to hear your thoughts on this since I've been poking around at this for a while now and am not sure who I could chat with to get some ideas. :) It's not clear to me what the right solution is or even what the problem really is at this point...

Thanks!

[roberth-k]

Hi @jhebert-coursera 👋 and thank you for reporting this issue!

This PR purposefully attempted to stick as close to the AWS Go SDK v1 implementation as possible. As it's using the same Identity Store API-s as before, I'm almost certain that the behaviour you're seeing is due to internal differences between the AWS Go SDK v1 and AWS Go SDK v2 that we didn't foresee.

After #26598 is complete, which should not be long, the plan is to re-factor the data sources using the new GetUserId and GetGroupId API-s. There is a chance that these API-s will be more efficient and not hit the rate limiting behaviour that you're seeing.

Alternatively, if you're running bulk queries on users and groups, it's worth keeping an eye out on #26770, which is likely to become the most efficient data source for this purpose.

I couldn't find any documented ways of manipulating this "retry quota" at the SDK level.

@ewbankkit it is possible that the AWS Go SDK v2 is too conservative in its retry strategy, and that this is going to start biting more and more as services start using v2.

[jhebert-coursera]

Hey @roberth-k, thanks for the thoughtful reply! I did end up digging through all of your code and, as far as I could tell, it looks like you were using the standard Retryer functionality correctly. I think you're correct that this is just related to the internal difference that you noted. I also couldn't find a good way to configure the Retryer although we still probably wouldn't be able to pass that config through to Terraform.

After concluding that your code was okay and that I was hitting artificial token bucket limits (I was not hitting any rate limits on the service itself), I tried reducing the parallelism in our Terraform invocations and that resolved my issue.

I am inclined to agree with the sentiment of your question about the rate limiter being too conservative, though. It seems incorrect for the client to rate limit calls if we're not actually hitting AWS limits. We might prefer to disable the rate limiter entirely since we are typically anticipating these failure modes and adding back-pressure accordingly if we hit actual AWS service limits.

#26770 looks great as we are doing bulk queries as you mentioned, to fetch all User IDs for permission set attachments. Thanks for the note about that!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.