Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 51 additions & 8 deletions aws/internal/keyvaluetags/generators/updatetags/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ var serviceNames = []string{
"directoryservice",
"docdb",
"dynamodb",
"ec2",
"ecr",
"ecs",
"efs",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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 }}
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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 {
Expand All @@ -399,6 +440,8 @@ func ServiceUntagInputTagsField(serviceName string) string {
return "TagKeyList"
case "datasync":
return "Keys"
case "ec2":
return "Tags"
case "glue":
return "TagsToRemove"
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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":
Expand Down
37 changes: 37 additions & 0 deletions aws/internal/keyvaluetags/update_tags_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

112 changes: 103 additions & 9 deletions aws/resource_aws_vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)),
Expand All @@ -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")
}

Expand Down