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"
 
@@ -30,10 +31,17 @@ func resourceAwsNetworkAcl() *schema.Resource {
 				Computed: false,
 			},
 			"subnet_id": &schema.Schema{
-				Type:     schema.TypeString,
+				Type:       schema.TypeString,
+				Optional:   true,
+				ForceNew:   true,
+				Computed:   false,
+				Deprecated: "Attribute subnet_id is deprecated on network_acl resources. Use subnet_ids instead",
+			},
+			"subnet_ids": &schema.Schema{
+				Type:     schema.TypeList,
 				Optional: true,
-				ForceNew: true,
+				// Computed: true,
-				Computed: false,
+				Elem: &schema.Schema{Type: schema.TypeString},
 			},
 			"ingress": &schema.Schema{
 				Type:     schema.TypeSet,
@@ -168,6 +176,15 @@ func resourceAwsNetworkAclRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("vpc_id", networkAcl.VPCID)
 	d.Set("tags", tagsToMapSDK(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 +230,64 @@ func resourceAwsNetworkAclUpdate(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
+	if d.HasChange("subnet_ids") {
+		o, n := d.GetChange("subnet_ids")
+
+		ol := o.([]interface{})
+		var pre []string
+		for _, x := range ol {
+			pre = append(pre, x.(string))
+		}
+
+		nl := n.([]interface{})
+		var post []string
+		for _, x := range nl {
+			post = append(post, x.(string))
+		}
+
+		remove := diffSubnets(pre, post)
+		add := diffSubnets(post, pre)
+
+		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, 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, 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 := setTagsSDK(conn, d); err != nil {
 		return err
 	} else {
@@ -326,18 +401,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 err != nil {
+				if v, ok := d.GetOk("subnet_id"); ok {
-					return resource.RetryError{Err: fmt.Errorf("Dependency violation: Cannot delete acl %s: %s", d.Id(), err)}
+					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.([]interface{})
+					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)}
 				}
-				_, err = conn.ReplaceNetworkACLAssociation(&ec2.ReplaceNetworkACLAssociationInput{
+
-					AssociationID: association.NetworkACLAssociationID,
+				for _, a := range associations {
-					NetworkACLID:  defaultAcl.NetworkACLID,
+					_, err = conn.ReplaceNetworkACLAssociation(&ec2.ReplaceNetworkACLAssociationInput{
-				})
+						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 +510,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
@@ -451,3 +544,22 @@ func networkAclEntriesToMapList(networkAcls []*ec2.NetworkACLEntry) []map[string
 
 	return result
 }
+
+func diffSubnets(o, n []string) []string {
+	m := make(map[string]int)
+
+	for _, y := range n {
+		m[y]++
+	}
+
+	var ret []string
+	for _, x := range o {
+		if m[x] > 0 {
+			m[x]--
+			continue
+		}
+		ret = append(ret, x)
+	}
+
+	return ret
+}

builtin/providers/aws/resource_aws_network_acl_test.go

@@ -10,6 +10,72 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
+func TestDiffSubnets(t *testing.T) {
+	removal := []struct {
+		First  []string
+		Second []string
+		Output []string
+	}{
+		{[]string{"old"}, []string{"new"}, []string{"old"}},
+		{[]string{"new"}, []string{"old"}, []string{"new"}},
+		{[]string{"one", "two"}, []string{"two"}, []string{"one"}},
+		{
+			[]string{"subnet-1234", "subnet-5667", "subnet-897"},
+			[]string{"subnet-1234", "subnet-897"},
+			[]string{"subnet-5667"},
+		},
+		{
+			[]string{"subnet-1234", "subnet-897"},
+			[]string{"subnet-1234", "subnet-5667", "subnet-897"},
+			[]string{},
+		},
+	}
+	for _, tc := range removal {
+		actual := diffSubnets(tc.First, tc.Second)
+		if len(actual) != len(tc.Output) {
+			t.Fatalf("\n\texpected: %#v \n\tactual: %#v\n", tc.Output, actual)
+		}
+
+		for i, x := range tc.Output {
+			if x != actual[i] {
+				t.Fatalf("Network ACL diff does not match, expected %#v, got %#v", x, actual[i])
+			}
+		}
+	}
+
+	addition := []struct {
+		First  []string
+		Second []string
+		Output []string
+	}{
+		{[]string{"old"}, []string{"new"}, []string{"new"}},
+		{[]string{"new"}, []string{"old"}, []string{"old"}},
+		{[]string{"one", "two"}, []string{"two"}, []string{}},
+		{
+			[]string{"subnet-1234", "subnet-5667", "subnet-897"},
+			[]string{"subnet-1234", "subnet-897"},
+			[]string{},
+		},
+		{
+			[]string{"subnet-1234", "subnet-897"},
+			[]string{"subnet-1234", "subnet-5667", "subnet-897"},
+			[]string{"subnet-5667"},
+		},
+	}
+	for _, tc := range addition {
+		actual := diffSubnets(tc.Second, tc.First)
+		if len(actual) != len(tc.Output) {
+			t.Fatalf("\n\texpected: %#v \n\tactual: %#v\n", tc.Output, actual)
+		}
+
+		for i, x := range tc.Output {
+			if x != actual[i] {
+				t.Fatalf("Network ACL diff does not match, expected %#v, got %#v", x, actual[i])
+			}
+		}
+	}
+}
+
 func TestAccAWSNetworkAcl_EgressAndIngressRules(t *testing.T) {
 	var networkAcl ec2.NetworkACL
 
@@ -181,6 +247,24 @@ func TestAccAWSNetworkAcl_SubnetChange(t *testing.T) {
 
 }
 
+func TestAccAWSNetworkAcl_Subnets(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSNetworkAclDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSNetworkAclSubnet_SubnetIds,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckSubnetIsAssociatedWithAcl("aws_network_acl.bar", "aws_subnet.one"),
+					testAccCheckSubnetIsAssociatedWithAcl("aws_network_acl.bar", "aws_subnet.two"),
+				),
+			},
+		},
+	})
+
+}
+
 func testAccCheckAWSNetworkAclDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).ec2conn
 
@@ -281,10 +365,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 +574,24 @@ 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}"]
+}
+`