From 6807f3d6fb080af5ff3dfedb984d6c92af6576eb Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Fri, 28 Apr 2023 14:29:24 +0100 Subject: [PATCH 1/6] Add test for firewall policy data source with override This currently fails with the following error: panic: Invalid address to set: []string{"stateful_rule_group_reference", "0", "override"} I tried to include an aws_networkfirewall_rule_group resource in the test configuration to avoid depending on the rule group existing, however override blocks are only available on AWS-managed rule group references. --- .../firewall_policy_data_source_test.go | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/internal/service/networkfirewall/firewall_policy_data_source_test.go b/internal/service/networkfirewall/firewall_policy_data_source_test.go index 7fc7bc0ca301..adb2a7150681 100644 --- a/internal/service/networkfirewall/firewall_policy_data_source_test.go +++ b/internal/service/networkfirewall/firewall_policy_data_source_test.go @@ -103,6 +103,39 @@ func TestAccNetworkFirewallFirewallPolicyDataSource_nameAndARN(t *testing.T) { }) } +func TestAccNetworkFirewallFirewallPolicyDataSource_withOverriddenManagedRuleGroup(t *testing.T) { + ctx := acctest.Context(t) + var firewallPolicy networkfirewall.DescribeFirewallPolicyOutput + rName := sdkacctest.RandomWithPrefix("resource-test-terraform") + resourceName := "aws_networkfirewall_firewall_policy.test" + datasourceName := "data.aws_networkfirewall_firewall_policy.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, networkfirewall.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + Steps: []resource.TestStep{ + { + Config: testAccFirewallPolicyDataSourceConfig_withOverriddenManagedRuleGroup(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy), + resource.TestCheckResourceAttrPair(datasourceName, "arn", resourceName, "arn"), resource.TestCheckResourceAttrPair(datasourceName, "description", resourceName, "description"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.#", resourceName, "firewall_policy.#"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.0.stateless_fragment_default_actions.#", resourceName, "firewall_policy.0.stateless_fragment_default_actions.#"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.0.stateless_fragment_default_actions.0", resourceName, "firewall_policy.0.stateless_fragment_default_actions.0"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.0.stateless_default_actions.#", resourceName, "firewall_policy.0.stateless_default_actions.#"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.0.stateless_default_actions.0", resourceName, "firewall_policy.0.stateless_default_actions.0"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.0.stateful_rule_group_reference.#", resourceName, "firewall_policy.0.stateful_rule_group_reference.#"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.0.stateful_rule_group_reference.0", resourceName, "firewall_policy.0.stateful_rule_group_reference.0"), + resource.TestCheckResourceAttrPair(datasourceName, "firewall_policy.0.stateful_rule_group_reference.override.action", resourceName, "firewall_policy.0.stateful_rule_group_reference.override.action"), + resource.TestCheckResourceAttrPair(datasourceName, "name", resourceName, "name"), + resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"), + ), + }, + }, + }) +} + func testAccFirewallPolicyDataSourceConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_networkfirewall_firewall_policy" "test" { @@ -142,3 +175,27 @@ data "aws_networkfirewall_firewall_policy" "test" { name = aws_networkfirewall_firewall_policy.test.name }`) } + +func testAccFirewallPolicyDataSourceConfig_withOverriddenManagedRuleGroup(rName string) string { + return fmt.Sprintf(` +resource "aws_networkfirewall_firewall_policy" "test" { + name = %[1]q + + firewall_policy { + stateless_default_actions = ["aws:forward_to_sfe"] + stateless_fragment_default_actions = ["aws:forward_to_sfe"] + + # Managed rule group required for override block + stateful_rule_group_reference { + resource_arn = "arn:aws:network-firewall:eu-west-1:aws-managed:stateful-rulegroup/MalwareDomainsActionOrder" + override { + action = "DROP_TO_ALERT" + } + } + } +} + +data "aws_networkfirewall_firewall_policy" "test" { + arn = aws_networkfirewall_firewall_policy.test.arn +}`, rName) +} From 0bbd3587dc4e7dd20424ac89438e21773e2f8cfc Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Mon, 1 May 2023 15:42:11 +0100 Subject: [PATCH 2/6] Add override to firewall policy data source schema This previously caused a panic if an override is configured. --- .../networkfirewall/firewall_policy_data_source.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/service/networkfirewall/firewall_policy_data_source.go b/internal/service/networkfirewall/firewall_policy_data_source.go index 1b851ac0a32a..2dec652e28ba 100644 --- a/internal/service/networkfirewall/firewall_policy_data_source.go +++ b/internal/service/networkfirewall/firewall_policy_data_source.go @@ -56,6 +56,18 @@ func DataSourceFirewallPolicy() *schema.Resource { Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ + "override": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "action": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, "priority": { Type: schema.TypeInt, Computed: true, From 8ae6eb2b446cca217d6c270e70a92666d19c32fb Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Mon, 1 May 2023 16:22:54 +0100 Subject: [PATCH 3/6] Add changelog --- .changelog/31089.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/31089.txt diff --git a/.changelog/31089.txt b/.changelog/31089.txt new file mode 100644 index 000000000000..68e2df1bdd4b --- /dev/null +++ b/.changelog/31089.txt @@ -0,0 +1,3 @@ +```release-note:bug +data-source/aws_networkfirewall_firewall_policy: Fix crash in when a policy has an overridden action on a statefule_rule_group_reference +``` From acd083e432c2e4fe9c11ef184294320faa015c61 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 2 May 2023 16:18:12 -0400 Subject: [PATCH 4/6] Tweak CHANGELOG entry. --- .changelog/31089.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/31089.txt b/.changelog/31089.txt index 68e2df1bdd4b..8873169d9d62 100644 --- a/.changelog/31089.txt +++ b/.changelog/31089.txt @@ -1,3 +1,3 @@ ```release-note:bug -data-source/aws_networkfirewall_firewall_policy: Fix crash in when a policy has an overridden action on a statefule_rule_group_reference +data-source/aws_networkfirewall_firewall_policy: Add `firewall_policy.stateful_rule_group_reference.override` attribute ``` From 1442720a1a3dd48e7f03ac26eb2b8a30672940de Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 2 May 2023 16:25:11 -0400 Subject: [PATCH 5/6] Update 31089.txt --- .changelog/31089.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/31089.txt b/.changelog/31089.txt index 8873169d9d62..4cbf0e1c8ed2 100644 --- a/.changelog/31089.txt +++ b/.changelog/31089.txt @@ -1,3 +1,3 @@ ```release-note:bug -data-source/aws_networkfirewall_firewall_policy: Add `firewall_policy.stateful_rule_group_reference.override` attribute +data-source/aws_networkfirewall_firewall_policy: Add `firewall_policy.stateful_rule_group_reference.override` attribute, fixing `setting firewall_policy: Invalid address to set` error ``` From 78f451c6041dd9fe71201081655fcff049173098 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 2 May 2023 16:35:38 -0400 Subject: [PATCH 6/6] Fix 'testAccFirewallPolicyDataSourceConfig_withOverriddenManagedRuleGroup'. --- .../networkfirewall/firewall_policy_data_source_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/service/networkfirewall/firewall_policy_data_source_test.go b/internal/service/networkfirewall/firewall_policy_data_source_test.go index adb2a7150681..bc56d4749cf2 100644 --- a/internal/service/networkfirewall/firewall_policy_data_source_test.go +++ b/internal/service/networkfirewall/firewall_policy_data_source_test.go @@ -178,6 +178,9 @@ data "aws_networkfirewall_firewall_policy" "test" { func testAccFirewallPolicyDataSourceConfig_withOverriddenManagedRuleGroup(rName string) string { return fmt.Sprintf(` +data "aws_region" "current" {} +data "aws_partition" "current" {} + resource "aws_networkfirewall_firewall_policy" "test" { name = %[1]q @@ -185,9 +188,10 @@ resource "aws_networkfirewall_firewall_policy" "test" { stateless_default_actions = ["aws:forward_to_sfe"] stateless_fragment_default_actions = ["aws:forward_to_sfe"] - # Managed rule group required for override block + # Managed rule group required for override block. stateful_rule_group_reference { - resource_arn = "arn:aws:network-firewall:eu-west-1:aws-managed:stateful-rulegroup/MalwareDomainsActionOrder" + resource_arn = "arn:${data.aws_partition.current.partition}:network-firewall:${data.aws_region.current.name}:aws-managed:stateful-rulegroup/MalwareDomainsActionOrder" + override { action = "DROP_TO_ALERT" }