Skip to content

Commit

Permalink
service/ec2: Switch tagging during resource creation to keyvaluetags.…
Browse files Browse the repository at this point in the history
…CreateEc2Tags implementation

Reference: hashicorp#11060
Reference: hashicorp#12427
Reference: https://github.com/terraform-providers/terraform-provider-aws/blob/0c56c9bea1291e77f28ae99c79748251a2e23517/aws/tags.go#L198

The EC2 service has special considerations during resource creation to retry for eventual consistency within the API itself on "NotFound" errors. This switches Terraform resources that cannot tag-on-create due to the lack of EC2 API support to the keyvaluetags implementation that handles this eventual consistency automatically for 5 minutes (or max retries if API is throttling).

This retry logic was present prior to the service refactoring to keyvaluetags (although errantly retrying on "NotFound" errors on every update, which may never succeed) and was still present in the `aws_vpc` and `aws_subnet` resources manually in the logic after the refactoring.

This refactor also catches cases where the resource `Create` function was depending on the `Update` logic to handle tagging on creation logic. We discourage the usage of `Update` after `Create` for new resources, but this refactor only guards against running the tag update logic rather than bundling more complex resource refactoring into this changeset.

Now all EC2 resources are consolidated to similar tagging on creation logic.

Output from acceptance testing:

```

```
  • Loading branch information
