Skip to content

Commit

Permalink
resource/aws_subnet: Refactor to use keyvaluetags library and call Re…
Browse files Browse the repository at this point in the history
…ad after Create (#10411)

Reference: #7926
Similar in nature to: #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)
```
  • Loading branch information
bflad authored Oct 25, 2019
1 parent 62c6095 commit f1b17f4
Showing 1 changed file with 77 additions and 8 deletions.
85 changes: 77 additions & 8 deletions aws/resource_aws_subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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")
}

Expand All @@ -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
Expand Down

0 comments on commit f1b17f4

Please sign in to comment.