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

Fix update behavior for Cognito User Pool Clients #5478

Merged
merged 2 commits into from
Aug 9, 2018
Merged

Fix update behavior for Cognito User Pool Clients #5478

merged 2 commits into from
Aug 9, 2018

Conversation

jbergknoff-rival
Copy link
Contributor

Fixes #5029

Changes proposed in this pull request:

  • The UpdateUserPoolClient API call appears to require a full specification of the client. Currently the provider sends a partial specification, including only the fields that have changed. As a result, any fields that aren't being updated are wiped out by terraform apply. This PR changes the provider to always send a full specification based on the Terraform config.

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoUserPoolClient_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoUserPoolClient_ -timeout 120m
=== RUN   TestAccAWSCognitoUserPoolClient_basic
--- PASS: TestAccAWSCognitoUserPoolClient_basic (15.03s)
=== RUN   TestAccAWSCognitoUserPoolClient_importBasic
--- PASS: TestAccAWSCognitoUserPoolClient_importBasic (16.81s)
=== RUN   TestAccAWSCognitoUserPoolClient_RefreshTokenValidity
--- PASS: TestAccAWSCognitoUserPoolClient_RefreshTokenValidity (24.37s)
=== RUN   TestAccAWSCognitoUserPoolClient_allFields
--- PASS: TestAccAWSCognitoUserPoolClient_allFields (14.49s)
=== RUN   TestAccAWSCognitoUserPoolClient_allFieldsUpdatingOneField
--- PASS: TestAccAWSCognitoUserPoolClient_allFieldsUpdatingOneField (24.25s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	94.979s

The newly added acceptance test TestAccAWSCognitoUserPoolClient_allFieldsUpdatingOneField passes here but fails on the master branch. The failure on master looks ilke:

(Note: failure from master branch. The issue is fixed in this PR)

make testacc TEST=./aws TESTARGS='-run=TestAccAWSCognitoUserPoolClient_allFields'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCognitoUserPoolClient_allFields -timeout 120m
=== RUN   TestAccAWSCognitoUserPoolClient_allFields
--- PASS: TestAccAWSCognitoUserPoolClient_allFields (15.28s)
=== RUN   TestAccAWSCognitoUserPoolClient_allFieldsUpdatingOneField
--- FAIL: TestAccAWSCognitoUserPoolClient_allFieldsUpdatingOneField (19.63s)
	testing.go:518: Step 1 error: Check failed: 24 errors occurred:
			* Check 3/28 error: aws_cognito_user_pool_client.client: Attribute 'explicit_auth_flows.#' expected "3", got "0"
			* Check 4/28 error: aws_cognito_user_pool_client.client: Attribute 'explicit_auth_flows.1728632605' not found
			* Check 5/28 error: aws_cognito_user_pool_client.client: Attribute 'explicit_auth_flows.1860959087' not found
			* Check 6/28 error: aws_cognito_user_pool_client.client: Attribute 'explicit_auth_flows.245201344' not found
			* Check 8/28 error: aws_cognito_user_pool_client.client: Attribute 'read_attributes.#' expected "1", got "0"
			* Check 9/28 error: aws_cognito_user_pool_client.client: Attribute 'read_attributes.881205744' not found
			* Check 10/28 error: aws_cognito_user_pool_client.client: Attribute 'write_attributes.#' expected "1", got "0"
			* Check 11/28 error: aws_cognito_user_pool_client.client: Attribute 'write_attributes.881205744' not found
			* Check 13/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_flows.#' expected "2", got "0"
			* Check 14/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_flows.2645166319' not found
			* Check 15/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_flows.3465961881' not found
			* Check 16/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_flows_user_pool_client' expected "true", got "false"
			* Check 17/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_scopes.#' expected "5", got "0"
			* Check 18/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_scopes.2517049750' not found
			* Check 19/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_scopes.881205744' not found
			* Check 20/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_scopes.2603607895' not found
			* Check 21/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_scopes.380129571' not found
			* Check 22/28 error: aws_cognito_user_pool_client.client: Attribute 'allowed_oauth_scopes.4080487570' not found
			* Check 23/28 error: aws_cognito_user_pool_client.client: Attribute 'callback_urls.#' expected "2", got "0"
			* Check 24/28 error: aws_cognito_user_pool_client.client: Attribute 'callback_urls.0' not found
			* Check 25/28 error: aws_cognito_user_pool_client.client: Attribute 'callback_urls.1' not found
			* Check 26/28 error: aws_cognito_user_pool_client.client: Attribute 'default_redirect_uri' expected "https://www.example.com/redirect", got ""
			* Check 27/28 error: aws_cognito_user_pool_client.client: Attribute 'logout_urls.#' expected "1", got "0"
			* Check 28/28 error: aws_cognito_user_pool_client.client: Attribute 'logout_urls.0' not found
		
		
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	34.930s

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Aug 8, 2018
@bflad bflad added bug Addresses a defect in current functionality. service/cognito labels Aug 8, 2018
@bflad bflad added this to the v1.32.0 milestone Aug 9, 2018
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.

LGTM -- thanks for this fix @jbergknoff-rival! 🚀

5 tests passed (all tests)
=== RUN   TestAccAWSCognitoUserPoolClient_basic
--- PASS: TestAccAWSCognitoUserPoolClient_basic (5.09s)
=== RUN   TestAccAWSCognitoUserPoolClient_allFields
--- PASS: TestAccAWSCognitoUserPoolClient_allFields (5.11s)
=== RUN   TestAccAWSCognitoUserPoolClient_importBasic
--- PASS: TestAccAWSCognitoUserPoolClient_importBasic (5.86s)
=== RUN   TestAccAWSCognitoUserPoolClient_allFieldsUpdatingOneField
--- PASS: TestAccAWSCognitoUserPoolClient_allFieldsUpdatingOneField (7.28s)
=== RUN   TestAccAWSCognitoUserPoolClient_RefreshTokenValidity
--- PASS: TestAccAWSCognitoUserPoolClient_RefreshTokenValidity (7.42s)

@bflad bflad merged commit fa841a3 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 3, 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 3, 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.

AWS Cognito app client resource unchecks options that have been enabled on alternative applies
2 participants