bflad authored and adamdecaf committed May 28, 2020
1 parent ee4535c commit ad15ccf
Show file tree
Hide file tree
Showing 23 changed files with 51 additions and 62 deletions.
2 changes: 1 addition & 1 deletion aws/resource_aws_ami.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func resourceAwsAmiCreate(d *schema.ResourceData, meta interface{}) error {
d.SetId(id)

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(client, id, nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(client, id, v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_ami_copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func resourceAwsAmiCopyCreate(d *schema.ResourceData, meta interface{}) error {
d.Set("manage_ebs_snapshots", true)

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(client, id, nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(client, id, v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_ami_from_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func resourceAwsAmiFromInstanceCreate(d *schema.ResourceData, meta interface{})
d.Set("manage_ebs_snapshots", true)

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(client, id, nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(client, id, v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_customer_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func resourceAwsCustomerGatewayCreate(d *schema.ResourceData, meta interface{})
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding EC2 Customer Gateway (%s) tags: %s", d.Id(), err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_default_security_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func resourceAwsDefaultSecurityGroupCreate(d *schema.ResourceData, meta interfac
log.Printf("[INFO] Default Security Group ID: %s", d.Id())

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding EC2 Default Security Group (%s) tags: %s", d.Id(), err)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func resourceAwsEc2TransitGatewayVpcAttachmentAccepterCreate(d *schema.ResourceD
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error updating EC2 Transit Gateway VPC Attachment (%s) tags: %s", d.Id(), err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_egress_only_internet_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func resourceAwsEgressOnlyInternetGatewayCreate(d *schema.ResourceData, meta int
d.SetId(aws.StringValue(resp.EgressOnlyInternetGateway.EgressOnlyInternetGatewayId))

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions aws/resource_aws_eip.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func resourceAwsEipCreate(d *schema.ResourceData, meta interface{}) error {
log.Printf("[INFO] EIP ID: %s (domain: %v)", d.Id(), *allocResp.Domain)

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(ec2conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(ec2conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down Expand Up @@ -358,7 +358,7 @@ func resourceAwsEipUpdate(d *schema.ResourceData, meta interface{}) error {
}
}

if d.HasChange("tags") {
if d.HasChange("tags") && !d.IsNewResource() {
o, n := d.GetChange("tags")
if err := keyvaluetags.Ec2UpdateTags(ec2conn, d.Id(), o, n); err != nil {
return fmt.Errorf("error updating EIP (%s) tags: %s", d.Id(), err)
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_internet_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func resourceAwsInternetGatewayCreate(d *schema.ResourceData, meta interface{})
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding EC2 Internet Gateway (%s) tags: %s", d.Id(), err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_key_pair.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func resourceAwsKeyPairCreate(d *schema.ResourceData, meta interface{}) error {

for _, keyPair := range readResp.KeyPairs {
if *keyPair.KeyName == d.Id() {
if err := keyvaluetags.Ec2UpdateTags(conn, aws.StringValue(keyPair.KeyPairId), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, aws.StringValue(keyPair.KeyPairId), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_network_acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ func resourceAwsNetworkAclUpdate(d *schema.ResourceData, meta interface{}) error

}

if d.HasChange("tags") {
if d.HasChange("tags") && !d.IsNewResource() {
o, n := d.GetChange("tags")

if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil {
Expand Down
10 changes: 8 additions & 2 deletions aws/resource_aws_network_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,13 @@ func resourceAwsNetworkInterfaceCreate(d *schema.ResourceData, meta interface{})
}

d.SetId(*resp.NetworkInterface.NetworkInterfaceId)
log.Printf("[INFO] ENI ID: %s", d.Id())

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}

return resourceAwsNetworkInterfaceUpdate(d, meta)
}

Expand Down Expand Up @@ -404,7 +410,7 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{})
}
}

if d.HasChange("tags") {
if d.HasChange("tags") && !d.IsNewResource() {
o, n := d.GetChange("tags")

if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_placement_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func resourceAwsPlacementGroupCreate(d *schema.ResourceData, meta interface{}) e
return err
}
pg := out.PlacementGroups[0]
if err := keyvaluetags.Ec2UpdateTags(conn, aws.StringValue(pg.GroupId), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, aws.StringValue(pg.GroupId), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
8 changes: 7 additions & 1 deletion aws/resource_aws_route_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ func resourceAwsRouteTableCreate(d *schema.ResourceData, meta interface{}) error
d.Id(), err)
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}

return resourceAwsRouteTableUpdate(d, meta)
}

Expand Down Expand Up @@ -405,7 +411,7 @@ func resourceAwsRouteTableUpdate(d *schema.ResourceData, meta interface{}) error
}
}

if d.HasChange("tags") {
if d.HasChange("tags") && !d.IsNewResource() {
o, n := d.GetChange("tags")

if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_security_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding EC2 Security Group (%s) tags: %s", d.Id(), err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_spot_instance_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func resourceAwsSpotInstanceRequestCreate(d *schema.ResourceData, meta interface
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding EC2 Spot Instance Request (%s) tags: %s", d.Id(), err)
}
}
Expand Down
21 changes: 1 addition & 20 deletions aws/resource_aws_subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,26 +142,7 @@ func resourceAwsSubnetCreate(d *schema.ResourceData, meta interface{}) error {
}

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 {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
21 changes: 1 addition & 20 deletions aws/resource_aws_vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,26 +221,7 @@ func resourceAwsVpcCreate(d *schema.ResourceData, meta interface{}) error {
}

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 {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_vpc_dhcp_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func resourceAwsVpcDhcpOptionsCreate(d *schema.ResourceData, meta interface{}) e
}

if v, ok := d.GetOk("tags"); ok {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v.(map[string]interface{})); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v.(map[string]interface{})); err != nil {
return fmt.Errorf("error updating tags: %s", err)
}
}
Expand Down
8 changes: 7 additions & 1 deletion aws/resource_aws_vpc_peering_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@ func resourceAwsVPCPeeringCreate(d *schema.ResourceData, meta interface{}) error
return fmt.Errorf("Error waiting for VPC Peering Connection to become available: %s", err)
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}

return resourceAwsVPCPeeringUpdate(d, meta)
}

Expand Down Expand Up @@ -203,7 +209,7 @@ func resourceAwsVpcPeeringConnectionModifyOptions(d *schema.ResourceData, meta i
func resourceAwsVPCPeeringUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

if d.HasChange("tags") {
if d.HasChange("tags") && !d.IsNewResource() {
o, n := d.GetChange("tags")

if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil {
Expand Down
9 changes: 9 additions & 0 deletions aws/resource_aws_vpc_peering_connection_accepter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"log"

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func resourceAwsVpcPeeringConnectionAccepter() *schema.Resource {
Expand Down Expand Up @@ -59,6 +60,8 @@ func resourceAwsVpcPeeringConnectionAccepter() *schema.Resource {
}

func resourceAwsVPCPeeringAccepterCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

id := d.Get("vpc_peering_connection_id").(string)
d.SetId(id)

Expand All @@ -69,6 +72,12 @@ func resourceAwsVPCPeeringAccepterCreate(d *schema.ResourceData, meta interface{
return fmt.Errorf("VPC Peering Connection %q not found", id)
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding tags: %s", err)
}
}

return resourceAwsVPCPeeringUpdate(d, meta)
}

Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_vpn_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func resourceAwsVpnConnectionCreate(d *schema.ResourceData, meta interface{}) er
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding EC2 VPN Connection (%s) tags: %s", d.Id(), err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_aws_vpn_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func resourceAwsVpnGatewayCreate(d *schema.ResourceData, meta interface{}) error
}

if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), nil, v); err != nil {
if err := keyvaluetags.Ec2CreateTags(conn, d.Id(), v); err != nil {
return fmt.Errorf("error adding EC2 VPN Gateway (%s) tags: %s", d.Id(), err)
}
}
Expand Down

0 comments on commit ad15ccf

Please sign in to comment.