Storage: adding a shim layer for Resource Manager

[magodo]

Adding shim mgmt layer for container, queue, shares. Due to the mgmt plane API of the table still lacks of some feature (namely the signedIdentities), the table resource is still data plane only.

This PR also introduced a new feature toggle for the storage in the provider block: use_resource_manager, which controls whether to use data plane API or the mgmt plane API for the affected resources.

The affected resources include:

• azurerm_storage_container
• azurerm_storage_share
• azurerm_storage_queue

Remark

1. After using the mgmt plane API, the azurerm_storage_container is not able to destroy a "root" container. The test case is commented. The tracking issue is: https://github.com/Azure/azure-rest-api-specs/issues/16783
2. After using the mgmt plane API, there are some properties of the azurerm_storage_share doesn't work. The related test cases are commented. The tracking issue is: https://github.com/Azure/azure-rest-api-specs/issues/16782
3. This PR changes the interface definition of the StorageQueuesWrapper to remove the UpdateServiceProperties() and GetServiceProperties(). The reason is that these two methods don't technically belong to the queue resource, but to the queue service. The only place that is using these two methods are in the azurerm_storage_account. Currently, due to the mgmt API of the queue service lacks of the minuteMetrics, hourMetrics and the logging, we have to keep using the data plane API. However, once this gap disappears, we can use the mgmt API of the queue service, just as what has been done for the file service and the blob service in the implementation of the azurerm_storage_account (where they are using the mgmt plane client to set/get service properties). Until then, we can further remove these two methods from the DataPlaneStorageQueueWrapper.

