From 793664ea71eee25f252da2f6c4e02d2c1db36482 Mon Sep 17 00:00:00 2001 From: The Magician Date: Fri, 1 Nov 2024 09:50:06 -0700 Subject: [PATCH] Fix address group ordering for network firewall policy rule (#12182) (#20148) [upstream:b41c48e34de620d6042a2af0a1e905de9d66c211] Signed-off-by: Modular Magician --- .changelog/12182.txt | 3 + ...ce_compute_network_firewall_policy_rule.go | 48 +++++++- ...mpute_network_firewall_policy_rule_test.go | 104 ++++++++++++++++++ 3 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 .changelog/12182.txt diff --git a/.changelog/12182.txt b/.changelog/12182.txt new file mode 100644 index 00000000000..fcebe141d5d --- /dev/null +++ b/.changelog/12182.txt @@ -0,0 +1,3 @@ +```release-note:bug +compute: fixed a diff based on server-side reordering of `match.src_address_groups` and `match.dest_address_groups` in `google_compute_network_firewall_policy_rule` +``` \ No newline at end of file diff --git a/google/services/compute/resource_compute_network_firewall_policy_rule.go b/google/services/compute/resource_compute_network_firewall_policy_rule.go index 8d29528cc5a..cd327ae63dc 100644 --- a/google/services/compute/resource_compute_network_firewall_policy_rule.go +++ b/google/services/compute/resource_compute_network_firewall_policy_rule.go @@ -874,11 +874,55 @@ func flattenComputeNetworkFirewallPolicyRuleMatchSrcSecureTagsState(v interface{ } func flattenComputeNetworkFirewallPolicyRuleMatchDestAddressGroups(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { - return v + rawConfigValue := d.Get("match.0.dest_address_groups") + + // Convert config value to []string + configValue, err := tpgresource.InterfaceSliceToStringSlice(rawConfigValue) + if err != nil { + log.Printf("[ERROR] Failed to convert config value: %s", err) + return v + } + + // Convert v to []string + apiStringValue, err := tpgresource.InterfaceSliceToStringSlice(v) + if err != nil { + log.Printf("[ERROR] Failed to convert API value: %s", err) + return v + } + + sortedStrings, err := tpgresource.SortStringsByConfigOrder(configValue, apiStringValue) + if err != nil { + log.Printf("[ERROR] Could not sort API response value: %s", err) + return v + } + + return sortedStrings } func flattenComputeNetworkFirewallPolicyRuleMatchSrcAddressGroups(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { - return v + rawConfigValue := d.Get("match.0.src_address_groups") + + // Convert config value to []string + configValue, err := tpgresource.InterfaceSliceToStringSlice(rawConfigValue) + if err != nil { + log.Printf("[ERROR] Failed to convert config value: %s", err) + return v + } + + // Convert v to []string + apiStringValue, err := tpgresource.InterfaceSliceToStringSlice(v) + if err != nil { + log.Printf("[ERROR] Failed to convert API value: %s", err) + return v + } + + sortedStrings, err := tpgresource.SortStringsByConfigOrder(configValue, apiStringValue) + if err != nil { + log.Printf("[ERROR] Could not sort API response value: %s", err) + return v + } + + return sortedStrings } func flattenComputeNetworkFirewallPolicyRuleMatchSrcFqdns(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { diff --git a/google/services/compute/resource_compute_network_firewall_policy_rule_test.go b/google/services/compute/resource_compute_network_firewall_policy_rule_test.go index 37a06dc745b..6a6ffcb7169 100644 --- a/google/services/compute/resource_compute_network_firewall_policy_rule_test.go +++ b/google/services/compute/resource_compute_network_firewall_policy_rule_test.go @@ -142,6 +142,41 @@ func TestAccComputeNetworkFirewallPolicyRule_multipleRules(t *testing.T) { }) } +func TestAccComputeNetworkFirewallPolicyRule_addressGroupOrder(t *testing.T) { + t.Parallel() + + context := map[string]interface{}{ + "random_suffix": acctest.RandString(t, 10), + "project": envvar.GetTestProjectFromEnv(), + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeNetworkFirewallPolicyRule_addressGroupOrder(context), + }, + { + ResourceName: "google_compute_network_firewall_policy_rule.src_test", + ImportState: true, + ImportStateVerify: true, + // Referencing using ID causes import to fail + // Client-side reordering doesn't work with no state, so ignore on import + ImportStateVerifyIgnore: []string{"firewall_policy", "match.0.src_address_groups"}, + }, + { + ResourceName: "google_compute_network_firewall_policy_rule.dest_test", + ImportState: true, + ImportStateVerify: true, + // Referencing using ID causes import to fail + // Client-side reordering doesn't work with no state, so ignore on import + ImportStateVerifyIgnore: []string{"firewall_policy", "match.0.dest_address_groups"}, + }, + }, + }) +} + func TestAccComputeNetworkFirewallPolicyRule_securityProfileGroup_update(t *testing.T) { t.Parallel() @@ -898,3 +933,72 @@ resource "google_compute_network_firewall_policy_rule" "fw_policy_rule3" { } `, context) } + +func testAccComputeNetworkFirewallPolicyRule_addressGroupOrder(context map[string]interface{}) string { + return acctest.Nprintf(` +resource "google_compute_network_firewall_policy" "policy" { + name = "tf-test-policy-%{random_suffix}" + description = "Resource created for Terraform acceptance testing" +} + +resource "google_network_security_address_group" "add-group1" { + name = "tf-test-group-1-%{random_suffix}" + parent = "projects/%{project}" + location = "global" + type = "IPV4" + capacity = "10" + items = ["10.0.1.1/32"] +} +resource "google_network_security_address_group" "add-group2" { + name = "tf-test-group-2-%{random_suffix}" + parent = "projects/%{project}" + location = "global" + type = "IPV4" + capacity = "10" + items = ["10.0.2.2/32"] +} +resource "google_network_security_address_group" "add-group3" { + name = "tf-test-group-3-%{random_suffix}" + parent = "projects/%{project}" + location = "global" + type = "IPV4" + capacity = "10" + items = ["10.0.3.3/32"] +} + +resource "google_compute_network_firewall_policy_rule" "src_test" { + firewall_policy = google_compute_network_firewall_policy.policy.id + action = "allow" + priority = 1000 + description = "Testing address group order issue" + direction = "INGRESS" + enable_logging = true + match { + src_address_groups = [google_network_security_address_group.add-group2.id, + google_network_security_address_group.add-group1.id] + dest_ip_ranges = ["192.168.2.0/24", "10.0.3.4/32"] + layer4_configs { + ip_protocol = "all" + } + } +} + +resource "google_compute_network_firewall_policy_rule" "dest_test" { + firewall_policy = google_compute_network_firewall_policy.policy.id + action = "allow" + priority = 1100 + description = "Testing address group order issue" + direction = "EGRESS" + enable_logging = true + match { + dest_address_groups = [google_network_security_address_group.add-group3.id, + google_network_security_address_group.add-group2.id] + src_ip_ranges = ["192.168.2.0/24", "10.0.3.4/32"] + layer4_configs { + ip_protocol = "all" + } + } +} + +`, context) +}