provider/aws: Allows aws_alb security_groups to be updated (#9804)

Fixes #9658
Fixes #8728

Originally, this would ForceNew as follows:

```
-/+ aws_alb.alb_test
    arn:                        "arn:aws:elasticloadbalancing:us-west-2:187416307283:loadbalancer/app/test-alb-9658/3459cd2446b76901" => "<computed>"
    arn_suffix:                 "app/test-alb-9658/3459cd2446b76901" => "<computed>"
    dns_name:                   "test-alb-9658-1463108301.us-west-2.elb.amazonaws.com" => "<computed>"
    enable_deletion_protection: "false" => "false"
    idle_timeout:               "30" => "30"
    internal:                   "false" => "false"
    name:                       "test-alb-9658" => "test-alb-9658"
    security_groups.#:          "2" => "1" (forces new resource)
    security_groups.1631253634: "sg-3256274b" => "" (forces new resource)
    security_groups.3505955000: "sg-1e572667" => "sg-1e572667" (forces new resource)
    subnets.#:                  "2" => "2"
    subnets.2407170741:         "subnet-ee536498" => "subnet-ee536498"
    subnets.2414619308:         "subnet-f1a7b595" => "subnet-f1a7b595"
    tags.%:                     "1" => "1"
    tags.TestName:              "TestAccAWSALB_basic" => "TestAccAWSALB_basic"
    vpc_id:                     "vpc-dd0ff9ba" => "<computed>"
    zone_id:                    "Z1H1FL5HABSF5" => "<computed>"

Plan: 1 to add, 0 to change, 1 to destroy.
```

When the ALB was ForceNew, the ARN changed. The test has been updated to include a check to make sure that the ARNs are the same after the update

After this change, it looks as follows:

```
~ aws_alb.alb_test
    security_groups.#:          "1" => "2"
    security_groups.1631253634: "" => "sg-3256274b"
    security_groups.3505955000: "sg-1e572667" => "sg-1e572667"

Plan: 0 to add, 1 to change, 0 to destroy.
```

Test Results:

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSALB_'                                                                                                                                ✹
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/11/02 12:20:58 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSALB_ -timeout 120m
=== RUN   TestAccAWSALB_basic
--- PASS: TestAccAWSALB_basic (64.25s)
=== RUN   TestAccAWSALB_generatedName
--- PASS: TestAccAWSALB_generatedName (65.04s)
=== RUN   TestAccAWSALB_namePrefix
--- PASS: TestAccAWSALB_namePrefix (67.02s)
=== RUN   TestAccAWSALB_tags
--- PASS: TestAccAWSALB_tags (96.06s)
=== RUN   TestAccAWSALB_updatedSecurityGroups
--- PASS: TestAccAWSALB_updatedSecurityGroups (101.61s)
=== RUN   TestAccAWSALB_noSecurityGroup
--- PASS: TestAccAWSALB_noSecurityGroup (59.83s)
=== RUN   TestAccAWSALB_accesslogs
--- PASS: TestAccAWSALB_accesslogs (162.65s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	616.489s
```

builtin/providers/aws/resource_aws_alb.go

@@ -61,7 +61,6 @@ func resourceAwsAlb() *schema.Resource {
 				Type:     schema.TypeSet,
 				Elem:     &schema.Schema{Type: schema.TypeString},
 				Computed: true,
-				ForceNew: true,
 				Optional: true,
 				Set:      schema.HashString,
 			},
@@ -331,6 +330,20 @@ func resourceAwsAlbUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
+	if d.HasChange("security_groups") {
+		sgs := expandStringList(d.Get("security_groups").(*schema.Set).List())
+
+		params := &elbv2.SetSecurityGroupsInput{
+			LoadBalancerArn: aws.String(d.Id()),
+			SecurityGroups:  sgs,
+		}
+		_, err := elbconn.SetSecurityGroups(params)
+		if err != nil {
+			return fmt.Errorf("Failure Setting ALB Security Groups: %s", err)
+		}
+
+	}
+
 	return resourceAwsAlbRead(d, meta)
 }
 

builtin/providers/aws/resource_aws_alb_test.go

@@ -150,6 +150,35 @@ func TestAccAWSALB_tags(t *testing.T) {
 	})
 }
 
+func TestAccAWSALB_updatedSecurityGroups(t *testing.T) {
+	var pre, post elbv2.LoadBalancer
+	albName := fmt.Sprintf("testaccawsalb-basic-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:      func() { testAccPreCheck(t) },
+		IDRefreshName: "aws_alb.alb_test",
+		Providers:     testAccProviders,
+		CheckDestroy:  testAccCheckAWSALBDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSALBConfig_basic(albName),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckAWSALBExists("aws_alb.alb_test", &pre),
+					resource.TestCheckResourceAttr("aws_alb.alb_test", "security_groups.#", "1"),
+				),
+			},
+			{
+				Config: testAccAWSALBConfig_updateSecurityGroups(albName),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckAWSALBExists("aws_alb.alb_test", &post),
+					resource.TestCheckResourceAttr("aws_alb.alb_test", "security_groups.#", "2"),
+					testAccCheckAWSAlbARNs(&pre, &post),
+				),
+			},
+		},
+	})
+}
+
 // TestAccAWSALB_noSecurityGroup regression tests the issue in #8264,
 // where if an ALB is created without a security group, a default one
 // is assigned.
@@ -259,6 +288,16 @@ func TestAccAWSALB_accesslogs(t *testing.T) {
 	})
 }
 
+func testAccCheckAWSAlbARNs(pre, post *elbv2.LoadBalancer) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if *pre.LoadBalancerArn != *post.LoadBalancerArn {
+			return errors.New("ALB has been recreated. ARNs are different")
+		}
+
+		return nil
+	}
+}
+
 func testAccCheckAWSALBExists(n string, res *elbv2.LoadBalancer) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -741,3 +780,87 @@ resource "aws_subnet" "alb_test" {
   }
 }`, albName)
 }
