Failure to delete the parent Azure Firewall Policy

KuznecovSemen commented 1 year ago

Is there an existing issue for this?

[X] I have searched the existing issues

Community Note

Please vote on this issue by adding a :thumbsup: reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

1.4.6

AzureRM Provider Version

3.35.0

Affected Resource(s)/Data Source(s)

resource "azurerm_firewall_policy"

Terraform Configuration Files

# Root module:
  module "firewall_policy_parent" {
  source                        = "git::https://XXX/XXX/XXX/_git/terraform.azurerm.firewallpolicy"
  for_each                      = { for firewall_policy in var.firewall_policies_parent : firewall_policy.firewall_policy_name => firewall_policy }
  firewall_policy_name          = each.value.firewall_policy_name
  resource_group_name           = each.value.resource_group_name
  sku                           = each.value.sku
  rule_collection_groups        = lookup(each.value, "rule_collection_groups", [])
  tls_certificate               = lookup(each.value, "tls_certificate", [])
  dns                           = lookup(each.value, "dns", null)
  intrusion_detection           = lookup(each.value, "intrusion_detection", null)
  threat_intelligence_mode      = lookup(each.value, "threat_intelligence_mode", "Alert")
  threat_intelligence_allowlist = lookup(each.value, "threat_intelligence_allowlist", null)
  parent_policy_name            = lookup(each.value, "parent_policy_name", null)
  tags                          = lookup(each.value, "tags", {})
}

module "firewall_policy_child" {
  source                        = "git::https://XXX/XXX/XXX/_git/terraform.azurerm.firewallpolicy"
  depends_on                    = [module.firewall_policy_parent]
  for_each                      = { for firewall_policy in var.firewall_policies_child : firewall_policy.firewall_policy_name => firewall_policy }
  firewall_policy_name          = each.value.firewall_policy_name
  resource_group_name           = each.value.resource_group_name
  sku                           = each.value.sku
  rule_collection_groups        = lookup(each.value, "rule_collection_groups", [])
  tls_certificate               = lookup(each.value, "tls_certificate", [])
  dns                           = lookup(each.value, "dns", null)
  intrusion_detection           = lookup(each.value, "intrusion_detection", null)
  threat_intelligence_mode      = lookup(each.value, "threat_intelligence_mode", "Alert")
  threat_intelligence_allowlist = lookup(each.value, "threat_intelligence_allowlist", null)
  parent_policy_name            = lookup(each.value, "parent_policy_name", null)
  tags                          = lookup(each.value, "tags", {})
}

variable "firewall_policies_parent" {
  description = "Parent firewall policies parameters"
  type        = any
}
variable "firewall_policies_child" {
  description = "Child firewall policies parameters"
  type        = any
}

# Child module: 
# Get resource group data 
data "azurerm_resource_group" "rg" {
  name = var.resource_group_name
}

# Get the parent firewall policy data
data "azurerm_firewall_policy" "parent_policy" {
  count               = var.parent_policy_name != null ? 1 : 0
  name                = var.parent_policy_name
  resource_group_name = data.azurerm_resource_group.rg.name
}

# Create firewall policy
resource "azurerm_firewall_policy" "policy" {
  name                     = var.firewall_policy_name
  resource_group_name      = data.azurerm_resource_group.rg.name
  location                 = data.azurerm_resource_group.rg.location
  sku                      = var.sku
  base_policy_id           = try(data.azurerm_firewall_policy.parent_policy[0].id, null)
  threat_intelligence_mode = var.threat_intelligence_mode
  tags                     = var.tags

  dynamic "tls_certificate" {
    for_each = local.tls_certificate
    content {
      name                = tls_certificate.value.kv_cert_name
      key_vault_secret_id = data.azurerm_key_vault_secret.kv_secret[tls_certificate.key].id
    }
  }

  dynamic "identity" {
    for_each = local.tls_certificate
    content {
      type         = "UserAssigned"
      identity_ids = [azurerm_user_assigned_identity.ua_identity[identity.key].id]
    }
  }

  dynamic "dns" {
    for_each = var.dns != null ? [1] : []
    content {
      proxy_enabled = var.dns.proxy_enabled
      servers       = var.dns.servers
    }
  }

  dynamic "intrusion_detection" {
    for_each = var.intrusion_detection != null ? [1] : []
    content {
      mode = var.intrusion_detection.mode

      dynamic "traffic_bypass" {
        for_each = { for traffic in var.intrusion_detection.traffic_bypass : traffic.name => traffic }
        content {
          name                  = traffic_bypass.value.name
          protocol              = traffic_bypass.value.protocol
          description           = lookup(traffic_bypass.value, "description", null)
          destination_addresses = lookup(traffic_bypass.value, "destination_addresses", [])
          destination_ip_groups = lookup(traffic_bypass.value, "destination_ip_groups", [])
          destination_ports     = lookup(traffic_bypass.value, "destination_ports", [])
          source_addresses      = lookup(traffic_bypass.value, "source_addresses", [])
          source_ip_groups      = lookup(traffic_bypass.value, "source_ip_groups", [])
        }
      }

      dynamic "signature_overrides" {
        for_each = { for signature in var.intrusion_detection.signature_overrides : signature.id => signature }
        content {
          id    = signature_overrides.value.id
          state = signature_overrides.value.state
        }
      }
    }
  }

  dynamic "threat_intelligence_allowlist" {
    for_each = var.threat_intelligence_allowlist != null ? [1] : []
    content {
      fqdns        = var.threat_intelligence_allowlist.fqdns != [] ? var.threat_intelligence_allowlist.fqdns : null
      ip_addresses = var.threat_intelligence_allowlist.ip_addresses != [] ? var.threat_intelligence_allowlist.ip_addresses : null
    }

  }
}

