Skip to content

Commit 08c0ac6

Browse files
ewbankkitstack72
authored andcommitted
Correct handling of network ACL default IPv6 ingress/egress rules. (hashicorp#12835)
1 parent 9210222 commit 08c0ac6

4 files changed

Lines changed: 122 additions & 22 deletions

File tree

‎builtin/providers/aws/resource_aws_default_network_acl.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,14 @@ import (
99
"github.com/hashicorp/terraform/helper/schema"
1010
)
1111

12-
// ACL Network ACLs all contain an explicit deny-all rule that cannot be
13-
// destroyed or changed by users. This rule is numbered very high to be a
12+
// ACL Network ACLs all contain explicit deny-all rules that cannot be
13+
// destroyed or changed by users. This rules are numbered very high to be a
1414
// catch-all.
1515
// See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html#default-network-acl
16-
const awsDefaultAclRuleNumber = 32767
16+
const (
17+
awsDefaultAclRuleNumberIpv4 = 32767
18+
awsDefaultAclRuleNumberIpv6 = 32768
19+
)
1720

1821
func resourceAwsDefaultNetworkAcl() *schema.Resource {
1922
return &schema.Resource{
@@ -258,7 +261,8 @@ func revokeAllNetworkACLEntries(netaclId string, meta interface{}) error {
258261
for _, e := range networkAcl.Entries {
259262
// Skip the default rules added by AWS. They can be neither
260263
// configured or deleted by users. See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html#default-network-acl
261-
if *e.RuleNumber == awsDefaultAclRuleNumber {
264+
if *e.RuleNumber == awsDefaultAclRuleNumberIpv4 ||
265+
*e.RuleNumber == awsDefaultAclRuleNumberIpv6 {
262266
continue
263267
}
264268

‎builtin/providers/aws/resource_aws_default_network_acl_test.go‎

Lines changed: 58 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,27 @@ func TestAccAWSDefaultNetworkAcl_basic(t *testing.T) {
3636
resource.TestStep{
3737
Config: testAccAWSDefaultNetworkConfig_basic,
3838
Check: resource.ComposeTestCheckFunc(
39-
testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
40-
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0),
39+
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
40+
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2),
41+
),
42+
},
43+
},
44+
})
45+
}
46+
47+
func TestAccAWSDefaultNetworkAcl_basicIpv6Vpc(t *testing.T) {
48+
var networkAcl ec2.NetworkAcl
49+
50+
resource.Test(t, resource.TestCase{
51+
PreCheck: func() { testAccPreCheck(t) },
52+
Providers: testAccProviders,
53+
CheckDestroy: testAccCheckAWSDefaultNetworkAclDestroy,
54+
Steps: []resource.TestStep{
55+
resource.TestStep{
56+
Config: testAccAWSDefaultNetworkConfig_basicIpv6Vpc,
57+
Check: resource.ComposeTestCheckFunc(
58+
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
59+
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 4),
4160
),
4261
},
4362
},
@@ -58,8 +77,8 @@ func TestAccAWSDefaultNetworkAcl_deny_ingress(t *testing.T) {
5877
resource.TestStep{
5978
Config: testAccAWSDefaultNetworkConfig_deny_ingress,
6079
Check: resource.ComposeTestCheckFunc(
61-
testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
62-
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{defaultEgressAcl}, 0),
80+
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
81+
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{defaultEgressAcl}, 0, 2),
6382
),
6483
},
6584
},
@@ -77,8 +96,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) {
7796
resource.TestStep{
7897
Config: testAccAWSDefaultNetworkConfig_Subnets,
7998
Check: resource.ComposeTestCheckFunc(
80-
testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
81-
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2),
99+
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
100+
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2),
82101
),
83102
},
84103

