From 8b460a8c62216ab48cb72d737b18324002eaa40b Mon Sep 17 00:00:00 2001
From: Brian Flad <bflad417@gmail.com>
Date: Mon, 7 Oct 2019 12:11:03 -0400
Subject: [PATCH] resource/aws_subnet: Refactor to use keyvaluetags library and
 call Read after Create

Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/7926
Similar in nature to: https://github.com/terraform-providers/terraform-provider-aws/pull/10315

In preparation for provider-wide ignore and default tag logic, here we refactor this resource to use the consistent `keyvaluetags` handling. The previous `setTags()` logic was always performing retries only necessary for resource creation and the resource itself was not following recommended practices to call the Read function after Create.

Ouput from acceptance testing:

```
--- PASS: TestAccAWSSubnet_availabilityZoneId (25.22s)
--- PASS: TestAccAWSSubnet_basic (25.66s)
--- PASS: TestAccAWSSubnet_enableIpv6 (41.83s)
--- PASS: TestAccAWSSubnet_ipv6 (66.62s)
```
---
 aws/resource_aws_subnet.go | 85 ++++++++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 8 deletions(-)

diff --git a/aws/resource_aws_subnet.go b/aws/resource_aws_subnet.go
index 2ce8e8fee31..56c8c402d1c 100644
--- a/aws/resource_aws_subnet.go
+++ b/aws/resource_aws_subnet.go
@@ -10,6 +10,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
 )
 
 func resourceAwsSubnet() *schema.Resource {
@@ -140,7 +141,69 @@ func resourceAwsSubnetCreate(d *schema.ResourceData, meta interface{}) error {
 			d.Id(), err)
 	}
 
-	return resourceAwsSubnetUpdate(d, meta)
+	if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
+		// Handle EC2 eventual consistency on creation
+		err := resource.Retry(5*time.Minute, func() *resource.RetryError {
+			err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v)
+
+			if isAWSErr(err, "InvalidSubnetID.NotFound", "") {
+				return resource.RetryableError(err)
+			}
+
+			if err != nil {
+				return resource.NonRetryableError(err)
+			}
+
+			return nil
+		})
+
+		if isResourceTimeoutError(err) {
+			err = keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v)
+		}
+
+		if err != nil {
+			return fmt.Errorf("error adding tags: %s", err)
+		}
+
+		d.SetPartial("tags")
+	}
+
+	// You cannot modify multiple subnet attributes in the same request.
+	// Reference: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_ModifySubnetAttribute.html
+
+	if d.Get("assign_ipv6_address_on_creation").(bool) {
+		input := &ec2.ModifySubnetAttributeInput{
+			AssignIpv6AddressOnCreation: &ec2.AttributeBooleanValue{
+				Value: aws.Bool(true),
+			},
+			SubnetId: aws.String(d.Id()),
+		}
+
+		if _, err := conn.ModifySubnetAttribute(input); err != nil {
+			return fmt.Errorf("error enabling EC2 Subnet (%s) assign IPv6 address on creation: %s", d.Id(), err)
+		}
+
+		d.SetPartial("assign_ipv6_address_on_creation")
+	}
+
+	if d.Get("map_public_ip_on_launch").(bool) {
+		input := &ec2.ModifySubnetAttributeInput{
+			MapPublicIpOnLaunch: &ec2.AttributeBooleanValue{
+				Value: aws.Bool(true),
+			},
+			SubnetId: aws.String(d.Id()),
+		}
+
+		if _, err := conn.ModifySubnetAttribute(input); err != nil {
+			return fmt.Errorf("error enabling EC2 Subnet (%s) map public IP on launch: %s", d.Id(), err)
+		}
+
+		d.SetPartial("map_public_ip_on_launch")
+	}
+
+	d.Partial(false)
+
+	return resourceAwsSubnetRead(d, meta)
 }
 
 func resourceAwsSubnetRead(d *schema.ResourceData, meta interface{}) error {
@@ -184,7 +247,11 @@ func resourceAwsSubnetRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	d.Set("arn", subnet.SubnetArn)
-	d.Set("tags", tagsToMap(subnet.Tags))
+
+	if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(subnet.Tags).IgnoreAws().Map()); err != nil {
+		return fmt.Errorf("error setting tags: %s", err)
+	}
+
 	d.Set("owner_id", subnet.OwnerId)
 
 	return nil
@@ -195,9 +262,13 @@ func resourceAwsSubnetUpdate(d *schema.ResourceData, meta interface{}) error {
 
 	d.Partial(true)
 
-	if err := setTags(conn, d); err != nil {
-		return err
-	} else {
+	if d.HasChange("tags") {
+		o, n := d.GetChange("tags")
+
+		if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil {
+			return fmt.Errorf("error updating EC2 Subnet (%s) tags: %s", d.Id(), err)
+		}
+
 		d.SetPartial("tags")
 	}
 
@@ -220,9 +291,7 @@ 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() {
+	if d.HasChange("ipv6_cidr_block") {
 		// 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