service/ec2: Handle read-after-write eventual consistency issues in N…

…etwork ACL resources (#18388)

* service/ec2: Handle read-after-write eventual consistency issues in Network ACL resources

Reference: #16796
Reference: https://github.com/hashicorp/terraform-provider-aws/blob/main/docs/contributing/retries-and-waiters.md#resource-lifecycle-retries

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSNetworkAcl_basic (55.36s)
--- PASS: TestAccAWSNetworkAcl_CaseSensitivityNoChanges (49.96s)
--- PASS: TestAccAWSNetworkAcl_disappears (32.88s)
--- PASS: TestAccAWSNetworkAcl_Egress_ConfigMode (86.30s)
--- PASS: TestAccAWSNetworkAcl_EgressAndIngressRules (43.19s)
--- PASS: TestAccAWSNetworkAcl_espProtocol (44.05s)
--- PASS: TestAccAWSNetworkAcl_Ingress_ConfigMode (83.59s)
--- PASS: TestAccAWSNetworkAcl_ipv6ICMPRules (40.67s)
--- PASS: TestAccAWSNetworkAcl_ipv6Rules (64.78s)
--- PASS: TestAccAWSNetworkAcl_ipv6VpcRules (52.74s)
--- PASS: TestAccAWSNetworkAcl_OnlyEgressRules (43.49s)
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_basic (50.78s)
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_update (72.14s)
--- PASS: TestAccAWSNetworkAcl_SubnetChange (74.65s)
--- PASS: TestAccAWSNetworkAcl_Subnets (87.92s)
--- PASS: TestAccAWSNetworkAcl_SubnetsDelete (81.74s)
--- PASS: TestAccAWSNetworkAcl_tags (74.60s)

--- PASS: TestAccAWSNetworkAclRule_allProtocol (69.48s)
--- PASS: TestAccAWSNetworkAclRule_basic (54.04s)
--- PASS: TestAccAWSNetworkAclRule_disappears (30.99s)
--- PASS: TestAccAWSNetworkAclRule_disappears_IngressEgressSameNumber (41.45s)
--- PASS: TestAccAWSNetworkAclRule_disappears_NetworkAcl (40.04s)
--- PASS: TestAccAWSNetworkAclRule_ipv6 (45.12s)
--- PASS: TestAccAWSNetworkAclRule_ipv6ICMP (47.00s)
--- PASS: TestAccAWSNetworkAclRule_ipv6VpcAssignGeneratedIpv6CidrBlockUpdate (72.25s)
--- PASS: TestAccAWSNetworkAclRule_tcpProtocol (61.69s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- PASS: TestAccAWSNetworkAcl_basic (58.57s)
--- PASS: TestAccAWSNetworkAcl_CaseSensitivityNoChanges (94.47s)
--- PASS: TestAccAWSNetworkAcl_disappears (60.54s)
--- PASS: TestAccAWSNetworkAcl_Egress_ConfigMode (99.32s)
--- PASS: TestAccAWSNetworkAcl_EgressAndIngressRules (74.30s)
--- PASS: TestAccAWSNetworkAcl_espProtocol (63.01s)
--- PASS: TestAccAWSNetworkAcl_Ingress_ConfigMode (129.73s)
--- PASS: TestAccAWSNetworkAcl_ipv6ICMPRules (64.84s)
--- PASS: TestAccAWSNetworkAcl_ipv6Rules (95.18s)
--- PASS: TestAccAWSNetworkAcl_ipv6VpcRules (61.43s)
--- PASS: TestAccAWSNetworkAcl_OnlyEgressRules (86.34s)
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_basic (87.57s)
--- PASS: TestAccAWSNetworkAcl_OnlyIngressRules_update (129.92s)
--- PASS: TestAccAWSNetworkAcl_SubnetChange (144.88s)
--- PASS: TestAccAWSNetworkAcl_Subnets (144.73s)
--- PASS: TestAccAWSNetworkAcl_SubnetsDelete (120.00s)
--- PASS: TestAccAWSNetworkAcl_tags (122.44s)

--- PASS: TestAccAWSNetworkAclRule_allProtocol (72.14s)
--- PASS: TestAccAWSNetworkAclRule_basic (95.37s)
--- PASS: TestAccAWSNetworkAclRule_disappears (61.95s)
--- PASS: TestAccAWSNetworkAclRule_disappears_IngressEgressSameNumber (56.73s)
--- PASS: TestAccAWSNetworkAclRule_disappears_NetworkAcl (65.84s)
--- PASS: TestAccAWSNetworkAclRule_ipv6 (89.03s)
--- PASS: TestAccAWSNetworkAclRule_ipv6ICMP (81.90s)
--- PASS: TestAccAWSNetworkAclRule_ipv6VpcAssignGeneratedIpv6CidrBlockUpdate (123.78s)
--- PASS: TestAccAWSNetworkAclRule_missingParam (27.16s)
--- PASS: TestAccAWSNetworkAclRule_tcpProtocol (88.83s)
```

* Update CHANGELOG for #18388

.changelog/18388.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_network_acl: Handle EC2 eventual consistency errors on creation
+```
+
+```release-note:bug
+resource/aws_network_acl_rule: Handle EC2 eventual consistency errors on creation
+```

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

@@ -98,6 +98,88 @@ func InstanceByID(conn *ec2.EC2, id string) (*ec2.Instance, error) {
  return output.Reservations[0].Instances[0], nil
 }
 
+// NetworkAclByID looks up a NetworkAcl by ID. When not found, returns nil and potentially an API error.
+func NetworkAclByID(conn *ec2.EC2, id string) (*ec2.NetworkAcl, error) {
+ input := &ec2.DescribeNetworkAclsInput{
+ NetworkAclIds: aws.StringSlice([]string{id}),
+ }
+
+ output, err := conn.DescribeNetworkAcls(input)
+
+ if err != nil {
+ return nil, err
+ }
+
+ if output == nil {
+ return nil, nil
+ }
+
+ for _, networkAcl := range output.NetworkAcls {
+ if networkAcl == nil {
+ continue
+ }
+
+ if aws.StringValue(networkAcl.NetworkAclId) != id {
+ continue
+ }
+
+ return networkAcl, nil
+ }
+
+ return nil, nil
+}
+
+// NetworkAclEntry looks up a NetworkAclEntry by Network ACL ID, Egress, and Rule Number. When not found, returns nil and potentially an API error.
+func NetworkAclEntry(conn *ec2.EC2, networkAclID string, egress bool, ruleNumber int) (*ec2.NetworkAclEntry, error) {
+ input := &ec2.DescribeNetworkAclsInput{
+ Filters: []*ec2.Filter{
+ {
+ Name: aws.String("entry.egress"),
+ Values: aws.StringSlice([]string{fmt.Sprintf("%t", egress)}),
+ },
+ {
+ Name: aws.String("entry.rule-number"),
+ Values: aws.StringSlice([]string{fmt.Sprintf("%d", ruleNumber)}),
+ },
+ },
+ NetworkAclIds: aws.StringSlice([]string{networkAclID}),
+ }
+
+ output, err := conn.DescribeNetworkAcls(input)
+
+ if err != nil {
+ return nil, err
+ }
+
+ if output == nil {
+ return nil, nil
+ }
+
+ for _, networkAcl := range output.NetworkAcls {
+ if networkAcl == nil {
+ continue
+ }
+
+ if aws.StringValue(networkAcl.NetworkAclId) != networkAclID {
+ continue
+ }
+
+ for _, entry := range output.NetworkAcls[0].Entries {
+ if entry == nil {
+ continue
+ }
+
+ if aws.BoolValue(entry.Egress) != egress || aws.Int64Value(entry.RuleNumber) != int64(ruleNumber) {
+ continue
+ }
+
+ return entry, nil
+ }
+ }
+
+ return nil, nil
+}
+
 // 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/internal/service/ec2/waiter/waiter.go

@@ -252,6 +252,11 @@ func InstanceIamInstanceProfileUpdated(conn *ec2.EC2, instanceID string, expecte
  return nil, err
 }
 
+const (
+ NetworkAclPropagationTimeout = 2 * time.Minute
+ NetworkAclEntryPropagationTimeout = 5 * time.Minute
+)
+
 func SecurityGroupCreated(conn *ec2.EC2, id string, timeout time.Duration) (*ec2.SecurityGroup, error) {
  stateConf := &resource.StateChangeConf{
  Pending: []string{SecurityGroupStatusNotFound},

aws/resource_aws_network_acl.go

@@ -11,11 +11,15 @@ import (
  "github.com/aws/aws-sdk-go/aws"
  "github.com/aws/aws-sdk-go/aws/arn"
  "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"
  "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
  "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode"
  "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
+ "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 resourceAwsNetworkAcl() *schema.Resource {
@@ -196,39 +200,119 @@ func resourceAwsNetworkAclCreate(d *schema.ResourceData, meta interface{}) error
 
  log.Printf("[DEBUG] Network Acl create config: %#v", createOpts)
  resp, err := conn.CreateNetworkAcl(createOpts)
+
  if err != nil {
- return fmt.Errorf("Error creating network acl: %s", err)
+ return fmt.Errorf("error creating EC2 Network ACL: %w", err)
+ }
+
+ if resp == nil || resp.NetworkAcl == nil {
+ return fmt.Errorf("error creating EC2 Network ACL: empty response")
+ }
+
+ d.SetId(aws.StringValue(resp.NetworkAcl.NetworkAclId))
+
+ if v, ok := d.GetOk("egress"); ok && v.(*schema.Set).Len() > 0 {
+ err := updateNetworkAclEntries(d, "egress", conn)
+
+ if err != nil {
+ return fmt.Errorf("error updating EC2 Network ACL (%s) Egress Entries: %w", d.Id(), err)
+ }
+ }
+
+ if v, ok := d.GetOk("ingress"); ok && v.(*schema.Set).Len() > 0 {
+ err := updateNetworkAclEntries(d, "ingress", conn)
+
+ if err != nil {
+ return fmt.Errorf("error updating EC2 Network ACL (%s) Ingress Entries: %w", d.Id(), err)
+ }
  }
 
- // Get the ID and store it
- networkAcl := resp.NetworkAcl
- d.SetId(aws.StringValue(networkAcl.NetworkAclId))
+ if v, ok := d.GetOk("subnet_ids"); ok && v.(*schema.Set).Len() > 0 {
+ for _, subnetIDRaw := range v.(*schema.Set).List() {
+ subnetID, ok := subnetIDRaw.(string)
+
+ if !ok {
+ continue
+ }
+
+ association, err := findNetworkAclAssociation(subnetID, conn)
+
+ if err != nil {
+ return fmt.Errorf("error finding existing EC2 Network ACL association for Subnet (%s): %w", subnetID, err)
+ }
 
- // Update rules and subnet association once acl is created
- return resourceAwsNetworkAclUpdate(d, meta)
+ if association == nil {
+ return fmt.Errorf("error finding existing EC2 Network ACL association for Subnet (%s): empty response", subnetID)
+ }
+
+ input := &ec2.ReplaceNetworkAclAssociationInput{
+ AssociationId: association.NetworkAclAssociationId,
+ NetworkAclId: aws.String(d.Id()),
+ }
+
+ _, err = conn.ReplaceNetworkAclAssociation(input)
+
+ if err != nil {
+ return fmt.Errorf("error replacing existing EC2 Network ACL association for Subnet (%s): %w", subnetID, err)
+ }
+ }
+ }
+
+ return resourceAwsNetworkAclRead(d, meta)
 }
 
 func resourceAwsNetworkAclRead(d *schema.ResourceData, meta interface{}) error {
  conn := meta.(*AWSClient).ec2conn
  ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig
 
- resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{
- NetworkAclIds: []*string{aws.String(d.Id())},
+ var networkAcl *ec2.NetworkAcl
+
+ err := resource.Retry(waiter.NetworkAclPropagationTimeout, func() *resource.RetryError {
+ var err error
+
+ networkAcl, err = finder.NetworkAclByID(conn, d.Id())
+
+ if d.IsNewResource() && tfawserr.ErrCodeEquals(err, "InvalidNetworkAclID.NotFound") {
+ return resource.RetryableError(err)
+ }
+
+ if err != nil {
+ return resource.NonRetryableError(err)
+ }
+
+ if d.IsNewResource() && networkAcl == nil {
+ return resource.RetryableError(&resource.NotFoundError{
+ LastError: fmt.Errorf("EC2 Network ACL (%s) not found", d.Id()),
+ })
+ }
+
+ return nil
  })
 
+ if tfresource.TimedOut(err) {
+ networkAcl, err = finder.NetworkAclByID(conn, d.Id())
+ }
+
+ if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, "InvalidNetworkAclID.NotFound") {
+ log.Printf("[WARN] EC2 Network ACL (%s) not found, removing from state", d.Id())
+ d.SetId("")
+ return nil
+ }
+
  if err != nil {
- if isAWSErr(err, "InvalidNetworkAclID.NotFound", "") {
- log.Printf("[WARN] Network ACL (%s) not found, removing from state", d.Id())
- d.SetId("")
- return nil
- }
- return err
+ return fmt.Errorf("error reading EC2 Network ACL (%s): %w", d.Id(), err)
  }
- if resp == nil {
+
+ if networkAcl == nil {
+ if d.IsNewResource() {
+ return fmt.Errorf("error reading EC2 Network ACL (%s): not found after creation", d.Id())
+ }
+
+ log.Printf("[WARN] EC2 Network ACL (%s) not found, removing from state", d.Id())
+ d.SetId("")
  return nil
  }
 
- networkAcl := resp.NetworkAcls[0]
  var ingressEntries []*ec2.NetworkAclEntry
  var egressEntries []*ec2.NetworkAclEntry