provider/aws: Allow AWS Subnet to change IPv6 CIDR Block without ForceNew (#13909)

Fixes: #13588

It was pointed out in #13588 that we don't need to ForceNew on a change
of IPv6 CIDR block. The logic I decided to implement here was to
disassociate then associate. We should only be able to be associated to
1 IPv6 CIDR block at once. This feels like a risky move. We can
disassociate and then error on the associate. This would leave us in a
situation where we have no IPv6 CIDR block associated

The alternative here would be that the failure of association, triggers
a reassociation with the old IPv6 CIDR block

I added a test to make sure that the subnet Ids don't change as the ipv6
block changes. Before removing the ForceNew from the ipv6_cidr_block,
the test results in the following:

```
=== RUN   TestAccAWSSubnet_ipv6
--- FAIL: TestAccAWSSubnet_ipv6 (92.09s)
	resource_aws_subnet_test.go:105: Expected SubnetIDs not to change, but both got before: subnet-0d2b6a6a and after: subnet-742c6d13
```

After the removal of ForceNew, the test result looks as follows:

```
=== RUN   TestAccAWSSubnet_ipv6
--- PASS: TestAccAWSSubnet_ipv6 (188.34s)
```

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSubnet_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/04/24 21:26:36 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSubnet_ -timeout 120m
=== RUN   TestAccAWSSubnet_importBasic
--- PASS: TestAccAWSSubnet_importBasic (85.63s)
=== RUN   TestAccAWSSubnet_basic
--- PASS: TestAccAWSSubnet_basic (80.28s)
=== RUN   TestAccAWSSubnet_ipv6
--- PASS: TestAccAWSSubnet_ipv6 (188.34s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	354.283s
```

builtin/providers/aws/resource_aws_subnet.go

@@ -41,7 +41,6 @@ func resourceAwsSubnet() *schema.Resource {
 			"ipv6_cidr_block": {
 				Type:     schema.TypeString,
 				Optional: true,
-				ForceNew: true,
 			},
 
 			"availability_zone": {
@@ -148,6 +147,7 @@ func resourceAwsSubnetRead(d *schema.ResourceData, meta interface{}) error {
 		if *a.Ipv6CidrBlockState.State == "associated" { //we can only ever have 1 IPv6 block associated at once
 			d.Set("ipv6_cidr_block_association_id", a.AssociationId)
 			d.Set("ipv6_cidr_block", a.Ipv6CidrBlock)
+			break
 		} else {
 			d.Set("ipv6_cidr_block_association_id", "") // we blank these out to remove old entries
 			d.Set("ipv6_cidr_block", "")
@@ -207,6 +207,73 @@ func resourceAwsSubnetUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
+	// We have to be careful here to not go through a change of association if this is a new resource
+	// A New resource here would denote that the Update func is called by the Create func
+	if d.HasChange("ipv6_cidr_block") && !d.IsNewResource() {
+		// We need to handle that we disassociate the IPv6 CIDR block before we try and associate the new one
+		// This could be an issue as, we could error out when we try and add the new one
+		// We may need to roll back the state and reattach the old one if this is the case
+
+		_, new := d.GetChange("ipv6_cidr_block")
+
+		//Firstly we have to disassociate the old IPv6 CIDR Block
+		disassociateOps := &ec2.DisassociateSubnetCidrBlockInput{
+			AssociationId: aws.String(d.Get("ipv6_cidr_block_association_id").(string)),
+		}
+
+		_, err := conn.DisassociateSubnetCidrBlock(disassociateOps)
+		if err != nil {
+			return err
+		}
+
+		// Wait for the CIDR to become disassociated
+		log.Printf(
+			"[DEBUG] Waiting for IPv6 CIDR (%s) to become disassociated",
+			d.Id())
+		stateConf := &resource.StateChangeConf{
+			Pending: []string{"disassociating", "associated"},
+			Target:  []string{"disassociated"},
+			Refresh: SubnetIpv6CidrStateRefreshFunc(conn, d.Id(), d.Get("ipv6_cidr_block_association_id").(string)),
+			Timeout: 1 * time.Minute,
+		}
+		if _, err := stateConf.WaitForState(); err != nil {
+			return fmt.Errorf(
+				"Error waiting for IPv6 CIDR (%s) to become disassociated: %s",
+				d.Id(), err)
+		}
+
+		//Now we need to try and associate the new CIDR block
+		associatesOpts := &ec2.AssociateSubnetCidrBlockInput{
+			SubnetId:      aws.String(d.Id()),
+			Ipv6CidrBlock: aws.String(new.(string)),
+		}
+
+		resp, err := conn.AssociateSubnetCidrBlock(associatesOpts)
+		if err != nil {
+			//The big question here is, do we want to try and reassociate the old one??
+			//If we have a failure here, then we may be in a situation that we have nothing associated
+			return err
+		}
+
+		// Wait for the CIDR to become associated
+		log.Printf(
+			"[DEBUG] Waiting for IPv6 CIDR (%s) to become associated",
+			d.Id())
+		stateConf = &resource.StateChangeConf{
+			Pending: []string{"associating", "disassociated"},
+			Target:  []string{"associated"},
+			Refresh: SubnetIpv6CidrStateRefreshFunc(conn, d.Id(), *resp.Ipv6CidrBlockAssociation.AssociationId),
+			Timeout: 1 * time.Minute,
+		}
+		if _, err := stateConf.WaitForState(); err != nil {
+			return fmt.Errorf(
+				"Error waiting for IPv6 CIDR (%s) to become associated: %s",
+				d.Id(), err)
+		}
+
+		d.SetPartial("ipv6_cidr_block")
+	}
+
 	d.Partial(false)
 
 	return resourceAwsSubnetRead(d, meta)
@@ -279,3 +346,38 @@ func SubnetStateRefreshFunc(conn *ec2.EC2, id string) resource.StateRefreshFunc
 		return subnet, *subnet.State, nil
 	}
 }
+
+func SubnetIpv6CidrStateRefreshFunc(conn *ec2.EC2, id string, associationId string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		opts := &ec2.DescribeSubnetsInput{
+			SubnetIds: []*string{aws.String(id)},
+		}
+		resp, err := conn.DescribeSubnets(opts)
+		if err != nil {
+			if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidSubnetID.NotFound" {
+				resp = nil
+			} else {
+				log.Printf("Error on SubnetIpv6CidrStateRefreshFunc: %s", err)
+				return nil, "", err
+			}
+		}
+
+		if resp == nil {
+			// Sometimes AWS just has consistency issues and doesn't see
+			// our instance yet. Return an empty state.
+			return nil, "", nil
+		}
+
+		if resp.Subnets[0].Ipv6CidrBlockAssociationSet == nil {
+			return nil, "", nil
+		}
+
+		for _, association := range resp.Subnets[0].Ipv6CidrBlockAssociationSet {
+			if *association.AssociationId == associationId {
+				return association, *association.Ipv6CidrBlockState.State, nil
+			}
+		}
+
+		return nil, "", nil
+	}
+}

builtin/providers/aws/resource_aws_subnet_test.go

@@ -45,27 +45,7 @@ func TestAccAWSSubnet_basic(t *testing.T) {
 }
 
 func TestAccAWSSubnet_ipv6(t *testing.T) {
-	var v ec2.Subnet
+	var before, after ec2.Subnet
-
-	testCheck := func(*terraform.State) error {
-		if v.Ipv6CidrBlockAssociationSet == nil {
-			return fmt.Errorf("Expected IPV6 CIDR Block Association")
-		}
-
-		if *v.AssignIpv6AddressOnCreation != true {
-			return fmt.Errorf("bad AssignIpv6AddressOnCreation: %t", *v.AssignIpv6AddressOnCreation)
-		}
-
-		return nil
-	}
-
-	testCheckUpdated := func(*terraform.State) error {
-		if *v.AssignIpv6AddressOnCreation != false {
-			return fmt.Errorf("bad AssignIpv6AddressOnCreation: %t", *v.AssignIpv6AddressOnCreation)
-		}
-
-		return nil
-	}
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:      func() { testAccPreCheck(t) },
@@ -77,22 +57,65 @@ func TestAccAWSSubnet_ipv6(t *testing.T) {
 				Config: testAccSubnetConfigIpv6,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckSubnetExists(
-						"aws_subnet.foo", &v),
+						"aws_subnet.foo", &before),
-					testCheck,
+					testAccCheckAwsSubnetIpv6BeforeUpdate(t, &before),
 				),
 			},
 			{
-				Config: testAccSubnetConfigIpv6Updated,
+				Config: testAccSubnetConfigIpv6UpdateAssignIpv6OnCreation,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckSubnetExists(
-						"aws_subnet.foo", &v),
+						"aws_subnet.foo", &after),
-					testCheckUpdated,
+					testAccCheckAwsSubnetIpv6AfterUpdate(t, &after),
+				),
+			},
+			{
+				Config: testAccSubnetConfigIpv6UpdateIpv6Cidr,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckSubnetExists(
+						"aws_subnet.foo", &after),
+
+					testAccCheckAwsSubnetNotRecreated(t, &before, &after),
 				),
 			},
 		},
 	})
 }
 
+func testAccCheckAwsSubnetIpv6BeforeUpdate(t *testing.T, subnet *ec2.Subnet) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if subnet.Ipv6CidrBlockAssociationSet == nil {
+			return fmt.Errorf("Expected IPV6 CIDR Block Association")
+		}
+
+		if *subnet.AssignIpv6AddressOnCreation != true {
+			return fmt.Errorf("bad AssignIpv6AddressOnCreation: %t", *subnet.AssignIpv6AddressOnCreation)
+		}
+
+		return nil
+	}
+}
+
+func testAccCheckAwsSubnetIpv6AfterUpdate(t *testing.T, subnet *ec2.Subnet) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if *subnet.AssignIpv6AddressOnCreation != false {
+			return fmt.Errorf("bad AssignIpv6AddressOnCreation: %t", *subnet.AssignIpv6AddressOnCreation)
+		}
+
+		return nil
+	}
+}
+
+func testAccCheckAwsSubnetNotRecreated(t *testing.T,
+	before, after *ec2.Subnet) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if *before.SubnetId != *after.SubnetId {
+			t.Fatalf("Expected SubnetIDs not to change, but both got before: %s and after: %s", *before.SubnetId, *after.SubnetId)
+		}
+		return nil
+	}
+}
+
 func testAccCheckSubnetDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).ec2conn
 
@@ -187,7 +210,25 @@ resource "aws_subnet" "foo" {
 }
 `
 
-const testAccSubnetConfigIpv6Updated = `
+const testAccSubnetConfigIpv6UpdateAssignIpv6OnCreation = `
+resource "aws_vpc" "foo" {
+	cidr_block = "10.10.0.0/16"
+	assign_generated_ipv6_cidr_block = true
+}
+
+resource "aws_subnet" "foo" {
+	cidr_block = "10.10.1.0/24"
+	vpc_id = "${aws_vpc.foo.id}"
+	ipv6_cidr_block = "${cidrsubnet(aws_vpc.foo.ipv6_cidr_block, 8, 1)}"
+	map_public_ip_on_launch = true
+	assign_ipv6_address_on_creation = false
+	tags {
+		Name = "tf-subnet-acc-test"
+	}
+}
+`
+
+const testAccSubnetConfigIpv6UpdateIpv6Cidr = `
 resource "aws_vpc" "foo" {
 	cidr_block = "10.10.0.0/16"
 	assign_generated_ipv6_cidr_block = true