hashicorp / terraform-provider-azurerm

Terraform provider for Azure Resource Manager
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs
Mozilla Public License 2.0
4.6k stars 4.65k forks source link

Replacing a Storage Account Does Not Replace Dependent Resources #13106

Open owenfarrell opened 3 years ago

owenfarrell commented 3 years ago

Community Note

Terraform (and AzureRM Provider) Version

terraform -v
Terraform v0.14.11
+ provider registry.terraform.io/hashicorp/azurerm v2.73.0
+ provider registry.terraform.io/hashicorp/random v3.1.0

Affected Resource(s)

All of the above resources have a schema that references the name of a storage account (and not the ID). I suspect that some/all of these will not be replaced in the event a storage account is replaced.

Terraform Configuration Files

provider "azurerm" {
  features {}
}

resource "random_integer" "id" {
  min = 1
  max = 99999
}

resource "azurerm_resource_group" "test" {
  name     = "acctestsa${random_integer.id.result}"
  location = "eastus"
}

resource "azurerm_storage_account" "sa" {
  name                     = "acctestsa${random_integer.id.result}"
  resource_group_name      = azurerm_resource_group.test.name
  location                 = azurerm_resource_group.test.location
  account_tier             = "Standard"
  account_replication_type = "LRS"
}

resource "azurerm_storage_account_network_rules" "sa" {
  resource_group_name  = azurerm_resource_group.test.name
  storage_account_name = azurerm_storage_account.sa.name

  default_action = "Deny"
  bypass         = ["AzureServices"]
}

Expected Behaviour

Terraform recreates storage account network rules when the underlying storage account is replaced.

Actual Behaviour

Replaced storage accounts have default network rules in place

Steps to Reproduce

  1. terraform apply
  2. terraform taint azurerm_storage_account.sa
  3. terraform apply
alxy commented 2 years ago

@tombuildsstuff Would you accept a similar PR as #13307 (i.e. deprecating the old way with RG/name, and add an option to provide the account id instead) for azurerm_storage_container?

owenfarrell commented 2 years ago

@alxy - this comment reminded me that I was sitting on a half-baked contribution to move this forward. I saw that my original PR was adapted to better leverage the feature flags, and specifically the flags that were leveraged as part of the 3.0 upgrade. I took a similar approach and used 4.0 flags to wrap up backwards-compatible functionality and deprecation warnings.

The downside is that I found another 3 resources that are bitten by this same bug. πŸ™

drjwelch commented 2 years ago

Confirming this problem affects azurerm_storage_share.

To reproduce:

drjwelch commented 1 year ago

A little more on the above azurerm_storage_share issue. As in the above comment, this is only dependent on the storage_account_name so any action that re-creates the storage account without changing its name looks like no change to this resource.

The comment above isn't accurate though: the resource remains in the tf state but the share in Azure is destroyed along with the storage account.

A second plan and apply notices the fileshare is in the state but not in Azure; it thinks you deleted it manually and then re-creates it.

If anyone else gets here with the same issue, you can work around, not with depends_on, that didn't work for me, but with a lifecycle statement. The below will handle the share correctly if you e.g. rename the RG.

resource "azurerm_resource_group" "test_rg" {
  name      = "test_rg"
  location  = "UK South"
}

resource "azurerm_storage_account" "test_store" {
  name                  = "myuniquelynamedteststore"
  resource_group_name   = azurerm_resource_group.test_rg.name
  location              = "UK South"
  account_kind             = "StorageV2"
  account_tier             = "Standard"
  account_replication_type = "LRS"
  is_hns_enabled           = true
}

resource "azurerm_storage_share" "test_share" {
  name                 = "myuniquelynamedtestshare"
  storage_account_name = azurerm_storage_account.test_store.name
  quota                = 100
  lifecycle {
    replace_triggered_by = [azurerm_storage_account.test_store]
  }
}
nesteves commented 10 months ago

This also affects azurerm_storage_container resources.

Unfortunately, using lifecycle.replace_triggered_by suggested above is not a viable workaround, as it will trigger a replace if the targeted resource(s) is updated in place. At the moment, running a plan & apply twice is how we get around this issue.