`azurerm_storage_account` - Support for `min_tls_version` #7879

giotab · 2020-07-23T21:12:42Z

set the min_tls_version when creating a storage account.

This closes #7778, closes #7966.

Merge 2.20.0

magodo

Hi @giotab Thanks for this PR!
I've taken a look through and left some comments inline, but this is mostly looking good to me 👍

azurerm/internal/services/storage/resource_arm_storage_account.go

.teamcity/components/generated/services.kt

azurerm/internal/services/storage/data_source_storage_account.go

azurerm/internal/services/storage/resource_arm_storage_account.go

azurerm/internal/services/storage/tests/resource_arm_storage_account_test.go

…form-provider-azurerm

magodo · 2020-08-06T02:11:39Z

Hey @giotab
The change is LGTM now. Whilst there seems to be some test failures:

=== RUN   TestAccAzureRMStorageAccount_writeLock
=== PAUSE TestAccAzureRMStorageAccount_writeLock
=== CONT  TestAccAzureRMStorageAccount_writeLock
--- FAIL: TestAccAzureRMStorageAccount_writeLock (71.81s)
=== RUN   TestAccAzureRMStorageAccount_minTLSVersion
=== PAUSE TestAccAzureRMStorageAccount_minTLSVersion
=== CONT  TestAccAzureRMStorageAccount_minTLSVersion
--- FAIL: TestAccAzureRMStorageAccount_minTLSVersion (204.07s)

Once that is addressed, we shall be ready to merge it 🚀

giotab · 2020-08-06T05:27:35Z

Hey @giotab
The change is LGTM now. Whilst there seems to be some test failures:

=== RUN   TestAccAzureRMStorageAccount_writeLock
=== PAUSE TestAccAzureRMStorageAccount_writeLock
=== CONT  TestAccAzureRMStorageAccount_writeLock
--- FAIL: TestAccAzureRMStorageAccount_writeLock (71.81s)
=== RUN   TestAccAzureRMStorageAccount_minTLSVersion
=== PAUSE TestAccAzureRMStorageAccount_minTLSVersion
=== CONT  TestAccAzureRMStorageAccount_minTLSVersion
--- FAIL: TestAccAzureRMStorageAccount_minTLSVersion (204.07s)

Once that is addressed, we shall be ready to merge it 🚀

Turns out typos in the tests :)