@@ -88,8 +107,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) {
88107
resource.TestStep{
89108
Config: testAccAWSDefaultNetworkConfig_Subnets_remove,
90109
Check: resource.ComposeTestCheckFunc(
91-
testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
92-
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2),
110+
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
111+
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2),
93112
),
94113
ExpectNonEmptyPlan: true,
95114
},
@@ -108,8 +127,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetReassign(t *testing.T) {
108127
resource.TestStep{
109128
Config: testAccAWSDefaultNetworkConfig_Subnets,
110129
Check: resource.ComposeTestCheckFunc(
111-
testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
112-
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2),
130+
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
131+
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2),
113132
),
114133
},
115134

@@ -128,8 +147,8 @@ func TestAccAWSDefaultNetworkAcl_SubnetReassign(t *testing.T) {
128147
resource.TestStep{
129148
Config: testAccAWSDefaultNetworkConfig_Subnets_move,
130149
Check: resource.ComposeTestCheckFunc(
131-
testAccGetWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
132-
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0),
150+
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
151+
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2),
133152
),
134153
},
135154
},
@@ -141,14 +160,14 @@ func testAccCheckAWSDefaultNetworkAclDestroy(s *terraform.State) error {
141160
return nil
142161
}
143162

144-
func testAccCheckAWSDefaultACLAttributes(acl *ec2.NetworkAcl, rules []*ec2.NetworkAclEntry, subnetCount int) resource.TestCheckFunc {
163+
func testAccCheckAWSDefaultACLAttributes(acl *ec2.NetworkAcl, rules []*ec2.NetworkAclEntry, subnetCount int, hiddenRuleCount int) resource.TestCheckFunc {
145164
return func(s *terraform.State) error {
146165

147166
aclEntriesCount := len(acl.Entries)
148167
ruleCount := len(rules)
149168

150-
// Default ACL has 2 hidden rules we can't do anything about
151-
ruleCount = ruleCount + 2
169+
// Default ACL has hidden rules we can't do anything about
170+
ruleCount = ruleCount + hiddenRuleCount
152171

153172
if ruleCount != aclEntriesCount {
154173
return fmt.Errorf("Expected (%d) Rules, got (%d)", ruleCount, aclEntriesCount)
@@ -162,7 +181,7 @@ func testAccCheckAWSDefaultACLAttributes(acl *ec2.NetworkAcl, rules []*ec2.Netwo
162181
}
163182
}
164183

165-
func testAccGetWSDefaultNetworkAcl(n string, networkAcl *ec2.NetworkAcl) resource.TestCheckFunc {
184+
func testAccGetAWSDefaultNetworkAcl(n string, networkAcl *ec2.NetworkAcl) resource.TestCheckFunc {
166185
return func(s *terraform.State) error {
167186
rs, ok := s.RootModule().Resources[n]
168187
if !ok {
@@ -426,3 +445,26 @@ resource "aws_default_network_acl" "default" {
426445
}
427446
}
428447
`
448+
449+
const testAccAWSDefaultNetworkConfig_basicIpv6Vpc = `
450+
provider "aws" {
451+
region = "us-east-2"
452+
}
453+
454+
resource "aws_vpc" "tftestvpc" {
455+
cidr_block = "10.1.0.0/16"
456+
assign_generated_ipv6_cidr_block = true
457+
458+
tags {
459+
Name = "TestAccAWSDefaultNetworkAcl_basicIpv6Vpc"
460+
}
461+
}
462+
463+
resource "aws_default_network_acl" "default" {
464+
default_network_acl_id = "${aws_vpc.tftestvpc.default_network_acl_id}"
465+
466+
tags {
467+
Name = "TestAccAWSDefaultNetworkAcl_basicIpv6Vpc"
468+
}
469+
}
470+
`

‎builtin/providers/aws/resource_aws_network_acl.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ func resourceAwsNetworkAclRead(d *schema.ResourceData, meta interface{}) error {
201201
for _, e := range networkAcl.Entries {
202202
// Skip the default rules added by AWS. They can be neither
203203
// configured or deleted by users.
204-
if *e.RuleNumber == awsDefaultAclRuleNumber {
204+
if *e.RuleNumber == awsDefaultAclRuleNumberIpv4 ||
205+
*e.RuleNumber == awsDefaultAclRuleNumberIpv6 {
205206
continue
206207
}
207208

@@ -358,7 +359,8 @@ func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2
358359
// neither modified nor destroyed. They have a custom rule
359360
// number that is out of bounds for any other rule. If we
360361
// encounter it, just continue. There's no work to be done.
361-
if *remove.RuleNumber == awsDefaultAclRuleNumber {
362+
if *remove.RuleNumber == awsDefaultAclRuleNumberIpv4 ||
363+
*remove.RuleNumber == awsDefaultAclRuleNumberIpv6 {
362364
continue
363365
}
364366

‎builtin/providers/aws/resource_aws_network_acl_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,8 @@ func TestAccAWSNetworkAcl_ipv6Rules(t *testing.T) {
242242
Config: testAccAWSNetworkAclIpv6Config,
243243
Check: resource.ComposeTestCheckFunc(
244244
testAccCheckAWSNetworkAclExists("aws_network_acl.foos", &networkAcl),
245+
resource.TestCheckResourceAttr(
246+
"aws_network_acl.foos", "ingress.#", "1"),
245247
resource.TestCheckResourceAttr(
246248
"aws_network_acl.foos", "ingress.1976110835.protocol", "6"),
247249
resource.TestCheckResourceAttr(
@@ -260,6 +262,29 @@ func TestAccAWSNetworkAcl_ipv6Rules(t *testing.T) {
260262
})
261263
}
262264

265+
func TestAccAWSNetworkAcl_ipv6VpcRules(t *testing.T) {
266+
var networkAcl ec2.NetworkAcl
267+
268+
resource.Test(t, resource.TestCase{
269+
PreCheck: func() { testAccPreCheck(t) },
270+
IDRefreshName: "aws_network_acl.foos",
271+
Providers: testAccProviders,
272+
CheckDestroy: testAccCheckAWSNetworkAclDestroy,
273+
Steps: []resource.TestStep{
274+
{
275+
Config: testAccAWSNetworkAclIpv6VpcConfig,
276+
Check: resource.ComposeTestCheckFunc(
277+
testAccCheckAWSNetworkAclExists("aws_network_acl.foos", &networkAcl),
278+
resource.TestCheckResourceAttr(
279+
"aws_network_acl.foos", "ingress.#", "1"),
280+
resource.TestCheckResourceAttr(
281+
"aws_network_acl.foos", "ingress.1296304962.ipv6_cidr_block", "2600:1f16:d1e:9a00::/56"),
282+
),
283+
},
284+
},
285+
})
286+
}
287+
263288
func TestAccAWSNetworkAcl_espProtocol(t *testing.T) {
264289
var networkAcl ec2.NetworkAcl
265290

@@ -436,6 +461,33 @@ resource "aws_network_acl" "foos" {
436461
}
437462
`
438463

464+
const testAccAWSNetworkAclIpv6VpcConfig = `
465+
provider "aws" {
466+
region = "us-east-2"
467+
}
468+
469+
resource "aws_vpc" "foo" {
470+
cidr_block = "10.1.0.0/16"
471+
assign_generated_ipv6_cidr_block = true
472+
473+
tags {
474+
Name = "TestAccAWSNetworkAcl_ipv6VpcRules"
475+
}
476+
}
477+
478+
resource "aws_network_acl" "foos" {
479+
vpc_id = "${aws_vpc.foo.id}"
480+
ingress = {
481+
protocol = "tcp"
482+
rule_no = 1
483+
action = "allow"
484+
ipv6_cidr_block = "2600:1f16:d1e:9a00::/56"
485+
from_port = 0
486+
to_port = 22
487+
}
488+
}
489+
`
490+
439491
const testAccAWSNetworkAclIngressConfig = `
440492
resource "aws_vpc" "foo" {
441493
cidr_block = "10.1.0.0/16"

0 commit comments

Comments
 (0)