From 08c0ac68e98cc595414d094104c05b1bf3eb91de Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 27 Mar 2017 05:42:49 -0400 Subject: [PATCH] Correct handling of network ACL default IPv6 ingress/egress rules. (#12835) --- .../aws/resource_aws_default_network_acl.go | 12 ++- .../resource_aws_default_network_acl_test.go | 74 +++++++++++++++---- .../providers/aws/resource_aws_network_acl.go | 6 +- .../aws/resource_aws_network_acl_test.go | 52 +++++++++++++ 4 files changed, 122 insertions(+), 22 deletions(-) diff --git a/builtin/providers/aws/resource_aws_default_network_acl.go b/builtin/providers/aws/resource_aws_default_network_acl.go index 44443e924..419972b18 100644 --- a/builtin/providers/aws/resource_aws_default_network_acl.go +++ b/builtin/providers/aws/resource_aws_default_network_acl.go @@ -9,11 +9,14 @@ import ( "github.com/hashicorp/terraform/helper/schema" ) -// ACL Network ACLs all contain an explicit deny-all rule that cannot be -// destroyed or changed by users. This rule is numbered very high to be a +// ACL Network ACLs all contain explicit deny-all rules that cannot be +// destroyed or changed by users. This rules are numbered very high to be a // catch-all. // See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html#default-network-acl -const awsDefaultAclRuleNumber = 32767 +const ( + awsDefaultAclRuleNumberIpv4 = 32767 + awsDefaultAclRuleNumberIpv6 = 32768 +) func resourceAwsDefaultNetworkAcl() *schema.Resource { return &schema.Resource{ @@ -258,7 +261,8 @@ func revokeAllNetworkACLEntries(netaclId string, meta interface{}) error { for _, e := range networkAcl.Entries { // Skip the default rules added by AWS. They can be neither // configured or deleted by users. See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html#default-network-acl - if *e.RuleNumber == awsDefaultAclRuleNumber { + if *e.RuleNumber == awsDefaultAclRuleNumberIpv4 || + *e.RuleNumber == awsDefaultAclRuleNumberIpv6 { continue } diff --git a/builtin/providers/aws/resource_aws_default_network_acl_test.go b/builtin/providers/aws/resource_aws_default_network_acl_test.go index 628943634..c5f9e02d1 100644 --- a/builtin/providers/aws/resource_aws_default_network_acl_test.go +++ b/builtin/providers/aws/resource_aws_default_network_acl_test.go @@ -36,8 +36,27 @@ func TestAccAWSDefaultNetworkAcl_basic(t *testing.T) { resource.TestStep{ Config: testAccAWSDefaultNetworkConfig_basic, Check: resource.ComposeTestCheckFunc( - testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), - testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0), + testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), + testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2), + ), + }, + }, + }) +} + +func TestAccAWSDefaultNetworkAcl_basicIpv6Vpc(t *testing.T) { + var networkAcl ec2.NetworkAcl + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSDefaultNetworkAclDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSDefaultNetworkConfig_basicIpv6Vpc, + Check: resource.ComposeTestCheckFunc( + testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), + testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 4), ), }, }, @@ -58,8 +77,8 @@ func TestAccAWSDefaultNetworkAcl_deny_ingress(t *testing.T) { resource.TestStep{ Config: testAccAWSDefaultNetworkConfig_deny_ingress, Check: resource.ComposeTestCheckFunc( - testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), - testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{defaultEgressAcl}, 0), + testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), + testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{defaultEgressAcl}, 0, 2), ), }, }, @@ -77,8 +96,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) { resource.TestStep{ Config: testAccAWSDefaultNetworkConfig_Subnets, Check: resource.ComposeTestCheckFunc( - testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), - testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2), + testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), + testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2), ), }, @@ -88,8 +107,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) { resource.TestStep{ Config: testAccAWSDefaultNetworkConfig_Subnets_remove, Check: resource.ComposeTestCheckFunc( - testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), - testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2), + testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), + testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2), ), ExpectNonEmptyPlan: true, }, @@ -108,8 +127,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetReassign(t *testing.T) { resource.TestStep{ Config: testAccAWSDefaultNetworkConfig_Subnets, Check: resource.ComposeTestCheckFunc( - testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), - testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2), + testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), + testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2), ), }, @@ -128,8 +147,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetReassign(t *testing.T) { resource.TestStep{ Config: testAccAWSDefaultNetworkConfig_Subnets_move, Check: resource.ComposeTestCheckFunc( - testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), - testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0), + testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl), + testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2), ), }, }, @@ -141,14 +160,14 @@ func testAccCheckAWSDefaultNetworkAclDestroy(s *terraform.State) error { return nil } -func testAccCheckAWSDefaultACLAttributes(acl *ec2.NetworkAcl, rules []*ec2.NetworkAclEntry, subnetCount int) resource.TestCheckFunc { +func testAccCheckAWSDefaultACLAttributes(acl *ec2.NetworkAcl, rules []*ec2.NetworkAclEntry, subnetCount int, hiddenRuleCount int) resource.TestCheckFunc { return func(s *terraform.State) error { aclEntriesCount := len(acl.Entries) ruleCount := len(rules) - // Default ACL has 2 hidden rules we can't do anything about - ruleCount = ruleCount + 2 + // Default ACL has hidden rules we can't do anything about + ruleCount = ruleCount + hiddenRuleCount if ruleCount != aclEntriesCount { return fmt.Errorf("Expected (%d) Rules, got (%d)", ruleCount, aclEntriesCount) @@ -162,7 +181,7 @@ func testAccCheckAWSDefaultACLAttributes(acl *ec2.NetworkAcl, rules []*ec2.Netwo } } -func testAccGetWSDefaultNetworkAcl(n string, networkAcl *ec2.NetworkAcl) resource.TestCheckFunc { +func testAccGetAWSDefaultNetworkAcl(n string, networkAcl *ec2.NetworkAcl) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -426,3 +445,26 @@ resource "aws_default_network_acl" "default" { } } ` + +const testAccAWSDefaultNetworkConfig_basicIpv6Vpc = ` +provider "aws" { + region = "us-east-2" +} + +resource "aws_vpc" "tftestvpc" { + cidr_block = "10.1.0.0/16" + assign_generated_ipv6_cidr_block = true + + tags { + Name = "TestAccAWSDefaultNetworkAcl_basicIpv6Vpc" + } +} + +resource "aws_default_network_acl" "default" { + default_network_acl_id = "${aws_vpc.tftestvpc.default_network_acl_id}" + + tags { + Name = "TestAccAWSDefaultNetworkAcl_basicIpv6Vpc" + } +} +` diff --git a/builtin/providers/aws/resource_aws_network_acl.go b/builtin/providers/aws/resource_aws_network_acl.go index 7a525e299..4777f4707 100644 --- a/builtin/providers/aws/resource_aws_network_acl.go +++ b/builtin/providers/aws/resource_aws_network_acl.go @@ -201,7 +201,8 @@ func resourceAwsNetworkAclRead(d *schema.ResourceData, meta interface{}) error { for _, e := range networkAcl.Entries { // Skip the default rules added by AWS. They can be neither // configured or deleted by users. - if *e.RuleNumber == awsDefaultAclRuleNumber { + if *e.RuleNumber == awsDefaultAclRuleNumberIpv4 || + *e.RuleNumber == awsDefaultAclRuleNumberIpv6 { continue } @@ -358,7 +359,8 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2 // neither modified nor destroyed. They have a custom rule // number that is out of bounds for any other rule. If we // encounter it, just continue. There's no work to be done. - if *remove.RuleNumber == awsDefaultAclRuleNumber { + if *remove.RuleNumber == awsDefaultAclRuleNumberIpv4 || + *remove.RuleNumber == awsDefaultAclRuleNumberIpv6 { continue } diff --git a/builtin/providers/aws/resource_aws_network_acl_test.go b/builtin/providers/aws/resource_aws_network_acl_test.go index bc589267f..b97568b65 100644 --- a/builtin/providers/aws/resource_aws_network_acl_test.go +++ b/builtin/providers/aws/resource_aws_network_acl_test.go @@ -242,6 +242,8 @@ func TestAccAWSNetworkAcl_ipv6Rules(t *testing.T) { Config: testAccAWSNetworkAclIpv6Config, Check: resource.ComposeTestCheckFunc( testAccCheckAWSNetworkAclExists("aws_network_acl.foos", &networkAcl), + resource.TestCheckResourceAttr( + "aws_network_acl.foos", "ingress.#", "1"), resource.TestCheckResourceAttr( "aws_network_acl.foos", "ingress.1976110835.protocol", "6"), resource.TestCheckResourceAttr( @@ -260,6 +262,29 @@ func TestAccAWSNetworkAcl_ipv6Rules(t *testing.T) { }) } +func TestAccAWSNetworkAcl_ipv6VpcRules(t *testing.T) { + var networkAcl ec2.NetworkAcl + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "aws_network_acl.foos", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSNetworkAclDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSNetworkAclIpv6VpcConfig, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSNetworkAclExists("aws_network_acl.foos", &networkAcl), + resource.TestCheckResourceAttr( + "aws_network_acl.foos", "ingress.#", "1"), + resource.TestCheckResourceAttr( + "aws_network_acl.foos", "ingress.1296304962.ipv6_cidr_block", "2600:1f16:d1e:9a00::/56"), + ), + }, + }, + }) +} + func TestAccAWSNetworkAcl_espProtocol(t *testing.T) { var networkAcl ec2.NetworkAcl @@ -436,6 +461,33 @@ resource "aws_network_acl" "foos" { } ` +const testAccAWSNetworkAclIpv6VpcConfig = ` +provider "aws" { + region = "us-east-2" +} + +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" + assign_generated_ipv6_cidr_block = true + + tags { + Name = "TestAccAWSNetworkAcl_ipv6VpcRules" + } +} + +resource "aws_network_acl" "foos" { + vpc_id = "${aws_vpc.foo.id}" + ingress = { + protocol = "tcp" + rule_no = 1 + action = "allow" + ipv6_cidr_block = "2600:1f16:d1e:9a00::/56" + from_port = 0 + to_port = 22 + } +} +` + const testAccAWSNetworkAclIngressConfig = ` resource "aws_vpc" "foo" { cidr_block = "10.1.0.0/16"