From a62287cbfaad811c5e67bf5c1f08479ae50f9ea0 Mon Sep 17 00:00:00 2001 From: Dave DeRicco <30156588+ddericco@users.noreply.github.com> Date: Wed, 5 Jul 2023 21:04:26 -0400 Subject: [PATCH 1/9] Add resource schema, CRUD support --- .../networkfirewall/firewall_policy.go | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/internal/service/networkfirewall/firewall_policy.go b/internal/service/networkfirewall/firewall_policy.go index 5a6bbfa71f8..3524e38a07e 100644 --- a/internal/service/networkfirewall/firewall_policy.go +++ b/internal/service/networkfirewall/firewall_policy.go @@ -6,6 +6,7 @@ package networkfirewall import ( "context" "log" + "regexp" "time" "github.com/aws/aws-sdk-go/aws" @@ -53,6 +54,46 @@ func ResourceFirewallPolicy() *schema.Resource { MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ + "policy_variables": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "rule_variables": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "key": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.All( + validation.StringLenBetween(1, 32), + validation.StringMatch(regexp.MustCompile(`^[A-Za-z]`), "must begin with alphabetic character"), + validation.StringMatch(regexp.MustCompile(`^[A-Za-z0-9_]+$`), "must contain only alphanumeric and underscore characters"), + ), + }, + "ip_set": { + Type: schema.TypeList, + Required: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "definition": { + Type: schema.TypeSet, + Required: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, "stateful_default_actions": { Type: schema.TypeSet, Optional: true, @@ -335,6 +376,25 @@ func waitFirewallPolicyDeleted(ctx context.Context, conn *networkfirewall.Networ return nil, err } +func expandPolicyVariables(tfMap map[string]interface{}) *networkfirewall.PolicyVariables { + if tfMap == nil { + return nil + } + + policyVariables := &networkfirewall.PolicyVariables{} + + if tfList, ok := tfMap["rule_variables"].([]interface{}); ok && len(tfList) > 0 && tfList[0] != nil { + pvMap, ok := tfList[0].(map[string]interface{}) + if ok { + if v, ok := pvMap["ip_sets"].(*schema.Set); ok && v.Len() > 0 { + policyVariables.RuleVariables = expandIPSets(v.List()) + } + } + } + + return policyVariables +} + func expandStatefulEngineOptions(l []interface{}) *networkfirewall.StatefulEngineOptions { if len(l) == 0 || l[0] == nil { return nil @@ -429,6 +489,14 @@ func expandFirewallPolicy(l []interface{}) *networkfirewall.FirewallPolicy { StatelessFragmentDefaultActions: flex.ExpandStringSet(lRaw["stateless_fragment_default_actions"].(*schema.Set)), } + // if v, ok := lRaw["policy_variables"].([]interface{}); ok && len(v) > 0 { + // policy.PolicyVariables = expandPolicyVariables(v) + // } + + if v, ok := lRaw["policy_variables"]; ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + policy.PolicyVariables = expandPolicyVariables(v.([]interface{})[0].(map[string]interface{})) + } + if v, ok := lRaw["stateful_default_actions"].(*schema.Set); ok && v.Len() > 0 { policy.StatefulDefaultActions = flex.ExpandStringSet(v) } @@ -457,6 +525,9 @@ func flattenFirewallPolicy(policy *networkfirewall.FirewallPolicy) []interface{} return []interface{}{} } p := map[string]interface{}{} + if policy.PolicyVariables != nil { + p["policy_variables"] = flattenPolicyVariables(policy.PolicyVariables) + } if policy.StatefulDefaultActions != nil { p["stateful_default_actions"] = flex.FlattenStringSet(policy.StatefulDefaultActions) } @@ -482,6 +553,19 @@ func flattenFirewallPolicy(policy *networkfirewall.FirewallPolicy) []interface{} return []interface{}{p} } +func flattenPolicyVariables(variables *networkfirewall.PolicyVariables) []interface{} { + if variables == nil { + return []interface{}{} + } + + m := map[string]interface{}{ + "rule_variables": flattenIPSets(variables.RuleVariables), + } + + return []interface{}{m} + +} + func flattenStatefulEngineOptions(options *networkfirewall.StatefulEngineOptions) []interface{} { if options == nil { return []interface{}{} From 1f54c443375ad1a2aad5011fa37817e3654b0182 Mon Sep 17 00:00:00 2001 From: Dave DeRicco <30156588+ddericco@users.noreply.github.com> Date: Thu, 6 Jul 2023 13:38:15 -0400 Subject: [PATCH 2/9] Add acceptance tests --- .../networkfirewall/firewall_policy.go | 9 +-- .../networkfirewall/firewall_policy_test.go | 74 +++++++++++++++++++ 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/internal/service/networkfirewall/firewall_policy.go b/internal/service/networkfirewall/firewall_policy.go index 3524e38a07e..00d56fcf7ed 100644 --- a/internal/service/networkfirewall/firewall_policy.go +++ b/internal/service/networkfirewall/firewall_policy.go @@ -383,13 +383,8 @@ func expandPolicyVariables(tfMap map[string]interface{}) *networkfirewall.Policy policyVariables := &networkfirewall.PolicyVariables{} - if tfList, ok := tfMap["rule_variables"].([]interface{}); ok && len(tfList) > 0 && tfList[0] != nil { - pvMap, ok := tfList[0].(map[string]interface{}) - if ok { - if v, ok := pvMap["ip_sets"].(*schema.Set); ok && v.Len() > 0 { - policyVariables.RuleVariables = expandIPSets(v.List()) - } - } + if rvMap, ok := tfMap["rule_variables"].(*schema.Set); ok && rvMap.Len() > 0 { + policyVariables.RuleVariables = expandIPSets(rvMap.List()) } return policyVariables diff --git a/internal/service/networkfirewall/firewall_policy_test.go b/internal/service/networkfirewall/firewall_policy_test.go index a2eea19b7df..2477cda9e6c 100644 --- a/internal/service/networkfirewall/firewall_policy_test.go +++ b/internal/service/networkfirewall/firewall_policy_test.go @@ -116,6 +116,59 @@ func TestAccNetworkFirewallFirewallPolicy_encryptionConfiguration(t *testing.T) }) } +func TestAccNetworkFirewallFirewallPolicy_policyVariables(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_policyVariables(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.policy_variables.#", "1"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.policy_variables.0.rule_variables.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "firewall_policy.0.policy_variables.0.rule_variables.*", map[string]string{ + "key": "HOME_NET", + "ip_set.#": "1", + "ip_set.0.definition.#": "2", + }), + resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.policy_variables.0.rule_variables.*.ip_set.0.definition.*", "10.0.0.0/16"), + resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.policy_variables.0.rule_variables.*.ip_set.0.definition.*", "10.0.1.0/24"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.*", "aws:drop"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_default_actions.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.stateless_default_actions.*", "aws:pass"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccFirewallPolicyConfig_basic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.policy_variables.#", "0"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.*", "aws:drop"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_default_actions.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.stateless_default_actions.*", "aws:pass"), + ), + }, + }, + }) +} + func TestAccNetworkFirewallFirewallPolicy_statefulDefaultActions(t *testing.T) { ctx := acctest.Context(t) var firewallPolicy networkfirewall.DescribeFirewallPolicyOutput @@ -1175,6 +1228,27 @@ resource "aws_networkfirewall_firewall_policy" "test" { `, rName, ruleOrder, streamExceptionPolicy) } +func testAccFirewallPolicyConfig_policyVariables(rName string) string { + return fmt.Sprintf(` +resource "aws_networkfirewall_firewall_policy" "test" { + name = %[1]q + + firewall_policy { + policy_variables { + rule_variables { + key = "HOME_NET" + ip_set { + definition = ["10.0.0.0/16", "10.0.1.0/24"] + } + } + } + stateless_fragment_default_actions = ["aws:drop"] + stateless_default_actions = ["aws:pass"] + } +} +`, rName) +} + func testAccFirewallPolicyConfig_ruleOrderOnly(rName, ruleOrder string) string { return fmt.Sprintf(` resource "aws_networkfirewall_firewall_policy" "test" { From 457ea3861c16f218045bef26cece4f26c25455c9 Mon Sep 17 00:00:00 2001 From: Dave DeRicco <30156588+ddericco@users.noreply.github.com> Date: Thu, 6 Jul 2023 13:51:11 -0400 Subject: [PATCH 3/9] Update MaxItems on rule_variables --- internal/service/networkfirewall/firewall_policy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/networkfirewall/firewall_policy.go b/internal/service/networkfirewall/firewall_policy.go index 00d56fcf7ed..a22c4300a60 100644 --- a/internal/service/networkfirewall/firewall_policy.go +++ b/internal/service/networkfirewall/firewall_policy.go @@ -63,6 +63,7 @@ func ResourceFirewallPolicy() *schema.Resource { "rule_variables": { Type: schema.TypeSet, Optional: true, + MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "key": { From 0c1c07bb3b921aae038a4c3779fb19e001738285 Mon Sep 17 00:00:00 2001 From: Dave DeRicco <30156588+ddericco@users.noreply.github.com> Date: Thu, 6 Jul 2023 14:23:49 -0400 Subject: [PATCH 4/9] Update docs with new arguments, example --- ...workfirewall_firewall_policy.html.markdown | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/website/docs/r/networkfirewall_firewall_policy.html.markdown b/website/docs/r/networkfirewall_firewall_policy.html.markdown index 7216826551d..d5a4efc3991 100644 --- a/website/docs/r/networkfirewall_firewall_policy.html.markdown +++ b/website/docs/r/networkfirewall_firewall_policy.html.markdown @@ -32,6 +32,36 @@ resource "aws_networkfirewall_firewall_policy" "example" { } ``` +## Policy with a HOME_NET Override + +```terraform +resource "aws_networkfirewall_firewall_policy" "example" { + name = "example" + + firewall_policy { + policy_variables { + rule_variables { + key = "HOME_NET" + ip_set { + definition = ["10.0.0.0/16", "10.1.0.0/24"] + } + } + } + stateless_default_actions = ["aws:pass"] + stateless_fragment_default_actions = ["aws:drop"] + stateless_rule_group_reference { + priority = 1 + resource_arn = aws_networkfirewall_rule_group.example.arn + } + } + + tags = { + Tag1 = "Value1" + Tag2 = "Value2" + } +} +``` + ## Policy with a Custom Action for Stateless Inspection ```terraform @@ -81,6 +111,8 @@ The following arguments are supported: The `firewall_policy` block supports the following arguments: +* `policy_variables` - (Optional). Contains variables that you can use to override default Suricata settings in your firewall policy. See [Rule Variables](#rule-variables) for details. + * `stateful_default_actions` - (Optional) Set of actions to take on a packet if it does not match any stateful rules in the policy. This can only be specified if the policy has a `stateful_engine_options` block with a `rule_order` value of `STRICT_ORDER`. You can specify one of either or neither values of `aws:drop_strict` or `aws:drop_established`, as well as any combination of `aws:alert_strict` and `aws:alert_established`. * `stateful_engine_options` - (Optional) A configuration block that defines options on how the policy handles stateful rules. See [Stateful Engine Options](#stateful-engine-options) below for details. @@ -97,6 +129,18 @@ In addition, you can specify custom actions that are compatible with your standa * `stateless_rule_group_reference` - (Optional) Set of configuration blocks containing references to the stateless rule groups that are used in the policy. See [Stateless Rule Group Reference](#stateless-rule-group-reference) below for details. +### Rule Variables +The `rule_variables` block supports the following arguments: + +* `key` - (Required) An alphanumeric string to identify the `ip_set`. Valid values: `HOME_NET` + +* `ip_set` - (Required) A configuration block that defines a set of IP addresses. See [IP Set](#ip-set) below for details. + +### IP Set +The `ip_set` block supports the following arguement: + +* `definition` - (Required) Set of IPv4 or IPv6 addresses in CIDR notation to use for the Suricata `HOME_NET` variable. + ### Stateful Engine Options The `stateful_engine_options` block supports the following argument: From ce57a50a44905b3ae925e4adf44f79898a7a9389 Mon Sep 17 00:00:00 2001 From: Dave DeRicco <30156588+ddericco@users.noreply.github.com> Date: Thu, 6 Jul 2023 15:14:47 -0400 Subject: [PATCH 5/9] Run linters --- internal/service/networkfirewall/firewall_policy.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/service/networkfirewall/firewall_policy.go b/internal/service/networkfirewall/firewall_policy.go index a22c4300a60..e533976d8bf 100644 --- a/internal/service/networkfirewall/firewall_policy.go +++ b/internal/service/networkfirewall/firewall_policy.go @@ -63,7 +63,6 @@ func ResourceFirewallPolicy() *schema.Resource { "rule_variables": { Type: schema.TypeSet, Optional: true, - MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "key": { @@ -559,7 +558,6 @@ func flattenPolicyVariables(variables *networkfirewall.PolicyVariables) []interf } return []interface{}{m} - } func flattenStatefulEngineOptions(options *networkfirewall.StatefulEngineOptions) []interface{} { From 76288e0dfeb748b610c34b02d34f54293e994a82 Mon Sep 17 00:00:00 2001 From: Dave DeRicco <30156588+ddericco@users.noreply.github.com> Date: Thu, 6 Jul 2023 15:39:09 -0400 Subject: [PATCH 6/9] Add changelog --- .changelog/32400.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/32400.txt diff --git a/.changelog/32400.txt b/.changelog/32400.txt new file mode 100644 index 00000000000..b401ba57cba --- /dev/null +++ b/.changelog/32400.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_networkfirewall_firewall_policy: Add policy_variables argument to support HOME_NET override +``` \ No newline at end of file From 0795b1f4596c61935f8dd3016f988656a0b54bc3 Mon Sep 17 00:00:00 2001 From: Dave DeRicco <30156588+ddericco@users.noreply.github.com> Date: Thu, 6 Jul 2023 15:42:30 -0400 Subject: [PATCH 7/9] Guess whose 'make docs-lint' doesn't work? --- 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 d5a4efc3991..2970b6f7c27 100644 --- a/website/docs/r/networkfirewall_firewall_policy.html.markdown +++ b/website/docs/r/networkfirewall_firewall_policy.html.markdown @@ -130,6 +130,7 @@ In addition, you can specify custom actions that are compatible with your standa * `stateless_rule_group_reference` - (Optional) Set of configuration blocks containing references to the stateless rule groups that are used in the policy. See [Stateless Rule Group Reference](#stateless-rule-group-reference) below for details. ### Rule Variables + The `rule_variables` block supports the following arguments: * `key` - (Required) An alphanumeric string to identify the `ip_set`. Valid values: `HOME_NET` @@ -137,7 +138,8 @@ The `rule_variables` block supports the following arguments: * `ip_set` - (Required) A configuration block that defines a set of IP addresses. See [IP Set](#ip-set) below for details. ### IP Set -The `ip_set` block supports the following arguement: + +The `ip_set` block supports the following argument: * `definition` - (Required) Set of IPv4 or IPv6 addresses in CIDR notation to use for the Suricata `HOME_NET` variable. From 1be3b5f3604402f96a9ee1aa756c331fbcfb04c5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 26 Jul 2023 09:41:31 -0400 Subject: [PATCH 8/9] Tweak CHANGELOG entry. --- .changelog/32400.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/32400.txt b/.changelog/32400.txt index b401ba57cba..8211d9baea4 100644 --- a/.changelog/32400.txt +++ b/.changelog/32400.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -resource/aws_networkfirewall_firewall_policy: Add policy_variables argument to support HOME_NET override +resource/aws_networkfirewall_firewall_policy: Add `firewall_policy.policy_variables` configuration block to support Suricata HOME_NET variable override ``` \ No newline at end of file From e960202f34808bd7767703438da256f2941b0311 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 26 Jul 2023 09:47:34 -0400 Subject: [PATCH 9/9] Additional checks in 'TestAccNetworkFirewallFirewallPolicy_basic'. --- internal/service/networkfirewall/firewall_policy.go | 4 ---- .../service/networkfirewall/firewall_policy_test.go | 12 +++++++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/service/networkfirewall/firewall_policy.go b/internal/service/networkfirewall/firewall_policy.go index e533976d8bf..301a1835474 100644 --- a/internal/service/networkfirewall/firewall_policy.go +++ b/internal/service/networkfirewall/firewall_policy.go @@ -484,10 +484,6 @@ func expandFirewallPolicy(l []interface{}) *networkfirewall.FirewallPolicy { StatelessFragmentDefaultActions: flex.ExpandStringSet(lRaw["stateless_fragment_default_actions"].(*schema.Set)), } - // if v, ok := lRaw["policy_variables"].([]interface{}); ok && len(v) > 0 { - // policy.PolicyVariables = expandPolicyVariables(v) - // } - if v, ok := lRaw["policy_variables"]; ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { policy.PolicyVariables = expandPolicyVariables(v.([]interface{})[0].(map[string]interface{})) } diff --git a/internal/service/networkfirewall/firewall_policy_test.go b/internal/service/networkfirewall/firewall_policy_test.go index 2477cda9e6c..d92381aba7d 100644 --- a/internal/service/networkfirewall/firewall_policy_test.go +++ b/internal/service/networkfirewall/firewall_policy_test.go @@ -33,15 +33,21 @@ func TestAccNetworkFirewallFirewallPolicy_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccFirewallPolicyConfig_basic(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckFirewallPolicyExists(ctx, resourceName, &firewallPolicy), acctest.CheckResourceAttrRegionalARN(resourceName, "arn", "network-firewall", fmt.Sprintf("firewall-policy/%s", rName)), resource.TestCheckResourceAttr(resourceName, "description", ""), resource.TestCheckResourceAttr(resourceName, "firewall_policy.#", "1"), - resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.#", "1"), - resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.*", "aws:drop"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.policy_variables.#", "0"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_default_actions.#", "0"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_engine_options.#", "0"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateful_rule_group_reference.#", "0"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_custom_action.#", "0"), resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_default_actions.#", "1"), resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.stateless_default_actions.*", "aws:pass"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "firewall_policy.0.stateless_fragment_default_actions.*", "aws:drop"), + resource.TestCheckResourceAttr(resourceName, "firewall_policy.0.stateless_rule_group_reference.#", "0"), resource.TestCheckResourceAttr(resourceName, "name", rName), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ),