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

azurerm_mysql_server ssl_enforcement_enabled update fail #7307

Merged
merged 8 commits into from
Jul 13, 2020

Conversation

yupwei68
Copy link
Contributor

Fix: #7228

=== RUN TestAccAzureRMMySQLServer_createReplica
=== PAUSE TestAccAzureRMMySQLServer_createReplica
=== CONT TestAccAzureRMMySQLServer_createReplica
--- PASS: TestAccAzureRMMySQLServer_createReplica (807.43s)
=== RUN TestAccAzureRMMySQLServer_basicFiveSix
=== PAUSE TestAccAzureRMMySQLServer_basicFiveSix
=== CONT TestAccAzureRMMySQLServer_basicFiveSix
--- PASS: TestAccAzureRMMySQLServer_basicFiveSix (165.97s)
=== RUN TestAccAzureRMMySQLServer_basicFiveSixDeprecated
=== PAUSE TestAccAzureRMMySQLServer_basicFiveSixDeprecated
=== CONT TestAccAzureRMMySQLServer_basicFiveSixDeprecated
--- PASS: TestAccAzureRMMySQLServer_basicFiveSixDeprecated (165.41s)
=== RUN TestAccAzureRMMySQLServer_basicFiveSeven
=== PAUSE TestAccAzureRMMySQLServer_basicFiveSeven
=== CONT TestAccAzureRMMySQLServer_basicFiveSeven
--- PASS: TestAccAzureRMMySQLServer_basicFiveSeven (319.59s)
=== RUN TestAccAzureRMMySQLServer_basicEightZero
=== PAUSE TestAccAzureRMMySQLServer_basicEightZero
=== CONT TestAccAzureRMMySQLServer_basicEightZero
--- PASS: TestAccAzureRMMySQLServer_basicEightZero (227.11s)
=== RUN TestAccAzureRMMySQLServer_autogrowOnly
=== PAUSE TestAccAzureRMMySQLServer_autogrowOnly
=== CONT TestAccAzureRMMySQLServer_autogrowOnly
--- PASS: TestAccAzureRMMySQLServer_autogrowOnly (252.00s)
=== RUN TestAccAzureRMMySQLServer_requiresImport
=== PAUSE TestAccAzureRMMySQLServer_requiresImport
=== CONT TestAccAzureRMMySQLServer_requiresImport
--- PASS: TestAccAzureRMMySQLServer_requiresImport (238.74s)
=== RUN TestAccAzureRMMySQLServer_complete
=== PAUSE TestAccAzureRMMySQLServer_complete
=== CONT TestAccAzureRMMySQLServer_complete
--- PASS: TestAccAzureRMMySQLServer_complete (227.11s)
=== RUN TestAccAzureRMMySQLServer_update
=== PAUSE TestAccAzureRMMySQLServer_update
=== CONT TestAccAzureRMMySQLServer_update
--- PASS: TestAccAzureRMMySQLServer_update (345.75s)
=== RUN TestAccAzureRMMySQLServer_completeDeprecatedMigrate
=== PAUSE TestAccAzureRMMySQLServer_completeDeprecatedMigrate
=== CONT TestAccAzureRMMySQLServer_completeDeprecatedMigrate
--- PASS: TestAccAzureRMMySQLServer_completeDeprecatedMigrate (194.79s)
=== RUN TestAccAzureRMMySQLServer_updateDeprecated
=== PAUSE TestAccAzureRMMySQLServer_updateDeprecated
=== CONT TestAccAzureRMMySQLServer_updateDeprecated
--- PASS: TestAccAzureRMMySQLServer_updateDeprecated (224.12s)
=== RUN TestAccAzureRMMySQLServer_updateSKU
=== PAUSE TestAccAzureRMMySQLServer_updateSKU
=== CONT TestAccAzureRMMySQLServer_updateSKU
--- PASS: TestAccAzureRMMySQLServer_updateSKU (317.73s)
=== RUN TestAccAzureRMMySQLServer_createPointInTimeRestore
=== PAUSE TestAccAzureRMMySQLServer_createPointInTimeRestore
=== CONT TestAccAzureRMMySQLServer_createPointInTimeRestore
--- PASS: TestAccAzureRMMySQLServer_createPointInTimeRestore (1449.75s)

