From bab154528de48aa0b9ff3bbd92ab0ad89bed6039 Mon Sep 17 00:00:00 2001 From: Thayne McCombs Date: Tue, 4 Oct 2022 23:56:36 -0600 Subject: [PATCH 1/4] aws_networkfirewall_rule_group: Use list instead of set for stateful_rule Order matters for the StatefulRules, at least when rule_order is set to STRICT_ORDER, so change the types from TypeSet to TypeList, so AWS doesn't change the order when it writes it up. Fixes: #24977 --- .../service/networkfirewall/rule_group.go | 6 +- .../networkfirewall/rule_group_test.go | 134 ++++++++---------- 2 files changed, 61 insertions(+), 79 deletions(-) diff --git a/internal/service/networkfirewall/rule_group.go b/internal/service/networkfirewall/rule_group.go index ba99aa6b31e2..84affc0581e3 100644 --- a/internal/service/networkfirewall/rule_group.go +++ b/internal/service/networkfirewall/rule_group.go @@ -168,7 +168,7 @@ func ResourceRuleGroup() *schema.Resource { Optional: true, }, "stateful_rule": { - Type: schema.TypeSet, + Type: schema.TypeList, Optional: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -749,8 +749,8 @@ func expandRuleGroup(l []interface{}) *networkfirewall.RuleGroup { if v, ok := rsMap["rules_string"].(string); ok && v != "" { rulesSource.RulesString = aws.String(v) } - if v, ok := rsMap["stateful_rule"].(*schema.Set); ok && v.Len() > 0 { - rulesSource.StatefulRules = expandStatefulRules(v.List()) + if v, ok := rsMap["stateful_rule"].([]interface{}); ok && len(v) > 0 { + rulesSource.StatefulRules = expandStatefulRules(v) } if v, ok := rsMap["stateless_rules_and_custom_actions"].([]interface{}); ok && len(v) > 0 && v[0] != nil { rulesSource.StatelessRulesAndCustomActions = expandStatelessRulesAndCustomActions(v) diff --git a/internal/service/networkfirewall/rule_group_test.go b/internal/service/networkfirewall/rule_group_test.go index 0a50b23881ab..ccd991cb2fab 100644 --- a/internal/service/networkfirewall/rule_group_test.go +++ b/internal/service/networkfirewall/rule_group_test.go @@ -78,18 +78,16 @@ func TestAccNetworkFirewallRuleGroup_Basic_statefulRule(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "rule_group.#", "1"), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.#", "1"), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionPass, - "header.#": "1", - "header.0.destination": "124.1.1.24/32", - "header.0.destination_port": "53", - "header.0.direction": networkfirewall.StatefulRuleDirectionAny, - "header.0.protocol": networkfirewall.StatefulRuleProtocolTcp, - "header.0.source": "1.2.3.4/32", - "header.0.source_port": "53", - "rule_option.#": "1", - }), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*.rule_option.*", map[string]string{ + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionPass), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination", "124.1.1.24/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination_port", "53"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.direction", networkfirewall.StatefulRuleDirectionAny), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.protocol", networkfirewall.StatefulRuleProtocolTcp), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source", "1.2.3.4/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source_port", "53"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.rule_option.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.rule_option.*", map[string]string{ "keyword": "sid:1", }), resource.TestCheckResourceAttr(resourceName, "rule_group.0.stateful_rule_options.#", "0"), @@ -501,17 +499,15 @@ func TestAccNetworkFirewallRuleGroup_updateStatefulRule(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionDrop, - "header.#": "1", - "header.0.destination": "1.2.3.4/32", - "header.0.destination_port": "1001", - "header.0.direction": networkfirewall.StatefulRuleDirectionForward, - "header.0.protocol": networkfirewall.StatefulRuleProtocolIp, - "header.0.source": "124.1.1.24/32", - "header.0.source_port": "1001", - "rule_option.#": "1", - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionDrop), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination", "1.2.3.4/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination_port", "1001"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.direction", networkfirewall.StatefulRuleDirectionForward), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.protocol", networkfirewall.StatefulRuleProtocolIp), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source", "124.1.1.24/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source_port", "1001"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.rule_option.#", "1"), ), }, { @@ -549,17 +545,15 @@ func TestAccNetworkFirewallRuleGroup_updateMultipleStatefulRules(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "2"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionPass, - "header.#": "1", - "header.0.destination": "124.1.1.24/32", - "header.0.destination_port": "53", - "header.0.direction": networkfirewall.StatefulRuleDirectionAny, - "header.0.protocol": networkfirewall.StatefulRuleProtocolTcp, - "header.0.source": "1.2.3.4/32", - "header.0.source_port": "53", - "rule_option.#": "1", - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionPass), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination", "124.1.1.24/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination_port", "53"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.direction", networkfirewall.StatefulRuleDirectionAny), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.protocol", networkfirewall.StatefulRuleProtocolTcp), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source", "1.2.3.4/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source_port", "53"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.rule_option.#", "1"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ "action": networkfirewall.StatefulActionAlert, "header.#": "1", @@ -583,17 +577,15 @@ func TestAccNetworkFirewallRuleGroup_updateMultipleStatefulRules(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionDrop, - "header.#": "1", - "header.0.destination": "1.2.3.4/32", - "header.0.destination_port": "1001", - "header.0.direction": networkfirewall.StatefulRuleDirectionForward, - "header.0.protocol": networkfirewall.StatefulRuleProtocolIp, - "header.0.source": "124.1.1.24/32", - "header.0.source_port": "1001", - "rule_option.#": "1", - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionDrop), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination", "1.2.3.4/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination_port", "1001"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.direction", networkfirewall.StatefulRuleDirectionForward), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.protocol", networkfirewall.StatefulRuleProtocolIp), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source", "124.1.1.24/32"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source_port", "1001"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.rule_option.#", "1"), ), }, { @@ -624,9 +616,7 @@ func TestAccNetworkFirewallRuleGroup_StatefulRule_action(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionAlert, - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionAlert), ), }, { @@ -639,9 +629,7 @@ func TestAccNetworkFirewallRuleGroup_StatefulRule_action(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionPass, - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionPass), ), }, { @@ -654,9 +642,7 @@ func TestAccNetworkFirewallRuleGroup_StatefulRule_action(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionDrop, - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", networkfirewall.StatefulActionDrop), ), }, { @@ -685,17 +671,15 @@ func TestAccNetworkFirewallRuleGroup_StatefulRule_header(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionPass, - "header.#": "1", - "header.0.destination": "ANY", - "header.0.destination_port": "1990", - "header.0.direction": networkfirewall.StatefulRuleDirectionAny, - "header.0.protocol": networkfirewall.StatefulRuleProtocolTcp, - "header.0.source": "ANY", - "header.0.source_port": "1994", - "rule_option.#": "1", - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionPass), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination", "ANY"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination_port", "1990"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.direction", networkfirewall.StatefulRuleDirectionAny), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.protocol", networkfirewall.StatefulRuleProtocolTcp), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source", "ANY"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source_port", "1994"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.rule_option.#", "1"), ), }, { @@ -708,17 +692,15 @@ func TestAccNetworkFirewallRuleGroup_StatefulRule_header(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule_group.0.rules_source.0.stateful_rule.*", map[string]string{ - "action": networkfirewall.StatefulActionPass, - "header.#": "1", - "header.0.destination": "ANY", - "header.0.destination_port": "ANY", - "header.0.direction": networkfirewall.StatefulRuleDirectionAny, - "header.0.protocol": networkfirewall.StatefulRuleProtocolTcp, - "header.0.source": "ANY", - "header.0.source_port": "ANY", - "rule_option.#": "1", - }), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionPass), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.#", "1"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination", "ANY"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.destination_port", "ANY"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.direction", networkfirewall.StatefulRuleDirectionAny), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.protocol", networkfirewall.StatefulRuleProtocolTcp), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source", "ANY"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.header.0.source_port", "ANY"), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.rule_option.#", "1"), ), }, { From 51db9aa116e3ecd2d99cd65d7d31d015e6ff3dfc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 16 Dec 2022 15:37:01 -0500 Subject: [PATCH 2/4] Add CHANGELOG entry. --- .changelog/27102.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/27102.txt diff --git a/.changelog/27102.txt b/.changelog/27102.txt new file mode 100644 index 000000000000..bb2fa6cc26fe --- /dev/null +++ b/.changelog/27102.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_networkfirewall_rule_group: Change `rule_group.rules_source.stateful_rule` frmo `TypeSet` to `TypeList` to preserve rule order +``` \ No newline at end of file From 2bf8412a8197fc84bcd2618c5e9c61133e2951be Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 16 Dec 2022 15:45:34 -0500 Subject: [PATCH 3/4] Update 27102.txt --- .changelog/27102.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changelog/27102.txt b/.changelog/27102.txt index bb2fa6cc26fe..32cdba44fb98 100644 --- a/.changelog/27102.txt +++ b/.changelog/27102.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_networkfirewall_rule_group: Change `rule_group.rules_source.stateful_rule` frmo `TypeSet` to `TypeList` to preserve rule order -``` \ No newline at end of file +resource/aws_networkfirewall_rule_group: Change `rule_group.rules_source.stateful_rule` from `TypeSet` to `TypeList` to preserve rule order +``` From 7baa7c8a4bb629b1f0daa6533bfc61df69f4c170 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 16 Dec 2022 16:04:36 -0500 Subject: [PATCH 4/4] Fix typo in 'TestAccNetworkFirewallRuleGroup_StatefulRule_action'. --- internal/service/networkfirewall/rule_group_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/networkfirewall/rule_group_test.go b/internal/service/networkfirewall/rule_group_test.go index 66b77a6a916c..81d7d5fdc2bf 100644 --- a/internal/service/networkfirewall/rule_group_test.go +++ b/internal/service/networkfirewall/rule_group_test.go @@ -731,7 +731,7 @@ func TestAccNetworkFirewallRuleGroup_StatefulRule_action(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckRuleGroupExists(resourceName, &ruleGroup), resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "1"), - resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", networkfirewall.StatefulActionDrop), + resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.0.action", networkfirewall.StatefulActionDrop), ), }, {