$ make acctests SERVICE='storage' TESTARGS='-run=TestAccAzureRMStorageAccount' TESTTIMEOUT='60m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./azurerm/internal/services/storage/tests/ -run=TestAccAzureRMStorageAccount -timeout 60m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMStorageAccountCustomerManagedKey_basic
=== PAUSE TestAccAzureRMStorageAccountCustomerManagedKey_basic
=== RUN   TestAccAzureRMStorageAccountCustomerManagedKey_requiresImport
=== PAUSE TestAccAzureRMStorageAccountCustomerManagedKey_requiresImport
=== RUN   TestAccAzureRMStorageAccountCustomerManagedKey_updateKey
=== PAUSE TestAccAzureRMStorageAccountCustomerManagedKey_updateKey
=== RUN   TestAccAzureRMStorageAccountNetworkRules_basic
=== PAUSE TestAccAzureRMStorageAccountNetworkRules_basic
=== RUN   TestAccAzureRMStorageAccountNetworkRules_update
=== PAUSE TestAccAzureRMStorageAccountNetworkRules_update
=== RUN   TestAccAzureRMStorageAccountNetworkRules_empty
=== PAUSE TestAccAzureRMStorageAccountNetworkRules_empty
=== RUN   TestAccAzureRMStorageAccount_basic
=== PAUSE TestAccAzureRMStorageAccount_basic
=== RUN   TestAccAzureRMStorageAccount_requiresImport
=== PAUSE TestAccAzureRMStorageAccount_requiresImport
=== RUN   TestAccAzureRMStorageAccount_tagCount
=== PAUSE TestAccAzureRMStorageAccount_tagCount
=== RUN   TestAccAzureRMStorageAccount_writeLock
=== PAUSE TestAccAzureRMStorageAccount_writeLock
=== RUN   TestAccAzureRMStorageAccount_premium
=== PAUSE TestAccAzureRMStorageAccount_premium
=== RUN   TestAccAzureRMStorageAccount_disappears
=== PAUSE TestAccAzureRMStorageAccount_disappears
=== RUN   TestAccAzureRMStorageAccount_blobConnectionString
=== PAUSE TestAccAzureRMStorageAccount_blobConnectionString
=== RUN   TestAccAzureRMStorageAccount_enableHttpsTrafficOnly
=== PAUSE TestAccAzureRMStorageAccount_enableHttpsTrafficOnly
=== RUN   TestAccAzureRMStorageAccount_minTLSVersion
=== PAUSE TestAccAzureRMStorageAccount_minTLSVersion
=== RUN   TestAccAzureRMStorageAccount_allowBlobPublicAccess
=== PAUSE TestAccAzureRMStorageAccount_allowBlobPublicAccess
=== RUN   TestAccAzureRMStorageAccount_isHnsEnabled
=== PAUSE TestAccAzureRMStorageAccount_isHnsEnabled
=== RUN   TestAccAzureRMStorageAccount_blobStorageWithUpdate
=== PAUSE TestAccAzureRMStorageAccount_blobStorageWithUpdate
=== RUN   TestAccAzureRMStorageAccount_blockBlobStorage
=== PAUSE TestAccAzureRMStorageAccount_blockBlobStorage
=== RUN   TestAccAzureRMStorageAccount_fileStorageWithUpdate
=== PAUSE TestAccAzureRMStorageAccount_fileStorageWithUpdate
=== RUN   TestAccAzureRMStorageAccount_storageV2WithUpdate
=== PAUSE TestAccAzureRMStorageAccount_storageV2WithUpdate
=== RUN   TestAccAzureRMStorageAccount_storageV1ToV2Update
=== PAUSE TestAccAzureRMStorageAccount_storageV1ToV2Update
=== RUN   TestAccAzureRMStorageAccount_NonStandardCasing
=== PAUSE TestAccAzureRMStorageAccount_NonStandardCasing
=== RUN   TestAccAzureRMStorageAccount_enableIdentity
=== PAUSE TestAccAzureRMStorageAccount_enableIdentity
=== RUN   TestAccAzureRMStorageAccount_updateResourceByEnablingIdentity
=== PAUSE TestAccAzureRMStorageAccount_updateResourceByEnablingIdentity
=== RUN   TestAccAzureRMStorageAccount_networkRules
=== PAUSE TestAccAzureRMStorageAccount_networkRules
=== RUN   TestAccAzureRMStorageAccount_networkRulesDeleted
=== PAUSE TestAccAzureRMStorageAccount_networkRulesDeleted
=== RUN   TestAccAzureRMStorageAccount_blobProperties
=== PAUSE TestAccAzureRMStorageAccount_blobProperties
=== RUN   TestAccAzureRMStorageAccount_queueProperties
=== PAUSE TestAccAzureRMStorageAccount_queueProperties
=== RUN   TestAccAzureRMStorageAccount_staticWebsiteEnabled
=== PAUSE TestAccAzureRMStorageAccount_staticWebsiteEnabled
=== RUN   TestAccAzureRMStorageAccount_staticWebsiteProperties
=== PAUSE TestAccAzureRMStorageAccount_staticWebsiteProperties
=== RUN   TestAccAzureRMStorageAccount_replicationTypeGZRS
=== PAUSE TestAccAzureRMStorageAccount_replicationTypeGZRS
=== CONT  TestAccAzureRMStorageAccountCustomerManagedKey_basic
=== CONT  TestAccAzureRMStorageAccount_blobStorageWithUpdate
=== CONT  TestAccAzureRMStorageAccount_networkRules
=== CONT  TestAccAzureRMStorageAccount_replicationTypeGZRS
--- PASS: TestAccAzureRMStorageAccount_blobStorageWithUpdate (116.79s)
=== CONT  TestAccAzureRMStorageAccount_staticWebsiteProperties
--- PASS: TestAccAzureRMStorageAccount_replicationTypeGZRS (119.59s)
=== CONT  TestAccAzureRMStorageAccount_staticWebsiteEnabled
--- PASS: TestAccAzureRMStorageAccount_networkRules (157.24s)
=== CONT  TestAccAzureRMStorageAccount_queueProperties
--- PASS: TestAccAzureRMStorageAccount_staticWebsiteProperties (119.92s)
=== CONT  TestAccAzureRMStorageAccount_blobProperties
--- PASS: TestAccAzureRMStorageAccountCustomerManagedKey_basic (245.01s)
=== CONT  TestAccAzureRMStorageAccount_networkRulesDeleted
=== CONT  TestAccAzureRMStorageAccount_storageV1ToV2Update
--- PASS: TestAccAzureRMStorageAccount_staticWebsiteEnabled (146.08s)
--- PASS: TestAccAzureRMStorageAccount_queueProperties (123.50s)
=== CONT  TestAccAzureRMStorageAccount_updateResourceByEnablingIdentity
--- PASS: TestAccAzureRMStorageAccount_blobProperties (118.46s)
=== CONT  TestAccAzureRMStorageAccount_enableIdentity
--- PASS: TestAccAzureRMStorageAccount_storageV1ToV2Update (115.16s)
=== CONT  TestAccAzureRMStorageAccount_NonStandardCasing
--- PASS: TestAccAzureRMStorageAccount_networkRulesDeleted (157.44s)
=== CONT  TestAccAzureRMStorageAccount_writeLock
--- PASS: TestAccAzureRMStorageAccount_updateResourceByEnablingIdentity (129.86s)
=== CONT  TestAccAzureRMStorageAccount_isHnsEnabled
--- PASS: TestAccAzureRMStorageAccount_enableIdentity (93.37s)
=== CONT  TestAccAzureRMStorageAccount_allowBlobPublicAccess
--- PASS: TestAccAzureRMStorageAccount_NonStandardCasing (107.15s)
=== CONT  TestAccAzureRMStorageAccount_minTLSVersion
--- PASS: TestAccAzureRMStorageAccount_writeLock (115.96s)
=== CONT  TestAccAzureRMStorageAccount_enableHttpsTrafficOnly
--- PASS: TestAccAzureRMStorageAccount_isHnsEnabled (138.76s)
=== CONT  TestAccAzureRMStorageAccount_blobConnectionString
--- PASS: TestAccAzureRMStorageAccount_allowBlobPublicAccess (140.94s)
=== CONT  TestAccAzureRMStorageAccount_disappears
--- PASS: TestAccAzureRMStorageAccount_enableHttpsTrafficOnly (122.30s)
=== CONT  TestAccAzureRMStorageAccount_premium
--- PASS: TestAccAzureRMStorageAccount_blobConnectionString (91.38s)
=== CONT  TestAccAzureRMStorageAccountNetworkRules_empty
--- PASS: TestAccAzureRMStorageAccount_disappears (96.28s)
=== CONT  TestAccAzureRMStorageAccount_tagCount
--- PASS: TestAccAzureRMStorageAccount_minTLSVersion (198.18s)
=== CONT  TestAccAzureRMStorageAccount_requiresImport
--- PASS: TestAccAzureRMStorageAccount_premium (94.02s)
=== CONT  TestAccAzureRMStorageAccount_basic
--- PASS: TestAccAzureRMStorageAccountNetworkRules_empty (103.86s)
=== CONT  TestAccAzureRMStorageAccountNetworkRules_basic
--- PASS: TestAccAzureRMStorageAccount_tagCount (95.71s)
=== CONT  TestAccAzureRMStorageAccountNetworkRules_update
--- PASS: TestAccAzureRMStorageAccount_requiresImport (101.43s)
=== CONT  TestAccAzureRMStorageAccount_fileStorageWithUpdate
--- PASS: TestAccAzureRMStorageAccount_basic (121.26s)
=== CONT  TestAccAzureRMStorageAccount_storageV2WithUpdate
--- PASS: TestAccAzureRMStorageAccountNetworkRules_basic (118.30s)
=== CONT  TestAccAzureRMStorageAccountCustomerManagedKey_updateKey
--- PASS: TestAccAzureRMStorageAccount_fileStorageWithUpdate (110.44s)
=== CONT  TestAccAzureRMStorageAccountCustomerManagedKey_requiresImport
--- PASS: TestAccAzureRMStorageAccount_storageV2WithUpdate (115.70s)
=== CONT  TestAccAzureRMStorageAccount_blockBlobStorage
--- PASS: TestAccAzureRMStorageAccountNetworkRules_update (253.99s)
--- PASS: TestAccAzureRMStorageAccount_blockBlobStorage (98.78s)
--- PASS: TestAccAzureRMStorageAccountCustomerManagedKey_updateKey (262.99s)
--- PASS: TestAccAzureRMStorageAccountCustomerManagedKey_requiresImport (230.19s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/storage/tests       1128.908s

ghost · 2020-08-07T08:36:42Z

This has been released in version 2.22.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.22.0"
}
# ... other configuration ...