@ghost ghost added the size/XS label Jun 12, 2020
@tombuildsstuff tombuildsstuff self-assigned this Jun 12, 2020
@tombuildsstuff tombuildsstuff removed their assignment Jul 2, 2020
katbyte
katbyte previously requested changes Jul 8, 2020
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Hi @yupwei68, I don't think this is the right way to solve this issue. Instead can we use a diff suppress function to ignore the TLSEnforcementDisabled value? thanks!

@yupwei68
Copy link
Contributor Author

yupwei68 commented Jul 8, 2020

Hi @katbyte , thanks for your comments.
The customer issue is caused by once the ssl_enforcement = Disabled, we can't update this field to enabled.
There are two fields (ssl_enforcement_enabled and ssl_enforcement,ssl_enforcement is to be deprecated). In update function, if we have updated ssl_enforcement to enabled , d.Get("ssl_enforcement_enabled") = false, because in tf state, its value is false. It'll always be covered by its previous state value.
This PR is to fix this problem.

@ghost ghost removed the waiting-response label Jul 8, 2020
@yupwei68
Copy link
Contributor Author

yupwei68 commented Jul 8, 2020

I have merged the conflicts. I have changed this back, because I assume when ssl_enforcement_enabled is absent, d.Get("ssl_enforcement_enabled") will get false, ssl will be disabled. But by default, it should be enabled:

ssl := mysql.SslEnforcementEnumEnabled
if v := d.Get("ssl_enforcement_enabled").(bool); !v {
		ssl = mysql.SslEnforcementEnumDisabled
}

@jackofallops jackofallops modified the milestones: v2.18.0, v2.19.0 Jul 9, 2020
@yupwei68
Copy link
Contributor Author

Thanks for your comments. I think it's fixed by tom's PR #6809. I'll close this PR.

@yupwei68 yupwei68 closed this Jul 10, 2020
yupwei68 added 2 commits July 10, 2020 11:25
@yupwei68 yupwei68 reopened this Jul 10, 2020
@yupwei68
Copy link
Contributor Author

Sorry that I find that there is another problem with the issue #7228.

Tests have passed:
=== RUN TestAccAzureRMMySQLServer_basicFiveSix
=== PAUSE TestAccAzureRMMySQLServer_basicFiveSix
=== CONT TestAccAzureRMMySQLServer_basicFiveSix
--- PASS: TestAccAzureRMMySQLServer_basicFiveSix (177.01s)
=== RUN TestAccAzureRMMySQLServer_basicFiveSixDeprecated
=== PAUSE TestAccAzureRMMySQLServer_basicFiveSixDeprecated
=== CONT TestAccAzureRMMySQLServer_basicFiveSixDeprecated
--- PASS: TestAccAzureRMMySQLServer_basicFiveSixDeprecated (173.28s)
=== RUN TestAccAzureRMMySQLServer_basicFiveSeven
=== PAUSE TestAccAzureRMMySQLServer_basicFiveSeven
=== CONT TestAccAzureRMMySQLServer_basicFiveSeven
--- PASS: TestAccAzureRMMySQLServer_basicFiveSeven (237.26s)
=== RUN TestAccAzureRMMySQLServer_basicEightZero
=== PAUSE TestAccAzureRMMySQLServer_basicEightZero

