Skip to content

Commit

Permalink
Merge pull request #22137 from hashicorp/b-ec2-vpc-endpoint-policy-diffs
Browse files Browse the repository at this point in the history
ec2/vpc_endpoint: Fix erroneous diffs with equivalent policies
  • Loading branch information
YakDriver authored Dec 9, 2021
2 parents 904bc86 + a936249 commit 158025f
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 12 deletions.
3 changes: 3 additions & 0 deletions .changelog/22137.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_vpc_endpoint: Fix erroneous diffs in `policy` when no changes made or policies are equivalent
```
39 changes: 27 additions & 12 deletions internal/service/ec2/vpc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
awspolicy "github.com/hashicorp/awspolicyequivalence"
"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"
Expand Down Expand Up @@ -271,11 +272,21 @@ func resourceVPCEndpointRead(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error setting network_interface_ids: %s", err)
}
d.Set("owner_id", vpce.OwnerId)
policy, err := structure.NormalizeJsonString(aws.StringValue(vpce.PolicyDocument))

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

if err != nil {
return fmt.Errorf("while setting policy (%s), encountered: %w", policyToSet, err)
}

policyToSet, err = structure.NormalizeJsonString(policyToSet)

if err != nil {
return fmt.Errorf("policy contains an invalid JSON: %s", err)
return fmt.Errorf("policy (%s) is invalid JSON: %w", policyToSet, err)
}
d.Set("policy", policy)

d.Set("policy", policyToSet)

d.Set("private_dns_enabled", vpce.PrivateDnsEnabled)
err = d.Set("route_table_ids", flex.FlattenStringSet(vpce.RouteTableIds))
if err != nil {
Expand Down Expand Up @@ -326,15 +337,19 @@ func resourceVPCEndpointUpdate(d *schema.ResourceData, meta interface{}) error {
}

if d.HasChange("policy") {
policy, err := structure.NormalizeJsonString(d.Get("policy"))
if err != nil {
return fmt.Errorf("policy contains an invalid JSON: %s", err)
}

if policy == "" {
req.ResetPolicy = aws.Bool(true)
} else {
req.PolicyDocument = aws.String(policy)
o, n := d.GetChange("policy")

if equivalent, err := awspolicy.PoliciesAreEquivalent(o.(string), n.(string)); err != nil || !equivalent {
policy, err := structure.NormalizeJsonString(d.Get("policy"))
if err != nil {
return fmt.Errorf("policy contains an invalid JSON: %s", err)
}

if policy == "" {
req.ResetPolicy = aws.Bool(true)
} else {
req.PolicyDocument = aws.String(policy)
}
}
}

Expand Down
103 changes: 103 additions & 0 deletions internal/service/ec2/vpc_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,31 @@ func TestAccEC2VPCEndpoint_gatewayPolicy(t *testing.T) {
})
}

func TestAccEC2VPCEndpoint_ignoreEquivalent(t *testing.T) {
var endpoint ec2.VpcEndpoint
resourceName := "aws_vpc_endpoint.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckVpcEndpointDestroy,
Steps: []resource.TestStep{
{
Config: testAccVpcEndpointOrderPolicyConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckVpcEndpointExists(resourceName, &endpoint),
),
},
{
Config: testAccVpcEndpointNewOrderPolicyConfig(rName),
PlanOnly: true,
},
},
})
}

func TestAccEC2VPCEndpoint_interfaceBasic(t *testing.T) {
var endpoint ec2.VpcEndpoint
resourceName := "aws_vpc_endpoint.test"
Expand Down Expand Up @@ -1007,3 +1032,81 @@ resource "aws_vpc_endpoint" "test" {
}
`, rName))
}

func testAccVpcEndpointOrderPolicyConfig(rName string) string {
return fmt.Sprintf(`
data "aws_vpc_endpoint_service" "test" {
service = "dynamodb"
}
resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"
tags = {
Name = %[1]q
}
}
resource "aws_vpc_endpoint" "test" {
policy = jsonencode({
Version = "2012-10-17"
Statement = [{
Sid = "ReadOnly"
Principal = "*"
Action = [
"dynamodb:DescribeTable",
"dynamodb:ListTables",
"dynamodb:ListTagsOfResource",
]
Effect = "Allow"
Resource = "*"
}]
})
service_name = data.aws_vpc_endpoint_service.test.service_name
vpc_id = aws_vpc.test.id
tags = {
Name = %[1]q
}
}
`, rName)
}

func testAccVpcEndpointNewOrderPolicyConfig(rName string) string {
return fmt.Sprintf(`
data "aws_vpc_endpoint_service" "test" {
service = "dynamodb"
}
resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"
tags = {
Name = %[1]q
}
}
resource "aws_vpc_endpoint" "test" {
policy = jsonencode({
Version = "2012-10-17"
Statement = [{
Sid = "ReadOnly"
Principal = "*"
Action = [
"dynamodb:ListTables",
"dynamodb:ListTagsOfResource",
"dynamodb:DescribeTable",
]
Effect = "Allow"
Resource = "*"
}]
})
service_name = data.aws_vpc_endpoint_service.test.service_name
vpc_id = aws_vpc.test.id
tags = {
Name = %[1]q
}
}
`, rName)
}

0 comments on commit 158025f

Please sign in to comment.