SebastianAtWork · 2020-08-11T14:18:37Z

Unfortunately, this PR produces changes in already existing infrastructure. Shouldnt this be optional? If i upgrade the provider from 2.21 to 2.22 it shows this as a needed change for every storage account.

rmcolbert · 2020-08-11T14:23:43Z

The PR should only produces changes in existing infrastructure if you manually updated the storage account to use a different min_tls_version before Terraform started being able to track it. It should be documented as a change that requires remediation of existing storage account resource definitions that were manually updated outside of Terraform though.

SebastianAtWork · 2020-08-11T14:34:47Z

Its a storage account that i imported i.e. it existed and was created in azure and was imported via the import command. And this is an example of the current plan output :

# module.epaz_databases.azurerm_storage_account.backupStorage will be updated in-place
 ~ resource "azurerm_storage_account" "backupStorage" {
     + min_tls_version                  = "TLS1_0"

I dont want to apply these changes, since i dont know their effect, should i add this attribute to the ignore_changes or reimport the resource? Or is there a better way?

PS: I dont see min_tls_version as an attribute in the azure resource explorer for any of the storage accounts.

rmcolbert · 2020-08-11T21:14:26Z

Honestly, I would re-import the resource so you are consistent w/ the provider. The provider should pull the min_tls_version of the storage account and put it in the state file.

If you look in the Azure Portal for the storage account under Configuration, the option should appear there above "Access tier (default)" and below "Allow Blob public access". I haven't been able to find where that setting is surfaced through other clients (Azure CLI, PowerShell or Resource Explorer) yet.

SebastianAtWork · 2020-08-13T07:21:41Z

Unfortunatly, even after removing and importing the state of the storage account, min_tls_version is only written empty ( "min_tls_version": "",) and terraform plan still shows it as an addition

SebastianAtWork · 2020-08-13T08:29:09Z

I even tried to directly set the empty values into the state but as soon as the state refreshes, it gets emptied again. For now i will ignore changes to this attribute but that sounds like a strange behavior / bug

magodo · 2020-08-15T01:55:39Z

Hi @SebastianAtWork Apologize to the impact.

May I know whether you are using Azure Public or some other environment (e.g. Azure US Government)? If you are the latter case, then the issue is probably fixed by #8092.

Otherwise, this might because of the API issue, assuming your storage account is created by a quite old API version, where the min_tls_version is not introduced yet. Then when the newer API accesses this resource, the response model does not include min_tls_version (whilst it should have included).

SebastianAtWork · 2020-08-17T07:03:15Z

We´re on Azure Public. I dont know if the api is old, The storage account should have been created last year manually in the Portal and we imported them a few weeks ago into terraform. And then reimported it last week. Sounds like it should not happen with new azure storage accounts and theres not much i can do about the existing ones besides ignoring the change? (Or recreating them, which i cant do / would be too much work for such an attribute)

magodo · 2020-08-18T02:21:57Z

@SebastianAtWork I just open a PR #8152 to fix this, thank you for your report!

For those storage account created using old API version (at least 2018-02-01, before provider v1.29.0), the GET call of new API version will not return the `minimumTLSVersion` in response body. So those users will get a diff. This PR sets the default value for `min_tls_version` in case it is missing in response, using the same value as the default value defined in schema.

ghost · 2020-09-05T14:50:00Z

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!

giotab and others added 3 commits July 23, 2020 21:57

Merge pull request #1 from terraform-providers/master

b19525b

Merge 2.20.0

add min tls version for storage accounts

10fe670

fmt

93fd7b4

ghost added size/L documentation labels Jul 23, 2020

giotab added 4 commits July 23, 2020 23:16

correct documentation

bc7103f

remove computed

66c45ee

fmt

4e7ad17

add optional

7de3655

magodo requested changes Jul 31, 2020

View reviewed changes

add conversion and update tests

56ddc4b

ghost added size/M and removed size/L labels Jul 31, 2020

giotab added 3 commits July 31, 2020 10:51

Merge branch 'master' of https://github.com/terraform-providers/terra…

c6ee678

…form-provider-azurerm

fmt

f0e9fb1

remove attribute checks

d4f2bf6

magodo added service/storage enhancement labels Aug 4, 2020

magodo added this to the v2.22.0 milestone Aug 4, 2020

vaclav-dvorak mentioned this pull request Aug 4, 2020

Support for controlling the TLS version in a storage account. #7966

Closed

giotab requested a review from magodo August 5, 2020 11:12

giotab added 2 commits August 6, 2020 06:14

fix typo

81b1ed2

add quotes

a3783f7

magodo changed the title ~~Minimum TLS version for storage account~~ azurerm_storage_account - Support for min_tls_version Aug 6, 2020

magodo merged commit 861dc48 into hashicorp:master Aug 6, 2020

magodo added a commit that referenced this pull request Aug 6, 2020

update CHANGELOG.md for #7879

978ad76

daweins mentioned this pull request Aug 11, 2020

minimumTlsVersion causes storage account deployment to break in government environment #8083

Closed

magodo mentioned this pull request Aug 18, 2020

azurerm_storage_account: set default for min_tls_version #8152

Merged

pasqualedevita mentioned this pull request Sep 3, 2020

[#174257522] Update azure provider to version 2.22.0 pagopa-archive/io-infrastructure-modules-new#152

Merged

ghost locked and limited conversation to collaborators Sep 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_storage_account` - Support for `min_tls_version` #7879

`azurerm_storage_account` - Support for `min_tls_version` #7879

giotab commented Jul 23, 2020 •

edited by magodo

Loading

magodo left a comment

magodo commented Aug 6, 2020

giotab commented Aug 6, 2020

ghost commented Aug 7, 2020

SebastianAtWork commented Aug 11, 2020

rmcolbert commented Aug 11, 2020 •

edited

Loading

SebastianAtWork commented Aug 11, 2020 •

edited

Loading

rmcolbert commented Aug 11, 2020

SebastianAtWork commented Aug 13, 2020

SebastianAtWork commented Aug 13, 2020

magodo commented Aug 15, 2020

SebastianAtWork commented Aug 17, 2020

magodo commented Aug 18, 2020

ghost commented Sep 5, 2020

azurerm_storage_account - Support for min_tls_version #7879

azurerm_storage_account - Support for min_tls_version #7879

Conversation

giotab commented Jul 23, 2020 • edited by magodo Loading

magodo left a comment

Choose a reason for hiding this comment

magodo commented Aug 6, 2020

giotab commented Aug 6, 2020

ghost commented Aug 7, 2020

SebastianAtWork commented Aug 11, 2020

rmcolbert commented Aug 11, 2020 • edited Loading

SebastianAtWork commented Aug 11, 2020 • edited Loading

rmcolbert commented Aug 11, 2020

SebastianAtWork commented Aug 13, 2020

SebastianAtWork commented Aug 13, 2020

magodo commented Aug 15, 2020

SebastianAtWork commented Aug 17, 2020

magodo commented Aug 18, 2020

ghost commented Sep 5, 2020

`azurerm_storage_account` - Support for `min_tls_version` #7879

`azurerm_storage_account` - Support for `min_tls_version` #7879

giotab commented Jul 23, 2020 •

edited by magodo

Loading

rmcolbert commented Aug 11, 2020 •

edited

Loading

SebastianAtWork commented Aug 11, 2020 •

edited

Loading