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

resource/cognito_user_pool: Modify update params #3458

Merged
merged 3 commits into from
Aug 9, 2018

Conversation

atsushi-ishibashi
Copy link
Contributor

closed: #3389

TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoUserPool_update -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_update
--- PASS: TestAccAWSCognitoUserPool_update (107.53s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	107.564s

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 20, 2018
@bflad bflad added bug Addresses a defect in current functionality. service/cognito labels Feb 21, 2018
@atsushi-ishibashi
Copy link
Contributor Author

@Ninir @bflad Could you increase the priority of this PR?
We have a trouble that when you modify templates, auto_verified_attributes becomes empty.
So I hope the next release includes the fix🙇

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 22, 2018
@atsushi-ishibashi
Copy link
Contributor Author

@Ninir @radeksimko @bflad Please review this PR:bow:

Copy link
Contributor

@Ninir Ninir left a comment

Choose a reason for hiding this comment

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

Hey @atsushi-ishibashi

Thanks for the work here! 👍 🚀

However, these changes are bringing a few issues:

With your changes

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoUserPool_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoUserPool_ -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_importBasic
--- PASS: TestAccAWSCognitoUserPool_importBasic (26.36s)
=== RUN   TestAccAWSCognitoUserPool_basic
--- PASS: TestAccAWSCognitoUserPool_basic (17.83s)
=== RUN   TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration
--- PASS: TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration (28.09s)
=== RUN   TestAccAWSCognitoUserPool_withDeviceConfiguration
--- PASS: TestAccAWSCognitoUserPool_withDeviceConfiguration (29.52s)
=== RUN   TestAccAWSCognitoUserPool_withEmailVerificationMessage
--- FAIL: TestAccAWSCognitoUserPool_withEmailVerificationMessage (23.77s)
	testing.go:513: Step 1 error: Check failed: 2 error(s) occurred:
		
		* Check 1/2 error: aws_cognito_user_pool.pool: Attribute 'email_verification_subject' expected "kywb3brq9t", got "gfx6wxxtxl"
		* Check 2/2 error: aws_cognito_user_pool.pool: Attribute 'email_verification_message' expected "nslc6ysekw {####}", got "xm27mx2s8f {####}"
=== RUN   TestAccAWSCognitoUserPool_withSmsVerificationMessage
--- FAIL: TestAccAWSCognitoUserPool_withSmsVerificationMessage (23.91s)
	testing.go:513: Step 1 error: Check failed: 1 error(s) occurred:
		
		* Check 2/2 error: aws_cognito_user_pool.pool: Attribute 'sms_verification_message' expected "1akj41vkyb {####}", got "9600w67vku {####}"
=== RUN   TestAccAWSCognitoUserPool_withEmailConfiguration
--- PASS: TestAccAWSCognitoUserPool_withEmailConfiguration (27.23s)
=== RUN   TestAccAWSCognitoUserPool_withSmsConfiguration
--- PASS: TestAccAWSCognitoUserPool_withSmsConfiguration (39.99s)
=== RUN   TestAccAWSCognitoUserPool_withSmsConfigurationUpdated
--- PASS: TestAccAWSCognitoUserPool_withSmsConfigurationUpdated (48.31s)
=== RUN   TestAccAWSCognitoUserPool_withTags
--- PASS: TestAccAWSCognitoUserPool_withTags (27.78s)
=== RUN   TestAccAWSCognitoUserPool_withAliasAttributes
--- PASS: TestAccAWSCognitoUserPool_withAliasAttributes (30.22s)
=== RUN   TestAccAWSCognitoUserPool_withPasswordPolicy
--- PASS: TestAccAWSCognitoUserPool_withPasswordPolicy (27.88s)
=== RUN   TestAccAWSCognitoUserPool_withLambdaConfig
--- PASS: TestAccAWSCognitoUserPool_withLambdaConfig (51.75s)
=== RUN   TestAccAWSCognitoUserPool_withSchemaAttributes
--- PASS: TestAccAWSCognitoUserPool_withSchemaAttributes (27.90s)
=== RUN   TestAccAWSCognitoUserPool_withVerificationMessageTemplate
--- FAIL: TestAccAWSCognitoUserPool_withVerificationMessageTemplate (37.50s)
	testing.go:506: Step 2, expected error:
		
		After applying this step, the plan was not empty:
		
		DIFF:
		
		UPDATE: aws_cognito_user_pool.pool
		  sms_verification_message: "{####} Baz" => ""
		
		STATE:
		
		aws_cognito_user_pool.pool:
		  ID = eu-west-1_fAueoFfIy
		  provider = provider.aws
		  admin_create_user_config.# = 1
		  admin_create_user_config.0.allow_admin_create_user_only = false
		  admin_create_user_config.0.invite_message_template.# = 0
		  admin_create_user_config.0.unused_account_validity_days = 7
		  arn = arn:aws:cognito-idp:eu-west-1:512425551441:userpool/eu-west-1_fAueoFfIy
		  creation_date = 2018-03-12T11:08:40Z
		  device_configuration.# = 0
		  email_configuration.# = 0
		  email_verification_message = Foo {####} Bar
		  email_verification_subject = FooBar {####}
		  lambda_config.# = 0
		  last_modified_date = 2018-03-12T11:09:03Z
		  mfa_configuration = OFF
		  name = terraform-test-pool-d6pv9
		  password_policy.# = 1
		  password_policy.0.minimum_length = 8
		  password_policy.0.require_lowercase = true
		  password_policy.0.require_numbers = true
		  password_policy.0.require_symbols = true
		  password_policy.0.require_uppercase = true
		  sms_configuration.# = 0
		  sms_verification_message = {####} Baz
		  tags.% = 0
		  verification_message_template.# = 1
		  verification_message_template.0.default_email_option = CONFIRM_WITH_CODE
		  verification_message_template.0.email_message = Foo {####} Bar
		  verification_message_template.0.email_message_by_link = {##foobar##}
		  verification_message_template.0.email_subject = FooBar {####}
		  verification_message_template.0.email_subject_by_link = foobar
		  verification_message_template.0.sms_message = {####} Baz
		
		To match:
		
		cannot be set to nil
		
		
=== RUN   TestAccAWSCognitoUserPool_update
--- PASS: TestAccAWSCognitoUserPool_update (67.71s)

Without changes (on master branch)

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoUserPool_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoUserPool_ -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_importBasic
--- PASS: TestAccAWSCognitoUserPool_importBasic (20.33s)
=== RUN   TestAccAWSCognitoUserPool_basic
--- PASS: TestAccAWSCognitoUserPool_basic (18.95s)
=== RUN   TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration
--- PASS: TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration (29.21s)
=== RUN   TestAccAWSCognitoUserPool_withDeviceConfiguration
--- PASS: TestAccAWSCognitoUserPool_withDeviceConfiguration (30.48s)
=== RUN   TestAccAWSCognitoUserPool_withEmailVerificationMessage
--- PASS: TestAccAWSCognitoUserPool_withEmailVerificationMessage (28.69s)
=== RUN   TestAccAWSCognitoUserPool_withSmsVerificationMessage
--- PASS: TestAccAWSCognitoUserPool_withSmsVerificationMessage (29.18s)
=== RUN   TestAccAWSCognitoUserPool_withEmailConfiguration
--- PASS: TestAccAWSCognitoUserPool_withEmailConfiguration (28.99s)
=== RUN   TestAccAWSCognitoUserPool_withSmsConfiguration
--- PASS: TestAccAWSCognitoUserPool_withSmsConfiguration (35.29s)
=== RUN   TestAccAWSCognitoUserPool_withSmsConfigurationUpdated
--- FAIL: TestAccAWSCognitoUserPool_withSmsConfigurationUpdated (36.68s)
	testing.go:513: Step 1 error: Error applying: 1 error(s) occurred:
		
		* aws_cognito_user_pool.pool: 1 error(s) occurred:
		
		* aws_cognito_user_pool.pool: Error updating Cognito User pool: NotAuthorizedException: Error when calling SMS: The security token included in the request is invalid. (Service: AmazonSNS; Status Code: 403; Error Code: InvalidClientTokenId; Request ID: 715bdec9-5e67-5775-aadc-8de903a2889d)
			status code: 400, request id: 36c6016e-25e3-11e8-81bc-cd1354da4c20
=== RUN   TestAccAWSCognitoUserPool_withTags
--- PASS: TestAccAWSCognitoUserPool_withTags (29.06s)
=== RUN   TestAccAWSCognitoUserPool_withAliasAttributes
--- PASS: TestAccAWSCognitoUserPool_withAliasAttributes (30.99s)
=== RUN   TestAccAWSCognitoUserPool_withPasswordPolicy
--- PASS: TestAccAWSCognitoUserPool_withPasswordPolicy (28.53s)
=== RUN   TestAccAWSCognitoUserPool_withLambdaConfig
--- PASS: TestAccAWSCognitoUserPool_withLambdaConfig (43.26s)
=== RUN   TestAccAWSCognitoUserPool_withSchemaAttributes
--- PASS: TestAccAWSCognitoUserPool_withSchemaAttributes (29.86s)
=== RUN   TestAccAWSCognitoUserPool_withVerificationMessageTemplate
--- PASS: TestAccAWSCognitoUserPool_withVerificationMessageTemplate (34.39s)

Can you fix them before merging?

Thanks!

@Puneeth-n
Copy link
Contributor

Puneeth-n commented Mar 14, 2018

Is it a fix to what I am seeing in #3780 #3009

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Mar 28, 2018
@atsushi-ishibashi
Copy link
Contributor Author

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoUserPool_ -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_importBasic
--- PASS: TestAccAWSCognitoUserPool_importBasic (42.60s)
=== RUN   TestAccAWSCognitoUserPool_basic
--- PASS: TestAccAWSCognitoUserPool_basic (43.12s)
=== RUN   TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration
--- PASS: TestAccAWSCognitoUserPool_withAdminCreateUserConfiguration (68.92s)
=== RUN   TestAccAWSCognitoUserPool_withDeviceConfiguration
--- PASS: TestAccAWSCognitoUserPool_withDeviceConfiguration (66.87s)
=== RUN   TestAccAWSCognitoUserPool_withEmailVerificationMessage
--- PASS: TestAccAWSCognitoUserPool_withEmailVerificationMessage (70.04s)
=== RUN   TestAccAWSCognitoUserPool_withSmsVerificationMessage
--- PASS: TestAccAWSCognitoUserPool_withSmsVerificationMessage (59.70s)
=== RUN   TestAccAWSCognitoUserPool_withEmailConfiguration
--- PASS: TestAccAWSCognitoUserPool_withEmailConfiguration (62.97s)
=== RUN   TestAccAWSCognitoUserPool_withSmsConfiguration
--- PASS: TestAccAWSCognitoUserPool_withSmsConfiguration (62.66s)
=== RUN   TestAccAWSCognitoUserPool_withSmsConfigurationUpdated
--- PASS: TestAccAWSCognitoUserPool_withSmsConfigurationUpdated (86.23s)
=== RUN   TestAccAWSCognitoUserPool_withTags
--- PASS: TestAccAWSCognitoUserPool_withTags (55.70s)
=== RUN   TestAccAWSCognitoUserPool_withAliasAttributes
--- PASS: TestAccAWSCognitoUserPool_withAliasAttributes (58.67s)
=== RUN   TestAccAWSCognitoUserPool_withPasswordPolicy
--- PASS: TestAccAWSCognitoUserPool_withPasswordPolicy (56.31s)
=== RUN   TestAccAWSCognitoUserPool_withLambdaConfig
--- PASS: TestAccAWSCognitoUserPool_withLambdaConfig (82.43s)
=== RUN   TestAccAWSCognitoUserPool_withSchemaAttributes
--- PASS: TestAccAWSCognitoUserPool_withSchemaAttributes (57.57s)
=== RUN   TestAccAWSCognitoUserPool_withVerificationMessageTemplate
--- FAIL: TestAccAWSCognitoUserPool_withVerificationMessageTemplate (77.39s)
	testing.go:494: Step 2, no error received, but expected a match to:
		
		cannot be set to nil
		
		
=== RUN   TestAccAWSCognitoUserPool_update
--- PASS: TestAccAWSCognitoUserPool_update (109.77s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	1060.993s
make: *** [testacc] Error 1

What is the purpose of below test case? @Ninir

{
	Config:      testAccAWSCognitoUserPoolConfig_basic(name),
	ExpectError: regexp.MustCompile(`cannot be set to nil`),
},

@atsushi-ishibashi
Copy link
Contributor Author

@Ninir @bflad What is the status of this PR?

@loivis
Copy link
Contributor

loivis commented May 9, 2018

Hit the same issue in prod 😭. Call for attention @Ninir @bflad

@Puneeth-n
Copy link
Contributor

@loivis I also had the same issue. auto_verified_attributes = email was somehow overwritten in all our cognito pools and it was pretty random. We use aws lambda for generating custom_message. Somehow strangely adding IAM permissions to our lambda function solved it for us.

        {
            "Sid": "CognitoUserPoolAccess",
            "Action": "cognito-idp:*",
            "Effect": "Allow",
            "Resource": "*"
        }

@loivis
Copy link
Contributor

loivis commented Jun 25, 2018

@Ninir @bflad Calling again 🙌

Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Sorry for the delay here, folks. This PR seems good to me. We also just merged a similar fix for the aws_cognito_user_pool_client resource. The failing test after the code updates seems invalid to me given the new usage of d.GetOk(). I'm going to remove those cannot be set to nil conditionals as well as the failing test in a commit after these so we can get this bug fix in the next provider release.

Thanks @atsushi-ishibashi 🚀

@bflad bflad added this to the v1.32.0 milestone Aug 9, 2018
@bflad bflad merged commit bf5819f into hashicorp:master Aug 9, 2018
bflad added a commit that referenced this pull request Aug 9, 2018
@bflad
Copy link
Contributor

bflad commented Aug 16, 2018

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

@ghost
Copy link

ghost commented Apr 4, 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 and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

resource/cognito_user_pool change unexpected attributes every apply
5 participants