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

r/aws_cognito_user_pool - add support for account recovery setting #12444

Merged
merged 12 commits into from
Nov 30, 2020

Conversation

DrFaust92
Copy link
Collaborator

@DrFaust92 DrFaust92 commented Mar 18, 2020

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

Closes #11220
Relates #13826

Release note for CHANGELOG:

resouce_aws_cognito_user_pool: add support for account recovery setting

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSCognitoUserPool_'
--- PASS: TestAccAWSCognitoUserPool_basic (48.59s)
--- PASS: TestAccAWSCognitoUserPool_recovery (114.12s)
--- PASS: TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration (103.63s)
--- PASS: TestAccAWSCognitoUserPool_withAdminCreateUserConfigurationAndPasswordPolicy (47.56s)
--- PASS: TestAccAWSCognitoUserPool_withDeviceConfiguration (78.45s)
--- PASS: TestAccAWSCognitoUserPool_withEmailVerificationMessage (85.15s)
--- PASS: TestAccAWSCognitoUserPool_MfaConfiguration_SmsConfiguration (147.53s)
--- PASS: TestAccAWSCognitoUserPool_MfaConfiguration_SmsConfigurationAndSoftwareTokenMfaConfiguration (129.68s)
--- PASS: TestAccAWSCognitoUserPool_MfaConfiguration_SmsConfigurationToSoftwareTokenMfaConfiguration (91.63s)
--- PASS: TestAccAWSCognitoUserPool_MfaConfiguration_SoftwareTokenMfaConfiguration (108.50s)
--- PASS: TestAccAWSCognitoUserPool_MfaConfiguration_SoftwareTokenMfaConfigurationToSmsConfiguration (101.67s)
--- PASS: TestAccAWSCognitoUserPool_SmsAuthenticationMessage (81.63s)
--- PASS: TestAccAWSCognitoUserPool_SmsConfiguration (129.93s)
--- PASS: TestAccAWSCognitoUserPool_SmsConfiguration_ExternalId (121.34s)
--- PASS: TestAccAWSCognitoUserPool_SmsConfiguration_SnsCallerArn (109.63s)
--- PASS: TestAccAWSCognitoUserPool_SmsVerificationMessage (79.68s)
--- PASS: TestAccAWSCognitoUserPool_withTags (111.92s)
--- PASS: TestAccAWSCognitoUserPool_withAliasAttributes (79.91s)
--- PASS: TestAccAWSCognitoUserPool_withPasswordPolicy (79.13s)
--- PASS: TestAccAWSCognitoUserPool_withUsernameConfiguration (80.23s)
--- PASS: TestAccAWSCognitoUserPool_withLambdaConfig (126.40s)
--- PASS: TestAccAWSCognitoUserPool_withSchemaAttributes (84.25s)
--- PASS: TestAccAWSCognitoUserPool_withVerificationMessageTemplate (77.63s)
--- PASS: TestAccAWSCognitoUserPool_update (143.22s)

--- FAIL: TestAccAWSCognitoUserPool_withEmailConfiguration (63.24s)

TestAccAWSCognitoUserPool_withEmailConfiguration seems to have failed but i dont think its relevant the changes as it failed on:

2020/03/18 13:57:20 [ERROR] <root>: eval: *terraform.EvalApplyPost, err: Error updating Cognito User pool: InvalidParameterException: Unable to send email message, please try again
2020/03/18 13:57:20 [ERROR] <root>: eval: *terraform.EvalSequence, err: Error updating Cognito User pool: InvalidParameterException: Unable to send email message, please try again
2020/03/18 13:57:20 [DEBUG] New state was assigned lineage "b19a8bb8-2e21-41f7-1e30-be8390d53f9f"
    TestAccAWSCognitoUserPool_withEmailConfiguration: testing.go:654: Step 2 error: errors during apply:
        
        Error: Error updating Cognito User pool: InvalidParameterException: Unable to send email message, please try again
        
          on C:\Users\ilial\AppData\Local\Temp\tf-test650923783\main.tf line 2:
          (source code not available)

