welcome-to-palo-alto-networks[bot]
commented
2 years ago :tada: Thanks for opening your first issue here! Welcome to the community!
Open jasonjuenger opened 2 years ago
welcome-to-palo-alto-networks[bot]
commented
2 years ago :tada: Thanks for opening your first issue here! Welcome to the community!
shinmog
commented
2 years ago Ok, so I can reproduce this using the above steps, but I would argue that the above is "doing it wrong," and doing it "the right way" does not result in an error.
In the 1st stage of the above plan file, there are 4 security rules. In the 2nd stage, this gets trimmed down to 3 security rules, but because an emphasis is placed on the Terraform names of individual resources being meaningful to a human reading the plan file, removing a rule in the middle has a ripple effect of causing every other rule defined after it to be touched. If this was 100 rules, and you needed to delete the 2nd, that would be 1 (delete the 2nd rule) + 98 * 3 updates (you have to change 1) the Terraform name, 2) the position_reference, and 3) the depends_on). This feels wrong to me.
If, instead, the Terraform names are not locked to the placement of the rule within the rulebase, then deleting the 2nd security rule only requires a 1 (delete the 2nd rule) + 1 (update reference of the 3rd rule) changes. There's nothing wrong with having the Terraform name still being meaningful to humans reading the plan file, just don't tie the name to the rulebase location :)
Here is an initial deploy file that I'll be using. You'll note that I'm forgoing the depends_on in order to directly reference the rule name. If for some reason you really like using depends_on, then it would be 1 + 2 updates instead of 1 + 1. You'll also note that I'm using a NGFW instead of a Panorama here, but the concept is the same for Panorama and does not change anything other than resource naming.
Here is my initial deploy config:
resource "panos_security_rule_group" "top" {
position_keyword = "top"
rule {
name = "first"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}
resource "panos_security_rule_group" "one" {
position_keyword = "directly after"
position_reference = panos_security_rule_group.top.rule.0.name
rule {
name = "second"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}
resource "panos_security_rule_group" "two" {
position_keyword = "directly after"
position_reference = panos_security_rule_group.one.rule.0.name
rule {
name = "third"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}
resource "panos_security_rule_group" "three" {
position_keyword = "directly after"
position_reference = panos_security_rule_group.two.rule.0.name
rule {
name = "fourth"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}Updated deploy config (removes the 2nd rule):
resource "panos_security_rule_group" "top" {
position_keyword = "top"
rule {
name = "first"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}
resource "panos_security_rule_group" "two" {
position_keyword = "directly after"
position_reference = panos_security_rule_group.top.rule.0.name
rule {
name = "third"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}
resource "panos_security_rule_group" "three" {
position_keyword = "directly after"
position_reference = panos_security_rule_group.two.rule.0.name
rule {
name = "fourth"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}I don't know Terraform internals well enough to know why doing it your way is causing it to break. If I had to guess, I would say it's a combination of reusing the Terraform names while changing their contents, which in actuality overlaps with existing config on PAN-OS. But doing it the way that is mentioned in the opening of this issue doesn't seem right to me from a Terraform perspective.
From a PAN-OS perspective, I also would not recommend doing it that way even if Terraform wasn't breaking on it. This is because you are asking Terraform to delete and re-create every single security rule when you change the resource location like this. This not only will make deployment of the config take longer than it needs to, but also will affect the operational state of your security rules as well, such as hit count counters and the such.
Finally, if it is at all possible, I would highly recommend that if the intent is that security rules be back-to-back, they should all be in a single panos_security_rule_group / panos_panorama_security_rule_group resource, as there are additional performance gains that take place within the confines of a single resource (as of panos v1.9.0). AKA:
resource "panos_security_rule_group" "top" {
position_keyword = "top"
rule {
name = "first"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
rule {
name = "second"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
rule {
name = "third"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
rule {
name = "fourth"
source_zones = ["any"]
source_addresses = ["any"]
source_users = ["any"]
destination_zones = ["any"]
destination_addresses = ["any"]
applications = ["any"]
services = ["any"]
categories = ["any"]
log_end = true
action = "allow"
}
lifecycle {
create_before_destroy = true
}
}Provided single resource is an option for your use case, doing it this way also has the benefit of increasing the human-readable factor of the security rules.
Hope this helps!
Describe the bug
Removing a rule results in an error when placing rules dependent on other rules that were previously dependent on the removed ruled.
Expected behavior
Removing a rule should invalidate references to the removed rule so subsequent rules can be placed properly.
Current behavior
The following error is seen on an existing rule when a rule at least two spots above it is removed.
Trying to back out the change by adding the removed rule back in results in the same error message. The tfstate file has to be deleted first and the original main.tf file run to restore the policies to the previous state.
Possible solution
Steps to reproduce
The behavior can be reproduced using the following main.tf files. To reproduce:
Original state main.tf:
Contents of second main.tf that generate an error:
Context
We are no longer able to delete unused rules reliably via code. We have to delete the rules manually, remove references to them from our Terraform files, and delete TFstate files in order to maintain a proper working environment.
Your Environment