Merge branch 'f-aws-network-subnet-ids'

* f-aws-network-subnet-ids:
  document Network ACL Subnet IDs attribute
  provider/aws: Add tests for Network ACL subnets
  upgrade to use typeset for subnet_ids
  network acl cleanups
  provider/aws: Support multiple subnets in Network ACL

builtin/providers/aws/resource_aws_network_acl.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"log"
+	"sort"
 	"strconv"
 	"time"
 
@@ -34,6 +35,15 @@ func resourceAwsNetworkAcl() *schema.Resource {
 				Optional:      true,
 				ForceNew:      true,
 				Computed:      false,
+				ConflictsWith: []string{"subnet_ids"},
+				Deprecated:    "Attribute subnet_id is deprecated on network_acl resources. Use subnet_ids instead",
+			},
+			"subnet_ids": &schema.Schema{
+				Type:          schema.TypeSet,
+				Optional:      true,
+				ConflictsWith: []string{"subnet_id"},
+				Elem:          &schema.Schema{Type: schema.TypeString},
+				Set:           schema.HashString,
 			},
 			"ingress": &schema.Schema{
 				Type:     schema.TypeSet,
@@ -168,6 +178,15 @@ func resourceAwsNetworkAclRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("vpc_id", networkAcl.VPCID)
 	d.Set("tags", tagsToMap(networkAcl.Tags))
 
+	var s []string
+	for _, a := range networkAcl.Associations {
+		s = append(s, *a.SubnetID)
+	}
+	sort.Strings(s)
+	if err := d.Set("subnet_ids", s); err != nil {
+		return err
+	}
+
 	if err := d.Set("ingress", networkAclEntriesToMapList(ingressEntries)); err != nil {
 		return err
 	}
@@ -213,6 +232,61 @@ func resourceAwsNetworkAclUpdate(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
+	if d.HasChange("subnet_ids") {
+		o, n := d.GetChange("subnet_ids")
+		if o == nil {
+			o = new(schema.Set)
+		}
+		if n == nil {
+			n = new(schema.Set)
+		}
+
+		os := o.(*schema.Set)
+		ns := n.(*schema.Set)
+
+		remove := os.Difference(ns).List()
+		add := ns.Difference(os).List()
+
+		if len(remove) > 0 {
+			// A Network ACL is required for each subnet. In order to disassociate a
+			// subnet from this ACL, we must associate it with the default ACL.
+			defaultAcl, err := getDefaultNetworkAcl(d.Get("vpc_id").(string), conn)
+			if err != nil {
+				return fmt.Errorf("Failed to find Default ACL for VPC %s", d.Get("vpc_id").(string))
+			}
+			for _, r := range remove {
+				association, err := findNetworkAclAssociation(r.(string), conn)
+				if err != nil {
+					return fmt.Errorf("Failed to find acl association: acl %s with subnet %s: %s", d.Id(), r, err)
+				}
+				_, err = conn.ReplaceNetworkACLAssociation(&ec2.ReplaceNetworkACLAssociationInput{
+					AssociationID: association.NetworkACLAssociationID,
+					NetworkACLID:  defaultAcl.NetworkACLID,
+				})
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+		if len(add) > 0 {
+			for _, a := range add {
+				association, err := findNetworkAclAssociation(a.(string), conn)
+				if err != nil {
+					return fmt.Errorf("Failed to find acl association: acl %s with subnet %s: %s", d.Id(), a, err)
+				}
+				_, err = conn.ReplaceNetworkACLAssociation(&ec2.ReplaceNetworkACLAssociationInput{
+					AssociationID: association.NetworkACLAssociationID,
+					NetworkACLID:  aws.String(d.Id()),
+				})
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+	}
+
 	if err := setTags(conn, d); err != nil {
 		return err
 	} else {
@@ -326,18 +400,36 @@ func resourceAwsNetworkAclDelete(d *schema.ResourceData, meta interface{}) error
 			case "DependencyViolation":
 				// In case of dependency violation, we remove the association between subnet and network acl.
 				// This means the subnet is attached to default acl of vpc.
-				association, err := findNetworkAclAssociation(d.Get("subnet_id").(string), conn)
+				var associations []*ec2.NetworkACLAssociation
+				if v, ok := d.GetOk("subnet_id"); ok {
+					a, err := findNetworkAclAssociation(v.(string), conn)
 					if err != nil {
 						return resource.RetryError{Err: fmt.Errorf("Dependency violation: Cannot delete acl %s: %s", d.Id(), err)}
 					}
+					associations = append(associations, a)
+				}
+
+				if v, ok := d.GetOk("subnet_ids"); ok {
+					ids := v.(*schema.Set).List()
+					for _, i := range ids {
+						a, err := findNetworkAclAssociation(i.(string), conn)
+						if err != nil {
+							return resource.RetryError{Err: fmt.Errorf("Dependency violation: Cannot delete acl %s: %s", d.Id(), err)}
+						}
+						associations = append(associations, a)
+					}
+				}
 				defaultAcl, err := getDefaultNetworkAcl(d.Get("vpc_id").(string), conn)
 				if err != nil {
 					return resource.RetryError{Err: fmt.Errorf("Dependency violation: Cannot delete acl %s: %s", d.Id(), err)}
 				}
+
+				for _, a := range associations {
 					_, err = conn.ReplaceNetworkACLAssociation(&ec2.ReplaceNetworkACLAssociationInput{
-					AssociationID: association.NetworkACLAssociationID,
+						AssociationID: a.NetworkACLAssociationID,
 						NetworkACLID:  defaultAcl.NetworkACLID,
 					})
+				}
 				return resource.RetryError{Err: err}
 			default:
 				// Any other error, we want to quit the retry loop immediately
@@ -417,7 +509,7 @@ func findNetworkAclAssociation(subnetId string, conn *ec2.EC2) (networkAclAssoci
 			}
 		}
 	}
-	return nil, fmt.Errorf("could not find association for subnet %s ", subnetId)
+	return nil, fmt.Errorf("could not find association for subnet: %s ", subnetId)
 }
 
 // networkAclEntriesToMapList turns ingress/egress rules read from AWS into a list

builtin/providers/aws/resource_aws_network_acl_test.go

@@ -181,6 +181,49 @@ func TestAccAWSNetworkAcl_SubnetChange(t *testing.T) {
 
 }
 
+func TestAccAWSNetworkAcl_Subnets(t *testing.T) {
+	var networkAcl ec2.NetworkACL
+
+	checkACLSubnets := func(acl *ec2.NetworkACL, count int) resource.TestCheckFunc {
+		return func(*terraform.State) (err error) {
+			if count != len(acl.Associations) {
+				return fmt.Errorf("ACL association count does not match, expected %d, got %d", count, len(acl.Associations))
+			}
+
+			return nil
+		}
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSNetworkAclDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSNetworkAclSubnet_SubnetIds,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSNetworkAclExists("aws_network_acl.bar", &networkAcl),
+					testAccCheckSubnetIsAssociatedWithAcl("aws_network_acl.bar", "aws_subnet.one"),
+					testAccCheckSubnetIsAssociatedWithAcl("aws_network_acl.bar", "aws_subnet.two"),
+					checkACLSubnets(&networkAcl, 2),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccAWSNetworkAclSubnet_SubnetIdsUpdate,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSNetworkAclExists("aws_network_acl.bar", &networkAcl),
+					testAccCheckSubnetIsAssociatedWithAcl("aws_network_acl.bar", "aws_subnet.one"),
+					testAccCheckSubnetIsAssociatedWithAcl("aws_network_acl.bar", "aws_subnet.three"),
+					testAccCheckSubnetIsAssociatedWithAcl("aws_network_acl.bar", "aws_subnet.four"),
+					checkACLSubnets(&networkAcl, 3),
+				),
+			},
+		},
+	})
+
+}
+
 func testAccCheckAWSNetworkAclDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).ec2conn
 