@DrFaust92 DrFaust92 requested a review from a team March 18, 2020 11:42
@ghost ghost added needs-triage Waiting for first response or review from a maintainer. size/L Managed by automation to categorize the size of a PR. service/cognito tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. labels Mar 18, 2020
@DrFaust92 DrFaust92 changed the title [WIP]r/aws_cognito_user_pool - add support for account recovery setting r/aws_cognito_user_pool - add support for account recovery setting Mar 18, 2020
@ewbankkit
Copy link
Contributor

Verified acceptance test:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSCognitoUserPool_recovery'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSCognitoUserPool_recovery -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_recovery
=== PAUSE TestAccAWSCognitoUserPool_recovery
=== CONT  TestAccAWSCognitoUserPool_recovery
--- PASS: TestAccAWSCognitoUserPool_recovery (61.23s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	61.273s

@michaelgmcd
Copy link

Hello, what's the status on this?

@judedaryl
Copy link
Contributor

bump for status

@emulanob
Copy link

emulanob commented Jun 3, 2020

Hi, is this planned to be merged any time soon?

@seanlynch
Copy link

I've been using a build of the AWS provider with this PR merged to manage some rather complicated AWS infrastructure including multiple Cognito pools for over a month now with no problems. Can one of the maintainers please explain what still remains to be done for this to be mergeable? The lack of this feature is causing a lot of extra work for a lot of people.

@DrFaust92 DrFaust92 added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Jul 28, 2020
@DrFaust92 DrFaust92 force-pushed the r/cognito-user-pool-account-rec branch from 3906ad8 to 9a47702 Compare July 28, 2020 22:08
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. provider Pertains to the provider itself, rather than any interaction with AWS. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 29, 2020
@DrFaust92
Copy link
Collaborator Author

rebased and tested

@ewbankkit ewbankkit removed the request for review from a team July 29, 2020 13:16
@DrFaust92
Copy link
Collaborator Author

renamed recovery_mechanisms to recovery_mechanism and tested.

@DrFaust92 DrFaust92 requested a review from ewbankkit July 29, 2020 13:31
@DrFaust92 DrFaust92 force-pushed the r/cognito-user-pool-account-rec branch from 1472e0d to 112d638 Compare August 21, 2020 21:22
@jeffrystev
Copy link

Did the above issue resolved ? Do we have any new parameters in terraform in order to update the recovery of account configuration in cognito . If so kindly update the newly added parameter over here.

@cfernandez-outset
Copy link

is this merged? thank you

@seanlynch
Copy link

What's missing in order to merge this? It's costing us a lot to have to maintain our own build of the AWS provider just to get this one missing feature.

Copy link
Contributor

@bill-rich bill-rich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one minor capitalization fix. Other than that, looks good.

website/docs/r/cognito_user_pool.markdown Outdated Show resolved Hide resolved
@DrFaust92 DrFaust92 requested a review from a team as a code owner November 30, 2020 17:05
Copy link
Contributor

@bill-rich bill-rich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bill-rich bill-rich merged commit 9d66b99 into hashicorp:master Nov 30, 2020
@breathingdust breathingdust added this to the v3.19.0 milestone Dec 1, 2020
@ghost
Copy link

ghost commented Dec 1, 2020

This has been released in version 3.19.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!

@rpstreef
Copy link

rpstreef commented Dec 2, 2020

What's missing in order to merge this? It's costing us a lot to have to maintain our own build of the AWS provider just to get this one missing feature.

this is a regular issue with Terraform providers to get new features implemented in a timely manner.

Perhaps there's a good reason for it, but it might be worth reviewing why time to release is 6 months+ and sometimes even 12 months + and see if there are ways to reduce this.

@ghost
Copy link

ghost commented Dec 31, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 31, 2020
@DrFaust92 DrFaust92 deleted the r/cognito-user-pool-account-rec branch April 15, 2021 10:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. provider Pertains to the provider itself, rather than any interaction with AWS. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set account recovery preference