azurerm_site_recovery_protection_container_mapping - support automatic_update_extension.automatic_update_extensions_enabled and automatic_update_extension.automation_account_id properties

[ziyeqf]

closes #19691
closes #19407

Test

TF_ACC=1 go test -v ./internal/services/recoveryservices -run=TestAccSiteRecoveryProtectionContainerMapping -timeout=60m            [ tengzh/issue/vault_auto_update][$!][🐹 v1.19.4][⏱ 22m11s][16:57:14]
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_basic
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_basic
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_basic
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_basic (884.46s)
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension (1268.62s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/recoveryservices      1269.810s

[mbfrahry]

Hey @ziyeqf, thanks for this PR. It's mostly good but I think that we could lump these two attributes into a TypeList instead.

[ziyeqf]

Test after modification

TF_ACC=1 go test -v ./internal/services/recoveryservices -run=TestAccSiteRecoveryProtectionContainerMapping -timeout=60m
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_basic
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_basic
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_basic
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_basic (886.50s)
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension (1229.09s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/recoveryservices      1230.260s

[katbyte]

One comment left inline

[mbfrahry]

Hey @ziyeqf, thanks for making those changes. I was looking more into this and it looks like we are changing the ProviderSpecificInput type from the basic type ReplicationProviderSpecificContainerMappingInput to A2A. I also see that there is another type called VMwareCbt.

I don't like that we're favoring one type over the other in this resource and we are making that decision for the user based off their input whilst also not letting them use the other type if they want.

Do you know if this resource does anything? It doesn't appear so since most of the functionality comes from what you're adding here. If this resource isn't doing anything, it's probably worth creating a new resource and deprecating this one. That new resource will be azurerm_site_recovery_protection_container_mapping_a2a. That will also give us the flexibility to add the other type azurerm_site_recovery_protection_container_mapping_vmware_cbt if there are asks for it.

[ziyeqf]

Hi @mbfrahry , For the current resources azurerm_site_recovery_xxx, they are all for A2A(Azure To Azure) type.

For Site Recovery, there are basically four type A2A,V2A(Vmware To Azure) Classic,V2A Modern and H2A(HyperV To Azure). I have worked to add the rest three types and I think the current resources are all for A2A so it should be ok to specify a input type. For the others I'm going to name them as azure_site_recovery_xxx_v2a_classic and so on..

How do you like that?

I have also thought about renaming the current a2a resources to azurerm_site_recovery_xxx_a2a, while it seems to be breaking...

[mbfrahry]

That is fair. So with that being the case, we can nearly revert it back to the way you had it with those attributes being at the top level. But we should also force the type to be InstanceTypeBasicReplicationProviderSpecificContainerMappingInputInstanceTypeA2A as in the original PR submitted, the type was being changed back to ReplicationProviderSpecificContainerMappingInput if these new attributes weren't set.

Apologies for the run around on this!

[ziyeqf]

temporarily close this PR to reduce noise in refactoring.

[katbyte]

@ziyeqf - is there any progress on getting this merged so we don't need the hack? Azure/azure-rest-api-specs#21652

[ziyeqf]

@ziyeqf - is there any progress on getting this merged so we don't need the hack? Azure/azure-rest-api-specs#21652

oh, it's a mistake, Tom has explained it's not a Pandora issue. While the codes on main branch has been changed. I'm wondering if we can specify the property type to A2A in the create function then we will not need the workaround.

[ziyeqf]

new test result

TF_ACC=1 go test -v ./internal/services/recoveryservices -run=TestAccSiteRecoveryProtectionContainerMapping -timeout=60m
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_basic
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_basic
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_basic
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_basic (841.12s)
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension (1290.34s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/recoveryservices      1291.507s

[ziyeqf]

test result after rename

terraform-provider-azurerm ❯ TF_ACC=1 go test -v ./internal/services/recoveryservices -run=TestAccSiteRecoveryProtectionContainerMapping -timeout=60m
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_basic
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_basic
=== RUN   TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== PAUSE TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_basic
=== CONT  TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_basic (848.41s)
--- PASS: TestAccSiteRecoveryProtectionContainerMapping_withAutoUpdateExtension (1179.99s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/recoveryservices      1183.464s

[katbyte]

Thanks @ziyeqf - LGTM 🛠️

[github-actions]

This functionality has been released in v3.42.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

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.