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.

Sign up for GitHub

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

Jump to bottom

azurerm_storage_management_policy - Add existance check #22966

Merged
merged 4 commits into from Aug 17, 2023
Merged

azurerm_storage_management_policy - Add existance check #22966

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
68d83e2
`azurerm_storage_management_policy` - Add existance check
magodo Aug 16, 2023
2cdfb5d
typo
magodo Aug 16, 2023
875c8aa
Remove the locks
magodo Aug 17, 2023
beec3fe
Update internal/services/storage/storage_management_policy_resource_t…
magodo Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion internal/services/storage/storage_management_policy_resource.go
View file
Open in desktop
Expand Up @@ -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"
Expand Down Expand Up @@ -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 existence checking when two storage mgmt policies are being created at the same time.
locks.ByID(mgmtPolicyId.ID())
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we should go this route quite yet. We should do a separate PR with it's own test to confirm that this will fix that issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I've removed the locks.

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,
Expand Down
25 changes: 25 additions & 0 deletions internal/services/storage/storage_management_policy_resource_test.go
View file
Open in desktop
Expand Up @@ -46,6 +46,21 @@ func TestAccStorageManagementPolicy_basic(t *testing.T) {
})
}

func TestAccStorageManagementPolicy_requiersImport(t *testing.T) {
magodo marked this conversation as resolved.
Show resolved Hide resolved
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{}
Expand Down Expand Up @@ -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" {
Expand Down