check configuration changes in rules when determining update input

hashicorp · May 18, 2021 · 84dccc5 · 84dccc5
1 parent 002e91f
commit 84dccc5
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 5 deletions.
diff --git a/.changelog/19430.txt b/.changelog/19430.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_networkfirewall_rule_group: Correctly update resource on `rules` change
+```
diff --git a/aws/resource_aws_networkfirewall_rule_group.go b/aws/resource_aws_networkfirewall_rule_group.go
@@ -511,11 +511,15 @@ func resourceAwsNetworkFirewallRuleGroupUpdate(ctx context.Context, d *schema.Re
  if v, ok := d.GetOk("description"); ok {
  input.Description = aws.String(v.(string))
  }
- if v, ok := d.GetOk("rule_group"); ok {
- input.RuleGroup = expandNetworkFirewallRuleGroup(v.([]interface{}))
- }
- if v, ok := d.GetOk("rules"); ok {
- input.Rules = aws.String(v.(string))
+
+ // Network Firewall UpdateRuleGroup API method only allows one of Rules or RuleGroup
+ // else, request returns "InvalidRequestException: Exactly one of Rules or RuleGroup must be set";
+ // Here, "rules" takes precedence as "rule_group" is Computed from "rules" when configured
+ // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/19414
+ if d.HasChange("rules") {
+ input.Rules = aws.String(d.Get("rules").(string))
+ } else if d.HasChange("rule_group") {
+ input.RuleGroup = expandNetworkFirewallRuleGroup(d.Get("rule_group").([]interface{}))
  }
 
  _, err := conn.UpdateRuleGroupWithContext(ctx, input)

diff --git a/aws/resource_aws_networkfirewall_rule_group_test.go b/aws/resource_aws_networkfirewall_rule_group_test.go
@@ -289,6 +289,47 @@ func TestAccAwsNetworkFirewallRuleGroup_statelessRuleWithCustomAction(t *testing
  })
 }
 
+// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/19414
+func TestAccAwsNetworkFirewallRuleGroup_updateRules(t *testing.T) {
+ rName := acctest.RandomWithPrefix("tf-acc-test")
+ resourceName := "aws_networkfirewall_rule_group.test"
+
+ rules := `pass tls $HOME_NET any -> $EXTERNAL_NET 443 (tls.sni; content:"OLD.example.com"; msg:"FQDN test"; sid:1;)`
+ updatedRules := `pass tls $HOME_NET any -> $EXTERNAL_NET 443 (tls.sni; content:"NEW.example.com"; msg:"FQDN test"; sid:1;)`
+
+ resource.ParallelTest(t, resource.TestCase{
+ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAwsNetworkFirewall(t) },
+ ErrorCheck: testAccErrorCheck(t, networkfirewall.EndpointsID),
+ Providers: testAccProviders,
+ CheckDestroy: testAccCheckAwsNetworkFirewallRuleGroupDestroy,
+ Steps: []resource.TestStep{
+ {
+ Config: testAccNetworkFirewallRuleGroup_basic_rules(rName, rules),
+ Check: resource.ComposeTestCheckFunc(
+ testAccCheckAwsNetworkFirewallRuleGroupExists(resourceName),
+ ),
+ },
+ {
+ Config: testAccNetworkFirewallRuleGroup_basic_rules(rName, updatedRules),
+ Check: resource.ComposeTestCheckFunc(
+ testAccCheckAwsNetworkFirewallRuleGroupExists(resourceName),
+ resource.TestCheckResourceAttr(resourceName, "rules", updatedRules),
+ resource.TestCheckResourceAttr(resourceName, "rule_group.#", "1"),
+ resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.#", "1"),
+ resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.rules_string", updatedRules),
+ resource.TestCheckResourceAttr(resourceName, "rule_group.0.rules_source.0.stateful_rule.#", "0"),
+ ),
+ },
+ {
+ ResourceName: resourceName,
+ ImportState: true,
+ ImportStateVerify: true,
+ ImportStateVerifyIgnore: []string{"rules"}, // argument not returned in RuleGroup API response
+ },
+ },
+ })
+}
+
 func TestAccAwsNetworkFirewallRuleGroup_updateRulesSourceList(t *testing.T) {
  rName := acctest.RandomWithPrefix("tf-acc-test")
  resourceName := "aws_networkfirewall_rule_group.test"