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

email_verification_message of cognito_user_pool change every time #2423

Closed
atsushi-ishibashi opened this issue Nov 24, 2017 · 9 comments
Closed
Labels
bug Addresses a defect in current functionality.
Milestone

Comments

@atsushi-ishibashi
Copy link
Contributor

In testAccAWSCognitoUserPoolConfig_withVerificationMessageTemplate, I changed verification_message_template.email_message to "Foo {####} Hoge" and keep email_verification_message as "Foo {####} Bar". Of course I chaged TestCheckResourceAttr as well.
I run TestAccAWSCognitoUserPool_withVerificationMessageTemplate and the below error happened.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoUserPool_withVerificationMessageTemplate'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoUserPool_withVerificationMessageTemplate -timeout 120m
=== RUN   TestAccAWSCognitoUserPool_withVerificationMessageTemplate
--- FAIL: TestAccAWSCognitoUserPool_withVerificationMessageTemplate (22.51s)
	testing.go:503: Step 0 error: After applying this step, the plan was not empty:
		
		DIFF:
		
		UPDATE: aws_cognito_user_pool.pool
		  email_verification_message: "Foo {####} Hoge" => "Foo {####} Bar"
		
		STATE:
		
		aws_cognito_user_pool.pool:
		  ID = us-west-2_VqO35We5K
		  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
		  creation_date = 2017-11-24T04:10:17Z
		  device_configuration.# = 0
		  email_configuration.# = 0
		  email_verification_message = Foo {####} Hoge
		  email_verification_subject = FooBar {####}
		  lambda_config.# = 0
		  last_modified_date = 2017-11-24T04:10:17Z
		  mfa_configuration = OFF
		  name = terraform-test-pool-vg4qv
		  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_LINK
		  verification_message_template.0.email_message = Foo {####} Hoge
		  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
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	22.784s
make: *** [testacc] Error 1

This may be caused by the specification of the relation between verification_message_template.email_message and email_verification_message parameters...
Probably verification_message_template.email_message beats email_verification_message.

@a-suenami
Copy link
Contributor

a-suenami commented Nov 24, 2017

I have the same problem.

My environment is following:

$ terraform -v
Terraform v0.11.0
+ provider.aws v1.3.1

And my .tf file is:

resource "aws_cognito_user_pool" "pool" {
  name = "MyPool"

  auto_verified_attributes = ["email"]

  email_verification_subject = "email_verification_subject"
  email_verification_message = "email_verification_message {####}"

  verification_message_template {
    default_email_option = "CONFIRM_WITH_CODE"
    email_subject = "verification_message_template.email_subject"
    email_message = "verification_message_template.email_message {####}"
  }
}

I successfully created new Cognito User Pool by it, However, I found diff whenever I ran terraform plan. (The .tf file wasn't changed.)

~ aws_cognito_user_pool.pool
    email_verification_message: "verification_message_template.email_message {####}" => "email_verification_message {####}"
    email_verification_subject: "verification_message_template.email_subject" => "email_verification_subject"

And in this time, auto_verified_attributes is still ["email"], but the "email" attribute is no longer verified in actual. It's very important problem, I think.
According to my terraform.tfstate, "email" is still verified attribute.

"auto_verified_attributes.#": "1",
"auto_verified_attributes.881205744": "email",

@atsushi-ishibashi
Copy link
Contributor Author

To prevent confusion, using ConflictsWith is one choice but it will become a breaking-change...
So I think the reasonable solution is to update docs. What do you think? @Ninir

@Ninir Ninir added the bug Addresses a defect in current functionality. label Nov 24, 2017
@Ninir
Copy link
Contributor

Ninir commented Nov 24, 2017

HI folks,

Yeh stumbled upon this while making the resource: it appeared that both are synchronised.
However, the REST API exposes theverification_message_template structure while the Cloudformation section does not, so... 🤷‍♂️

Updating the documentation is fine for me I think. We just need to contact AWS and see if we can specify the 2 OR if one should specify verification_message_template or email_verification_subject.

@tomelliff
Copy link
Contributor

@Ninir I was seeing this issue with the sms_verification_message being nulled on a plan after an apply when I was setting it via the verification_message_template.sms_message. I spoke to AWS support about this and they replied with:

The fields at root were originally available with the API and the SMS verification messages sent the verification code. Recently, we added a feature to select verification by code or verification by link. Due to this new parameters were added to the API call (VerificationMessageTemplateType). The old one was not removed for backward compatibility. This is the same for other duplications. As to which one is best, both of them work well. But given the option, choose the newer (nested) one.

I'd suggest that we deprecate the root level attributes here (to be removed in version 2 of the provider) and set the root level ones to computed now.

If you're happy with that approach I can raise a pull request to cover that work.

@tomelliff
Copy link
Contributor

Too early this morning apparently and completely missed the linked pull request above my comment which mostly deals with this problem but doesn't fully cover things.

@atsushi-ishibashi could you add the SMS stuff to the pull request as well please?

@atsushi-ishibashi
Copy link
Contributor Author

@tomelliff Thank you!! I'll add it

@Ninir
Copy link
Contributor

Ninir commented Jan 5, 2018

@tomelliff Thank you for the feedback, this explains it all!

@atsushi-ishibashi thanks! ;)

@bflad bflad added this to the v2.0.0 milestone Feb 27, 2019
@bflad
Copy link
Contributor

bflad commented Feb 27, 2019

In version 2.0.0 of the Terraform AWS Provider, releasing later this week, the following changes have been made:

  • Ensure only email_verification_message argument or verification_message_template configuration block email_message argument is configured
  • Ensure only email_verification_subject argument or verification_message_template configuration block email_subject argument is configured
  • Ensure only sms_verification_message argument or verification_message_template configuration block sms_message argument is configured

Thanks for you patience with introducing this (potentially) breaking change. 👍

@ghost
Copy link

ghost commented Mar 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 and limited conversation to collaborators Mar 31, 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.
Projects
None yet
Development

No branches or pull requests

5 participants