@@ -281,10 +324,6 @@ func testAccCheckSubnetIsAssociatedWithAcl(acl string, sub string) resource.Test
 			return nil
 		}
 
-		// r, _ := conn.NetworkACLs([]string{}, ec2.NewFilter())
-		// fmt.Printf("\n\nall acls\n %#v\n\n", r.NetworkAcls)
-		// conn.NetworkAcls([]string{}, filter)
-
 		return fmt.Errorf("Network Acl %s is not associated with subnet %s", acl, sub)
 	}
 }
@@ -494,3 +533,58 @@ resource "aws_network_acl" "bar" {
 	subnet_id = "${aws_subnet.new.id}"
 }
 `
+
+const testAccAWSNetworkAclSubnet_SubnetIds = `
+resource "aws_vpc" "foo" {
+	cidr_block = "10.1.0.0/16"
+	tags {
+		Name = "acl-subnets-test"
+	}
+}
+resource "aws_subnet" "one" {
+	cidr_block = "10.1.111.0/24"
+	vpc_id = "${aws_vpc.foo.id}"
+}
+resource "aws_subnet" "two" {
+	cidr_block = "10.1.1.0/24"
+	vpc_id = "${aws_vpc.foo.id}"
+}
+resource "aws_network_acl" "bar" {
+	vpc_id = "${aws_vpc.foo.id}"
+	subnet_ids = ["${aws_subnet.one.id}", "${aws_subnet.two.id}"]
+}
+`
+
+const testAccAWSNetworkAclSubnet_SubnetIdsUpdate = `
+resource "aws_vpc" "foo" {
+	cidr_block = "10.1.0.0/16"
+	tags {
+		Name = "acl-subnets-test"
+	}
+}
+resource "aws_subnet" "one" {
+	cidr_block = "10.1.111.0/24"
+	vpc_id = "${aws_vpc.foo.id}"
+}
+resource "aws_subnet" "two" {
+	cidr_block = "10.1.1.0/24"
+	vpc_id = "${aws_vpc.foo.id}"
+}
+
+resource "aws_subnet" "three" {
+	cidr_block = "10.1.222.0/24"
+	vpc_id = "${aws_vpc.foo.id}"
+}
+resource "aws_subnet" "four" {
+	cidr_block = "10.1.4.0/24"
+	vpc_id = "${aws_vpc.foo.id}"
+}
+resource "aws_network_acl" "bar" {
+	vpc_id = "${aws_vpc.foo.id}"
+	subnet_ids = [
+		"${aws_subnet.one.id}", 
+		"${aws_subnet.three.id}",
+		"${aws_subnet.four.id}",
+	]
+}
+`

website/source/docs/providers/aws/r/network_acl.html.markdown

@@ -45,7 +45,9 @@ resource "aws_network_acl" "main" {
 The following arguments are supported:
 
 * `vpc_id` - (Required) The ID of the associated VPC.
-* `subnet_id` - (Optional) The ID of the associated subnet.
+* `subnet_ids` - (Optional) A list of Subnet IDs to apply the ACL to
+* `subnet_id` - (Optional, Deprecated) The ID of the associated Subnet. This
+attribute is deprecated, please use the `subnet_ids` attribute instead
 * `ingress` - (Optional) Specifies an ingress rule. Parameters defined below.
 * `egress` - (Optional) Specifies an egress rule. Parameters defined below.
 * `tags` - (Optional) A mapping of tags to assign to the resource.