From 68d83e2afe425bf4b458eda6c93461228866ccf3 Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 16 Aug 2023 09:33:08 +0800 Subject: [PATCH 1/4] `azurerm_storage_management_policy` - Add existance check --- .../storage_management_policy_resource.go | 18 ++++++++++++- ...storage_management_policy_resource_test.go | 25 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/internal/services/storage/storage_management_policy_resource.go b/internal/services/storage/storage_management_policy_resource.go index 02dcd69a277e..e88527d52315 100644 --- a/internal/services/storage/storage_management_policy_resource.go +++ b/internal/services/storage/storage_management_policy_resource.go @@ -10,7 +10,9 @@ import ( "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" // nolint: staticcheck "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" + "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" + "github.com/hashicorp/terraform-provider-azurerm/internal/locks" "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" @@ -285,7 +287,21 @@ func resourceStorageManagementPolicyCreateOrUpdate(d *pluginsdk.ResourceData, me // The name of the Storage Account Management Policy. It should always be 'default' (from https://docs.microsoft.com/en-us/rest/api/storagerp/managementpolicies/createorupdate) mgmtPolicyId := parse.NewStorageAccountManagementPolicyID(rid.SubscriptionId, rid.ResourceGroupName, rid.StorageAccountName, "default") - // TODO: support Requires Import + if d.IsNewResource() { + // This lock is to protect the existance checking when two storage mgmt policies are being created at the same time. + locks.ByID(mgmtPolicyId.ID()) + defer locks.UnlockByID(mgmtPolicyId.ID()) + + existing, err := client.Get(ctx, rid.ResourceGroupName, rid.StorageAccountName) + if err != nil { + if !utils.ResponseWasNotFound(existing.Response) { + return fmt.Errorf("checking for presence of existing %s: %s", mgmtPolicyId, err) + } + } + if !utils.ResponseWasNotFound(existing.Response) { + return tf.ImportAsExistsError("azurerm_storage_management_policy", mgmtPolicyId.ID()) + } + } parameters := storage.ManagementPolicy{ Name: &mgmtPolicyId.ManagementPolicyName, diff --git a/internal/services/storage/storage_management_policy_resource_test.go b/internal/services/storage/storage_management_policy_resource_test.go index b6a09b455fc5..20d3eb331c09 100644 --- a/internal/services/storage/storage_management_policy_resource_test.go +++ b/internal/services/storage/storage_management_policy_resource_test.go @@ -46,6 +46,21 @@ func TestAccStorageManagementPolicy_basic(t *testing.T) { }) } +func TestAccStorageManagementPolicy_requiersImport(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_storage_management_policy", "test") + r := StorageManagementPolicyResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.basic(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + ), + }, + data.RequiresImportErrorStep(r.requiresImport), + }) +} + func TestAccStorageManagementPolicy_singleAction(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_management_policy", "test") r := StorageManagementPolicyResource{} @@ -472,6 +487,16 @@ resource "azurerm_storage_management_policy" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomString) } +func (r StorageManagementPolicyResource) requiresImport(data acceptance.TestData) string { + return fmt.Sprintf(` +%s + +resource "azurerm_storage_management_policy" "import" { + storage_account_id = azurerm_storage_management_policy.test.storage_account_id +} +`, r.basic(data)) +} + func (r StorageManagementPolicyResource) singleAction(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { From 2cdfb5d8f6c40d89b5aea502b06e71677bec9f62 Mon Sep 17 00:00:00 2001 From: magodo Date: Wed, 16 Aug 2023 10:59:09 +0800 Subject: [PATCH 2/4] typo --- internal/services/storage/storage_management_policy_resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/storage/storage_management_policy_resource.go b/internal/services/storage/storage_management_policy_resource.go index e88527d52315..caf8d1e549fd 100644 --- a/internal/services/storage/storage_management_policy_resource.go +++ b/internal/services/storage/storage_management_policy_resource.go @@ -288,7 +288,7 @@ func resourceStorageManagementPolicyCreateOrUpdate(d *pluginsdk.ResourceData, me mgmtPolicyId := parse.NewStorageAccountManagementPolicyID(rid.SubscriptionId, rid.ResourceGroupName, rid.StorageAccountName, "default") if d.IsNewResource() { - // This lock is to protect the existance checking when two storage mgmt policies are being created at the same time. + // This lock is to protect the existence checking when two storage mgmt policies are being created at the same time. locks.ByID(mgmtPolicyId.ID()) defer locks.UnlockByID(mgmtPolicyId.ID()) From 875c8aae9e11111970b8b684522723312f3c0b4e Mon Sep 17 00:00:00 2001 From: magodo Date: Thu, 17 Aug 2023 10:47:30 +0800 Subject: [PATCH 3/4] Remove the locks --- .../services/storage/storage_management_policy_resource.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/services/storage/storage_management_policy_resource.go b/internal/services/storage/storage_management_policy_resource.go index caf8d1e549fd..5abc7321911c 100644 --- a/internal/services/storage/storage_management_policy_resource.go +++ b/internal/services/storage/storage_management_policy_resource.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" "github.com/hashicorp/terraform-provider-azurerm/internal/clients" - "github.com/hashicorp/terraform-provider-azurerm/internal/locks" "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/parse" "github.com/hashicorp/terraform-provider-azurerm/internal/services/storage/validate" "github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk" @@ -288,10 +287,6 @@ func resourceStorageManagementPolicyCreateOrUpdate(d *pluginsdk.ResourceData, me mgmtPolicyId := parse.NewStorageAccountManagementPolicyID(rid.SubscriptionId, rid.ResourceGroupName, rid.StorageAccountName, "default") if d.IsNewResource() { - // This lock is to protect the existence checking when two storage mgmt policies are being created at the same time. - locks.ByID(mgmtPolicyId.ID()) - defer locks.UnlockByID(mgmtPolicyId.ID()) - existing, err := client.Get(ctx, rid.ResourceGroupName, rid.StorageAccountName) if err != nil { if !utils.ResponseWasNotFound(existing.Response) { From beec3fe68a8d2772df5499b9997ada825af33e1e Mon Sep 17 00:00:00 2001 From: magodo Date: Thu, 17 Aug 2023 16:59:27 +0800 Subject: [PATCH 4/4] Update internal/services/storage/storage_management_policy_resource_test.go Co-authored-by: stephybun --- .../services/storage/storage_management_policy_resource_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/services/storage/storage_management_policy_resource_test.go b/internal/services/storage/storage_management_policy_resource_test.go index 20d3eb331c09..9f909d1ab184 100644 --- a/internal/services/storage/storage_management_policy_resource_test.go +++ b/internal/services/storage/storage_management_policy_resource_test.go @@ -46,7 +46,7 @@ func TestAccStorageManagementPolicy_basic(t *testing.T) { }) } -func TestAccStorageManagementPolicy_requiersImport(t *testing.T) { +func TestAccStorageManagementPolicy_requiresImport(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_management_policy", "test") r := StorageManagementPolicyResource{}