resource/aws_network_interface_sg_attachment: Handle read-after-creat…

…e eventual consistency (#18466)

* resource/aws_network_interface_sg_attachment: Handle read-after-create eventual consistency

Reference: #10363
Reference: #16201
Reference: #16796

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_disappears (59.24s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_basic (59.57s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Multiple (63.42s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Instance (93.21s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_DataSource (105.56s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_disappears (61.51s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_basic (62.15s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Multiple (65.05s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_DataSource (93.55s)
--- PASS: TestAccAWSNetworkInterfaceSGAttachment_Instance (114.74s)
```

* Update CHANGELOG for #18466

.changelog/18466.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_network_interface_sg_attachment: Handle read-after-create eventual consistency
+```

aws/internal/service/ec2/errors.go

@@ -17,6 +17,10 @@ const (
 	ErrCodeInvalidCarrierGatewayIDNotFound = "InvalidCarrierGatewayID.NotFound"
 )
 
+const (
+	ErrCodeInvalidNetworkInterfaceIDNotFound = "InvalidNetworkInterfaceID.NotFound"
+)
+
 const (
 	ErrCodeInvalidPrefixListIDNotFound = "InvalidPrefixListID.NotFound"
 )

aws/internal/service/ec2/finder/finder.go

@@ -180,6 +180,61 @@ func NetworkAclEntry(conn *ec2.EC2, networkAclID string, egress bool, ruleNumber
 	return nil, nil
 }
 
+// NetworkInterfaceByID looks up a NetworkInterface by ID. When not found, returns nil and potentially an API error.
+func NetworkInterfaceByID(conn *ec2.EC2, id string) (*ec2.NetworkInterface, error) {
+	input := &ec2.DescribeNetworkInterfacesInput{
+		NetworkInterfaceIds: aws.StringSlice([]string{id}),
+	}
+
+	output, err := conn.DescribeNetworkInterfaces(input)
+
+	if err != nil {
+		return nil, err
+	}
+
+	if output == nil {
+		return nil, nil
+	}
+
+	for _, networkInterface := range output.NetworkInterfaces {
+		if networkInterface == nil {
+			continue
+		}
+
+		if aws.StringValue(networkInterface.NetworkInterfaceId) != id {
+			continue
+		}
+
+		return networkInterface, nil
+	}
+
+	return nil, nil
+}
+
+// NetworkInterfaceSecurityGroup returns the associated GroupIdentifier if found
+func NetworkInterfaceSecurityGroup(conn *ec2.EC2, networkInterfaceID string, securityGroupID string) (*ec2.GroupIdentifier, error) {
+	var result *ec2.GroupIdentifier
+
+	networkInterface, err := NetworkInterfaceByID(conn, networkInterfaceID)
+
+	if err != nil {
+		return nil, err
+	}
+
+	if networkInterface == nil {
+		return nil, nil
+	}
+
+	for _, groupIdentifier := range networkInterface.Groups {
+		if aws.StringValue(groupIdentifier.GroupId) == securityGroupID {
+			result = groupIdentifier
+			break
+		}
+	}
+
+	return result, err
+}
+
 // RouteTableByID returns the route table corresponding to the specified identifier.
 // Returns NotFoundError if no route table is found.
 func RouteTableByID(conn *ec2.EC2, routeTableID string) (*ec2.RouteTable, error) {

aws/resource_aws_network_interface_sg_attachment.go

@@ -7,7 +7,13 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/hashicorp/aws-sdk-go-base/tfawserr"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
 )
 
 func resourceAwsNetworkInterfaceSGAttachment() *schema.Resource {
@@ -40,34 +46,38 @@ func resourceAwsNetworkInterfaceSGAttachmentCreate(d *schema.ResourceData, meta
 
 	conn := meta.(*AWSClient).ec2conn
 
-	// Fetch the network interface we will be working with.
-	iface, err := fetchNetworkInterface(conn, interfaceID)
+	iface, err := finder.NetworkInterfaceByID(conn, interfaceID)
+
 	if err != nil {
-		return err
+		return fmt.Errorf("error reading EC2 Network Interface (%s): %w", interfaceID, err)
 	}
 
-	// Add the security group to the network interface.
-	log.Printf("[DEBUG] Attaching security group %s to network interface ID %s", sgID, interfaceID)
+	groupIDs := []string{sgID}
 
-	if sgExistsInENI(sgID, iface) {
-		return fmt.Errorf("security group %s already attached to interface ID %s", sgID, *iface.NetworkInterfaceId)
-	}
-	var groupIDs []string
-	for _, v := range iface.Groups {
-		groupIDs = append(groupIDs, *v.GroupId)
+	for _, group := range iface.Groups {
+		if group == nil {
+			continue
+		}
+
+		if aws.StringValue(group.GroupId) == sgID {
+			return fmt.Errorf("EC2 Security Group (%s) already attached to EC2 Network Interface ID: %s", sgID, interfaceID)
+		}
+
+		groupIDs = append(groupIDs, aws.StringValue(group.GroupId))
 	}
-	groupIDs = append(groupIDs, sgID)
+
 	params := &ec2.ModifyNetworkInterfaceAttributeInput{
 		NetworkInterfaceId: iface.NetworkInterfaceId,
 		Groups:             aws.StringSlice(groupIDs),
 	}
 
 	_, err = conn.ModifyNetworkInterfaceAttribute(params)
+
 	if err != nil {
-		return err
+		return fmt.Errorf("error modifying EC2 Network Interface (%s): %w", interfaceID, err)
 	}
 
-	log.Printf("[DEBUG] Successful attachment of security group %s to network interface ID %s", sgID, interfaceID)
+	d.SetId(fmt.Sprintf("%s_%s", sgID, interfaceID))
 
 	return resourceAwsNetworkInterfaceSGAttachmentRead(d, meta)
 }
@@ -80,25 +90,57 @@ func resourceAwsNetworkInterfaceSGAttachmentRead(d *schema.ResourceData, meta in
 
 	conn := meta.(*AWSClient).ec2conn
 
-	iface, err := fetchNetworkInterface(conn, interfaceID)
+	var groupIdentifier *ec2.GroupIdentifier
 
-	if isAWSErr(err, "InvalidNetworkInterfaceID.NotFound", "") {
-		log.Printf("[WARN] EC2 Network Interface (%s) not found, removing from state", interfaceID)
+	err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
+		var err error
+
+		groupIdentifier, err = finder.NetworkInterfaceSecurityGroup(conn, interfaceID, sgID)
+
+		if d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidNetworkInterfaceIDNotFound) {
+			return resource.RetryableError(err)
+		}
+
+		if err != nil {
+			return resource.NonRetryableError(err)
+		}
+
+		if d.IsNewResource() && groupIdentifier == nil {
+			return resource.RetryableError(&resource.NotFoundError{
+				LastError: fmt.Errorf("EC2 Network Interface Security Group Attachment (%s) not found", d.Id()),
+			})
+		}
+
+		return nil
+	})
+
+	if tfresource.TimedOut(err) {
+		groupIdentifier, err = finder.NetworkInterfaceSecurityGroup(conn, interfaceID, sgID)
+	}
+
+	if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidNetworkInterfaceIDNotFound) {
+		log.Printf("[WARN] EC2 Network Interface Security Group Attachment (%s) not found, removing from state", d.Id())
 		d.SetId("")
 		return nil
 	}
 
 	if err != nil {
-		return err
+		return fmt.Errorf("error reading EC2 Network Interface Security Group Attachment (%s): %w", d.Id(), err)
 	}
 
-	if sgExistsInENI(sgID, iface) {
-		d.SetId(fmt.Sprintf("%s_%s", sgID, interfaceID))
-	} else {
-		// The association does not exist when it should, taint this resource.
-		log.Printf("[WARN] Security group %s not associated with network interface ID %s, tainting", sgID, interfaceID)
+	if groupIdentifier == nil {
+		if d.IsNewResource() {
+			return fmt.Errorf("error reading EC2 Network Interface Security Group Attachment (%s): not found after creation", d.Id())
+		}
+
+		log.Printf("[WARN] EC2 Network Interface Security Group Attachment (%s) not found, removing from state", d.Id())
 		d.SetId("")
+		return nil
 	}
+
+	d.Set("network_interface_id", interfaceID)
+	d.Set("security_group_id", groupIdentifier.GroupId)
+
 	return nil
 }
 
@@ -114,7 +156,7 @@ func resourceAwsNetworkInterfaceSGAttachmentDelete(d *schema.ResourceData, meta
 
 	conn := meta.(*AWSClient).ec2conn
 
-	iface, err := fetchNetworkInterface(conn, interfaceID)
+	iface, err := finder.NetworkInterfaceByID(conn, interfaceID)
 
 	if isAWSErr(err, "InvalidNetworkInterfaceID.NotFound", "") {
 		return nil
@@ -127,21 +169,6 @@ func resourceAwsNetworkInterfaceSGAttachmentDelete(d *schema.ResourceData, meta
 	return delSGFromENI(conn, sgID, iface)
 }
 
-// fetchNetworkInterface is a utility function used by Read and Delete to fetch
-// the full ENI details for a specific interface ID.
-func fetchNetworkInterface(conn *ec2.EC2, ifaceID string) (*ec2.NetworkInterface, error) {
-	log.Printf("[DEBUG] Fetching information for interface ID %s", ifaceID)
-	dniParams := &ec2.DescribeNetworkInterfacesInput{
-		NetworkInterfaceIds: aws.StringSlice([]string{ifaceID}),
-	}
-
-	dniResp, err := conn.DescribeNetworkInterfaces(dniParams)
-	if err != nil {
-		return nil, err
-	}
-	return dniResp.NetworkInterfaces[0], nil
-}
-
 func delSGFromENI(conn *ec2.EC2, sgID string, iface *ec2.NetworkInterface) error {
 	old := iface.Groups
 	var new []*string
@@ -169,14 +196,3 @@ func delSGFromENI(conn *ec2.EC2, sgID string, iface *ec2.NetworkInterface) error
 
 	return err
 }
-
-// sgExistsInENI  is a utility function that can be used to quickly check to
-// see if a security group exists in an *ec2.NetworkInterface.
-func sgExistsInENI(sgID string, iface *ec2.NetworkInterface) bool {
-	for _, v := range iface.Groups {
-		if *v.GroupId == sgID {
-			return true
-		}
-	}
-	return false
-}