From b8a1f98c6d1eb71d0bf5fd6af6d950d8c5ddfa6c Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Tue, 9 May 2023 08:54:28 +0100 Subject: [PATCH 1/6] Add Network Firewall stream exception policy --- .../service/networkfirewall/firewall_policy.go | 13 +++++++++++-- .../firewall_policy_data_source.go | 4 ++++ .../networkfirewall/firewall_policy_test.go | 16 ++++++++++------ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/internal/service/networkfirewall/firewall_policy.go b/internal/service/networkfirewall/firewall_policy.go index c99aee159e19..411f46d41c8a 100644 --- a/internal/service/networkfirewall/firewall_policy.go +++ b/internal/service/networkfirewall/firewall_policy.go @@ -63,9 +63,14 @@ func ResourceFirewallPolicy() *schema.Resource { Schema: map[string]*schema.Schema{ "rule_order": { Type: schema.TypeString, - Required: true, + Optional: true, ValidateFunc: validation.StringInSlice(networkfirewall.RuleOrder_Values(), false), }, + "stream_exception_policy": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice(networkfirewall.StreamExceptionPolicy_Values(), false), + }, }, }, }, @@ -338,6 +343,9 @@ func expandStatefulEngineOptions(l []interface{}) *networkfirewall.StatefulEngin if v, ok := m["rule_order"].(string); ok { options.RuleOrder = aws.String(v) } + if v, ok := m["stream_exception_policy"].(string); ok { + options.StreamExceptionPolicy = aws.String(v) + } return options } @@ -477,7 +485,8 @@ func flattenStatefulEngineOptions(options *networkfirewall.StatefulEngineOptions } m := map[string]interface{}{ - "rule_order": aws.StringValue(options.RuleOrder), + "rule_order": aws.StringValue(options.RuleOrder), + "stream_exception_policy": aws.StringValue(options.StreamExceptionPolicy), } return []interface{}{m} diff --git a/internal/service/networkfirewall/firewall_policy_data_source.go b/internal/service/networkfirewall/firewall_policy_data_source.go index 2dec652e28ba..838281f3349a 100644 --- a/internal/service/networkfirewall/firewall_policy_data_source.go +++ b/internal/service/networkfirewall/firewall_policy_data_source.go @@ -48,6 +48,10 @@ func DataSourceFirewallPolicy() *schema.Resource { Type: schema.TypeString, Computed: true, }, + "stream_exception_policy": { + Type: schema.TypeString, + Computed: true, + }, }, }, }, diff --git a/internal/service/networkfirewall/firewall_policy_test.go b/internal/service/networkfirewall/firewall_policy_test.go index 0542bb26f31c..edddc744557f 100644 --- a/internal/service/networkfirewall/firewall_policy_test.go +++ b/internal/service/networkfirewall/firewall_policy_test.go @@ -156,12 +156,13 @@ func TestAccNetworkFirewallFirewallPolicy_statefulEngineOption(t *testing.T) { CheckDestroy: testAccCheckFirewallPolicyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccFirewallPolicyConfig_statefulEngineOptions(rName, "STRICT_ORDER"), + Config: testAccFirewallPolicyConfig_statefulEngineOptions(rName, "STRICT_ORDER", "DROP"), Check: resource.ComposeTestCheckFunc( testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy), resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.#", "1"), resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.rule_order", networkfirewall.RuleOrderStrictOrder), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.stream_exception_policy", networkfirewall.StreamExceptionPolicyDrop), ), }, { @@ -186,12 +187,13 @@ func TestAccNetworkFirewallFirewallPolicy_updateStatefulEngineOption(t *testing. CheckDestroy: testAccCheckFirewallPolicyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccFirewallPolicyConfig_statefulEngineOptions(rName, "DEFAULT_ACTION_ORDER"), + Config: testAccFirewallPolicyConfig_statefulEngineOptions(rName, "DEFAULT_ACTION_ORDER", "CONTINUE"), Check: resource.ComposeTestCheckFunc( testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy1), resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.#", "1"), resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.rule_order", networkfirewall.RuleOrderDefaultActionOrder), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.stream_exception_policy", networkfirewall.StreamExceptionPolicyContinue), ), }, { @@ -203,13 +205,14 @@ func TestAccNetworkFirewallFirewallPolicy_updateStatefulEngineOption(t *testing. ), }, { - Config: testAccFirewallPolicyConfig_statefulEngineOptions(rName, "STRICT_ORDER"), + Config: testAccFirewallPolicyConfig_statefulEngineOptions(rName, "STRICT_ORDER", "REJECT"), Check: resource.ComposeTestCheckFunc( testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy3), testAccCheckFirewallPolicyRecreated(&firewallPolicy2, &firewallPolicy3), resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.#", "1"), resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.rule_order", networkfirewall.RuleOrderStrictOrder), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.stream_exception_policy", networkfirewall.StreamExceptionPolicyReject), ), }, { @@ -1110,7 +1113,7 @@ resource "aws_networkfirewall_firewall_policy" "test" { `, rName, tagKey1, tagValue1, tagKey2, tagValue2) } -func testAccFirewallPolicyConfig_statefulEngineOptions(rName, ruleOrder string) string { +func testAccFirewallPolicyConfig_statefulEngineOptions(rName, ruleOrder, streamExceptionPolicy string) string { return fmt.Sprintf(` resource "aws_networkfirewall_firewall_policy" "test" { name = %[1]q @@ -1120,11 +1123,12 @@ resource "aws_networkfirewall_firewall_policy" "test" { stateless_default_actions = ["aws:pass"] stateful_engine_options { - rule_order = %[2]q + rule_order = %[2]q + stream_exception_policy = %[3]q } } } -`, rName, ruleOrder) +`, rName, ruleOrder, streamExceptionPolicy) } func testAccFirewallPolicyConfig_statefulDefaultActions(rName string) string { From 654c4312e6d19597ca00ed1a7634c2404bf8d59c Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Fri, 19 May 2023 07:44:56 +0100 Subject: [PATCH 2/6] Handle the case where one of the stateful options isn't defined --- internal/service/networkfirewall/firewall_policy.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/service/networkfirewall/firewall_policy.go b/internal/service/networkfirewall/firewall_policy.go index 411f46d41c8a..6842a3c6ea23 100644 --- a/internal/service/networkfirewall/firewall_policy.go +++ b/internal/service/networkfirewall/firewall_policy.go @@ -340,10 +340,10 @@ func expandStatefulEngineOptions(l []interface{}) *networkfirewall.StatefulEngin options := &networkfirewall.StatefulEngineOptions{} m := l[0].(map[string]interface{}) - if v, ok := m["rule_order"].(string); ok { + if v, ok := m["rule_order"].(string); ok && v != "" { options.RuleOrder = aws.String(v) } - if v, ok := m["stream_exception_policy"].(string); ok { + if v, ok := m["stream_exception_policy"].(string); ok && v != "" { options.StreamExceptionPolicy = aws.String(v) } @@ -484,9 +484,12 @@ func flattenStatefulEngineOptions(options *networkfirewall.StatefulEngineOptions return []interface{}{} } - m := map[string]interface{}{ - "rule_order": aws.StringValue(options.RuleOrder), - "stream_exception_policy": aws.StringValue(options.StreamExceptionPolicy), + m := map[string]interface{}{} + if options.RuleOrder != nil { + m["rule_order"] = aws.StringValue(options.RuleOrder) + } + if options.StreamExceptionPolicy != nil { + m["stream_exception_policy"] = aws.StringValue(options.StreamExceptionPolicy) } return []interface{}{m} From ad211ec033fb3fd590bccc04a01fe984750b658d Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Mon, 22 May 2023 16:17:44 +0100 Subject: [PATCH 3/6] Add Network Firewall Stateful Engine Options test when only one defined It should be possible to configure just one of the stateful engine options, in which case the other will be missing from the state and AWS will use the default option for that configuration. --- .../networkfirewall/firewall_policy_test.go | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/internal/service/networkfirewall/firewall_policy_test.go b/internal/service/networkfirewall/firewall_policy_test.go index edddc744557f..63367aaf8cb1 100644 --- a/internal/service/networkfirewall/firewall_policy_test.go +++ b/internal/service/networkfirewall/firewall_policy_test.go @@ -224,6 +224,47 @@ func TestAccNetworkFirewallFirewallPolicy_updateStatefulEngineOption(t *testing. }) } +func TestAccNetworkFirewallFirewallPolicy_statefulEngineOptionsSingle(t *testing.T) { + ctx := acctest.Context(t) + var firewallPolicy networkfirewall.DescribeFirewallPolicyOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "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, + CheckDestroy: testAccCheckFirewallPolicyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccFirewallPolicyConfig_ruleOrderOnly(rName, "DEFAULT_ACTION_ORDER"), + Check: resource.ComposeTestCheckFunc( + testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.#", "1"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.rule_order", networkfirewall.RuleOrderDefaultActionOrder), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.stream_exception_policy", ""), + ), + }, + { + Config: testAccFirewallPolicyConfig_streamExceptionPolicyOnly(rName, "REJECT"), + Check: resource.ComposeTestCheckFunc( + testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.#", "1"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.rule_order", ""), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.0.stream_exception_policy", networkfirewall.StreamExceptionPolicyReject), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference(t *testing.T) { ctx := acctest.Context(t) var firewallPolicy networkfirewall.DescribeFirewallPolicyOutput @@ -1131,6 +1172,40 @@ resource "aws_networkfirewall_firewall_policy" "test" { `, rName, ruleOrder, streamExceptionPolicy) } +func testAccFirewallPolicyConfig_ruleOrderOnly(rName, ruleOrder string) string { + return fmt.Sprintf(` +resource "aws_networkfirewall_firewall_policy" "test" { + name = %[1]q + + firewall_policy { + stateless_fragment_default_actions = ["aws:drop"] + stateless_default_actions = ["aws:pass"] + + stateful_engine_options { + rule_order = %[2]q + } + } +} +`, rName, ruleOrder) +} + +func testAccFirewallPolicyConfig_streamExceptionPolicyOnly(rName, streamExceptionPolicy string) string { + return fmt.Sprintf(` +resource "aws_networkfirewall_firewall_policy" "test" { + name = %[1]q + + firewall_policy { + stateless_fragment_default_actions = ["aws:drop"] + stateless_default_actions = ["aws:pass"] + + stateful_engine_options { + stream_exception_policy = %[2]q + } + } +} +`, rName, streamExceptionPolicy) +} + func testAccFirewallPolicyConfig_statefulDefaultActions(rName string) string { return fmt.Sprintf(` resource "aws_networkfirewall_firewall_policy" "test" { From 382c45b8db578d2953530f9c2e075c3137e60447 Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Tue, 23 May 2023 14:30:35 +0100 Subject: [PATCH 4/6] Update Network Firewall Policy docs with stream_exception_policy rule_order is no longer required if that block is defined, it can be one or the other option, or both. --- website/docs/r/networkfirewall_firewall_policy.html.markdown | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/website/docs/r/networkfirewall_firewall_policy.html.markdown b/website/docs/r/networkfirewall_firewall_policy.html.markdown index ff82d96197d9..7216826551d4 100644 --- a/website/docs/r/networkfirewall_firewall_policy.html.markdown +++ b/website/docs/r/networkfirewall_firewall_policy.html.markdown @@ -103,7 +103,9 @@ The `stateful_engine_options` block supports the following argument: ~> **NOTE:** If the `STRICT_ORDER` rule order is specified, this firewall policy can only reference stateful rule groups that utilize `STRICT_ORDER`. -* `rule_order` - (Required) Indicates how to manage the order of stateful rule evaluation for the policy. Default value: `DEFAULT_ACTION_ORDER`. Valid values: `DEFAULT_ACTION_ORDER`, `STRICT_ORDER`. +* `rule_order` - Indicates how to manage the order of stateful rule evaluation for the policy. Default value: `DEFAULT_ACTION_ORDER`. Valid values: `DEFAULT_ACTION_ORDER`, `STRICT_ORDER`. + +* `stream_exception_policy` - Describes how to treat traffic which has broken midstream. Default value: `DROP`. Valid values: `DROP`, `CONTINUE`, `REJECT`. ### Stateful Rule Group Reference From 3dc855e4a04eeb6281296a0f0f6b6d7b4a412c35 Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Tue, 23 May 2023 16:42:12 +0100 Subject: [PATCH 5/6] Fix formatting in Terraform test --- internal/service/networkfirewall/firewall_policy_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/networkfirewall/firewall_policy_test.go b/internal/service/networkfirewall/firewall_policy_test.go index 63367aaf8cb1..55095e9736e2 100644 --- a/internal/service/networkfirewall/firewall_policy_test.go +++ b/internal/service/networkfirewall/firewall_policy_test.go @@ -1182,7 +1182,7 @@ resource "aws_networkfirewall_firewall_policy" "test" { stateless_default_actions = ["aws:pass"] stateful_engine_options { - rule_order = %[2]q + rule_order = %[2]q } } } From 53aee85c005348f853d6edb581867995fcf1262e Mon Sep 17 00:00:00 2001 From: Dan Corne Date: Tue, 23 May 2023 16:45:05 +0100 Subject: [PATCH 6/6] Add changelog 31541 --- .changelog/31541.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/31541.txt diff --git a/.changelog/31541.txt b/.changelog/31541.txt new file mode 100644 index 000000000000..17c9faae3ad1 --- /dev/null +++ b/.changelog/31541.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_networkfirewall_firewall_policy: Add `stream_exception_policy` option to `firewall_policy.stateful_engine_options` +```