From 0dda704cbfa5852f96afb53ff7f98c3e7837388f Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Tue, 12 May 2015 13:24:32 -0500 Subject: [PATCH] provider/aws: Support multiple subnets in Network ACL --- .../providers/aws/resource_aws_network_acl.go | 134 ++++++++++++++++-- .../aws/resource_aws_network_acl_test.go | 109 +++++++++++++- 2 files changed, 228 insertions(+), 15 deletions(-) diff --git a/builtin/providers/aws/resource_aws_network_acl.go b/builtin/providers/aws/resource_aws_network_acl.go index 0d44ff321..bc751cab7 100644 --- a/builtin/providers/aws/resource_aws_network_acl.go +++ b/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: false, + // Computed: true, + 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) - if err != nil { - return resource.RetryError{Err: fmt.Errorf("Dependency violation: Cannot delete acl %s: %s", d.Id(), err)} + 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.([]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, - NetworkACLID: defaultAcl.NetworkACLID, - }) + + for _, a := range associations { + _, 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 +} diff --git a/builtin/providers/aws/resource_aws_network_acl_test.go b/builtin/providers/aws/resource_aws_network_acl_test.go index 7a57a0012..32991f670 100644 --- a/builtin/providers/aws/resource_aws_network_acl_test.go +++ b/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}"] +} +`