Skip to content

Commit

Permalink
Merge pull request #22171 from hashicorp/b-networkfw-resource-policy-…
Browse files Browse the repository at this point in the history 
…diffs

networkfirewall/resource_policy: Fix equivalent policy diffs
  • Loading branch information
@YakDriver
YakDriver authored Dec 13, 2021
2 parents f61f7f9 + f7c8275 commit b26ba26
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 33 deletions.
3 changes: 3 additions & 0 deletions .changelog/22171.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_networkfirewall_resource_policy: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
```
29 changes: 24 additions & 5 deletions internal/service/networkfirewall/resource_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
Expand All @@ -31,7 +32,11 @@ func ResourceResourcePolicy() *schema.Resource {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.StringIsJSON,
DiffSuppressFunc: verify.SuppressEquivalentJSONDiffs,
DiffSuppressFunc: verify.SuppressEquivalentPolicyDiffs,
StateFunc: func(v interface{}) string {
json, _ := structure.NormalizeJsonString(v)
return json
},
},
"resource_arn": {
Type: schema.TypeString,
Expand All @@ -46,16 +51,23 @@ func ResourceResourcePolicy() *schema.Resource {
func resourceResourcePolicyPut(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
conn := meta.(*conns.AWSClient).NetworkFirewallConn
resourceArn := d.Get("resource_arn").(string)

policy, err := structure.NormalizeJsonString(d.Get("policy").(string))

if err != nil {
return diag.Errorf("policy (%s) is invalid JSON: %s", policy, err)
}

input := &networkfirewall.PutResourcePolicyInput{
ResourceArn: aws.String(resourceArn),
Policy: aws.String(d.Get("policy").(string)),
Policy: aws.String(policy),
}

log.Printf("[DEBUG] Putting NetworkFirewall Resource Policy for resource: %s", resourceArn)

_, err := conn.PutResourcePolicyWithContext(ctx, input)
_, err = conn.PutResourcePolicyWithContext(ctx, input)
if err != nil {
return diag.FromErr(fmt.Errorf("error putting NetworkFirewall Resource Policy (for resource: %s): %w", resourceArn, err))
return diag.Errorf("error putting NetworkFirewall Resource Policy (for resource: %s): %s", resourceArn, err)
}

d.SetId(resourceArn)
Expand Down Expand Up @@ -83,9 +95,16 @@ func resourceResourcePolicyRead(ctx context.Context, d *schema.ResourceData, met
return diag.FromErr(fmt.Errorf("error reading NetworkFirewall Resource Policy (for resource: %s): empty output", resourceArn))
}

d.Set("policy", policy)
d.Set("resource_arn", resourceArn)

policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(policy))

if err != nil {
return diag.Errorf("setting policy %s: %s", aws.StringValue(policy), err)
}

d.Set("policy", policyToSet)

return nil
}

Expand Down
77 changes: 49 additions & 28 deletions internal/service/networkfirewall/resource_policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
tfnetworkfirewall "github.com/hashicorp/terraform-provider-aws/internal/service/networkfirewall"
)

func TestAccNetworkFirewallResourcePolicy_firewallPolicy(t *testing.T) {
func TestAccNetworkFirewallResourcePolicy_basic(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_networkfirewall_resource_policy.test"

Expand All @@ -27,25 +27,46 @@ func TestAccNetworkFirewallResourcePolicy_firewallPolicy(t *testing.T) {
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccNetworkFirewallResourcePolicy_firewallPolicy(rName),
Config: testAccResourcePolicyConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "resource_arn", "aws_networkfirewall_firewall_policy.test", "arn"),
resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile(`"Action":\["network-firewall:CreateFirewall","network-firewall:UpdateFirewall","network-firewall:AssociateFirewallPolicy","network-firewall:ListFirewallPolicies"\]`)),
),
},
{
// Update the policy's Actions
Config: testAccNetworkFirewallResourcePolicy_firewallPolicy_updatePolicy(rName),
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccNetworkFirewallResourcePolicy_ignoreEquivalent(t *testing.T) {
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_networkfirewall_resource_policy.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t); testAccPreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, networkfirewall.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccResourcePolicyConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile(`"Action":\["network-firewall:UpdateFirewall","network-firewall:AssociateFirewallPolicy","network-firewall:ListFirewallPolicies","network-firewall:CreateFirewall"\]`)),
resource.TestCheckResourceAttrPair(resourceName, "resource_arn", "aws_networkfirewall_firewall_policy.test", "arn"),
resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile(`"Action":\["network-firewall:CreateFirewall","network-firewall:UpdateFirewall","network-firewall:AssociateFirewallPolicy","network-firewall:ListFirewallPolicies"\]`)),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
Config: testAccResourcePolicyEquivalentConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile(`"Action":\["network-firewall:CreateFirewall","network-firewall:UpdateFirewall","network-firewall:AssociateFirewallPolicy","network-firewall:ListFirewallPolicies"\]`)),
),
},
},
})
Expand All @@ -62,7 +83,7 @@ func TestAccNetworkFirewallResourcePolicy_ruleGroup(t *testing.T) {
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccNetworkFirewallResourcePolicy_ruleGroup(rName),
Config: testAccResourcePolicy_ruleGroup(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "resource_arn", "aws_networkfirewall_rule_group.test", "arn"),
Expand All @@ -71,10 +92,10 @@ func TestAccNetworkFirewallResourcePolicy_ruleGroup(t *testing.T) {
},
{
// Update the policy's Actions
Config: testAccNetworkFirewallResourcePolicy_ruleGroup_updatePolicy(rName),
Config: testAccResourcePolicy_ruleGroup_updatePolicy(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile(`"Action":\["network-firewall:UpdateFirewallPolicy","network-firewall:ListRuleGroups","network-firewall:CreateFirewallPolicy"\]`)),
resource.TestMatchResourceAttr(resourceName, "policy", regexp.MustCompile(`"Action":\["network-firewall:CreateFirewallPolicy","network-firewall:UpdateFirewallPolicy","network-firewall:ListRuleGroups"\]`)),
),
},
{
Expand All @@ -97,7 +118,7 @@ func TestAccNetworkFirewallResourcePolicy_disappears(t *testing.T) {
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccNetworkFirewallResourcePolicy_firewallPolicy(rName),
Config: testAccResourcePolicyConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
acctest.CheckResourceDisappears(acctest.Provider, tfnetworkfirewall.ResourceResourcePolicy(), resourceName),
Expand All @@ -119,7 +140,7 @@ func TestAccNetworkFirewallResourcePolicy_Disappears_firewallPolicy(t *testing.T
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccNetworkFirewallResourcePolicy_firewallPolicy(rName),
Config: testAccResourcePolicyConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
acctest.CheckResourceDisappears(acctest.Provider, tfnetworkfirewall.ResourceFirewallPolicy(), "aws_networkfirewall_firewall_policy.test"),
Expand All @@ -141,7 +162,7 @@ func TestAccNetworkFirewallResourcePolicy_Disappears_ruleGroup(t *testing.T) {
CheckDestroy: testAccCheckResourcePolicyDestroy,
Steps: []resource.TestStep{
{
Config: testAccNetworkFirewallResourcePolicy_ruleGroup(rName),
Config: testAccResourcePolicy_ruleGroup(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckResourcePolicyExists(resourceName),
acctest.CheckResourceDisappears(acctest.Provider, tfnetworkfirewall.ResourceRuleGroup(), "aws_networkfirewall_rule_group.test"),
Expand Down Expand Up @@ -200,14 +221,14 @@ func testAccCheckResourcePolicyExists(n string) resource.TestCheckFunc {
}
}

func testAccNetworkFirewallResourcePolicyFirewallPolicyBaseConfig(rName string) string {
func testAccResourcePolicyFirewallPolicyBaseConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}
data "aws_caller_identity" "current" {}
resource "aws_networkfirewall_firewall_policy" "test" {
name = %q
name = %[1]q
firewall_policy {
stateless_fragment_default_actions = ["aws:drop"]
stateless_default_actions = ["aws:pass"]
Expand All @@ -216,9 +237,9 @@ resource "aws_networkfirewall_firewall_policy" "test" {
`, rName)
}

