Merge pull request #10315 from terraform-providers/td-vpc-keyvaluetags

resource/aws_vpc: Refactor to use keyvaluetags and call Read after Create

aws/internal/keyvaluetags/generators/updatetags/main.go

@@ -43,6 +43,7 @@ var serviceNames = []string{
 	"directoryservice",
 	"docdb",
 	"dynamodb",
+	"ec2",
 	"ecr",
 	"ecs",
 	"efs",
@@ -101,14 +102,16 @@ func main() {
 		ServiceNames: serviceNames,
 	}
 	templateFuncMap := template.FuncMap{
-		"ClientType":                keyvaluetags.ServiceClientType,
-		"TagFunction":               ServiceTagFunction,
-		"TagInputIdentifierField":   ServiceTagInputIdentifierField,
-		"TagInputResourceTypeField": ServiceTagInputResourceTypeField,
-		"TagInputTagsField":         ServiceTagInputTagsField,
-		"Title":                     strings.Title,
-		"UntagFunction":             ServiceUntagFunction,
-		"UntagInputTagsField":       ServiceUntagInputTagsField,
+		"ClientType":                      keyvaluetags.ServiceClientType,
+		"TagFunction":                     ServiceTagFunction,
+		"TagInputIdentifierField":         ServiceTagInputIdentifierField,
+		"TagInputIdentifierRequiresSlice": ServiceTagInputIdentifierRequiresSlice,
+		"TagInputResourceTypeField":       ServiceTagInputResourceTypeField,
+		"TagInputTagsField":               ServiceTagInputTagsField,
+		"Title":                           strings.Title,
+		"UntagFunction":                   ServiceUntagFunction,
+		"UntagInputRequiresTagType":       ServiceUntagInputRequiresTagType,
+		"UntagInputTagsField":             ServiceUntagInputTagsField,
 	}
 
 	tmpl, err := template.New("updatetags").Funcs(templateFuncMap).Parse(templateBody)
@@ -169,11 +172,19 @@ func {{ . | Title }}UpdateTags(conn {{ . | ClientType }}, identifier string{{ if
 
 	if removedTags := oldTags.Removed(newTags); len(removedTags) > 0 {
 		input := &{{ . }}.{{ . | UntagFunction }}Input{
+			{{- if . | TagInputIdentifierRequiresSlice }}
+			{{ . | TagInputIdentifierField }}:   aws.StringSlice([]string{identifier}),
+			{{- else }}
 			{{ . | TagInputIdentifierField }}:   aws.String(identifier),
+			{{- end }}
 			{{- if . | TagInputResourceTypeField }}
 			{{ . | TagInputResourceTypeField }}: aws.String(resourceType),
 			{{- end }}
+			{{- if . | UntagInputRequiresTagType }}
+			{{ . | UntagInputTagsField }}:       removedTags.IgnoreAws().{{ . | Title }}Tags(),
+			{{- else }}
 			{{ . | UntagInputTagsField }}:       aws.StringSlice(removedTags.Keys()),
+			{{- end }}
 		}
 
 		_, err := conn.{{ . | UntagFunction }}(input)
@@ -185,7 +196,11 @@ func {{ . | Title }}UpdateTags(conn {{ . | ClientType }}, identifier string{{ if
 
 	if updatedTags := oldTags.Updated(newTags); len(updatedTags) > 0 {
 		input := &{{ . }}.{{ . | TagFunction }}Input{
+			{{- if . | TagInputIdentifierRequiresSlice }}
+			{{ . | TagInputIdentifierField }}:   aws.StringSlice([]string{identifier}),
+			{{- else }}
 			{{ . | TagInputIdentifierField }}:   aws.String(identifier),
+			{{- end }}
 			{{- if . | TagInputResourceTypeField }}
 			{{ . | TagInputResourceTypeField }}: aws.String(resourceType),
 			{{- end }}
@@ -215,6 +230,8 @@ func ServiceTagFunction(serviceName string) string {
 		return "AddTagsToResource"
 	case "docdb":
 		return "AddTagsToResource"
+	case "ec2":
+		return "CreateTags"
 	case "efs":
 		return "CreateTags"
 	case "elasticache":
@@ -269,6 +286,8 @@ func ServiceTagInputIdentifierField(serviceName string) string {
 		return "ResourceId"
 	case "docdb":
 		return "ResourceName"
+	case "ec2":
+		return "Resources"
 	case "efs":
 		return "FileSystemId"
 	case "elasticache":
@@ -322,6 +341,16 @@ func ServiceTagInputIdentifierField(serviceName string) string {
 	}
 }
 
+// ServiceTagInputIdentifierRequiresSlice determines if the service tagging resource field requires a slice.
+func ServiceTagInputIdentifierRequiresSlice(serviceName string) string {
+	switch serviceName {
+	case "ec2":
+		return "yes"
+	default:
+		return ""
+	}
+}
+
 // ServiceTagInputTagsField determines the service tagging tags field.
 func ServiceTagInputTagsField(serviceName string) string {
 	switch serviceName {
@@ -357,6 +386,8 @@ func ServiceUntagFunction(serviceName string) string {
 		return "RemoveTagsFromResource"
 	case "docdb":
 		return "RemoveTagsFromResource"
+	case "ec2":
+		return "DeleteTags"
 	case "efs":
 		return "DeleteTags"
 	case "elasticache":
@@ -390,6 +421,16 @@ func ServiceUntagFunction(serviceName string) string {
 	}
 }
 
+// ServiceUntagInputRequiresTagType determines if the service untagging requires full Tag type.
+func ServiceUntagInputRequiresTagType(serviceName string) string {
+	switch serviceName {
+	case "ec2":
+		return "yes"
+	default:
+		return ""
+	}
+}
+
 // ServiceUntagInputTagsField determines the service untagging tags field.
 func ServiceUntagInputTagsField(serviceName string) string {
 	switch serviceName {
@@ -399,6 +440,8 @@ func ServiceUntagInputTagsField(serviceName string) string {
 		return "TagKeyList"
 	case "datasync":
 		return "Keys"
+	case "ec2":
+		return "Tags"
 	case "glue":
 		return "TagsToRemove"
 	default:

aws/internal/keyvaluetags/service_generation_customizations.go

@@ -34,6 +34,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/directoryservice"
 	"github.com/aws/aws-sdk-go/service/docdb"
 	"github.com/aws/aws-sdk-go/service/dynamodb"
+	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ecr"
 	"github.com/aws/aws-sdk-go/service/ecs"
 	"github.com/aws/aws-sdk-go/service/efs"
@@ -147,6 +148,8 @@ func ServiceClientType(serviceName string) string {
 		funcType = reflect.TypeOf(docdb.New)
 	case "dynamodb":
 		funcType = reflect.TypeOf(dynamodb.New)
+	case "ec2":
+		funcType = reflect.TypeOf(ec2.New)
 	case "ecr":
 		funcType = reflect.TypeOf(ecr.New)
 	case "ecs":

aws/resource_aws_vpc.go

@@ -12,6 +12,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
 )
 
 func resourceAwsVpc() *schema.Resource {
@@ -171,8 +172,96 @@ func resourceAwsVpcCreate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
-	// Update our attributes and return
-	return resourceAwsVpcUpdate(d, meta)
+	// You cannot modify the DNS resolution and DNS hostnames attributes in the same request. Use separate requests for each attribute.
+	// Reference: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_ModifyVpcAttribute.html
+
+	if d.Get("enable_dns_hostnames").(bool) {
+		input := &ec2.ModifyVpcAttributeInput{
+			EnableDnsHostnames: &ec2.AttributeBooleanValue{
+				Value: aws.Bool(true),
+			},
+			VpcId: aws.String(d.Id()),
+		}
+
+		if _, err := conn.ModifyVpcAttribute(input); err != nil {
+			return fmt.Errorf("error enabling VPC (%s) DNS hostnames: %s", d.Id(), err)
+		}
+
+		d.SetPartial("enable_dns_hostnames")
+	}
+
+	// By default, only the enableDnsSupport attribute is set to true in a VPC created any other way.
+	// Reference: https://docs.aws.amazon.com/vpc/latest/userguide/vpc-dns.html#vpc-dns-support
+
+	if !d.Get("enable_dns_support").(bool) {
+		input := &ec2.ModifyVpcAttributeInput{
+			EnableDnsSupport: &ec2.AttributeBooleanValue{
+				Value: aws.Bool(false),
+			},
+			VpcId: aws.String(d.Id()),
+		}
+
+		if _, err := conn.ModifyVpcAttribute(input); err != nil {
+			return fmt.Errorf("error disabling VPC (%s) DNS support: %s", d.Id(), err)
+		}
+
+		d.SetPartial("enable_dns_support")
+	}
+
+	if d.Get("enable_classiclink").(bool) {
+		input := &ec2.EnableVpcClassicLinkInput{
+			VpcId: aws.String(d.Id()),
+		}
+
+		if _, err := conn.EnableVpcClassicLink(input); err != nil {
+			return fmt.Errorf("error enabling VPC (%s) ClassicLink: %s", d.Id(), err)
+		}
+
+		d.SetPartial("enable_classiclink")
+	}
+
+	if d.Get("enable_classiclink_dns_support").(bool) {
+		input := &ec2.EnableVpcClassicLinkDnsSupportInput{
+			VpcId: aws.String(d.Id()),
+		}
+
+		if _, err := conn.EnableVpcClassicLinkDnsSupport(input); err != nil {
+			return fmt.Errorf("error enabling VPC (%s) ClassicLink DNS support: %s", d.Id(), err)
+		}
+
+		d.SetPartial("enable_classiclink_dns_support")
+	}
+
+	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, "InvalidVpcID.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")
+	}
+
+	d.Partial(false)
+
+	return resourceAwsVpcRead(d, meta)
 }
 
 func resourceAwsVpcRead(d *schema.ResourceData, meta interface{}) error {
@@ -205,8 +294,9 @@ func resourceAwsVpcRead(d *schema.ResourceData, meta interface{}) error {
 	}.String()
 	d.Set("arn", arn)
 
-	// Tags
-	d.Set("tags", tagsToMap(vpc.Tags))
+	if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(vpc.Tags).IgnoreAws().Map()); err != nil {
+		return fmt.Errorf("error setting tags: %s", err)
+	}
 
 	d.Set("owner_id", vpc.OwnerId)
 
@@ -404,7 +494,7 @@ func resourceAwsVpcUpdate(d *schema.ResourceData, meta interface{}) error {
 		d.SetPartial("enable_classiclink_dns_support")
 	}
 
-	if d.HasChange("assign_generated_ipv6_cidr_block") && !d.IsNewResource() {
+	if d.HasChange("assign_generated_ipv6_cidr_block") {
 		toAssign := d.Get("assign_generated_ipv6_cidr_block").(bool)
 
 		log.Printf("[INFO] Modifying assign_generated_ipv6_cidr_block to %#v", toAssign)
@@ -445,7 +535,7 @@ func resourceAwsVpcUpdate(d *schema.ResourceData, meta interface{}) error {
 		d.SetPartial("assign_generated_ipv6_cidr_block")
 	}
 
-	if d.HasChange("instance_tenancy") && !d.IsNewResource() {
+	if d.HasChange("instance_tenancy") {
 		modifyOpts := &ec2.ModifyVpcTenancyInput{
 			VpcId:           aws.String(vpcid),
 			InstanceTenancy: aws.String(d.Get("instance_tenancy").(string)),
@@ -460,9 +550,13 @@ func resourceAwsVpcUpdate(d *schema.ResourceData, meta interface{}) error {
 		d.SetPartial("instance_tenancy")
 	}
 
-	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 tags: %s", err)
+		}
+
 		d.SetPartial("tags")
 	}