Debug Output/Panic Output

https://gist.github.com/KuznecovSemen/8a6ab22a13db3b86caebd71fe1122886

Expected Behaviour

When Terraform destroy is executed, the child policies should be deleted first, and then the parent policy

Actual Behaviour

The terraform output shows that all of the child policies have been deleted; they disappear from the tfstate file. In fact, one of the child policies is deleted and the other remains in existence, which leads to an error when deleting the parent policy.

Steps to Reproduce

No response

Important Factoids

No response

References

No response

wuxu92 commented 1 year ago

hi @KuznecovSemen , as the below log shows there two child policies and both of them deleted successfully, do you mean there are other child policies expcet these two?

In fact, one of the child policies is deleted and the other remains in existence

2023-05-03T12:54:30.4668476Z [0m[1mmodule.firewall_policy_child["test-azfwPolChild-noeu-p-network-02"].azurerm_firewall_policy.policy: Destruction complete after 2s[0m
2023-05-03T12:54:30.4737207Z [0m[1mmodule.firewall_policy_child["test-azfwPolChild-noeu-p-network-01"].azurerm_firewall_policy.policy: Destruction complete after 2s[0m

KuznecovSemen commented 1 year ago

@wuxu92 No, there are no other policies. In the terraform output we see that the policies have been deleted, but in fact one of them has been deleted, the other one still exists

geek-rb commented 1 year ago

Hi, @rcskosir I faced the same issue. Are there updates to the solution?

bewatersmsft commented 1 year ago

Hi all dev from Azure Firewall here,

Just a reminder that Hashicorp owns this code and we only contribute code when we can.

From a quick glance to the provider itself there are no checks / explicitly stated dependencies between child/parent policies in the terraform code

https://github.com/hashicorp/terraform-provider-azurerm/blob/10546e803094bc4b1ecf1f803075b5a9a6521e11/internal/services/firewall/firewall_policy_resource.go#LL247C1-L271C2

I'm not deeply familiar with the internals of terraform code, but I have a feeling that there isn't great dependency support between for loops and modules between these. This could be tested by not using modules and using a single, let's call it "block" of terraform. You'd want to make sure that each child policy has an explicit depends_on to the parent.

Also a HUGE HUGE help would be to use the TRACE logs explained here

https://developer.hashicorp.com/terraform/internals/debugging

Just make sure you clean out all your credentials before providing the logs. Basically for us to track when these request in ARM we need the correlation/trace ids that are provided by the arm client within the azurerm-provider and they cannot be accessed without making the log level more detailed. At the trace level we should also see all of the dependencies between nodes within terraform.

We may be able to "hack" the delete action of the resource by polling the delete state of the parent policy id before deleting the child, but these are just thoughts.

geek-rb commented 1 year ago

Hello @bewatersmsft , unfortunately this is not true, in our code each child policy depends on parent policy. Here log gist

bewatersmsft commented 1 year ago

@geek-rb can you provide a reference to your terraform as well, if it's not what @KuznecovSemen posted? Also I was expecting to see the actual execution of the destroy. When I had run it in the past I was able to see all the requests the the azurerm provider was making to ARM.

also for instance I want to make sure that a line like

https://gist.github.com/geek-rb/8e2f37a6a3323757851f166a9b0365e4#file-tf_output-log-L2485

is not an issue

geek-rb commented 1 year ago

@bewatersmsft terraform code the same as @KuznecovSemen was posted

geek-rb commented 10 months ago

update with test from 09/05/2023 with 3.71.0 version of provider, issue still exist

https://gist.github.com/geek-rb/626d052aac8884b9007de3203a9ad0bd

hashicorp / terraform-provider-azurerm