Test

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageContainer_cross|TestAccStorageContainer_mgmt"
=== RUN   TestAccStorageContainer_mgmtBasic
=== PAUSE TestAccStorageContainer_mgmtBasic
=== RUN   TestAccStorageContainer_mgmtUpdate
=== PAUSE TestAccStorageContainer_mgmtUpdate
=== RUN   TestAccStorageContainer_mgmtMetaData
=== PAUSE TestAccStorageContainer_mgmtMetaData
=== RUN   TestAccStorageContainer_mgmtWeb
=== PAUSE TestAccStorageContainer_mgmtWeb
=== RUN   TestAccStorageContainer_crossPlaneUpdate
=== PAUSE TestAccStorageContainer_crossPlaneUpdate
=== CONT  TestAccStorageContainer_mgmtBasic
=== CONT  TestAccStorageContainer_mgmtWeb
=== CONT  TestAccStorageContainer_crossPlaneUpdate
=== CONT  TestAccStorageContainer_mgmtMetaData
=== CONT  TestAccStorageContainer_mgmtUpdate
--- PASS: TestAccStorageContainer_mgmtWeb (175.09s)
--- PASS: TestAccStorageContainer_mgmtBasic (230.97s)
--- PASS: TestAccStorageContainer_crossPlaneUpdate (327.89s)
--- PASS: TestAccStorageContainer_mgmtUpdate (329.37s)
--- PASS: TestAccStorageContainer_mgmtMetaData (418.19s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/storage       418.242s


💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageShare_cross|TestAccStorageShare_mgmt"
=== RUN   TestAccStorageShare_mgmtBasic
=== PAUSE TestAccStorageShare_mgmtBasic
=== RUN   TestAccStorageShare_mgmtMetaData
=== PAUSE TestAccStorageShare_mgmtMetaData
=== RUN   TestAccStorageShare_mgmtUpdateQuota
=== PAUSE TestAccStorageShare_mgmtUpdateQuota
=== RUN   TestAccStorageShare_mgmtLargeQuota
=== PAUSE TestAccStorageShare_mgmtLargeQuota
=== RUN   TestAccStorageShare_mgmtNfsProtocol
=== PAUSE TestAccStorageShare_mgmtNfsProtocol
=== RUN   TestAccStorageShare_crossPlaneMetaData
=== PAUSE TestAccStorageShare_crossPlaneMetaData
=== RUN   TestAccStorageShare_crossPlaneAcl
=== PAUSE TestAccStorageShare_crossPlaneAcl
=== CONT  TestAccStorageShare_mgmtBasic
=== CONT  TestAccStorageShare_mgmtNfsProtocol
=== CONT  TestAccStorageShare_mgmtUpdateQuota
=== CONT  TestAccStorageShare_mgmtMetaData
=== CONT  TestAccStorageShare_mgmtLargeQuota
=== CONT  TestAccStorageShare_crossPlaneAcl
=== CONT  TestAccStorageShare_crossPlaneMetaData
--- PASS: TestAccStorageShare_mgmtNfsProtocol (215.44s)
--- PASS: TestAccStorageShare_mgmtBasic (240.89s)
--- PASS: TestAccStorageShare_mgmtUpdateQuota (306.46s)
--- PASS: TestAccStorageShare_mgmtLargeQuota (310.67s)
--- PASS: TestAccStorageShare_mgmtMetaData (332.07s)
--- PASS: TestAccStorageShare_crossPlaneMetaData (334.02s)
--- PASS: TestAccStorageShare_crossPlaneAcl (341.42s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/storage       341.459s

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageQueue_cross|TestAccStorageQueue_mgmt"
=== RUN   TestAccStorageQueue_mgmtBasic
=== PAUSE TestAccStorageQueue_mgmtBasic
=== RUN   TestAccStorageQueue_mgmtMetaData
=== PAUSE TestAccStorageQueue_mgmtMetaData
=== RUN   TestAccStorageQueue_crossPlaneMetaData
=== PAUSE TestAccStorageQueue_crossPlaneMetaData
=== CONT  TestAccStorageQueue_mgmtBasic
=== CONT  TestAccStorageQueue_crossPlaneMetaData
=== CONT  TestAccStorageQueue_mgmtMetaData
--- PASS: TestAccStorageQueue_mgmtBasic (230.27s)
--- PASS: TestAccStorageQueue_crossPlaneMetaData (303.89s)
--- PASS: TestAccStorageQueue_mgmtMetaData (315.50s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/storage       315.550s

---

Probably is able to fix issue #2977.

[magodo]

@tombuildsstuff Thank you for the review. I've resolved most of them, while keeping some for further discussion. Meanwhile, I've found that the cross planes tests were actually not taking effect, and I've made a fix for that.

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageContainer_resourceManager|TestAccStorageContainer_dataPlane"
=== RUN   TestAccStorageContainer_resourceManagerBasic
=== PAUSE TestAccStorageContainer_resourceManagerBasic
=== RUN   TestAccStorageContainer_resourceManagerUpdate
=== PAUSE TestAccStorageContainer_resourceManagerUpdate
=== RUN   TestAccStorageContainer_resourceManagerMetaData
=== PAUSE TestAccStorageContainer_resourceManagerMetaData
=== RUN   TestAccStorageContainer_resourceManagerRoot
    storage_container_resource_test.go:260:
--- SKIP: TestAccStorageContainer_resourceManagerRoot (0.00s)
=== RUN   TestAccStorageContainer_resourceManagerWeb
=== PAUSE TestAccStorageContainer_resourceManagerWeb
=== RUN   TestAccStorageContainer_dataPlaneThenResourceManager
=== PAUSE TestAccStorageContainer_dataPlaneThenResourceManager
=== RUN   TestAccStorageContainer_resourceManagerThenDataPlane
=== PAUSE TestAccStorageContainer_resourceManagerThenDataPlane
=== CONT  TestAccStorageContainer_resourceManagerBasic
=== CONT  TestAccStorageContainer_resourceManagerWeb
=== CONT  TestAccStorageContainer_resourceManagerThenDataPlane
=== CONT  TestAccStorageContainer_dataPlaneThenResourceManager
=== CONT  TestAccStorageContainer_resourceManagerMetaData
=== CONT  TestAccStorageContainer_resourceManagerUpdate
--- PASS: TestAccStorageContainer_resourceManagerWeb (105.00s)
--- PASS: TestAccStorageContainer_resourceManagerBasic (106.34s)
--- PASS: TestAccStorageContainer_dataPlaneThenResourceManager (124.17s)
--- PASS: TestAccStorageContainer_resourceManagerThenDataPlane (126.36s)
--- PASS: TestAccStorageContainer_resourceManagerUpdate (135.13s)
--- PASS: TestAccStorageContainer_resourceManagerMetaData (163.88s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/storage       163.943s

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageShare_resourceManager|TestAccStorageShare_dataPlane"

=== RUN   TestAccStorageShare_resourceManagerBasic
=== PAUSE TestAccStorageShare_resourceManagerBasic
=== RUN   TestAccStorageShare_resourceManagerMetaData
=== PAUSE TestAccStorageShare_resourceManagerMetaData
=== RUN   TestAccStorageShare_resourceManagerAcl
    storage_share_resource_test.go:244:
--- SKIP: TestAccStorageShare_resourceManagerAcl (0.00s)
=== RUN   TestAccStorageShare_resourceManagerAclGhostedRecall
    storage_share_resource_test.go:269:
--- SKIP: TestAccStorageShare_resourceManagerAclGhostedRecall (0.00s)
=== RUN   TestAccStorageShare_resourceManagerUpdateQuota
=== PAUSE TestAccStorageShare_resourceManagerUpdateQuota
=== RUN   TestAccStorageShare_resourceManagerLargeQuota
=== PAUSE TestAccStorageShare_resourceManagerLargeQuota
=== RUN   TestAccStorageShare_resourceManagerNfsProtocol
=== PAUSE TestAccStorageShare_resourceManagerNfsProtocol
=== RUN   TestAccStorageShare_dataPlaneThenResourceManagerMetaData
=== PAUSE TestAccStorageShare_dataPlaneThenResourceManagerMetaData
=== RUN   TestAccStorageShare_resourceManagerThenDataPlaneMetaData
=== PAUSE TestAccStorageShare_resourceManagerThenDataPlaneMetaData
=== RUN   TestAccStorageShare_dataPlaneThenResourceManagerAcl
    storage_share_resource_test.go:395:
--- SKIP: TestAccStorageShare_dataPlaneThenResourceManagerAcl (0.00s)
=== RUN   TestAccStorageShare_resourceManagerThenDataPlaneAcl
    storage_share_resource_test.go:423:
--- SKIP: TestAccStorageShare_resourceManagerThenDataPlaneAcl (0.00s)
=== CONT  TestAccStorageShare_resourceManagerBasic
=== CONT  TestAccStorageShare_resourceManagerNfsProtocol
=== CONT  TestAccStorageShare_dataPlaneThenResourceManagerMetaData
=== CONT  TestAccStorageShare_resourceManagerThenDataPlaneMetaData
=== CONT  TestAccStorageShare_resourceManagerUpdateQuota
=== CONT  TestAccStorageShare_resourceManagerLargeQuota
=== CONT  TestAccStorageShare_resourceManagerMetaData
--- PASS: TestAccStorageShare_resourceManagerNfsProtocol (108.46s)
--- PASS: TestAccStorageShare_resourceManagerBasic (110.00s)
--- PASS: TestAccStorageShare_resourceManagerLargeQuota (194.12s)
--- PASS: TestAccStorageShare_dataPlaneThenResourceManagerMetaData (204.61s)
--- PASS: TestAccStorageShare_resourceManagerThenDataPlaneMetaData (207.69s)
--- PASS: TestAccStorageShare_resourceManagerMetaData (211.44s)
--- PASS: TestAccStorageShare_resourceManagerUpdateQuota (377.89s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/storage       377.927s

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageQueue_resourceManager|TestAccStorageQueue_dataPlane"

=== RUN   TestAccStorageQueue_resourceManagerBasic
=== PAUSE TestAccStorageQueue_resourceManagerBasic
=== RUN   TestAccStorageQueue_resourceManagerMetaData
=== PAUSE TestAccStorageQueue_resourceManagerMetaData
=== RUN   TestAccStorageQueue_dataPlaneThenResourceManagerMetaData
=== PAUSE TestAccStorageQueue_dataPlaneThenResourceManagerMetaData
=== RUN   TestAccStorageQueue_resourceManagerThenDataPlaneMetaData
=== PAUSE TestAccStorageQueue_resourceManagerThenDataPlaneMetaData
=== CONT  TestAccStorageQueue_resourceManagerBasic
=== CONT  TestAccStorageQueue_dataPlaneThenResourceManagerMetaData
=== CONT  TestAccStorageQueue_resourceManagerMetaData
=== CONT  TestAccStorageQueue_resourceManagerThenDataPlaneMetaData
--- PASS: TestAccStorageQueue_resourceManagerBasic (97.85s)
--- PASS: TestAccStorageQueue_dataPlaneThenResourceManagerMetaData (117.89s)
--- PASS: TestAccStorageQueue_resourceManagerThenDataPlaneMetaData (180.53s)
--- PASS: TestAccStorageQueue_resourceManagerMetaData (187.26s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/storage       187.298s

[magodo]

@tombuildsstuff

I've resolved the left two comments:

1. For the feature toggle, I've now moved them to the v3.0 beta, and the related tests are now added the environment variable to opt in the beta so that they can be tested.
2. For the queues data plane client, now I'm directly using its data plane client.

Please take another look!

[tombuildsstuff]

@magodo is there any update from the service team as to when the Resource Manager API will be feature-compatible?

[magodo]

@tombuildsstuff Unfortunately, there is still no reply yet, I'll further check with them.

Meanwhile, I've summarized following issues/gaps for tracking:

• The queue_properties of the queue service are missing following properties, comparing to the data plane alternatives. See: https://github.com/Azure/azure-rest-api-specs/issues/17006
• The table mgmt API is missing the signedIdentites property, comparing to the data plane alternatives. See: https://github.com/Azure/azure-rest-api-specs/issues/17007
• The container mgmt API can't destroy a "root" container, due to lacks of permission. While the data plane API can. See: https://github.com/Azure/azure-rest-api-specs/issues/16783
• The file share mgmt API won't return the enabledProtocols in a GET if it is set to SMB. See: https://github.com/Azure/azure-rest-api-specs/issues/16782
• The file share mgmt API won't return the start, expiry of the accessPolicy in a GET. See: https://github.com/Azure/azure-rest-api-specs/issues/16782

If there is anything missing, please let me know so that I can submit another issue to track.

---

Update [Mar.9.2023]

• https://github.com/Azure/azure-rest-api-specs/issues/22984

[tombuildsstuff]

@magodo just checking in, any update from the service team here?

[magodo]

@tombuildsstuff I just saw them create a work item for this this morning, but seems no actual update yet.

[tombuildsstuff]

(as discussed offline) we also need Resource Manager API's for the other (non Blob File/Share File) API calls too e.g. https://github.com/hashicorp/terraform-provider-azurerm/blob/3f2512dc858799440978416022b5946d35b25e5b/internal/services/storage/storage_account_resource.go#L1219-L1244

Else we're unable to read these once Network Rules are enabled

[tombuildsstuff]

@magodo is there an update from the Service Team on when these fixes will go live?

[tombuildsstuff]

... I hit comment but 🙈

[dhirschfeld]

... I hit comment but 🙈

https://twitter.com/mariorod1/status/1542189296240713730

[magodo]

@magodo Zhaoting Weng FTE is there an update from the Service Team on when these fixes will go live?

Just saw an update in the internal ticket opened for resolving this the issues I listed above.

[magodo]

@tombuildsstuff Merged with the main branch, and all the related tests are passing. Though there is no news here as all those issues are still in progress, except https://github.com/Azure/azure-rest-api-specs/issues/17007, that seems to be fixed in the 2021-09-01, where I have another PR to update the API version to it: https://github.com/hashicorp/terraform-provider-azurerm/pull/17523.

Test

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageShare_resourceManager|TestAccStorageShare_dataPlane"

=== RUN TestAccStorageShare_resourceManagerBasic === PAUSE TestAccStorageShare_resourceManagerBasic === RUN TestAccStorageShare_resourceManagerMetaData === PAUSE TestAccStorageShare_resourceManagerMetaData === RUN TestAccStorageShare_resourceManagerAcl storage_share_resource_test.go:311: --- SKIP: TestAccStorageShare_resourceManagerAcl (0.00s) === RUN TestAccStorageShare_resourceManagerAclGhostedRecall storage_share_resource_test.go:336: --- SKIP: TestAccStorageShare_resourceManagerAclGhostedRecall (0.00s) === RUN TestAccStorageShare_resourceManagerUpdateQuota === PAUSE TestAccStorageShare_resourceManagerUpdateQuota === RUN TestAccStorageShare_resourceManagerLargeQuota === PAUSE TestAccStorageShare_resourceManagerLargeQuota === RUN TestAccStorageShare_resourceManagerNfsProtocol === PAUSE TestAccStorageShare_resourceManagerNfsProtocol === RUN TestAccStorageShare_dataPlaneThenResourceManagerMetaData === PAUSE TestAccStorageShare_dataPlaneThenResourceManagerMetaData === RUN TestAccStorageShare_resourceManagerThenDataPlaneMetaData === PAUSE TestAccStorageShare_resourceManagerThenDataPlaneMetaData === RUN TestAccStorageShare_dataPlaneThenResourceManagerAcl storage_share_resource_test.go:462: --- SKIP: TestAccStorageShare_dataPlaneThenResourceManagerAcl (0.00s) === RUN TestAccStorageShare_resourceManagerThenDataPlaneAcl storage_share_resource_test.go:490: --- SKIP: TestAccStorageShare_resourceManagerThenDataPlaneAcl (0.00s) === CONT TestAccStorageShare_resourceManagerBasic === CONT TestAccStorageShare_resourceManagerNfsProtocol === CONT TestAccStorageShare_resourceManagerUpdateQuota === CONT TestAccStorageShare_resourceManagerLargeQuota === CONT TestAccStorageShare_resourceManagerThenDataPlaneMetaData === CONT TestAccStorageShare_dataPlaneThenResourceManagerMetaData === CONT TestAccStorageShare_resourceManagerMetaData --- PASS: TestAccStorageShare_resourceManagerNfsProtocol (89.75s) --- PASS: TestAccStorageShare_resourceManagerBasic (96.84s) --- PASS: TestAccStorageShare_resourceManagerLargeQuota (110.31s) --- PASS: TestAccStorageShare_resourceManagerThenDataPlaneMetaData (176.03s) --- PASS: TestAccStorageShare_dataPlaneThenResourceManagerMetaData (178.06s) --- PASS: TestAccStorageShare_resourceManagerMetaData (183.42s) --- PASS: TestAccStorageShare_resourceManagerUpdateQuota (211.97s) PASS ok github.com/hashicorp/terraform-provider-azurerm/internal/services/storage 212.005s

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageContainer_resourceManager|TestAccStorageContainer_dataPlane"

=== RUN TestAccStorageContainer_resourceManagerBasic === PAUSE TestAccStorageContainer_resourceManagerBasic === RUN TestAccStorageContainer_resourceManagerUpdate === PAUSE TestAccStorageContainer_resourceManagerUpdate === RUN TestAccStorageContainer_resourceManagerMetaData === PAUSE TestAccStorageContainer_resourceManagerMetaData === RUN TestAccStorageContainer_resourceManagerRoot storage_container_resource_test.go:260: --- SKIP: TestAccStorageContainer_resourceManagerRoot (0.00s) === RUN TestAccStorageContainer_resourceManagerWeb === PAUSE TestAccStorageContainer_resourceManagerWeb === RUN TestAccStorageContainer_dataPlaneThenResourceManager === PAUSE TestAccStorageContainer_dataPlaneThenResourceManager === RUN TestAccStorageContainer_resourceManagerThenDataPlane === PAUSE TestAccStorageContainer_resourceManagerThenDataPlane === CONT TestAccStorageContainer_resourceManagerBasic === CONT TestAccStorageContainer_resourceManagerThenDataPlane === CONT TestAccStorageContainer_resourceManagerMetaData === CONT TestAccStorageContainer_resourceManagerWeb === CONT TestAccStorageContainer_resourceManagerUpdate === CONT TestAccStorageContainer_dataPlaneThenResourceManager --- PASS: TestAccStorageContainer_resourceManagerThenDataPlane (113.54s) --- PASS: TestAccStorageContainer_dataPlaneThenResourceManager (116.78s) --- PASS: TestAccStorageContainer_resourceManagerBasic (161.38s) --- PASS: TestAccStorageContainer_resourceManagerWeb (162.38s) --- PASS: TestAccStorageContainer_resourceManagerUpdate (187.95s) --- PASS: TestAccStorageContainer_resourceManagerMetaData (213.48s) PASS ok github.com/hashicorp/terraform-provider-azurerm/internal/services/storage 213.504s

💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/storage -run="TestAccStorageQueue_resourceManager|TestAccStorageQueue_dataPlane"

=== RUN TestAccStorageQueue_resourceManagerBasic === PAUSE TestAccStorageQueue_resourceManagerBasic === RUN TestAccStorageQueue_resourceManagerMetaData === PAUSE TestAccStorageQueue_resourceManagerMetaData === RUN TestAccStorageQueue_dataPlaneThenResourceManagerMetaData === PAUSE TestAccStorageQueue_dataPlaneThenResourceManagerMetaData === RUN TestAccStorageQueue_resourceManagerThenDataPlaneMetaData === PAUSE TestAccStorageQueue_resourceManagerThenDataPlaneMetaData === CONT TestAccStorageQueue_resourceManagerBasic === CONT TestAccStorageQueue_dataPlaneThenResourceManagerMetaData === CONT TestAccStorageQueue_resourceManagerMetaData === CONT TestAccStorageQueue_resourceManagerThenDataPlaneMetaData --- PASS: TestAccStorageQueue_resourceManagerBasic (154.83s) --- PASS: TestAccStorageQueue_dataPlaneThenResourceManagerMetaData (171.90s) --- PASS: TestAccStorageQueue_resourceManagerThenDataPlaneMetaData (174.94s) --- PASS: TestAccStorageQueue_resourceManagerMetaData (181.47s) PASS ok github.com/hashicorp/terraform-provider-azurerm/internal/services/storage 181.491s

[fardarter]

Is this going to be addressed?

[aghassemlouei]

Checking on the status of this pull request as its been while and the changes introduced would really be beneficial to the larger community.

[magodo]

@tombuildsstuff A tiny update to the https://github.com/hashicorp/terraform-provider-azurerm/pull/14220#issuecomment-1180161943. With #17523 merged, I now opt in the shim layer for the table client. However, the API has some issue, which makes the ACL update failed, which is tracked in https://github.com/Azure/azure-rest-api-specs/issues/17007#issuecomment-1272222690 (I've also asked the service team internally).

[katbyte]

@magodo - any updates from the service team on this?

[magodo]

@katbyte Unfortunately, there is no update from the service team at the moment.

[tombuildsstuff]

@magodo since this is still waiting on the Service Team, rather than leaving this PR open indefinitely I'm going to temporarily close this PR for the moment - once the Service Team's shipped the changes we can re-open this / fix up the merge conflicts and look to get this merged, however.

[github-actions[bot]]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.