func testAccNetworkFirewallResourcePolicy_firewallPolicy(rName string) string {
func testAccResourcePolicyConfig(rName string) string {
return acctest.ConfigCompose(
testAccNetworkFirewallResourcePolicyFirewallPolicyBaseConfig(rName), `
testAccResourcePolicyFirewallPolicyBaseConfig(rName), `
resource "aws_networkfirewall_resource_policy" "test" {
resource_arn = aws_networkfirewall_firewall_policy.test.arn
# policy's Action element must include all of the following operations
Expand All @@ -228,7 +249,7 @@ resource "aws_networkfirewall_resource_policy" "test" {
"network-firewall:CreateFirewall",
"network-firewall:UpdateFirewall",
"network-firewall:AssociateFirewallPolicy",
"network-firewall:ListFirewallPolicies"
"network-firewall:ListFirewallPolicies",
]
Effect = "Allow"
Resource = aws_networkfirewall_firewall_policy.test.arn
Expand All @@ -242,9 +263,9 @@ resource "aws_networkfirewall_resource_policy" "test" {
`)
}

func testAccNetworkFirewallResourcePolicy_firewallPolicy_updatePolicy(rName string) string {
func testAccResourcePolicyEquivalentConfig(rName string) string {
return acctest.ConfigCompose(
testAccNetworkFirewallResourcePolicyFirewallPolicyBaseConfig(rName), `
testAccResourcePolicyFirewallPolicyBaseConfig(rName), `
resource "aws_networkfirewall_resource_policy" "test" {
resource_arn = aws_networkfirewall_firewall_policy.test.arn
# policy's Action element must include all of the following operations
Expand All @@ -254,7 +275,7 @@ resource "aws_networkfirewall_resource_policy" "test" {
"network-firewall:UpdateFirewall",
"network-firewall:AssociateFirewallPolicy",
"network-firewall:ListFirewallPolicies",
"network-firewall:CreateFirewall"
"network-firewall:CreateFirewall",
]
Effect = "Allow"
Resource = aws_networkfirewall_firewall_policy.test.arn
Expand All @@ -268,7 +289,7 @@ resource "aws_networkfirewall_resource_policy" "test" {
`)
}

func testAccNetworkFirewallResourcePolicyRuleGroupBaseConfig(rName string) string {
func testAccResourcePolicyRuleGroupBaseConfig(rName string) string {
return fmt.Sprintf(`
data "aws_partition" "current" {}
Expand All @@ -291,9 +312,9 @@ resource "aws_networkfirewall_rule_group" "test" {
`, rName)
}

func testAccNetworkFirewallResourcePolicy_ruleGroup(rName string) string {
func testAccResourcePolicy_ruleGroup(rName string) string {
return acctest.ConfigCompose(
testAccNetworkFirewallResourcePolicyRuleGroupBaseConfig(rName), `
testAccResourcePolicyRuleGroupBaseConfig(rName), `
resource "aws_networkfirewall_resource_policy" "test" {
resource_arn = aws_networkfirewall_rule_group.test.arn
# policy's Action element must include all of the following operations
Expand All @@ -316,9 +337,9 @@ resource "aws_networkfirewall_resource_policy" "test" {
`)
}

func testAccNetworkFirewallResourcePolicy_ruleGroup_updatePolicy(rName string) string {
func testAccResourcePolicy_ruleGroup_updatePolicy(rName string) string {
return acctest.ConfigCompose(
testAccNetworkFirewallResourcePolicyRuleGroupBaseConfig(rName), `
testAccResourcePolicyRuleGroupBaseConfig(rName), `
resource "aws_networkfirewall_resource_policy" "test" {
resource_arn = aws_networkfirewall_rule_group.test.arn
# policy's Action element must include all of the following operations
Expand All @@ -327,7 +348,7 @@ resource "aws_networkfirewall_resource_policy" "test" {
Action = [
"network-firewall:UpdateFirewallPolicy",
"network-firewall:ListRuleGroups",
"network-firewall:CreateFirewallPolicy"
"network-firewall:CreateFirewallPolicy",
]
Effect = "Allow"
Resource = aws_networkfirewall_rule_group.test.arn
Expand Down

0 comments on commit b26ba26

Please sign in to comment.