Skip to content

Commit

Permalink
Correct handling of network ACL default IPv6 ingress/egress rules. (#…
Browse files Browse the repository at this point in the history 
…12835)
  • Loading branch information
@ewbankkit @stack72
ewbankkit authored and stack72 committed Mar 27, 2017
1 parent 9210222 commit 08c0ac6
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 22 deletions.
12 changes: 8 additions & 4 deletions builtin/providers/aws/resource_aws_default_network_acl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
}

Expand Down
74 changes: 58 additions & 16 deletions builtin/providers/aws/resource_aws_default_network_acl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
),
},
},
Expand All @@ -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),
),
},
},
Expand All @@ -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),
),
},

Expand All @@ -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,
},
Expand All @@ -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),
),
},

Expand All @@ -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),
),
},
},
Expand All @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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"
}
}
`
6 changes: 4 additions & 2 deletions builtin/providers/aws/resource_aws_network_acl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
52 changes: 52 additions & 0 deletions builtin/providers/aws/resource_aws_network_acl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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

Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 08c0ac6

Please sign in to comment.