Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct handling of network ACL default IPv6 ingress/egress rules #12835

Merged
merged 1 commit into from
Mar 27, 2017
Merged

Correct handling of network ACL default IPv6 ingress/egress rules #12835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions builtin/providers/aws/resource_aws_default_network_acl.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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