+
+func testAccAWSALBConfig_updateSecurityGroups(albName string) string {
+	return fmt.Sprintf(`resource "aws_alb" "alb_test" {
+  name            = "%s"
+  internal        = false
+  security_groups = ["${aws_security_group.alb_test.id}", "${aws_security_group.alb_test_2.id}"]
+  subnets         = ["${aws_subnet.alb_test.*.id}"]
+
+  idle_timeout = 30
+  enable_deletion_protection = false
+
+  tags {
+    TestName = "TestAccAWSALB_basic"
+  }
+}
+
+variable "subnets" {
+  default = ["10.0.1.0/24", "10.0.2.0/24"]
+  type    = "list"
+}
+
+data "aws_availability_zones" "available" {}
+
+resource "aws_vpc" "alb_test" {
+  cidr_block = "10.0.0.0/16"
+
+  tags {
+    TestName = "TestAccAWSALB_basic"
+  }
+}
+
+resource "aws_subnet" "alb_test" {
+  count                   = 2
+  vpc_id                  = "${aws_vpc.alb_test.id}"
+  cidr_block              = "${element(var.subnets, count.index)}"
+  map_public_ip_on_launch = true
+  availability_zone       = "${element(data.aws_availability_zones.available.names, count.index)}"
+
+  tags {
+    TestName = "TestAccAWSALB_basic"
+  }
+}
+
+resource "aws_security_group" "alb_test_2" {
+  name        = "allow_all_alb_test_2"
+  description = "Used for ALB Testing"
+  vpc_id      = "${aws_vpc.alb_test.id}"
+
+  ingress {
+    from_port   = 80
+    to_port     = 80
+    protocol    = "TCP"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+
+  tags {
+    TestName = "TestAccAWSALB_basic_2"
+  }
+}
+
+resource "aws_security_group" "alb_test" {
+  name        = "allow_all_alb_test"
+  description = "Used for ALB Testing"
+  vpc_id      = "${aws_vpc.alb_test.id}"
+
+  ingress {
+    from_port   = 0
+    to_port     = 0
+    protocol    = "-1"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+
+  egress {
+    from_port   = 0
+    to_port     = 0
+    protocol    = "-1"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+
+  tags {
+    TestName = "TestAccAWSALB_basic"
+  }
+}`, albName)
+}