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

r/network_acl + default_network_acl - add arn attribute + validations #13819

Merged
merged 6 commits into from
Jun 22, 2020
Merged

r/network_acl + default_network_acl - add arn attribute + validations #13819

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
29a1691
add plan time validations
DrFaust92 Jun 17, 2020
ea7d0aa
add missing attribute to default acl. fix case sensitivity for arguments
DrFaust92 Jun 18, 2020
7f95886
allow empty string for cidrs
DrFaust92 Jun 18, 2020
d42fb17
Update website/docs/index.html.markdown
bflad Jun 18, 2020
ea0d79d
Update website/docs/r/default_network_acl.html.markdown
bflad Jun 18, 2020
8bb7031
Update aws/resource_aws_network_acl_rule_test.go
bflad Jun 22, 2020
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
66 changes: 50 additions & 16 deletions aws/resource_aws_default_network_acl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

Expand Down Expand Up @@ -34,6 +35,10 @@ func resourceAwsDefaultNetworkAcl() *schema.Resource {
},

Schema: map[string]*schema.Schema{
"arn": {
Type: schema.TypeString,
Computed: true,
},
"vpc_id": {
Type: schema.TypeString,
Computed: true,
Expand All @@ -53,7 +58,6 @@ func resourceAwsDefaultNetworkAcl() *schema.Resource {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
Set: schema.HashString,
},
// We want explicit management of Rules here, so we do not allow them to be
// computed. Instead, an empty config will enforce just that; removal of the
Expand All @@ -64,20 +68,27 @@ func resourceAwsDefaultNetworkAcl() *schema.Resource {
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"from_port": {
Type: schema.TypeInt,
Required: true,
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IsPortNumberOrZero,
},
"to_port": {
Type: schema.TypeInt,
Required: true,
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IsPortNumberOrZero,
},
"rule_no": {
Type: schema.TypeInt,
Required: true,
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IntBetween(1, 32766),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Doesn't need to be this PR, but we should probably create a constant for this, e.g.

const (
	// Maximum number for EC2 Network ACL Rules. The range 32767 to 65535 is reserved for internal use.
	Ec2NetworkAclRuleNumberMaximum = 32766
)

We'll need to figure out the best place for these in the future when things are broken into separate packages.

},
"action": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
ec2.RuleActionAllow,
ec2.RuleActionDeny,
}, true),
},
"protocol": {
Type: schema.TypeString,
Expand All @@ -86,10 +97,18 @@ func resourceAwsDefaultNetworkAcl() *schema.Resource {
"cidr_block": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.Any(
validation.StringIsEmpty,
validation.IsCIDR,
),
},
"ipv6_cidr_block": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.Any(
validation.StringIsEmpty,
validation.IsCIDR,
),
},
"icmp_type": {
Type: schema.TypeInt,
Expand All @@ -109,20 +128,27 @@ func resourceAwsDefaultNetworkAcl() *schema.Resource {
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"from_port": {
Type: schema.TypeInt,
Required: true,
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IsPortNumberOrZero,
},
"to_port": {
Type: schema.TypeInt,
Required: true,
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IsPortNumberOrZero,
},
"rule_no": {
Type: schema.TypeInt,
Required: true,
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IntBetween(1, 32766),
},
"action": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
ec2.RuleActionAllow,
ec2.RuleActionDeny,
}, true),
},
"protocol": {
Type: schema.TypeString,
Expand All @@ -131,10 +157,18 @@ func resourceAwsDefaultNetworkAcl() *schema.Resource {
"cidr_block": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.Any(
validation.StringIsEmpty,
validation.IsCIDR,
),
},
"ipv6_cidr_block": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.Any(
validation.StringIsEmpty,
validation.IsCIDR,
),
},
"icmp_type": {
Type: schema.TypeInt,
Expand Down Expand Up @@ -277,14 +311,14 @@ 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 == awsDefaultAclRuleNumberIpv4 ||
*e.RuleNumber == awsDefaultAclRuleNumberIpv6 {
if aws.Int64Value(e.RuleNumber) == awsDefaultAclRuleNumberIpv4 ||
aws.Int64Value(e.RuleNumber) == awsDefaultAclRuleNumberIpv6 {
continue
}

// track if this is an egress or ingress rule, for logging purposes
rt := "ingress"
if *e.Egress {
if aws.BoolValue(e.Egress) {
rt = "egress"
}

Expand Down
45 changes: 27 additions & 18 deletions 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 @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -27,6 +28,7 @@ var ipv6IngressAcl = &ec2.NetworkAclEntry{

func TestAccAWSDefaultNetworkAcl_basic(t *testing.T) {
var networkAcl ec2.NetworkAcl
resourceName := "aws_default_network_acl.default"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -36,13 +38,14 @@ func TestAccAWSDefaultNetworkAcl_basic(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_basic,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`network-acl/acl-.+`)),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2),
testAccCheckResourceAttrAccountID("aws_default_network_acl.default", "owner_id"),
testAccCheckResourceAttrAccountID(resourceName, "owner_id"),
),
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand All @@ -52,6 +55,7 @@ func TestAccAWSDefaultNetworkAcl_basic(t *testing.T) {

func TestAccAWSDefaultNetworkAcl_basicIpv6Vpc(t *testing.T) {
var networkAcl ec2.NetworkAcl
resourceName := "aws_default_network_acl.default"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -61,12 +65,12 @@ func TestAccAWSDefaultNetworkAcl_basicIpv6Vpc(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_basicIpv6Vpc,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 4),
),
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand All @@ -79,6 +83,7 @@ func TestAccAWSDefaultNetworkAcl_deny_ingress(t *testing.T) {
// not Egress. We then expect there to be 3 rules, 2 AWS defaults and 1
// additional Egress.
var networkAcl ec2.NetworkAcl
resourceName := "aws_default_network_acl.default"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -88,12 +93,12 @@ func TestAccAWSDefaultNetworkAcl_deny_ingress(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_deny_ingress,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{defaultEgressAcl}, 0, 2),
),
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand All @@ -103,6 +108,7 @@ func TestAccAWSDefaultNetworkAcl_deny_ingress(t *testing.T) {

func TestAccAWSDefaultNetworkAcl_withIpv6Ingress(t *testing.T) {
var networkAcl ec2.NetworkAcl
resourceName := "aws_default_network_acl.default"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -112,12 +118,12 @@ func TestAccAWSDefaultNetworkAcl_withIpv6Ingress(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_includingIpv6Rule,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{ipv6IngressAcl}, 0, 2),
),
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand All @@ -127,6 +133,7 @@ func TestAccAWSDefaultNetworkAcl_withIpv6Ingress(t *testing.T) {

func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) {
var networkAcl ec2.NetworkAcl
resourceName := "aws_default_network_acl.default"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -136,12 +143,12 @@ func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_Subnets,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2),
),
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand All @@ -152,13 +159,13 @@ func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_Subnets_remove,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2),
),
ExpectNonEmptyPlan: true,
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand All @@ -168,6 +175,7 @@ func TestAccAWSDefaultNetworkAcl_SubnetRemoval(t *testing.T) {

func TestAccAWSDefaultNetworkAcl_SubnetReassign(t *testing.T) {
var networkAcl ec2.NetworkAcl
resourceName := "aws_default_network_acl.default"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -177,12 +185,12 @@ func TestAccAWSDefaultNetworkAcl_SubnetReassign(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_Subnets,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2),
),
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand All @@ -202,12 +210,12 @@ func TestAccAWSDefaultNetworkAcl_SubnetReassign(t *testing.T) {
{
Config: testAccAWSDefaultNetworkConfig_Subnets_move,
Check: resource.ComposeTestCheckFunc(
testAccGetAWSDefaultNetworkAcl("aws_default_network_acl.default", &networkAcl),
testAccGetAWSDefaultNetworkAcl(resourceName, &networkAcl),
testAccCheckAWSDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2),
),
},
{
ResourceName: "aws_default_network_acl.default",
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
Expand Down Expand Up @@ -260,7 +268,8 @@ func testAccGetAWSDefaultNetworkAcl(n string, networkAcl *ec2.NetworkAcl) resour
return err
}

if len(resp.NetworkAcls) > 0 && *resp.NetworkAcls[0].NetworkAclId == rs.Primary.ID {
if len(resp.NetworkAcls) > 0 &&
aws.StringValue(resp.NetworkAcls[0].NetworkAclId) == rs.Primary.ID {
*networkAcl = *resp.NetworkAcls[0]
return nil
}
Expand Down
Loading