=== CONT TestAccAzureRMMySQLServer_basicEightZero
--- PASS: TestAccAzureRMMySQLServer_basicEightZero (232.91s)
=== RUN TestAccAzureRMMySQLServer_autogrowOnly
=== PAUSE TestAccAzureRMMySQLServer_autogrowOnly
=== CONT TestAccAzureRMMySQLServer_autogrowOnly
--- PASS: TestAccAzureRMMySQLServer_autogrowOnly (300.61s)
=== RUN TestAccAzureRMMySQLServer_requiresImport
=== PAUSE TestAccAzureRMMySQLServer_requiresImport
=== CONT TestAccAzureRMMySQLServer_requiresImport
--- PASS: TestAccAzureRMMySQLServer_requiresImport (290.18s)
=== RUN TestAccAzureRMMySQLServer_complete
=== PAUSE TestAccAzureRMMySQLServer_complete
=== CONT TestAccAzureRMMySQLServer_complete
--- PASS: TestAccAzureRMMySQLServer_complete (300.80s)
=== RUN TestAccAzureRMMySQLServer_update
=== PAUSE TestAccAzureRMMySQLServer_update
=== CONT TestAccAzureRMMySQLServer_update
--- PASS: TestAccAzureRMMySQLServer_update (964.43s)
=== RUN TestAccAzureRMMySQLServer_completeDeprecatedMigrate
=== PAUSE TestAccAzureRMMySQLServer_completeDeprecatedMigrate
=== CONT TestAccAzureRMMySQLServer_completeDeprecatedMigrate
--- PASS: TestAccAzureRMMySQLServer_completeDeprecatedMigrate (276.24s)
=== RUN TestAccAzureRMMySQLServer_updateDeprecated
=== PAUSE TestAccAzureRMMySQLServer_updateDeprecated
=== CONT TestAccAzureRMMySQLServer_updateDeprecated
--- PASS: TestAccAzureRMMySQLServer_updateDeprecated (241.84s)
=== RUN TestAccAzureRMMySQLServer_updateSKU
=== PAUSE TestAccAzureRMMySQLServer_updateSKU
=== CONT TestAccAzureRMMySQLServer_updateSKU
--- PASS: TestAccAzureRMMySQLServer_updateSKU (335.69s)
=== RUN TestAccAzureRMMySQLServer_createReplica
=== PAUSE TestAccAzureRMMySQLServer_createReplica
=== CONT TestAccAzureRMMySQLServer_createReplica
--- PASS: TestAccAzureRMMySQLServer_createReplica (904.03s)
=== RUN TestAccAzureRMMySQLServer_createPointInTimeRestore
=== PAUSE TestAccAzureRMMySQLServer_createPointInTimeRestore
=== CONT TestAccAzureRMMySQLServer_createPointInTimeRestore
--- PASS: TestAccAzureRMMySQLServer_createPointInTimeRestore (1477.00s)

@yupwei68
Copy link
Contributor Author

@jackofallops , Sorry that I miss a minor change need to be solved. I have reopened the PR and add some tests.

@ghost ghost added size/M and removed size/XS labels Jul 10, 2020
Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

Thanks @yupwei68 - LGTM 👍

@yupwei68 yupwei68 requested a review from katbyte July 13, 2020 01:48
@yupwei68
Copy link
Contributor Author

Hi @katbyte , would you mind continue reviewing this PR ?

@jackofallops jackofallops self-assigned this Jul 13, 2020
@jackofallops jackofallops dismissed katbyte’s stale review July 13, 2020 11:02

Scope of PR changed.

@jackofallops
Copy link
Member

Tests Passing:
image

@jackofallops jackofallops merged commit 2724b4a into hashicorp:master Jul 13, 2020
jackofallops added a commit that referenced this pull request Jul 13, 2020
@yupwei68 yupwei68 deleted the wyp-mysql-server-ssl branch July 14, 2020 01:35
@ghost
Copy link

ghost commented Jul 16, 2020

This has been released in version 2.19.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.19.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Aug 12, 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. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Aug 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azurerm_mysql_server: ssl_minimal_tls_version_enforced is ignored
4 participants