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.

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

Failure to delete the parent Azure Firewall Policy #21641

Open
1 task done
KuznecovSemen opened this issue May 3, 2023 · 8 comments
Open
1 task done

Failure to delete the parent Azure Firewall Policy #21641

KuznecovSemen opened this issue May 3, 2023 · 8 comments

Comments

@KuznecovSemen
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 👍 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
Copy link
Contributor

wuxu92 commented May 4, 2023

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
Copy link
Author

KuznecovSemen commented May 4, 2023

@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
Copy link

geek-rb commented Jun 12, 2023

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

@bewatersmsft
Copy link

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
Copy link

geek-rb commented Jun 21, 2023

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

@bewatersmsft
Copy link

bewatersmsft commented Jun 21, 2023

@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
Copy link

geek-rb commented Jun 22, 2023

@bewatersmsft terraform code the same as @KuznecovSemen was posted

@geek-rb
Copy link

geek-rb commented Sep 5, 2023

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants