diff --git a/.changelog/18807.txt b/.changelog/18807.txt new file mode 100644 index 000000000000..257fbffb64b3 --- /dev/null +++ b/.changelog/18807.txt @@ -0,0 +1,3 @@ +```release-note:new-resource +aws_network_acl_association +``` diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 33cd8969176b..4e41958c3995 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -1177,6 +1177,7 @@ func Provider() *schema.Provider { "aws_main_route_table_association": ec2.ResourceMainRouteTableAssociation(), "aws_nat_gateway": ec2.ResourceNATGateway(), "aws_network_acl": ec2.ResourceNetworkACL(), + "aws_network_acl_association": ec2.ResourceNetworkACLAssociation(), "aws_network_acl_rule": ec2.ResourceNetworkACLRule(), "aws_network_interface": ec2.ResourceNetworkInterface(), "aws_network_interface_attachment": ec2.ResourceNetworkInterfaceAttachment(), diff --git a/internal/service/ec2/default_network_acl.go b/internal/service/ec2/default_network_acl.go index a019ca9044c6..fa0c554240da 100644 --- a/internal/service/ec2/default_network_acl.go +++ b/internal/service/ec2/default_network_acl.go @@ -5,9 +5,7 @@ import ( "log" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/verify" @@ -23,33 +21,51 @@ const ( ) func ResourceDefaultNetworkACL() *schema.Resource { + networkACLRuleSetSchema := &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Elem: networkACLRuleResource, + Set: networkACLRuleHash, + } + return &schema.Resource{ Create: resourceDefaultNetworkACLCreate, - // We reuse aws_network_acl's read method, the operations are the same Read: resourceNetworkACLRead, - Delete: resourceDefaultNetworkACLDelete, Update: resourceDefaultNetworkACLUpdate, + Delete: resourceDefaultNetworkACLDelete, + Importer: &schema.ResourceImporter{ State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { d.Set("default_network_acl_id", d.Id()) + return []*schema.ResourceData{d}, nil }, }, + // Keep in sync with aws_network_acl's schema with the following changes: + // - egress and ingress are not Computed and don't have "Attributes as Blocks" processing mode set + // - subnet_ids is not Computed + // and additions: + // - default_network_acl_id Required/ForceNew Schema: map[string]*schema.Schema{ "arn": { Type: schema.TypeString, Computed: true, }, - "vpc_id": { - Type: schema.TypeString, - Computed: true, - }, "default_network_acl_id": { Type: schema.TypeString, Required: true, ForceNew: true, }, + // We want explicit management of Rules here, so we do not allow them to be + // computed. Instead, an empty config will enforce just that; removal of the + // rules + "egress": networkACLRuleSetSchema, + "ingress": networkACLRuleSetSchema, + "owner_id": { + Type: schema.TypeString, + Computed: true, + }, // We want explicit management of Subnets here, so we do not allow them to be // computed. Instead, an empty config will enforce just that; removal of the // any Subnets that have been assigned to the Default Network ACL. Because we @@ -61,122 +77,9 @@ func ResourceDefaultNetworkACL() *schema.Resource { Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, }, - // We want explicit management of Rules here, so we do not allow them to be - // computed. Instead, an empty config will enforce just that; removal of the - // rules - "ingress": { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "from_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "to_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "rule_no": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IntBetween(1, 32766), - }, - "action": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{ - ec2.RuleActionAllow, - ec2.RuleActionDeny, - }, true), - }, - "protocol": { - Type: schema.TypeString, - Required: true, - }, - "cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "ipv6_cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "icmp_type": { - Type: schema.TypeInt, - Optional: true, - }, - "icmp_code": { - Type: schema.TypeInt, - Optional: true, - }, - }, - }, - Set: resourceNetworkACLEntryHash, - }, - "egress": { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "from_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "to_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "rule_no": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IntBetween(1, 32766), - }, - "action": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{ - ec2.RuleActionAllow, - ec2.RuleActionDeny, - }, true), - }, - "protocol": { - Type: schema.TypeString, - Required: true, - }, - "cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "ipv6_cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "icmp_type": { - Type: schema.TypeInt, - Optional: true, - }, - "icmp_code": { - Type: schema.TypeInt, - Optional: true, - }, - }, - }, - Set: resourceNetworkACLEntryHash, - }, - "tags": tftags.TagsSchema(), "tags_all": tftags.TagsSchemaComputed(), - - "owner_id": { + "vpc_id": { Type: schema.TypeString, Computed: true, }, @@ -187,144 +90,65 @@ func ResourceDefaultNetworkACL() *schema.Resource { } func resourceDefaultNetworkACLCreate(d *schema.ResourceData, meta interface{}) error { - d.SetId(d.Get("default_network_acl_id").(string)) - - // revoke all default and pre-existing rules on the default network acl. - // In the UPDATE method, we'll apply only the rules in the configuration. - log.Printf("[DEBUG] Revoking default ingress and egress rules for Default Network ACL for %s", d.Id()) - err := revokeAllNetworkACLEntries(d.Id(), meta) - if err != nil { - return err - } - - return resourceDefaultNetworkACLUpdate(d, meta) -} - -func resourceDefaultNetworkACLUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn - if d.HasChange("ingress") { - err := updateNetworkAclEntries(d, "ingress", conn) - if err != nil { - return err - } - } + naclID := d.Get("default_network_acl_id").(string) + nacl, err := FindNetworkACLByID(conn, naclID) - if d.HasChange("egress") { - err := updateNetworkAclEntries(d, "egress", conn) - if err != nil { - return err - } + if err != nil { + return fmt.Errorf("error reading EC2 Network ACL (%s): %w", d.Id(), err) } - if d.HasChange("subnet_ids") { - o, n := d.GetChange("subnet_ids") - if o == nil { - o = new(schema.Set) - } - if n == nil { - n = new(schema.Set) - } - - os := o.(*schema.Set) - ns := n.(*schema.Set) + if !aws.BoolValue(nacl.IsDefault) { + return fmt.Errorf("use the `aws_network_acl` resource instead") + } - remove := os.Difference(ns).List() - add := ns.Difference(os).List() + d.SetId(naclID) - if len(remove) > 0 { - // - // NO-OP - // - // Subnets *must* belong to a Network ACL. Subnets are not "removed" from - // Network ACLs, instead their association is replaced. In a normal - // Network ACL, any removal of a Subnet is done by replacing the - // Subnet/ACL association with an association between the Subnet and the - // Default Network ACL. Because we're managing the default here, we cannot - // do that, so we simply log a NO-OP. In order to remove the Subnet here, - // it must be destroyed, or assigned to different Network ACL. Those - // operations are not handled here - log.Printf("[WARN] Cannot remove subnets from the Default Network ACL. They must be re-assigned or destroyed") - } + // Revoke all default and pre-existing rules on the default network ACL. + if err := deleteNetworkAclEntries(conn, d.Id(), nacl.Entries); err != nil { + return err + } - if len(add) > 0 { - for _, a := range add { - association, err := findNetworkAclAssociation(a.(string), conn) - if err != nil { - return fmt.Errorf("Failed to find acl association: acl %s with subnet %s: %s", d.Id(), a, err) - } - log.Printf("[DEBUG] Updating Network Association for Default Network ACL (%s) and Subnet (%s)", d.Id(), a.(string)) - _, err = conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{ - AssociationId: association.NetworkAclAssociationId, - NetworkAclId: aws.String(d.Id()), - }) - if err != nil { - return err - } - } - } + if err := modifyNetworkACLAttributesOnCreate(conn, d); err != nil { + return err } - if d.HasChange("tags_all") { - o, n := d.GetChange("tags_all") + // Configure tags. + defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig + ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig + newTags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))).IgnoreConfig(ignoreTagsConfig) + oldTags := KeyValueTags(nacl.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) - if err := UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating EC2 Default Network ACL (%s) tags: %s", d.Id(), err) + if !oldTags.Equal(newTags) { + if err := UpdateTags(conn, d.Id(), oldTags, newTags); err != nil { + return fmt.Errorf("error updating EC2 Default Network ACL (%s) tags: %w", d.Id(), err) } } - // Re-use the exiting Network ACL Resources READ method return resourceNetworkACLRead(d, meta) } -func resourceDefaultNetworkACLDelete(d *schema.ResourceData, meta interface{}) error { - log.Printf("[WARN] Cannot destroy Default Network ACL. Terraform will remove this resource from the state file, however resources may remain.") - return nil -} - -// revokeAllNetworkACLEntries revoke all ingress and egress rules that the Default -// Network ACL currently has -func revokeAllNetworkACLEntries(netaclId string, meta interface{}) error { +func resourceDefaultNetworkACLUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn - resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(netaclId)}, - }) - - if err != nil { - log.Printf("[DEBUG] Error looking up Network ACL: %s", err) + // Subnets *must* belong to a Network ACL. Subnets are not "removed" from + // Network ACLs, instead their association is replaced. In a normal + // Network ACL, any removal of a Subnet is done by replacing the + // Subnet/ACL association with an association between the Subnet and the + // Default Network ACL. Because we're managing the default here, we cannot + // do that, so we simply log a NO-OP. In order to remove the Subnet here, + // it must be destroyed, or assigned to different Network ACL. Those + // operations are not handled here. + if err := modifyNetworkACLAttributesOnUpdate(conn, d, false); err != nil { return err } - if resp == nil { - return fmt.Errorf("Error looking up Default Network ACL Entries: No results") - } - - networkAcl := resp.NetworkAcls[0] - for _, e := range networkAcl.Entries { - // Skip the default rules added by AWS. They can be neither - // configured or deleted by users. See http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_ACLs.html#default-network-acl - if aws.Int64Value(e.RuleNumber) == defaultACLRuleNumberIPv4 || - aws.Int64Value(e.RuleNumber) == defaultACLRuleNumberIPv6 { - continue - } - - // track if this is an egress or ingress rule, for logging purposes - rt := "ingress" - if aws.BoolValue(e.Egress) { - rt = "egress" - } + return resourceNetworkACLRead(d, meta) +} - log.Printf("[DEBUG] Destroying Network ACL (%s) Entry number (%d)", rt, int(*e.RuleNumber)) - _, err := conn.DeleteNetworkAclEntry(&ec2.DeleteNetworkAclEntryInput{ - NetworkAclId: aws.String(netaclId), - RuleNumber: e.RuleNumber, - Egress: e.Egress, - }) - if err != nil { - return fmt.Errorf("Error deleting entry (%s): %s", e, err) - } - } +func resourceDefaultNetworkACLDelete(d *schema.ResourceData, meta interface{}) error { + log.Printf("[WARN] EC2 Default Network ACL (%s) not deleted, removing from state", d.Id()) return nil } diff --git a/internal/service/ec2/default_network_acl_test.go b/internal/service/ec2/default_network_acl_test.go index 4c0b3bbda825..fbff0c869cd4 100644 --- a/internal/service/ec2/default_network_acl_test.go +++ b/internal/service/ec2/default_network_acl_test.go @@ -7,30 +7,19 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfec2 "github.com/hashicorp/terraform-provider-aws/internal/service/ec2" ) -var defaultEgressAcl = &ec2.NetworkAclEntry{ - CidrBlock: aws.String("0.0.0.0/0"), - Egress: aws.Bool(true), - Protocol: aws.String("-1"), - RuleAction: aws.String("allow"), - RuleNumber: aws.Int64(100), -} -var ipv6IngressAcl = &ec2.NetworkAclEntry{ - Ipv6CidrBlock: aws.String("::/0"), - Egress: aws.Bool(false), - Protocol: aws.String("-1"), - RuleAction: aws.String("allow"), - RuleNumber: aws.Int64(101), -} - func TestAccEC2DefaultNetworkACL_basic(t *testing.T) { - var networkAcl ec2.NetworkAcl - resourceName := "aws_default_network_acl.default" + var v ec2.NetworkAcl + resourceName := "aws_default_network_acl.test" + vpcResourceName := "aws_vpc.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -39,12 +28,16 @@ func TestAccEC2DefaultNetworkACL_basic(t *testing.T) { CheckDestroy: testAccCheckDefaultNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccDefaultNetworkConfig_basic, + Config: testAccDefaultNetworkACLConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), + testAccCheckDefaultNetworkACLExists(resourceName, &v), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`network-acl/acl-.+`)), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2), + resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "0"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttrPair(resourceName, "vpc_id", vpcResourceName, "id"), ), }, { @@ -57,8 +50,37 @@ func TestAccEC2DefaultNetworkACL_basic(t *testing.T) { } func TestAccEC2DefaultNetworkACL_basicIPv6VPC(t *testing.T) { - var networkAcl ec2.NetworkAcl - resourceName := "aws_default_network_acl.default" + var v ec2.NetworkAcl + resourceName := "aws_default_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDefaultNetworkACLDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDefaultNetworkACLIPv6Config(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccEC2DefaultNetworkACL_tags(t *testing.T) { + var v ec2.NetworkAcl + resourceName := "aws_default_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -67,10 +89,11 @@ func TestAccEC2DefaultNetworkACL_basicIPv6VPC(t *testing.T) { CheckDestroy: testAccCheckDefaultNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccDefaultNetworkConfig_basicIPv6VPC, + Config: testAccDefaultNetworkACLTags1Config(rName, "key1", "value1"), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 4), + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), ), }, { @@ -78,16 +101,31 @@ func TestAccEC2DefaultNetworkACL_basicIPv6VPC(t *testing.T) { ImportState: true, ImportStateVerify: true, }, + { + Config: testAccDefaultNetworkACLTags2Config(rName, "key1", "value1updated", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccDefaultNetworkACLTags1Config(rName, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, }, }) } func TestAccEC2DefaultNetworkACL_Deny_ingress(t *testing.T) { - // TestAccEC2DefaultNetworkACL_Deny_ingress will deny all Ingress rules, but - // not Egress. We then expect there to be 3 rules, 2 AWS defaults and 1 - // additional Egress. - var networkAcl ec2.NetworkAcl - resourceName := "aws_default_network_acl.default" + var v ec2.NetworkAcl + resourceName := "aws_default_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -96,10 +134,19 @@ func TestAccEC2DefaultNetworkACL_Deny_ingress(t *testing.T) { CheckDestroy: testAccCheckDefaultNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccDefaultNetworkConfig_deny_ingress, + Config: testAccDefaultNetworkACLDenyIngressConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{defaultEgressAcl}, 0, 2), + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "egress.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "egress.*", map[string]string{ + "protocol": "-1", + "rule_no": "100", + "from_port": "0", + "to_port": "0", + "action": "allow", + "cidr_block": "0.0.0.0/0", + }), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), ), }, { @@ -112,8 +159,9 @@ func TestAccEC2DefaultNetworkACL_Deny_ingress(t *testing.T) { } func TestAccEC2DefaultNetworkACL_withIPv6Ingress(t *testing.T) { - var networkAcl ec2.NetworkAcl - resourceName := "aws_default_network_acl.default" + var v ec2.NetworkAcl + resourceName := "aws_default_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -122,10 +170,19 @@ func TestAccEC2DefaultNetworkACL_withIPv6Ingress(t *testing.T) { CheckDestroy: testAccCheckDefaultNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccDefaultNetworkConfig_includingIPv6Rule, + Config: testAccDefaultNetworkACLIncludingIPv6RuleConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{ipv6IngressAcl}, 0, 2), + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ + "protocol": "-1", + "rule_no": "101", + "from_port": "0", + "to_port": "0", + "action": "allow", + "ipv6_cidr_block": "::/0", + }), ), }, { @@ -138,8 +195,9 @@ func TestAccEC2DefaultNetworkACL_withIPv6Ingress(t *testing.T) { } func TestAccEC2DefaultNetworkACL_subnetRemoval(t *testing.T) { - var networkAcl ec2.NetworkAcl - resourceName := "aws_default_network_acl.default" + var v ec2.NetworkAcl + resourceName := "aws_default_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -148,10 +206,12 @@ func TestAccEC2DefaultNetworkACL_subnetRemoval(t *testing.T) { CheckDestroy: testAccCheckDefaultNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccDefaultNetworkConfig_Subnets, + Config: testAccDefaultNetworkACLSubnetsConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2), + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "2"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test1", "id"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test2", "id"), ), }, { @@ -159,15 +219,13 @@ func TestAccEC2DefaultNetworkACL_subnetRemoval(t *testing.T) { ImportState: true, ImportStateVerify: true, }, - // Here the Subnets have been removed from the Default Network ACL Config, // but have not been reassigned. The result is that the Subnets are still // there, and we have a non-empty plan { - Config: testAccDefaultNetworkConfig_Subnets_remove, + Config: testAccDefaultNetworkACLSubnetsRemoveConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2), + testAccCheckDefaultNetworkACLExists(resourceName, &v), ), ExpectNonEmptyPlan: true, }, @@ -181,8 +239,9 @@ func TestAccEC2DefaultNetworkACL_subnetRemoval(t *testing.T) { } func TestAccEC2DefaultNetworkACL_subnetReassign(t *testing.T) { - var networkAcl ec2.NetworkAcl - resourceName := "aws_default_network_acl.default" + var v ec2.NetworkAcl + resourceName := "aws_default_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -191,10 +250,12 @@ func TestAccEC2DefaultNetworkACL_subnetReassign(t *testing.T) { CheckDestroy: testAccCheckDefaultNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccDefaultNetworkConfig_Subnets, + Config: testAccDefaultNetworkACLSubnetsConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 2, 2), + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "2"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test1", "id"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test2", "id"), ), }, { @@ -202,7 +263,6 @@ func TestAccEC2DefaultNetworkACL_subnetReassign(t *testing.T) { ImportState: true, ImportStateVerify: true, }, - // Here we've reassigned the subnets to a different ACL. // Without any otherwise association between the `aws_network_acl` and // `aws_default_network_acl` resources, we cannot guarantee that the @@ -216,10 +276,10 @@ func TestAccEC2DefaultNetworkACL_subnetReassign(t *testing.T) { // update occurs first, and the former's READ will correctly read zero // subnets { - Config: testAccDefaultNetworkConfig_Subnets_move, + Config: testAccDefaultNetworkACLSubnetsMoveConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccGetDefaultNetworkACL(resourceName, &networkAcl), - testAccCheckDefaultACLAttributes(&networkAcl, []*ec2.NetworkAclEntry{}, 0, 2), + testAccCheckDefaultNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "0"), ), }, { @@ -232,32 +292,11 @@ func TestAccEC2DefaultNetworkACL_subnetReassign(t *testing.T) { } func testAccCheckDefaultNetworkACLDestroy(s *terraform.State) error { - // We can't destroy this resource; it comes and goes with the VPC itself. + // The default NACL is not deleted. return nil } -func testAccCheckDefaultACLAttributes(acl *ec2.NetworkAcl, rules []*ec2.NetworkAclEntry, subnetCount int, hiddenRuleCount int) resource.TestCheckFunc { - return func(s *terraform.State) error { - - aclEntriesCount := len(acl.Entries) - ruleCount := len(rules) - - // Default ACL has hidden rules we can't do anything about - ruleCount = ruleCount + hiddenRuleCount - - if ruleCount != aclEntriesCount { - return fmt.Errorf("Expected (%d) Rules, got (%d)", ruleCount, aclEntriesCount) - } - - if len(acl.Associations) != subnetCount { - return fmt.Errorf("Expected (%d) Subnets, got (%d)", subnetCount, len(acl.Associations)) - } - - return nil - } -} - -func testAccGetDefaultNetworkACL(n string, networkAcl *ec2.NetworkAcl) resource.TestCheckFunc { +func testAccCheckDefaultNetworkACLExists(n string, v *ec2.NetworkAcl) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -265,252 +304,233 @@ func testAccGetDefaultNetworkACL(n string, networkAcl *ec2.NetworkAcl) resource. } if rs.Primary.ID == "" { - return fmt.Errorf("No Network ACL is set") + return fmt.Errorf("No EC2 Default Network ACL ID is set: %s", n) } + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(rs.Primary.ID)}, - }) + output, err := tfec2.FindNetworkACLByID(conn, rs.Primary.ID) + if err != nil { return err } - if len(resp.NetworkAcls) > 0 && - aws.StringValue(resp.NetworkAcls[0].NetworkAclId) == rs.Primary.ID { - *networkAcl = *resp.NetworkAcls[0] - return nil + if !aws.BoolValue(output.IsDefault) { + return fmt.Errorf("EC2 Network ACL %s is not a default NACL", rs.Primary.ID) } - return fmt.Errorf("Network Acls not found") + *v = *output + + return nil } } -const testAccDefaultNetworkConfig_basic = ` -resource "aws_vpc" "tftestvpc" { +func testAccDefaultNetworkACLConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-default-network-acl-basic" + Name = %[1]q } } -resource "aws_default_network_acl" "default" { - default_network_acl_id = aws_vpc.tftestvpc.default_network_acl_id - - tags = { - Name = "tf-acc-default-acl-basic" - } +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id +} +`, rName) } -` -const testAccDefaultNetworkConfig_includingIPv6Rule = ` -resource "aws_vpc" "tftestvpc" { +func testAccDefaultNetworkACLTags1Config(rName, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-default-network-acl-including-ipv6-rule" + Name = %[1]q } } -resource "aws_default_network_acl" "default" { - default_network_acl_id = aws_vpc.tftestvpc.default_network_acl_id - - ingress { - protocol = -1 - rule_no = 101 - action = "allow" - ipv6_cidr_block = "::/0" - from_port = 0 - to_port = 0 - } +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id tags = { - Name = "tf-acc-default-acl-basic-including-ipv6-rule" + %[2]q = %[3]q } } -` +`, rName, tagKey1, tagValue1) +} -const testAccDefaultNetworkConfig_deny_ingress = ` -resource "aws_vpc" "tftestvpc" { +func testAccDefaultNetworkACLTags2Config(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-default-network-acl-deny-ingress" + Name = %[1]q } } -resource "aws_default_network_acl" "default" { - default_network_acl_id = aws_vpc.tftestvpc.default_network_acl_id - - egress { - protocol = -1 - rule_no = 100 - action = "allow" - cidr_block = "0.0.0.0/0" - from_port = 0 - to_port = 0 - } +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id tags = { - Name = "tf-acc-default-acl-deny-ingress" + %[2]q = %[3]q + %[4]q = %[5]q } } -` +`, rName, tagKey1, tagValue1, tagKey2, tagValue2) +} -const testAccDefaultNetworkConfig_Subnets = ` -resource "aws_vpc" "foo" { +func testAccDefaultNetworkACLIncludingIPv6RuleConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-default-network-acl-subnets" + Name = %[1]q } } -resource "aws_subnet" "one" { - cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id - tags = { - Name = "tf-acc-default-network-acl-one" + ingress { + protocol = -1 + rule_no = 101 + action = "allow" + ipv6_cidr_block = "::/0" + from_port = 0 + to_port = 0 } } - -resource "aws_subnet" "two" { - cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id - - tags = { - Name = "tf-acc-default-network-acl-two" - } +`, rName) } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +func testAccDefaultNetworkACLDenyIngressConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" tags = { - Name = "tf-acc-default-acl-subnets" + Name = %[1]q } } -resource "aws_default_network_acl" "default" { - default_network_acl_id = aws_vpc.foo.default_network_acl_id - - subnet_ids = [aws_subnet.one.id, aws_subnet.two.id] +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id - tags = { - Name = "tf-acc-default-acl-subnets" + egress { + protocol = -1 + rule_no = 100 + action = "allow" + cidr_block = "0.0.0.0/0" + from_port = 0 + to_port = 0 } } -` +`, rName) +} -const testAccDefaultNetworkConfig_Subnets_remove = ` -resource "aws_vpc" "foo" { +func testAccDefaultNetworkACLSubnetsBaseConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-default-network-acl-subnets-remove" + Name = %[1]q } } -resource "aws_subnet" "one" { +resource "aws_subnet" "test1" { cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-default-network-acl-subnets-remove-one" + Name = %[1]q } } -resource "aws_subnet" "two" { +resource "aws_subnet" "test2" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-default-network-acl-subnets-remove-two" + Name = %[1]q } } - -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id - - tags = { - Name = "tf-acc-default-acl-subnets-remove" - } +`, rName) } -resource "aws_default_network_acl" "default" { - default_network_acl_id = aws_vpc.foo.default_network_acl_id +func testAccDefaultNetworkACLSubnetsConfig(rName string) string { + return acctest.ConfigCompose(testAccDefaultNetworkACLSubnetsBaseConfig(rName), fmt.Sprintf(` +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-default-acl-subnets-remove" + Name = %[1]q } } -` -const testAccDefaultNetworkConfig_Subnets_move = ` -resource "aws_vpc" "foo" { - cidr_block = "10.1.0.0/16" +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id - tags = { - Name = "terraform-testacc-default-network-acl-subnets-move" - } + subnet_ids = [aws_subnet.test1.id, aws_subnet.test2.id] +} +`, rName)) } -resource "aws_subnet" "one" { - cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id +func testAccDefaultNetworkACLSubnetsRemoveConfig(rName string) string { + return acctest.ConfigCompose(testAccDefaultNetworkACLSubnetsBaseConfig(rName), fmt.Sprintf(` +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-default-network-acl-subnets-move-one" + Name = %[1]q } } -resource "aws_subnet" "two" { - cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id - tags = { - Name = "tf-acc-default-network-acl-subnets-move-two" - } + depends_on = [aws_network_acl.test] +} +`, rName)) } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +func testAccDefaultNetworkACLSubnetsMoveConfig(rName string) string { + return acctest.ConfigCompose(testAccDefaultNetworkACLSubnetsBaseConfig(rName), fmt.Sprintf(` +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id - subnet_ids = [aws_subnet.one.id, aws_subnet.two.id] + subnet_ids = [aws_subnet.test1.id, aws_subnet.test2.id] tags = { - Name = "tf-acc-default-acl-subnets-move" + Name = %[1]q } } -resource "aws_default_network_acl" "default" { - default_network_acl_id = aws_vpc.foo.default_network_acl_id - - depends_on = [aws_network_acl.bar] +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id - tags = { - Name = "tf-acc-default-acl-subnets-move" - } + depends_on = [aws_network_acl.test] +} +`, rName)) } -` -const testAccDefaultNetworkConfig_basicIPv6VPC = ` -resource "aws_vpc" "tftestvpc" { +func testAccDefaultNetworkACLIPv6Config(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" assign_generated_ipv6_cidr_block = true tags = { - Name = "terraform-testacc-default-network-acl-basic-ipv6-vpc" + Name = %[1]q } } -resource "aws_default_network_acl" "default" { - default_network_acl_id = aws_vpc.tftestvpc.default_network_acl_id - - tags = { - Name = "tf-acc-default-acl-subnets-basic-ipv6-vpc" - } +resource "aws_default_network_acl" "test" { + default_network_acl_id = aws_vpc.test.default_network_acl_id +} +`, rName) } -` diff --git a/internal/service/ec2/errors.go b/internal/service/ec2/errors.go index a3459abd609d..5c3d862d1cdb 100644 --- a/internal/service/ec2/errors.go +++ b/internal/service/ec2/errors.go @@ -34,6 +34,7 @@ const ( ErrCodeInvalidInstanceIDNotFound = "InvalidInstanceID.NotFound" ErrCodeInvalidInternetGatewayIDNotFound = "InvalidInternetGatewayID.NotFound" ErrCodeInvalidKeyPairNotFound = "InvalidKeyPair.NotFound" + ErrCodeInvalidNetworkAclEntryNotFound = "InvalidNetworkAclEntry.NotFound" ErrCodeInvalidNetworkAclIDNotFound = "InvalidNetworkAclID.NotFound" ErrCodeInvalidNetworkInterfaceIDNotFound = "InvalidNetworkInterfaceID.NotFound" ErrCodeInvalidParameter = "InvalidParameter" diff --git a/internal/service/ec2/find.go b/internal/service/ec2/find.go index 7c5ecde82f07..52002fb3eea1 100644 --- a/internal/service/ec2/find.go +++ b/internal/service/ec2/find.go @@ -2,6 +2,7 @@ package ec2 import ( "fmt" + "strconv" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -729,88 +730,93 @@ func FindNetworkACLs(conn *ec2.EC2, input *ec2.DescribeNetworkAclsInput) ([]*ec2 return output, nil } -// FindNetworkACLByID looks up a NetworkAcl by ID. When not found, returns nil and potentially an API error. func FindNetworkACLByID(conn *ec2.EC2, id string) (*ec2.NetworkAcl, error) { input := &ec2.DescribeNetworkAclsInput{ NetworkAclIds: aws.StringSlice([]string{id}), } - output, err := conn.DescribeNetworkAcls(input) + output, err := FindNetworkACL(conn, input) if err != nil { return nil, err } - if output == nil { - return nil, nil + // Eventual consistency check. + if aws.StringValue(output.NetworkAclId) != id { + return nil, &resource.NotFoundError{ + LastRequest: input, + } } - for _, networkAcl := range output.NetworkAcls { - if networkAcl == nil { - continue - } + return output, nil +} - if aws.StringValue(networkAcl.NetworkAclId) != id { - continue - } +func FindNetworkACLAssociationByID(conn *ec2.EC2, associationID string) (*ec2.NetworkAclAssociation, error) { + input := &ec2.DescribeNetworkAclsInput{ + Filters: BuildAttributeFilterList(map[string]string{ + "association.association-id": associationID, + }), + } + + output, err := FindNetworkACL(conn, input) - return networkAcl, nil + if err != nil { + return nil, err } - return nil, nil + for _, v := range output.Associations { + if aws.StringValue(v.NetworkAclAssociationId) == associationID { + return v, nil + } + } - // TODO: Layer on top of FindNetworkACL and modify callers to handle NotFoundError. + return nil, &resource.NotFoundError{} } -// FindNetworkACLEntry looks up a FindNetworkACLEntry by Network ACL ID, Egress, and Rule Number. When not found, returns nil and potentially an API error. -func FindNetworkACLEntry(conn *ec2.EC2, networkAclID string, egress bool, ruleNumber int) (*ec2.NetworkAclEntry, error) { +func FindNetworkACLAssociationBySubnetID(conn *ec2.EC2, subnetID string) (*ec2.NetworkAclAssociation, 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}), + Filters: BuildAttributeFilterList(map[string]string{ + "association.subnet-id": subnetID, + }), } - output, err := conn.DescribeNetworkAcls(input) + output, err := FindNetworkACL(conn, input) if err != nil { return nil, err } - if output == nil { - return nil, nil + for _, v := range output.Associations { + if aws.StringValue(v.SubnetId) == subnetID { + return v, nil + } } - for _, networkAcl := range output.NetworkAcls { - if networkAcl == nil { - continue - } + return nil, &resource.NotFoundError{} +} - if aws.StringValue(networkAcl.NetworkAclId) != networkAclID { - continue - } +func FindNetworkACLEntryByThreePartKey(conn *ec2.EC2, naclID string, egress bool, ruleNumber int) (*ec2.NetworkAclEntry, error) { + input := &ec2.DescribeNetworkAclsInput{ + Filters: BuildAttributeFilterList(map[string]string{ + "entry.egress": strconv.FormatBool(egress), + "entry.rule-number": strconv.Itoa(ruleNumber), + }), + NetworkAclIds: aws.StringSlice([]string{naclID}), + } - for _, entry := range output.NetworkAcls[0].Entries { - if entry == nil { - continue - } + output, err := FindNetworkACL(conn, input) - if aws.BoolValue(entry.Egress) != egress || aws.Int64Value(entry.RuleNumber) != int64(ruleNumber) { - continue - } + if err != nil { + return nil, err + } - return entry, nil + for _, v := range output.Entries { + if aws.BoolValue(v.Egress) == egress && aws.Int64Value(v.RuleNumber) == int64(ruleNumber) { + return v, nil } } - return nil, nil + return nil, &resource.NotFoundError{} } func FindNetworkInterface(conn *ec2.EC2, input *ec2.DescribeNetworkInterfacesInput) (*ec2.NetworkInterface, error) { diff --git a/internal/service/ec2/network_acl.go b/internal/service/ec2/network_acl.go index 6a65d6c7589a..2a1d253d5286 100644 --- a/internal/service/ec2/network_acl.go +++ b/internal/service/ec2/network_acl.go @@ -6,43 +6,66 @@ import ( "log" "strconv" "strings" - "time" "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/v2/awsv1shim/v2/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/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" - "github.com/hashicorp/terraform-provider-aws/internal/flex" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) func ResourceNetworkACL() *schema.Resource { + networkACLRuleSetSchema := &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Computed: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: networkACLRuleResource, + Set: networkACLRuleHash, + } return &schema.Resource{ Create: resourceNetworkACLCreate, Read: resourceNetworkACLRead, - Delete: resourceNetworkACLDelete, Update: resourceNetworkACLUpdate, + Delete: resourceNetworkACLDelete, + Importer: &schema.ResourceImporter{ - State: schema.ImportStatePassthrough, + State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + conn := meta.(*conns.AWSClient).EC2Conn + + nacl, err := FindNetworkACLByID(conn, d.Id()) + + if err != nil { + return nil, err + } + + if aws.BoolValue(nacl.IsDefault) { + return nil, fmt.Errorf("use the `aws_default_network_acl` resource instead") + } + + return []*schema.ResourceData{d}, nil + }, }, + // Keep in sync with aws_default_network_acl's schema. + // See notes in default_network_acl.go. Schema: map[string]*schema.Schema{ "arn": { Type: schema.TypeString, Computed: true, }, - "vpc_id": { + "egress": networkACLRuleSetSchema, + "ingress": networkACLRuleSetSchema, + "owner_id": { Type: schema.TypeString, - Required: true, - ForceNew: true, + Computed: true, }, "subnet_ids": { Type: schema.TypeSet, @@ -50,129 +73,12 @@ func ResourceNetworkACL() *schema.Resource { Computed: true, Elem: &schema.Schema{Type: schema.TypeString}, }, - "ingress": { - Type: schema.TypeSet, - Optional: true, - Computed: true, - ConfigMode: schema.SchemaConfigModeAttr, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "from_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "to_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "rule_no": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IntBetween(1, 32766), - }, - "action": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - return strings.EqualFold(old, new) - }, - ValidateFunc: validation.StringInSlice([]string{ - ec2.RuleActionAllow, - ec2.RuleActionDeny, - }, true), - }, - "protocol": { - Type: schema.TypeString, - Required: true, - }, - "cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "ipv6_cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "icmp_type": { - Type: schema.TypeInt, - Optional: true, - }, - "icmp_code": { - Type: schema.TypeInt, - Optional: true, - }, - }, - }, - Set: resourceNetworkACLEntryHash, - }, - "egress": { - Type: schema.TypeSet, - Optional: true, - Computed: true, - ConfigMode: schema.SchemaConfigModeAttr, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "from_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "to_port": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IsPortNumberOrZero, - }, - "rule_no": { - Type: schema.TypeInt, - Required: true, - ValidateFunc: validation.IntBetween(1, 32766), - }, - "action": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - return strings.EqualFold(old, new) - }, - ValidateFunc: validation.StringInSlice([]string{ - ec2.RuleActionAllow, - ec2.RuleActionDeny, - }, true), - }, - "protocol": { - Type: schema.TypeString, - Required: true, - }, - "cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "ipv6_cidr_block": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsCIDR, - }, - "icmp_type": { - Type: schema.TypeInt, - Optional: true, - }, - "icmp_code": { - Type: schema.TypeInt, - Optional: true, - }, - }, - }, - Set: resourceNetworkACLEntryHash, - }, "tags": tftags.TagsSchema(), "tags_all": tftags.TagsSchemaComputed(), - "owner_id": { + "vpc_id": { Type: schema.TypeString, - Computed: true, + Required: true, + ForceNew: true, }, }, @@ -180,76 +86,88 @@ func ResourceNetworkACL() *schema.Resource { } } -func resourceNetworkACLCreate(d *schema.ResourceData, meta interface{}) error { +// NACL rule Resource definition. +// Used in aws_network_acl and aws_default_network_acl ingress and egress rule sets. +var networkACLRuleResource = &schema.Resource{ + Schema: map[string]*schema.Schema{ + "action": { + Type: schema.TypeString, + Required: true, + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + return strings.EqualFold(old, new) + }, + ValidateFunc: validation.StringInSlice(ec2.RuleAction_Values(), true), + }, + "cidr_block": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: verify.ValidIPv4CIDRNetworkAddress, + }, + "from_port": { + Type: schema.TypeInt, + Required: true, + ValidateFunc: validation.IsPortNumberOrZero, + }, + "icmp_code": { + Type: schema.TypeInt, + Optional: true, + }, + "icmp_type": { + Type: schema.TypeInt, + Optional: true, + }, + "ipv6_cidr_block": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: verify.ValidIPv6CIDRNetworkAddress, + }, + "protocol": { + Type: schema.TypeString, + Required: true, + ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { + _, err := networkACLProtocolNumber(v.(string)) + + if err != nil { + errors = append(errors, fmt.Errorf("%q : %w", k, err)) + } + return + }, + }, + "rule_no": { + Type: schema.TypeInt, + Required: true, + ValidateFunc: validation.IntBetween(1, 32766), + }, + "to_port": { + Type: schema.TypeInt, + Required: true, + ValidateFunc: validation.IsPortNumberOrZero, + }, + }, +} + +func resourceNetworkACLCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) - // Create the Network Acl - createOpts := &ec2.CreateNetworkAclInput{ - VpcId: aws.String(d.Get("vpc_id").(string)), + input := &ec2.CreateNetworkAclInput{ TagSpecifications: ec2TagSpecificationsFromKeyValueTags(tags, ec2.ResourceTypeNetworkAcl), + VpcId: aws.String(d.Get("vpc_id").(string)), } - log.Printf("[DEBUG] Network Acl create config: %#v", createOpts) - resp, err := conn.CreateNetworkAcl(createOpts) + log.Printf("[DEBUG] Creating EC2 Network ACL: %s", input) + output, err := conn.CreateNetworkAcl(input) if err != nil { 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) - } - } - - 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) - } - - 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()), - } + d.SetId(aws.StringValue(output.NetworkAcl.NetworkAclId)) - _, err = conn.ReplaceNetworkAclAssociation(input) - - if err != nil { - return fmt.Errorf("error replacing existing EC2 Network ACL association for Subnet (%s): %w", subnetID, err) - } - } + if err := modifyNetworkACLAttributesOnCreate(conn, d); err != nil { + return err } return resourceNetworkACLRead(d, meta) @@ -260,36 +178,12 @@ func resourceNetworkACLRead(d *schema.ResourceData, meta interface{}) error { defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig - var networkAcl *ec2.NetworkAcl - - err := resource.Retry(NetworkACLPropagationTimeout, func() *resource.RetryError { - var err error - - networkAcl, err = FindNetworkACLByID(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()), - }) - } + outputRaw, err := tfresource.RetryWhenNewResourceNotFound(PropagationTimeout, func() (interface{}, error) { + return FindNetworkACLByID(conn, d.Id()) + }, d.IsNewResource()) - return nil - }) - - if tfresource.TimedOut(err) { - networkAcl, err = FindNetworkACLByID(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()) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] EC2 Network ACL %s not found, removing from state", d.Id()) d.SetId("") return nil } @@ -298,38 +192,50 @@ func resourceNetworkACLRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error reading EC2 Network ACL (%s): %w", d.Id(), err) } - if networkAcl == nil { - if d.IsNewResource() { - return fmt.Errorf("error reading EC2 Network ACL (%s): not found after creation", d.Id()) - } + nacl := outputRaw.(*ec2.NetworkAcl) - log.Printf("[WARN] EC2 Network ACL (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil + ownerID := aws.StringValue(nacl.OwnerId) + arn := arn.ARN{ + Partition: meta.(*conns.AWSClient).Partition, + Service: ec2.ServiceName, + Region: meta.(*conns.AWSClient).Region, + AccountID: ownerID, + Resource: fmt.Sprintf("network-acl/%s", d.Id()), + }.String() + d.Set("arn", arn) + d.Set("owner_id", ownerID) + + var subnetIDs []string + for _, v := range nacl.Associations { + subnetIDs = append(subnetIDs, aws.StringValue(v.SubnetId)) } + d.Set("subnet_ids", subnetIDs) - var ingressEntries []*ec2.NetworkAclEntry - var egressEntries []*ec2.NetworkAclEntry + d.Set("vpc_id", nacl.VpcId) - // separate the ingress and egress rules - for _, e := range networkAcl.Entries { + var egressEntries []*ec2.NetworkAclEntry + var ingressEntries []*ec2.NetworkAclEntry + for _, v := range nacl.Entries { // Skip the default rules added by AWS. They can be neither // configured or deleted by users. - if aws.Int64Value(e.RuleNumber) == defaultACLRuleNumberIPv4 || - aws.Int64Value(e.RuleNumber) == defaultACLRuleNumberIPv6 { + if v := aws.Int64Value(v.RuleNumber); v == defaultACLRuleNumberIPv4 || v == defaultACLRuleNumberIPv6 { continue } - if aws.BoolValue(e.Egress) { - egressEntries = append(egressEntries, e) + if aws.BoolValue(v.Egress) { + egressEntries = append(egressEntries, v) } else { - ingressEntries = append(ingressEntries, e) + ingressEntries = append(ingressEntries, v) } } + if err := d.Set("egress", flattenNetworkAclEntries(egressEntries)); err != nil { + return fmt.Errorf("error setting egress: %w", err) + } + if err := d.Set("ingress", flattenNetworkAclEntries(ingressEntries)); err != nil { + return fmt.Errorf("error setting ingress: %w", err) + } - d.Set("vpc_id", networkAcl.VpcId) - - tags := KeyValueTags(networkAcl.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) + tags := KeyValueTags(nacl.Tags).IgnoreAWS().IgnoreConfig(ignoreTagsConfig) //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { @@ -340,459 +246,601 @@ func resourceNetworkACLRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error setting tags_all: %w", err) } - d.Set("owner_id", networkAcl.OwnerId) + return nil +} - var subnetIds []*string - for _, a := range networkAcl.Associations { - subnetIds = append(subnetIds, a.SubnetId) - } - if err := d.Set("subnet_ids", flex.FlattenStringSet(subnetIds)); err != nil { +func resourceNetworkACLUpdate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*conns.AWSClient).EC2Conn + + if err := modifyNetworkACLAttributesOnUpdate(conn, d, true); err != nil { return err } - if err := d.Set("ingress", networkAclEntriesToMapList(ingressEntries)); err != nil { - return err + return resourceNetworkACLRead(d, meta) +} + +func resourceNetworkACLDelete(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*conns.AWSClient).EC2Conn + + // Delete all NACL/Subnet associations, even if they are managed via aws_network_acl_association resources. + nacl, err := FindNetworkACLByID(conn, d.Id()) + + if err != nil { + return fmt.Errorf("error reading EC2 Network ACL (%s): %w", d.Id(), err) } - if err := d.Set("egress", networkAclEntriesToMapList(egressEntries)); err != nil { - return err + + var subnetIDs []interface{} + for _, v := range nacl.Associations { + subnetIDs = append(subnetIDs, aws.StringValue(v.SubnetId)) + } + if len(subnetIDs) > 0 { + if err := networkACLAssociationsDelete(conn, d.Get("vpc_id").(string), subnetIDs); err != nil { + return err + } } - arn := arn.ARN{ - Partition: meta.(*conns.AWSClient).Partition, - Service: ec2.ServiceName, - Region: meta.(*conns.AWSClient).Region, - AccountID: aws.StringValue(networkAcl.OwnerId), - Resource: fmt.Sprintf("network-acl/%s", d.Id()), - }.String() + input := &ec2.DeleteNetworkAclInput{ + NetworkAclId: aws.String(d.Id()), + } - d.Set("arn", arn) + log.Printf("[INFO] Deleting EC2 Network ACL: %s", d.Id()) + _, err = tfresource.RetryWhenAWSErrCodeEquals(PropagationTimeout, func() (interface{}, error) { + return conn.DeleteNetworkAcl(input) + }, ErrCodeDependencyViolation) + + if tfawserr.ErrCodeEquals(err, ErrCodeInvalidNetworkAclIDNotFound) { + return nil + } + + if err != nil { + return fmt.Errorf("error deleting EC2 Network ACL (%s): %w", d.Id(), err) + } return nil } -func resourceNetworkACLUpdate(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*conns.AWSClient).EC2Conn +// modifyNetworkACLAttributesOnCreate sets NACL attributes on resource Create. +// Called after new NACL creation or existing default NACL adoption. +// Tags are not configured. +func modifyNetworkACLAttributesOnCreate(conn *ec2.EC2, d *schema.ResourceData) error { + if v, ok := d.GetOk("egress"); ok && v.(*schema.Set).Len() > 0 { + if err := createNetworkACLEntries(conn, d.Id(), v.(*schema.Set).List(), true); err != nil { + return err + } + } + + if v, ok := d.GetOk("ingress"); ok && v.(*schema.Set).Len() > 0 { + if err := createNetworkACLEntries(conn, d.Id(), v.(*schema.Set).List(), false); err != nil { + return err + } + } + + if v, ok := d.GetOk("subnet_ids"); ok && v.(*schema.Set).Len() > 0 { + for _, v := range v.(*schema.Set).List() { + if _, err := networkACLAssociationCreate(conn, d.Id(), v.(string)); err != nil { + return err + } + } + } + return nil +} + +// modifyNetworkACLAttributesOnUpdate sets NACL attributes on resource Update. +// Tags are configured. +func modifyNetworkACLAttributesOnUpdate(conn *ec2.EC2, d *schema.ResourceData, deleteAssociations bool) error { if d.HasChange("ingress") { - err := updateNetworkAclEntries(d, "ingress", conn) - if err != nil { + o, n := d.GetChange("ingress") + os, ns := o.(*schema.Set), n.(*schema.Set) + + if err := updateNetworkACLEntries(conn, d.Id(), os, ns, false); err != nil { return err } } if d.HasChange("egress") { - err := updateNetworkAclEntries(d, "egress", conn) - if err != nil { + o, n := d.GetChange("egress") + os, ns := o.(*schema.Set), n.(*schema.Set) + + if err := updateNetworkACLEntries(conn, d.Id(), os, ns, true); err != nil { return err } } if d.HasChange("subnet_ids") { o, n := d.GetChange("subnet_ids") - if o == nil { - o = new(schema.Set) - } - if n == nil { - n = new(schema.Set) - } - - os := o.(*schema.Set) - ns := n.(*schema.Set) + os, ns := o.(*schema.Set), n.(*schema.Set) + add, del := ns.Difference(os).List(), os.Difference(ns).List() - remove := os.Difference(ns).List() - add := ns.Difference(os).List() - - if len(remove) > 0 { - // A Network ACL is required for each subnet. In order to disassociate a - // subnet from this ACL, we must associate it with the default ACL. - defaultAcl, err := GetDefaultNetworkACL(d.Get("vpc_id").(string), conn) - if err != nil { - return fmt.Errorf("Failed to find Default ACL for VPC %s", d.Get("vpc_id").(string)) - } - for _, r := range remove { - association, err := findNetworkAclAssociation(r.(string), conn) - if err != nil { - if tfresource.NotFound(err) { - // Subnet has been deleted. - continue - } - return fmt.Errorf("Failed to find acl association: acl %s with subnet %s: %s", d.Id(), r, err) - } - log.Printf("[DEBUG] Replacing Network Acl Association (%s) with Default Network ACL ID (%s)", *association.NetworkAclAssociationId, *defaultAcl.NetworkAclId) - _, err = conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{ - AssociationId: association.NetworkAclAssociationId, - NetworkAclId: defaultAcl.NetworkAclId, - }) - if err != nil { - if tfawserr.ErrMessageContains(err, "InvalidAssociationID.NotFound", "") { - continue - } - return fmt.Errorf("Error Replacing Default Network Acl Association: %s", err) - } + if len(del) > 0 && deleteAssociations { + if err := networkACLAssociationsDelete(conn, d.Get("vpc_id").(string), del); err != nil { + return err } } if len(add) > 0 { - for _, a := range add { - association, err := findNetworkAclAssociation(a.(string), conn) - if err != nil { - return fmt.Errorf("Failed to find acl association: acl %s with subnet %s: %s", d.Id(), a, err) - } - _, err = conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{ - AssociationId: association.NetworkAclAssociationId, - NetworkAclId: aws.String(d.Id()), - }) - if err != nil { - return err - } + if err := networkACLAssociationsCreate(conn, d.Id(), add); err != nil { + return err } } - } - if d.HasChange("tags_all") && !d.IsNewResource() { + if d.HasChange("tags_all") { o, n := d.GetChange("tags_all") if err := UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating EC2 Network ACL (%s) tags: %s", d.Id(), err) + return fmt.Errorf("error updating EC2 Network ACL (%s) tags: %w", d.Id(), err) } } - return resourceNetworkACLRead(d, meta) + return nil } -func updateNetworkAclEntries(d *schema.ResourceData, entryType string, conn *ec2.EC2) error { - if d.HasChange(entryType) { - o, n := d.GetChange(entryType) +func networkACLRuleHash(v interface{}) int { + var buf bytes.Buffer - if o == nil { - o = new(schema.Set) - } - if n == nil { - n = new(schema.Set) - } + tfMap := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%d-", tfMap["from_port"].(int))) + buf.WriteString(fmt.Sprintf("%d-", tfMap["to_port"].(int))) + buf.WriteString(fmt.Sprintf("%d-", tfMap["rule_no"].(int))) + buf.WriteString(fmt.Sprintf("%s-", strings.ToLower(tfMap["action"].(string)))) - os := o.(*schema.Set) - ns := n.(*schema.Set) + // The AWS network ACL API only speaks protocol numbers, and that's + // all we store. Never hash a protocol name. + protocolNumber, _ := networkACLProtocolNumber(tfMap["protocol"].(string)) + buf.WriteString(fmt.Sprintf("%d-", protocolNumber)) - toBeDeleted, err := ExpandNetworkACLEntries(os.Difference(ns).List(), entryType) - if err != nil { - return err - } - for _, remove := range toBeDeleted { - // AWS includes default rules with all network ACLs that can be - // neither modified nor destroyed. They have a custom rule - // number that is out of bounds for any other rule. If we - // encounter it, just continue. There's no work to be done. - if aws.Int64Value(remove.RuleNumber) == defaultACLRuleNumberIPv4 || - aws.Int64Value(remove.RuleNumber) == defaultACLRuleNumberIPv6 { - continue - } + if v, ok := tfMap["cidr_block"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + if v, ok := tfMap["ipv6_cidr_block"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + if v, ok := tfMap["icmp_type"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } + if v, ok := tfMap["icmp_code"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } - // Delete old Acl - log.Printf("[DEBUG] Destroying Network ACL Entry number (%d)", int(aws.Int64Value(remove.RuleNumber))) - _, err := conn.DeleteNetworkAclEntry(&ec2.DeleteNetworkAclEntryInput{ - NetworkAclId: aws.String(d.Id()), - RuleNumber: remove.RuleNumber, - Egress: remove.Egress, - }) - if err != nil { - return fmt.Errorf("Error deleting %s entry: %s", entryType, err) - } - } + return create.StringHashcode(buf.String()) +} - toBeCreated, err := ExpandNetworkACLEntries(ns.Difference(os).List(), entryType) - if err != nil { - return err +func createNetworkACLEntries(conn *ec2.EC2, naclID string, tfList []interface{}, egress bool) error { + naclEntries := expandNetworkAclEntries(tfList, egress) + + for _, naclEntry := range naclEntries { + if naclEntry == nil { + continue } - for _, add := range toBeCreated { + + if aws.StringValue(naclEntry.Protocol) == "-1" { // Protocol -1 rules don't store ports in AWS. Thus, they'll always // hash differently when being read out of the API. Force the user // to set from_port and to_port to 0 for these rules, to keep the // hashing consistent. - if aws.StringValue(add.Protocol) == "-1" { - to := aws.Int64Value(add.PortRange.To) - from := aws.Int64Value(add.PortRange.From) - expected := &ExpectedPortPair{ - to_port: 0, - from_port: 0, - } - if ok := ValidPorts(to, from, *expected); !ok { - return fmt.Errorf( - "to_port (%d) and from_port (%d) must both be 0 to use the the 'all' \"-1\" protocol!", - to, from) - } - } - - // AWS mutates the CIDR block into a network implied by the IP and - // mask provided. This results in hashing inconsistencies between - // the local config file and the state returned by the API. Error - // if the user provides a CIDR block with an inappropriate mask - if cidrBlock := aws.StringValue(add.CidrBlock); cidrBlock != "" { - if err := verify.ValidateIPv4CIDRBlock(cidrBlock); err != nil { - return err - } - } - if ipv6CidrBlock := aws.StringValue(add.Ipv6CidrBlock); ipv6CidrBlock != "" { - if err := verify.ValidateIPv6CIDRBlock(ipv6CidrBlock); err != nil { - return err - } - } - - createOpts := &ec2.CreateNetworkAclEntryInput{ - NetworkAclId: aws.String(d.Id()), - Egress: add.Egress, - PortRange: add.PortRange, - Protocol: add.Protocol, - RuleAction: add.RuleAction, - RuleNumber: add.RuleNumber, - IcmpTypeCode: add.IcmpTypeCode, + if from, to := aws.Int64Value(naclEntry.PortRange.From), aws.Int64Value(naclEntry.PortRange.To); from != 0 || to != 0 { + return fmt.Errorf("to_port (%d) and from_port (%d) must both be 0 to use the the 'all' \"-1\" protocol!", to, from) } + } - if add.CidrBlock != nil && aws.StringValue(add.CidrBlock) != "" { - createOpts.CidrBlock = add.CidrBlock - } + input := &ec2.CreateNetworkAclEntryInput{ + CidrBlock: naclEntry.CidrBlock, + Egress: naclEntry.Egress, + IcmpTypeCode: naclEntry.IcmpTypeCode, + Ipv6CidrBlock: naclEntry.Ipv6CidrBlock, + NetworkAclId: aws.String(naclID), + PortRange: naclEntry.PortRange, + Protocol: naclEntry.Protocol, + RuleAction: naclEntry.RuleAction, + RuleNumber: naclEntry.RuleNumber, + } - if add.Ipv6CidrBlock != nil && aws.StringValue(add.Ipv6CidrBlock) != "" { - createOpts.Ipv6CidrBlock = add.Ipv6CidrBlock - } + log.Printf("[INFO] Creating EC2 Network ACL Entry: %s", input) + _, err := conn.CreateNetworkAclEntry(input) - // Add new Acl entry - _, connErr := conn.CreateNetworkAclEntry(createOpts) - if connErr != nil { - return fmt.Errorf("Error creating %s entry: %s", entryType, connErr) - } + if err != nil { + return fmt.Errorf("error creating EC2 Network ACL (%s) Entry: %w", naclID, err) } } + return nil } -func resourceNetworkACLDelete(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*conns.AWSClient).EC2Conn - - log.Printf("[INFO] Deleting Network Acl: %s", d.Id()) - input := &ec2.DeleteNetworkAclInput{ - NetworkAclId: aws.String(d.Id()), - } - err := resource.Retry(5*time.Minute, func() *resource.RetryError { - _, err := conn.DeleteNetworkAcl(input) - if err != nil { - if tfawserr.ErrMessageContains(err, "InvalidNetworkAclID.NotFound", "") { - return nil - } - if tfawserr.ErrMessageContains(err, "DependencyViolation", "") { - err = cleanUpDependencyViolations(d, conn) - if err != nil { - return resource.NonRetryableError(err) - } - return resource.RetryableError(fmt.Errorf("Dependencies found and cleaned up, retrying")) - } +func deleteNetworkACLEntries(conn *ec2.EC2, naclID string, tfList []interface{}, egress bool) error { + return deleteNetworkAclEntries(conn, naclID, expandNetworkAclEntries(tfList, egress)) +} - return resource.NonRetryableError(err) +func deleteNetworkAclEntries(conn *ec2.EC2, naclID string, naclEntries []*ec2.NetworkAclEntry) error { + for _, naclEntry := range naclEntries { + if naclEntry == nil { + continue + } + // AWS includes default rules with all network ACLs that can be + // neither modified nor destroyed. They have a custom rule + // number that is out of bounds for any other rule. If we + // encounter it, just continue. There's no work to be done. + if v := aws.Int64Value(naclEntry.RuleNumber); v == defaultACLRuleNumberIPv4 || v == defaultACLRuleNumberIPv6 { + continue } - log.Printf("[Info] Deleted network ACL %s successfully", d.Id()) - return nil - }) - if tfresource.TimedOut(err) { - _, err = conn.DeleteNetworkAcl(input) - if err != nil && tfawserr.ErrMessageContains(err, "InvalidNetworkAclID.NotFound", "") { - return nil + + input := &ec2.DeleteNetworkAclEntryInput{ + Egress: naclEntry.Egress, + NetworkAclId: aws.String(naclID), + RuleNumber: naclEntry.RuleNumber, } - err = cleanUpDependencyViolations(d, conn) + + log.Printf("[INFO] Deleting EC2 Network ACL Entry: %s", input) + _, err := conn.DeleteNetworkAclEntry(input) + if err != nil { - // This seems excessive but is probably the best way to make sure it's actually deleted - _, err = conn.DeleteNetworkAcl(input) - if err != nil && tfawserr.ErrMessageContains(err, "InvalidNetworkAclID.NotFound", "") { - return nil - } + return fmt.Errorf("error deleting EC2 Network ACL (%s) Entry: %w", naclID, err) } } - if err != nil { - return fmt.Errorf("Error destroying Network ACL (%s): %s", d.Id(), err) - } + return nil } -func cleanUpDependencyViolations(d *schema.ResourceData, conn *ec2.EC2) error { - // In case of dependency violation, we remove the association between subnet and network acl. - // This means the subnet is attached to default acl of vpc. - var associations []*ec2.NetworkAclAssociation - if v, ok := d.GetOk("subnet_ids"); ok { - ids := v.(*schema.Set).List() - for _, i := range ids { - a, err := findNetworkAclAssociation(i.(string), conn) - if err != nil { - if tfresource.NotFound(err) { - continue - } - return fmt.Errorf("Error finding network ACL association: %s", err) - } - associations = append(associations, a) - } +func updateNetworkACLEntries(conn *ec2.EC2, naclID string, os, ns *schema.Set, egress bool) error { + if err := deleteNetworkACLEntries(conn, naclID, os.Difference(ns).List(), egress); err != nil { + return err } - log.Printf("[DEBUG] Replacing network associations for Network ACL (%s): %s", d.Id(), associations) - defaultAcl, err := GetDefaultNetworkACL(d.Get("vpc_id").(string), conn) - if err != nil { - return fmt.Errorf("Error getting default network ACL: %s", err) - } - - for _, a := range associations { - log.Printf("DEBUG] Replacing Network Acl Association (%s) with Default Network ACL ID (%s)", - aws.StringValue(a.NetworkAclAssociationId), aws.StringValue(defaultAcl.NetworkAclId)) - _, replaceErr := conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{ - AssociationId: a.NetworkAclAssociationId, - NetworkAclId: defaultAcl.NetworkAclId, - }) - if replaceErr != nil { - // It's possible that during an attempt to replace this - // association, the Subnet in question has already been moved to - // another ACL. This can happen if you're destroying a network acl - // and simultaneously re-associating it's subnet(s) with another - // ACL; Terraform may have already re-associated the subnet(s) by - // the time we attempt to destroy them, even between the time we - // list them and then try to destroy them. In this case, the - // association we're trying to replace will no longer exist and - // this call will fail. Here we trap that error and fail - // gracefully; the association we tried to replace gone, we trust - // someone else has taken ownership. - if tfawserr.ErrMessageContains(replaceErr, "InvalidAssociationID.NotFound", "") { - log.Printf("[WARN] Network Association (%s) no longer found; Network Association likely updated or removed externally, removing from state", aws.StringValue(a.NetworkAclAssociationId)) - continue - } - log.Printf("[ERR] Non retry-able error in replacing associations for Network ACL (%s): %s", d.Id(), replaceErr) - return fmt.Errorf("Error replacing network ACL associations: %s", err) - } + if err := createNetworkACLEntries(conn, naclID, ns.Difference(os).List(), egress); err != nil { + return err } + return nil } -func resourceNetworkACLEntryHash(v interface{}) int { - var buf bytes.Buffer - m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%d-", m["from_port"].(int))) - buf.WriteString(fmt.Sprintf("%d-", m["to_port"].(int))) - buf.WriteString(fmt.Sprintf("%d-", m["rule_no"].(int))) - buf.WriteString(fmt.Sprintf("%s-", strings.ToLower(m["action"].(string)))) +func expandNetworkAclEntry(tfMap map[string]interface{}, egress bool) *ec2.NetworkAclEntry { + if tfMap == nil { + return nil + } - // The AWS network ACL API only speaks protocol numbers, and that's - // all we store. Never hash a protocol name. - protocol := m["protocol"].(string) - if _, err := strconv.Atoi(m["protocol"].(string)); err != nil { - // We're a protocol name. Look up the number. - buf.WriteString(fmt.Sprintf("%d-", protocolIntegers()[protocol])) - } else { - // We're a protocol number. Pass the value through. - buf.WriteString(fmt.Sprintf("%s-", protocol)) + apiObject := &ec2.NetworkAclEntry{ + Egress: aws.Bool(egress), + PortRange: &ec2.PortRange{}, } - if v, ok := m["cidr_block"]; ok { - buf.WriteString(fmt.Sprintf("%s-", v.(string))) + if v, ok := tfMap["rule_no"].(int); ok { + apiObject.RuleNumber = aws.Int64(int64(v)) } - if v, ok := m["ipv6_cidr_block"]; ok { - buf.WriteString(fmt.Sprintf("%s-", v.(string))) + if v, ok := tfMap["action"].(string); ok && v != "" { + apiObject.RuleAction = aws.String(v) } - if v, ok := m["ssl_certificate_id"]; ok { - buf.WriteString(fmt.Sprintf("%s-", v.(string))) + if v, ok := tfMap["cidr_block"].(string); ok && v != "" { + apiObject.CidrBlock = aws.String(v) } - if v, ok := m["icmp_type"]; ok { - buf.WriteString(fmt.Sprintf("%d-", v.(int))) + if v, ok := tfMap["ipv6_cidr_block"].(string); ok && v != "" { + apiObject.Ipv6CidrBlock = aws.String(v) } - if v, ok := m["icmp_code"]; ok { - buf.WriteString(fmt.Sprintf("%d-", v.(int))) + + if v, ok := tfMap["from_port"].(int); ok { + apiObject.PortRange.From = aws.Int64(int64(v)) } - return create.StringHashcode(buf.String()) -} + if v, ok := tfMap["to_port"].(int); ok { + apiObject.PortRange.To = aws.Int64(int64(v)) + } -func GetDefaultNetworkACL(vpcId string, conn *ec2.EC2) (defaultAcl *ec2.NetworkAcl, err error) { - resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{ - Filters: []*ec2.Filter{ - { - Name: aws.String("default"), - Values: []*string{aws.String("true")}, - }, - { - Name: aws.String("vpc-id"), - Values: []*string{aws.String(vpcId)}, - }, - }, - }) + if v, ok := tfMap["protocol"].(string); ok && v != "" { + protocolNumber, err := networkACLProtocolNumber(v) - if err != nil { - return nil, err + if err != nil { + log.Printf("[WARN] %s", err) + return nil + } + + apiObject.Protocol = aws.String(strconv.Itoa(protocolNumber)) + + // Specify additional required fields for ICMP. + if protocolNumber == 1 || protocolNumber == 58 { + apiObject.IcmpTypeCode = &ec2.IcmpTypeCode{} + + if v, ok := tfMap["icmp_code"].(int); ok { + apiObject.IcmpTypeCode.Code = aws.Int64(int64(v)) + } + + if v, ok := tfMap["icmp_type"].(int); ok { + apiObject.IcmpTypeCode.Type = aws.Int64(int64(v)) + } + } } - return resp.NetworkAcls[0], nil + + return apiObject } -func findNetworkAclAssociation(subnetId string, conn *ec2.EC2) (networkAclAssociation *ec2.NetworkAclAssociation, err error) { - req := &ec2.DescribeNetworkAclsInput{} - req.Filters = BuildAttributeFilterList( - map[string]string{ - "association.subnet-id": subnetId, - }, - ) - resp, err := conn.DescribeNetworkAcls(req) - if err != nil { - return nil, err +func expandNetworkAclEntries(tfList []interface{}, egress bool) []*ec2.NetworkAclEntry { + if len(tfList) == 0 { + return nil } - if len(resp.NetworkAcls) > 0 { - for _, association := range resp.NetworkAcls[0].Associations { - if aws.StringValue(association.SubnetId) == subnetId { - return association, nil - } + var apiObjects []*ec2.NetworkAclEntry + + for _, tfMapRaw := range tfList { + tfMap, ok := tfMapRaw.(map[string]interface{}) + + if !ok { + continue } - } - return nil, &resource.NotFoundError{ - LastRequest: req, - LastResponse: resp, - Message: fmt.Sprintf("could not find association for subnet: %s ", subnetId), + apiObject := expandNetworkAclEntry(tfMap, egress) + + if apiObject == nil { + continue + } + + apiObjects = append(apiObjects, apiObject) } + + return apiObjects } -// networkAclEntriesToMapList turns ingress/egress rules read from AWS into a list -// of maps. -func networkAclEntriesToMapList(networkAcls []*ec2.NetworkAclEntry) []map[string]interface{} { - result := make([]map[string]interface{}, 0, len(networkAcls)) - for _, entry := range networkAcls { - acl := make(map[string]interface{}) - acl["rule_no"] = aws.Int64Value(entry.RuleNumber) - acl["action"] = aws.StringValue(entry.RuleAction) - if entry.CidrBlock != nil { - acl["cidr_block"] = aws.StringValue(entry.CidrBlock) +func flattenNetworkAclEntry(apiObject *ec2.NetworkAclEntry) map[string]interface{} { + if apiObject == nil { + return nil + } + + tfMap := map[string]interface{}{} + + if v := apiObject.RuleNumber; v != nil { + tfMap["rule_no"] = aws.Int64Value(v) + } + + if v := apiObject.RuleAction; v != nil { + tfMap["action"] = aws.StringValue(v) + } + + if v := apiObject.CidrBlock; v != nil { + tfMap["cidr_block"] = aws.StringValue(v) + } + + if v := apiObject.Ipv6CidrBlock; v != nil { + tfMap["ipv6_cidr_block"] = aws.StringValue(v) + } + + if apiObject := apiObject.PortRange; apiObject != nil { + if v := apiObject.From; v != nil { + tfMap["from_port"] = aws.Int64Value(v) } - if entry.Ipv6CidrBlock != nil { - acl["ipv6_cidr_block"] = aws.StringValue(entry.Ipv6CidrBlock) + + if v := apiObject.To; v != nil { + tfMap["to_port"] = aws.Int64Value(v) } + } + + if v := aws.StringValue(apiObject.Protocol); v != "" { // The AWS network ACL API only speaks protocol numbers, and // that's all we record. - if _, err := strconv.Atoi(aws.StringValue(entry.Protocol)); err != nil { - // We're a protocol name. Look up the number. - acl["protocol"] = protocolIntegers()[aws.StringValue(entry.Protocol)] - } else { - // We're a protocol number. Pass through. - acl["protocol"] = aws.StringValue(entry.Protocol) + protocolNumber, err := networkACLProtocolNumber(v) + + if err != nil { + log.Printf("[WARN] %s", err) + return nil + } + + tfMap["protocol"] = strconv.Itoa(protocolNumber) + } + + if apiObject := apiObject.IcmpTypeCode; apiObject != nil { + if v := apiObject.Code; v != nil { + tfMap["icmp_code"] = aws.Int64Value(v) + } + + if v := apiObject.Type; v != nil { + tfMap["icmp_type"] = aws.Int64Value(v) + } + } + + return tfMap +} + +func flattenNetworkAclEntries(apiObjects []*ec2.NetworkAclEntry) []interface{} { + if len(apiObjects) == 0 { + return nil + } + + var tfList []interface{} + + for _, apiObject := range apiObjects { + if apiObject == nil { + continue } - acl["protocol"] = aws.StringValue(entry.Protocol) - if entry.PortRange != nil { - acl["from_port"] = aws.Int64Value(entry.PortRange.From) - acl["to_port"] = aws.Int64Value(entry.PortRange.To) + tfList = append(tfList, flattenNetworkAclEntry(apiObject)) + } + + return tfList +} + +func networkACLProtocolNumber(v string) (int, error) { + i, err := strconv.Atoi(v) + + if err != nil { + // Lookup number by name. + var ok bool + i, ok = ianaProtocolAToI[v] + + if !ok { + return 0, fmt.Errorf("unsupported NACL protocol: %s", v) } + } else { + _, ok := ianaProtocolIToA[i] - if entry.IcmpTypeCode != nil { - acl["icmp_type"] = aws.Int64Value(entry.IcmpTypeCode.Type) - acl["icmp_code"] = aws.Int64Value(entry.IcmpTypeCode.Code) + if !ok { + return 0, fmt.Errorf("unsupported NACL protocol: %d", i) } + } + + return i, nil +} - result = append(result, acl) +type ianaProtocolAToIType map[string]int +type ianaProtocolIToAType map[int]string + +func (m ianaProtocolAToIType) invert() ianaProtocolIToAType { + output := make(map[int]string, len(m)) + + for k, v := range m { + output[v] = k } - return result + return output } + +var ( + ianaProtocolAToI = ianaProtocolAToIType(map[string]int{ + // Defined at https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml. + "all": -1, + "hopopt": 0, + "icmp": 1, + "igmp": 2, + "ggp": 3, + "ipv4": 4, + "st": 5, + "tcp": 6, + "cbt": 7, + "egp": 8, + "igp": 9, + "bbn-rcc-mon": 10, + "nvp-ii": 11, + "pup": 12, + "argus": 13, + "emcon": 14, + "xnet": 15, + "chaos": 16, + "udp": 17, + "mux": 18, + "dcn-meas": 19, + "hmp": 20, + "prm": 21, + "xns-idp": 22, + "trunk-1": 23, + "trunk-2": 24, + "leaf-1": 25, + "leaf-2": 26, + "rdp": 27, + "irtp": 28, + "iso-tp4": 29, + "netblt": 30, + "mfe-nsp": 31, + "merit-inp": 32, + "dccp": 33, + "3pc": 34, + "idpr": 35, + "xtp": 36, + "ddp": 37, + "idpr-cmtp": 38, + "tp++": 39, + "il": 40, + "ipv6": 41, + "sdrp": 42, + "ipv6-route": 43, + "ipv6-frag": 44, + "idrp": 45, + "rsvp": 46, + "gre": 47, + "dsr": 48, + "bna": 49, + "esp": 50, + "ah": 51, + "i-nlsp": 52, + "swipe": 53, + "narp": 54, + "mobile": 55, + "tlsp": 56, + "ipv6-icmp": 58, + "ipv6-nonxt": 59, + "ipv6-opts": 60, + "61": 61, + "cftp": 62, + "63": 63, + "sat-expak": 64, + "kryptolan": 65, + "rvd": 66, + "ippc": 67, + "68": 68, + "sat-mon": 69, + "visa": 70, + "ipcv": 71, + "cpnx": 72, + "cphb": 73, + "wsn": 74, + "pvp": 75, + "br-sat-mon": 76, + "sun-nd": 77, + "wb-mon": 78, + "wb-expak": 79, + "iso-ip": 80, + "vmtp": 81, + "secure-vmtp": 82, + "vines": 83, + "ttp": 84, + "nsfnet-igp": 85, + "dgp": 86, + "tcf": 87, + "eigrp": 88, + "ospfigp": 89, + "sprite-rpc": 90, + "larp": 91, + "mtp": 92, + "ax.25": 93, + "ipip": 94, + "micp": 95, + "scc-sp": 96, + "etherip": 97, + "encap": 98, + "99": 99, + "gmtp": 100, + "ifmp": 101, + "pnni": 102, + "pim": 103, + "aris": 104, + "scps": 105, + "qnx": 106, + "a/n": 107, + "ipcomp": 108, + "snp": 109, + "compaq-peer": 110, + "ipx-in-ip": 111, + "vrrp": 112, + "pgm": 113, + "114": 114, + "l2tp": 115, + "dd": 116, + "iatp": 117, + "stp": 118, + "srp": 119, + "uti": 120, + "smp": 121, + "sm": 122, + "ptp": 123, + "isis-over-ipv4": 124, + "fire": 125, + "crtp": 126, + "crudp": 127, + "sscopmce": 128, + "iplt": 129, + "sps": 130, + "pipe": 131, + "sctp": 132, + "fc": 133, + "rsvp-e2e-ignore": 134, + "mobility-header": 135, + "udplite": 136, + "mpls-in-ip": 137, + "manet": 138, + "hip": 139, + "shim6": 140, + "wesp": 141, + "rohc": 142, + "253": 253, + "254": 254, + }) + ianaProtocolIToA = ianaProtocolAToI.invert() +) diff --git a/internal/service/ec2/network_acl_association.go b/internal/service/ec2/network_acl_association.go new file mode 100644 index 000000000000..b890876f9d9f --- /dev/null +++ b/internal/service/ec2/network_acl_association.go @@ -0,0 +1,198 @@ +package ec2 + +import ( + "fmt" + "log" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" +) + +func ResourceNetworkACLAssociation() *schema.Resource { + return &schema.Resource{ + Create: resourceNetworkACLAssociationCreate, + Read: resourceNetworkACLAssociationRead, + Delete: resourceNetworkACLAssociationDelete, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, + + Schema: map[string]*schema.Schema{ + "network_acl_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + "subnet_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + }, + } +} + +func resourceNetworkACLAssociationCreate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*conns.AWSClient).EC2Conn + + associationID, err := networkACLAssociationCreate(conn, d.Get("network_acl_id").(string), d.Get("subnet_id").(string)) + + if err != nil { + return err + } + + d.SetId(associationID) + + return resourceNetworkACLAssociationRead(d, meta) +} + +func resourceNetworkACLAssociationRead(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*conns.AWSClient).EC2Conn + + association, err := FindNetworkACLAssociationByID(conn, d.Id()) + + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] EC2 Network ACL Association (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error reading EC2 Network ACL Association (%s): %w", d.Id(), err) + } + + d.Set("network_acl_id", association.NetworkAclId) + d.Set("subnet_id", association.SubnetId) + + return nil +} + +func resourceNetworkACLAssociationDelete(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*conns.AWSClient).EC2Conn + + input := &ec2.DescribeNetworkAclsInput{ + Filters: BuildAttributeFilterList(map[string]string{ + "association.association-id": d.Id(), + }), + } + + nacl, err := FindNetworkACL(conn, input) + + if err != nil { + return fmt.Errorf("error reading EC2 Network ACL for Association (%s): %w", d.Id(), err) + } + + vpcID := aws.StringValue(nacl.VpcId) + defaultNACL, err := FindVPCDefaultNetworkACL(conn, vpcID) + + if err != nil { + return fmt.Errorf("error reading EC2 VPC (%s) default NACL: %w", vpcID, err) + } + + if err := networkACLAssociationDelete(conn, d.Id(), aws.StringValue(defaultNACL.NetworkAclId)); err != nil { + return err + } + + return nil +} + +// networkACLAssociationCreate creates an association between the specified NACL and subnet. +// The subnet's current association is replaced and the new association's ID is returned. +func networkACLAssociationCreate(conn *ec2.EC2, naclID, subnetID string) (string, error) { + association, err := FindNetworkACLAssociationBySubnetID(conn, subnetID) + + if err != nil { + return "", fmt.Errorf("error reading EC2 Network ACL Association for EC2 Subnet (%s): %w", subnetID, err) + } + + input := &ec2.ReplaceNetworkAclAssociationInput{ + AssociationId: association.NetworkAclAssociationId, + NetworkAclId: aws.String(naclID), + } + + log.Printf("[DEBUG] Creating EC2 Network ACL Association: %s", input) + outputRaw, err := tfresource.RetryWhenAWSErrCodeEquals(PropagationTimeout, func() (interface{}, error) { + return conn.ReplaceNetworkAclAssociation(input) + }, ErrCodeInvalidAssociationIDNotFound) + + if err != nil { + return "", fmt.Errorf("error creating EC2 Network ACL (%s) Association: %w", naclID, err) + } + + return aws.StringValue(outputRaw.(*ec2.ReplaceNetworkAclAssociationOutput).NewAssociationId), nil +} + +// networkACLAssociationsCreate creates associations between the specified NACL and subnets. +func networkACLAssociationsCreate(conn *ec2.EC2, naclID string, subnetIDs []interface{}) error { + for _, v := range subnetIDs { + subnetID := v.(string) + _, err := networkACLAssociationCreate(conn, naclID, subnetID) + + if tfresource.NotFound(err) { + // Subnet has been deleted. + continue + } + + if err != nil { + return err + } + } + + return nil +} + +// networkACLAssociationDelete deletes the specified association between a NACL and subnet. +// The subnet's current association is replaced by an association with the VPC's default NACL. +func networkACLAssociationDelete(conn *ec2.EC2, associationID, naclID string) error { + input := &ec2.ReplaceNetworkAclAssociationInput{ + AssociationId: aws.String(associationID), + NetworkAclId: aws.String(naclID), + } + + log.Printf("[DEBUG] Deleting EC2 Network ACL Association: %s", associationID) + _, err := conn.ReplaceNetworkAclAssociation(input) + + if tfawserr.ErrCodeEquals(err, ErrCodeInvalidAssociationIDNotFound) { + return nil + } + + if err != nil { + return fmt.Errorf("error deleting EC2 Network ACL Association (%s): %w", associationID, err) + } + + return nil +} + +// networkACLAssociationDelete deletes the specified NACL associations for the specified subnets. +// Each subnet's current association is replaced by an association with the specified VPC's default NACL. +func networkACLAssociationsDelete(conn *ec2.EC2, vpcID string, subnetIDs []interface{}) error { + defaultNACL, err := FindVPCDefaultNetworkACL(conn, vpcID) + + if err != nil { + return fmt.Errorf("error reading EC2 VPC (%s) default NACL: %w", vpcID, err) + } + + for _, v := range subnetIDs { + subnetID := v.(string) + association, err := FindNetworkACLAssociationBySubnetID(conn, subnetID) + + if tfresource.NotFound(err) { + // Subnet has been deleted. + continue + } + + if err != nil { + return fmt.Errorf("error reading EC2 Network ACL Association for EC2 Subnet (%s): %w", subnetID, err) + } + + if err := networkACLAssociationDelete(conn, aws.StringValue(association.NetworkAclAssociationId), aws.StringValue(defaultNACL.NetworkAclId)); err != nil { + return err + } + } + + return nil +} diff --git a/internal/service/ec2/network_acl_association_test.go b/internal/service/ec2/network_acl_association_test.go new file mode 100644 index 000000000000..53b5d0562571 --- /dev/null +++ b/internal/service/ec2/network_acl_association_test.go @@ -0,0 +1,344 @@ +package ec2_test + +import ( + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/service/ec2" + sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/hashicorp/terraform-provider-aws/internal/acctest" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfec2 "github.com/hashicorp/terraform-provider-aws/internal/service/ec2" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" +) + +func TestAccEC2NetworkACLAssociation_basic(t *testing.T) { + var v ec2.NetworkAclAssociation + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_network_acl_association.test" + naclResourceName := "aws_network_acl.test" + subnetResourceName := "aws_subnet.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckNetworkACLAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetworkACLAssociationConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLAssociationExists(resourceName, &v), + resource.TestCheckResourceAttrPair(resourceName, "network_acl_id", naclResourceName, "id"), + resource.TestCheckResourceAttrPair(resourceName, "subnet_id", subnetResourceName, "id"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccEC2NetworkACLAssociation_disappears(t *testing.T) { + var v ec2.NetworkAclAssociation + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_network_acl_association.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckNetworkACLAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetworkACLAssociationConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLAssociationExists(resourceName, &v), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACLAssociation(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func TestAccEC2NetworkACLAssociation_disappears_NACL(t *testing.T) { + var v ec2.NetworkAclAssociation + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_network_acl_association.test" + naclResourceName := "aws_network_acl.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckNetworkACLAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetworkACLAssociationConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLAssociationExists(resourceName, &v), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACL(), naclResourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func TestAccEC2NetworkACLAssociation_disappears_Subnet(t *testing.T) { + var v ec2.NetworkAclAssociation + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_network_acl_association.test" + subnetResourceName := "aws_subnet.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckNetworkACLAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetworkACLAssociationConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLAssociationExists(resourceName, &v), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceSubnet(), subnetResourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func TestAccEC2NetworkACLAssociation_twoAssociations(t *testing.T) { + var v1, v2 ec2.NetworkAclAssociation + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resource1Name := "aws_network_acl_association.test1" + resource2Name := "aws_network_acl_association.test2" + naclResourceName := "aws_network_acl.test" + subnet1ResourceName := "aws_subnet.test1" + subnet2ResourceName := "aws_subnet.test2" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckNetworkACLAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetworkACLAssociationTwoAssociationsConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLAssociationExists(resource1Name, &v1), + testAccCheckNetworkACLAssociationExists(resource1Name, &v2), + resource.TestCheckResourceAttrPair(resource1Name, "network_acl_id", naclResourceName, "id"), + resource.TestCheckResourceAttrPair(resource1Name, "subnet_id", subnet1ResourceName, "id"), + resource.TestCheckResourceAttrPair(resource2Name, "network_acl_id", naclResourceName, "id"), + resource.TestCheckResourceAttrPair(resource2Name, "subnet_id", subnet2ResourceName, "id"), + ), + }, + { + ResourceName: resource1Name, + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: resource2Name, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccEC2NetworkACLAssociation_associateWithDefaultNACL(t *testing.T) { + var v ec2.NetworkAclAssociation + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_network_acl_association.test" + subnetResourceName := "aws_subnet.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckNetworkACLAssociationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetworkACLAssociationDefaultNACLConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLAssociationExists(resourceName, &v), + resource.TestCheckResourceAttrPair(resourceName, "subnet_id", subnetResourceName, "id"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} +func testAccCheckNetworkACLAssociationDestroy(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_network_acl_association" { + continue + } + + _, err := tfec2.FindNetworkACLAssociationByID(conn, rs.Primary.ID) + + if tfresource.NotFound(err) { + continue + } + + if err != nil { + return err + } + + return fmt.Errorf("EC2 Network ACL Association %s still exists", rs.Primary.ID) + } + + return nil +} + +func testAccCheckNetworkACLAssociationExists(n string, v *ec2.NetworkAclAssociation) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No EC2 Network ACL Association ID is set") + } + + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn + + output, err := tfec2.FindNetworkACLAssociationByID(conn, rs.Primary.ID) + + if err != nil { + return err + } + + *v = *output + + return nil + } +} + +func testAccNetworkACLAssociationConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test" { + vpc_id = aws_vpc.test.id + + cidr_block = "10.1.33.0/24" + + tags = { + Name = %[1]q + } +} + +resource "aws_network_acl_association" "test" { + network_acl_id = aws_network_acl.test.id + subnet_id = aws_subnet.test.id +} +`, rName) +} + +func testAccNetworkACLAssociationTwoAssociationsConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test1" { + vpc_id = aws_vpc.test.id + + cidr_block = "10.1.33.0/24" + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test2" { + vpc_id = aws_vpc.test.id + + cidr_block = "10.1.34.0/24" + + tags = { + Name = %[1]q + } +} + +resource "aws_network_acl_association" "test1" { + network_acl_id = aws_network_acl.test.id + subnet_id = aws_subnet.test1.id +} + +resource "aws_network_acl_association" "test2" { + network_acl_id = aws_network_acl.test.id + subnet_id = aws_subnet.test2.id +} +`, rName) +} + +func testAccNetworkACLAssociationDefaultNACLConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test" { + vpc_id = aws_vpc.test.id + + cidr_block = "10.1.33.0/24" + + tags = { + Name = %[1]q + } +} + +resource "aws_network_acl_association" "test" { + network_acl_id = aws_vpc.test.default_network_acl_id + subnet_id = aws_subnet.test.id +} +`, rName) +} diff --git a/internal/service/ec2/network_acl_entry.go b/internal/service/ec2/network_acl_entry.go deleted file mode 100644 index ecfc5cc731c5..000000000000 --- a/internal/service/ec2/network_acl_entry.go +++ /dev/null @@ -1,234 +0,0 @@ -package ec2 - -import ( - "fmt" - "strconv" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" -) - -func ExpandNetworkACLEntries(configured []interface{}, entryType string) ([]*ec2.NetworkAclEntry, error) { - entries := make([]*ec2.NetworkAclEntry, 0, len(configured)) - for _, eRaw := range configured { - data := eRaw.(map[string]interface{}) - protocol := data["protocol"].(string) - p, err := strconv.Atoi(protocol) - if err != nil { - var ok bool - p, ok = protocolIntegers()[protocol] - if !ok { - return nil, fmt.Errorf("Invalid Protocol %s for rule %#v", protocol, data) - } - } - - e := &ec2.NetworkAclEntry{ - Protocol: aws.String(strconv.Itoa(p)), - PortRange: &ec2.PortRange{ - From: aws.Int64(int64(data["from_port"].(int))), - To: aws.Int64(int64(data["to_port"].(int))), - }, - Egress: aws.Bool(entryType == "egress"), - RuleAction: aws.String(data["action"].(string)), - RuleNumber: aws.Int64(int64(data["rule_no"].(int))), - } - - if v, ok := data["ipv6_cidr_block"]; ok { - e.Ipv6CidrBlock = aws.String(v.(string)) - } - - if v, ok := data["cidr_block"]; ok { - e.CidrBlock = aws.String(v.(string)) - } - - // Specify additional required fields for ICMP - if p == 1 || p == 58 { - e.IcmpTypeCode = &ec2.IcmpTypeCode{} - if v, ok := data["icmp_code"]; ok { - e.IcmpTypeCode.Code = aws.Int64(int64(v.(int))) - } - if v, ok := data["icmp_type"]; ok { - e.IcmpTypeCode.Type = aws.Int64(int64(v.(int))) - } - } - - entries = append(entries, e) - } - return entries, nil -} - -func protocolStrings(protocolIntegers map[string]int) map[int]string { - protocolStrings := make(map[int]string, len(protocolIntegers)) - for k, v := range protocolIntegers { - protocolStrings[v] = k - } - - return protocolStrings -} - -func protocolIntegers() map[string]int { - return map[string]int{ - // defined at https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml - "all": -1, - "hopopt": 0, - "icmp": 1, - "igmp": 2, - "ggp": 3, - "ipv4": 4, - "st": 5, - "tcp": 6, - "cbt": 7, - "egp": 8, - "igp": 9, - "bbn-rcc-mon": 10, - "nvp-ii": 11, - "pup": 12, - "argus": 13, - "emcon": 14, - "xnet": 15, - "chaos": 16, - "udp": 17, - "mux": 18, - "dcn-meas": 19, - "hmp": 20, - "prm": 21, - "xns-idp": 22, - "trunk-1": 23, - "trunk-2": 24, - "leaf-1": 25, - "leaf-2": 26, - "rdp": 27, - "irtp": 28, - "iso-tp4": 29, - "netblt": 30, - "mfe-nsp": 31, - "merit-inp": 32, - "dccp": 33, - "3pc": 34, - "idpr": 35, - "xtp": 36, - "ddp": 37, - "idpr-cmtp": 38, - "tp++": 39, - "il": 40, - "ipv6": 41, - "sdrp": 42, - "ipv6-route": 43, - "ipv6-frag": 44, - "idrp": 45, - "rsvp": 46, - "gre": 47, - "dsr": 48, - "bna": 49, - "esp": 50, - "ah": 51, - "i-nlsp": 52, - "swipe": 53, - "narp": 54, - "mobile": 55, - "tlsp": 56, - "ipv6-icmp": 58, - "ipv6-nonxt": 59, - "ipv6-opts": 60, - "61": 61, - "cftp": 62, - "63": 63, - "sat-expak": 64, - "kryptolan": 65, - "rvd": 66, - "ippc": 67, - "68": 68, - "sat-mon": 69, - "visa": 70, - "ipcv": 71, - "cpnx": 72, - "cphb": 73, - "wsn": 74, - "pvp": 75, - "br-sat-mon": 76, - "sun-nd": 77, - "wb-mon": 78, - "wb-expak": 79, - "iso-ip": 80, - "vmtp": 81, - "secure-vmtp": 82, - "vines": 83, - "ttp": 84, - "nsfnet-igp": 85, - "dgp": 86, - "tcf": 87, - "eigrp": 88, - "ospfigp": 89, - "sprite-rpc": 90, - "larp": 91, - "mtp": 92, - "ax.25": 93, - "ipip": 94, - "micp": 95, - "scc-sp": 96, - "etherip": 97, - "encap": 98, - "99": 99, - "gmtp": 100, - "ifmp": 101, - "pnni": 102, - "pim": 103, - "aris": 104, - "scps": 105, - "qnx": 106, - "a/n": 107, - "ipcomp": 108, - "snp": 109, - "compaq-peer": 110, - "ipx-in-ip": 111, - "vrrp": 112, - "pgm": 113, - "114": 114, - "l2tp": 115, - "dd": 116, - "iatp": 117, - "stp": 118, - "srp": 119, - "uti": 120, - "smp": 121, - "sm": 122, - "ptp": 123, - "isis-over-ipv4": 124, - "fire": 125, - "crtp": 126, - "crudp": 127, - "sscopmce": 128, - "iplt": 129, - "sps": 130, - "pipe": 131, - "sctp": 132, - "fc": 133, - "rsvp-e2e-ignore": 134, - "mobility-header": 135, - "udplite": 136, - "mpls-in-ip": 137, - "manet": 138, - "hip": 139, - "shim6": 140, - "wesp": 141, - "rohc": 142, - "253": 253, - "254": 254, - } -} - -// ExpectedPortPair stores a pair of ports we expect to see together. -type ExpectedPortPair struct { - to_port int64 - from_port int64 -} - -// ValidPorts ensures the ports and protocol match expected -// values. -func ValidPorts(to int64, from int64, expected ExpectedPortPair) bool { - if to != expected.to_port || from != expected.from_port { - return false - } - - return true -} diff --git a/internal/service/ec2/network_acl_entry_test.go b/internal/service/ec2/network_acl_entry_test.go deleted file mode 100644 index f0bdc8b8e0fa..000000000000 --- a/internal/service/ec2/network_acl_entry_test.go +++ /dev/null @@ -1,100 +0,0 @@ -package ec2 - -import ( - "reflect" - "testing" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" -) - -func Test_expandNetworkACLEntry(t *testing.T) { - input := []interface{}{ - map[string]interface{}{ - "protocol": "tcp", - "from_port": 22, - "to_port": 22, - "cidr_block": "0.0.0.0/0", - "action": "deny", - "rule_no": 1, - }, - map[string]interface{}{ - "protocol": "tcp", - "from_port": 443, - "to_port": 443, - "cidr_block": "0.0.0.0/0", - "action": "deny", - "rule_no": 2, - }, - map[string]interface{}{ - "protocol": "-1", - "from_port": 443, - "to_port": 443, - "cidr_block": "0.0.0.0/0", - "action": "deny", - "rule_no": 2, - }, - } - expanded, _ := ExpandNetworkACLEntries(input, "egress") - - expected := []*ec2.NetworkAclEntry{ - { - Protocol: aws.String("6"), - PortRange: &ec2.PortRange{ - From: aws.Int64(22), - To: aws.Int64(22), - }, - RuleAction: aws.String("deny"), - RuleNumber: aws.Int64(1), - CidrBlock: aws.String("0.0.0.0/0"), - Egress: aws.Bool(true), - }, - { - Protocol: aws.String("6"), - PortRange: &ec2.PortRange{ - From: aws.Int64(443), - To: aws.Int64(443), - }, - RuleAction: aws.String("deny"), - RuleNumber: aws.Int64(2), - CidrBlock: aws.String("0.0.0.0/0"), - Egress: aws.Bool(true), - }, - { - Protocol: aws.String("-1"), - PortRange: &ec2.PortRange{ - From: aws.Int64(443), - To: aws.Int64(443), - }, - RuleAction: aws.String("deny"), - RuleNumber: aws.Int64(2), - CidrBlock: aws.String("0.0.0.0/0"), - Egress: aws.Bool(true), - }, - } - - if !reflect.DeepEqual(expanded, expected) { - t.Fatalf( - "Got:\n\n%#v\n\nExpected:\n\n%#v\n", - expanded, - expected) - } - -} - -func Test_validatePorts(t *testing.T) { - for _, ts := range []struct { - to int64 - from int64 - expected *ExpectedPortPair - wanted bool - }{ - {0, 0, &ExpectedPortPair{0, 0}, true}, - {0, 1, &ExpectedPortPair{0, 0}, false}, - } { - got := ValidPorts(ts.to, ts.from, *ts.expected) - if got != ts.wanted { - t.Fatalf("Got: %t; Expected: %t\n", got, ts.wanted) - } - } -} diff --git a/internal/service/ec2/network_acl_rule.go b/internal/service/ec2/network_acl_rule.go index 98139fe0829e..504ba92bb848 100644 --- a/internal/service/ec2/network_acl_rule.go +++ b/internal/service/ec2/network_acl_rule.go @@ -10,8 +10,8 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/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/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" @@ -22,84 +22,86 @@ func ResourceNetworkACLRule() *schema.Resource { Create: resourceNetworkACLRuleCreate, Read: resourceNetworkACLRuleRead, Delete: resourceNetworkACLRuleDelete, + Importer: &schema.ResourceImporter{ - State: func(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - idParts := strings.Split(d.Id(), ":") - if len(idParts) != 4 || idParts[0] == "" || idParts[1] == "" || idParts[2] == "" || idParts[3] == "" { - return nil, fmt.Errorf("unexpected format of ID (%q), expected NETWORK_ACL_ID:RULE_NUMBER:PROTOCOL:EGRESS", d.Id()) - } - networkAclID := idParts[0] - ruleNumber, err := strconv.Atoi(idParts[1]) - if err != nil { - return nil, err - } - protocol := idParts[2] - egress, err := strconv.ParseBool(idParts[3]) - if err != nil { - return nil, err - } - - d.Set("network_acl_id", networkAclID) - d.Set("rule_number", ruleNumber) - d.Set("egress", egress) - d.SetId(networkAclIdRuleNumberEgressHash(networkAclID, ruleNumber, egress, protocol)) - return []*schema.ResourceData{d}, nil - }, + State: resourceNetworkACLRuleImport, }, Schema: map[string]*schema.Schema{ - "network_acl_id": { - Type: schema.TypeString, - Required: true, + "cidr_block": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ExactlyOneOf: []string{"cidr_block", "ipv6_cidr_block"}, + }, + "egress": { + Type: schema.TypeBool, + Optional: true, ForceNew: true, + Default: false, }, - "rule_number": { + "from_port": { Type: schema.TypeInt, - Required: true, + Optional: true, ForceNew: true, }, - "egress": { - Type: schema.TypeBool, + "icmp_code": { + Type: schema.TypeInt, Optional: true, ForceNew: true, - Default: false, + }, + "icmp_type": { + Type: schema.TypeInt, + Optional: true, + ForceNew: true, + }, + "ipv6_cidr_block": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ExactlyOneOf: []string{"cidr_block", "ipv6_cidr_block"}, + }, + "network_acl_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, }, "protocol": { Type: schema.TypeString, Required: true, ForceNew: true, DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { - pi := protocolIntegers() - if val, ok := pi[old]; ok { - old = strconv.Itoa(val) + if v, ok := ianaProtocolAToI[old]; ok { + old = strconv.Itoa(v) } - if val, ok := pi[new]; ok { - new = strconv.Itoa(val) + if v, ok := ianaProtocolAToI[new]; ok { + new = strconv.Itoa(v) } return old == new }, + ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { + _, err := networkACLProtocolNumber(v.(string)) + + if err != nil { + errors = append(errors, fmt.Errorf("%q : %w", k, err)) + } + + return + }, }, "rule_action": { Type: schema.TypeString, Required: true, ForceNew: true, + DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { + return strings.EqualFold(old, new) + }, + ValidateFunc: validation.StringInSlice(ec2.RuleAction_Values(), true), }, - "cidr_block": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - ConflictsWith: []string{"ipv6_cidr_block"}, - }, - "ipv6_cidr_block": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - ConflictsWith: []string{"cidr_block"}, - }, - "from_port": { + "rule_number": { Type: schema.TypeInt, - Optional: true, + Required: true, ForceNew: true, }, "to_port": { @@ -107,179 +109,114 @@ func ResourceNetworkACLRule() *schema.Resource { Optional: true, ForceNew: true, }, - "icmp_type": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - ValidateFunc: ValidICMPArgumentValue, - }, - "icmp_code": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - ValidateFunc: ValidICMPArgumentValue, - }, }, } } func resourceNetworkACLRuleCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn - egress := d.Get("egress").(bool) - networkAclID := d.Get("network_acl_id").(string) - ruleNumber := d.Get("rule_number").(int) protocol := d.Get("protocol").(string) - p, protocolErr := strconv.Atoi(protocol) - if protocolErr != nil { - var ok bool - p, ok = protocolIntegers()[protocol] - if !ok { - return fmt.Errorf("Invalid Protocol %s for rule %d", protocol, d.Get("rule_number").(int)) - } + protocolNumber, err := networkACLProtocolNumber(protocol) + + if err != nil { + return err } - log.Printf("[INFO] Transformed Protocol %s into %d", protocol, p) - params := &ec2.CreateNetworkAclEntryInput{ - NetworkAclId: aws.String(networkAclID), + egress := d.Get("egress").(bool) + naclID := d.Get("network_acl_id").(string) + ruleNumber := d.Get("rule_number").(int) + + input := &ec2.CreateNetworkAclEntryInput{ Egress: aws.Bool(egress), - RuleNumber: aws.Int64(int64(ruleNumber)), - Protocol: aws.String(strconv.Itoa(p)), - RuleAction: aws.String(d.Get("rule_action").(string)), + NetworkAclId: aws.String(naclID), PortRange: &ec2.PortRange{ From: aws.Int64(int64(d.Get("from_port").(int))), To: aws.Int64(int64(d.Get("to_port").(int))), }, + Protocol: aws.String(strconv.Itoa(protocolNumber)), + RuleAction: aws.String(d.Get("rule_action").(string)), + RuleNumber: aws.Int64(int64(ruleNumber)), } - cidr, hasCidr := d.GetOk("cidr_block") - ipv6Cidr, hasIpv6Cidr := d.GetOk("ipv6_cidr_block") - - if !hasCidr && !hasIpv6Cidr { - return fmt.Errorf("Either `cidr_block` or `ipv6_cidr_block` must be defined") + if v, ok := d.GetOk("cidr_block"); ok { + input.CidrBlock = aws.String(v.(string)) } - if hasCidr { - params.CidrBlock = aws.String(cidr.(string)) - } - - if hasIpv6Cidr { - params.Ipv6CidrBlock = aws.String(ipv6Cidr.(string)) + if v, ok := d.GetOk("ipv6_cidr_block"); ok { + input.Ipv6CidrBlock = aws.String(v.(string)) } // Specify additional required fields for ICMP. For the list // of ICMP codes and types, see: https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml - if p == 1 || p == 58 { - params.IcmpTypeCode = &ec2.IcmpTypeCode{} - if v, ok := d.GetOk("icmp_type"); ok { - icmpType, err := strconv.Atoi(v.(string)) - if err != nil { - return fmt.Errorf("Unable to parse ICMP type %s for rule %d", v, ruleNumber) - } - params.IcmpTypeCode.Type = aws.Int64(int64(icmpType)) - log.Printf("[DEBUG] Got ICMP type %d for rule %d", icmpType, ruleNumber) - } - if v, ok := d.GetOk("icmp_code"); ok { - icmpCode, err := strconv.Atoi(v.(string)) - if err != nil { - return fmt.Errorf("Unable to parse ICMP code %s for rule %d", v, ruleNumber) - } - params.IcmpTypeCode.Code = aws.Int64(int64(icmpCode)) - log.Printf("[DEBUG] Got ICMP code %d for rule %d", icmpCode, ruleNumber) + if protocolNumber == 1 || protocolNumber == 58 { + input.IcmpTypeCode = &ec2.IcmpTypeCode{ + Code: aws.Int64(int64(d.Get("icmp_code").(int))), + Type: aws.Int64(int64(d.Get("icmp_type").(int))), } } - log.Printf("[INFO] Creating Network Acl Rule: %d (%t)", ruleNumber, egress) - _, err := conn.CreateNetworkAclEntry(params) + log.Printf("[DEBUG] Creating EC2 Network ACL Rule: %s", input) + _, err = conn.CreateNetworkAclEntry(input) if err != nil { - return fmt.Errorf("error creating Network ACL (%s) Egress (%t) Rule (%d): %w", networkAclID, egress, ruleNumber, err) + return fmt.Errorf("error creating EC2 Network ACL (%s) Rule (egress:%t)(%d): %w", naclID, egress, ruleNumber, err) } - d.SetId(networkAclIdRuleNumberEgressHash(networkAclID, ruleNumber, egress, d.Get("protocol").(string))) + d.SetId(NetworkACLRuleCreateResourceID(naclID, ruleNumber, egress, protocol)) return resourceNetworkACLRuleRead(d, meta) } func resourceNetworkACLRuleRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn + egress := d.Get("egress").(bool) - networkAclID := d.Get("network_acl_id").(string) + naclID := d.Get("network_acl_id").(string) ruleNumber := d.Get("rule_number").(int) - var resp *ec2.NetworkAclEntry + outputRaw, err := tfresource.RetryWhenNewResourceNotFound(PropagationTimeout, func() (interface{}, error) { + return FindNetworkACLEntryByThreePartKey(conn, naclID, egress, ruleNumber) + }, d.IsNewResource()) - err := resource.Retry(NetworkACLEntryPropagationTimeout, func() *resource.RetryError { - var err error - - resp, err = FindNetworkACLEntry(conn, networkAclID, egress, ruleNumber) - - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, "InvalidNetworkAclID.NotFound") { - return resource.RetryableError(err) - } - - if err != nil { - return resource.NonRetryableError(err) - } - - if d.IsNewResource() && resp == nil { - return resource.RetryableError(&resource.NotFoundError{ - LastError: fmt.Errorf("EC2 Network ACL (%s) Egress (%t) Rule (%d) not found", networkAclID, egress, ruleNumber), - }) - } - - return nil - }) - - if tfresource.TimedOut(err) { - resp, err = FindNetworkACLEntry(conn, networkAclID, egress, ruleNumber) - } - - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, "InvalidNetworkAclID.NotFound") { - log.Printf("[WARN] EC2 Network ACL (%s) not found, removing from state", networkAclID) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] EC2 Network ACL Rule %s not found, removing from state", d.Id()) d.SetId("") return nil } if err != nil { - return fmt.Errorf("error reading EC2 Network ACL (%s) Egress (%t) Rule (%d): %w", networkAclID, egress, ruleNumber, err) + return fmt.Errorf("error reading EC2 Network ACL Rule (%s): %w", d.Id(), err) } - if resp == nil { - if d.IsNewResource() { - return fmt.Errorf("error reading EC2 Network ACL (%s) Egress (%t) Rule (%d): not found after creation", networkAclID, egress, ruleNumber) - } + naclEntry := outputRaw.(*ec2.NetworkAclEntry) - log.Printf("[WARN] EC2 Network ACL (%s) Egress (%t) Rule (%d) not found, removing from state", networkAclID, egress, ruleNumber) - d.SetId("") - return nil - } - - d.Set("rule_number", resp.RuleNumber) - d.Set("cidr_block", resp.CidrBlock) - d.Set("ipv6_cidr_block", resp.Ipv6CidrBlock) - d.Set("egress", resp.Egress) - if resp.IcmpTypeCode != nil { - d.Set("icmp_code", strconv.FormatInt(aws.Int64Value(resp.IcmpTypeCode.Code), 10)) - d.Set("icmp_type", strconv.FormatInt(aws.Int64Value(resp.IcmpTypeCode.Type), 10)) + d.Set("cidr_block", naclEntry.CidrBlock) + d.Set("egress", naclEntry.Egress) + d.Set("ipv6_cidr_block", naclEntry.Ipv6CidrBlock) + if naclEntry.IcmpTypeCode != nil { + d.Set("icmp_code", naclEntry.IcmpTypeCode.Code) + d.Set("icmp_type", naclEntry.IcmpTypeCode.Type) } - if resp.PortRange != nil { - d.Set("from_port", resp.PortRange.From) - d.Set("to_port", resp.PortRange.To) + if naclEntry.PortRange != nil { + d.Set("from_port", naclEntry.PortRange.From) + d.Set("to_port", naclEntry.PortRange.To) } + d.Set("rule_action", naclEntry.RuleAction) + d.Set("rule_number", naclEntry.RuleNumber) - d.Set("rule_action", resp.RuleAction) + if v := aws.StringValue(naclEntry.Protocol); v != "" { + // The AWS network ACL API only speaks protocol numbers, and + // that's all we record. + protocolNumber, err := networkACLProtocolNumber(v) - p, protocolErr := strconv.Atoi(*resp.Protocol) - log.Printf("[INFO] Converting the protocol %v", p) - if protocolErr == nil { - var ok bool - protocol, ok := protocolStrings(protocolIntegers())[p] - if !ok { - return fmt.Errorf("Invalid Protocol %s for rule %d", *resp.Protocol, d.Get("rule_number").(int)) + if err != nil { + return err } - log.Printf("[INFO] Transformed Protocol %s back into %s", *resp.Protocol, protocol) - d.Set("protocol", protocol) + + d.Set("protocol", strconv.Itoa(protocolNumber)) + } else { + d.Set("protocol", nil) } return nil @@ -288,35 +225,60 @@ func resourceNetworkACLRuleRead(d *schema.ResourceData, meta interface{}) error func resourceNetworkACLRuleDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).EC2Conn - params := &ec2.DeleteNetworkAclEntryInput{ + log.Printf("[INFO] Deleting EC2 Network ACL Rule: %s", d.Id()) + _, err := conn.DeleteNetworkAclEntry(&ec2.DeleteNetworkAclEntryInput{ + Egress: aws.Bool(d.Get("egress").(bool)), NetworkAclId: aws.String(d.Get("network_acl_id").(string)), RuleNumber: aws.Int64(int64(d.Get("rule_number").(int))), - Egress: aws.Bool(d.Get("egress").(bool)), + }) + + if tfawserr.ErrCodeEquals(err, ErrCodeInvalidNetworkAclEntryNotFound) { + return nil } - log.Printf("[INFO] Deleting Network Acl Rule: %s", d.Id()) - _, err := conn.DeleteNetworkAclEntry(params) if err != nil { - return fmt.Errorf("Error Deleting Network Acl Rule: %s", err.Error()) + return fmt.Errorf("error deleting EC2 Network ACL Rule (%s): %w", d.Id(), err) } return nil } -func networkAclIdRuleNumberEgressHash(networkAclId string, ruleNumber int, egress bool, protocol string) string { +func resourceNetworkACLRuleImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { + parts := strings.Split(d.Id(), NetworkACLRuleImportIDSeparator) + + if len(parts) != 4 || parts[0] == "" || parts[1] == "" || parts[2] == "" || parts[3] == "" { + return nil, fmt.Errorf("unexpected format of ID (%[1]s), expected NETWORK_ACL_ID%[2]sRULE_NUMBER%[2]sPROTOCOL%[2]sEGRESS", d.Id(), NetworkACLRuleImportIDSeparator) + } + + naclID := parts[0] + ruleNumber, err := strconv.Atoi(parts[1]) + + if err != nil { + return nil, err + } + + protocol := parts[2] + egress, err := strconv.ParseBool(parts[3]) + + if err != nil { + return nil, err + } + + d.SetId(NetworkACLRuleCreateResourceID(naclID, ruleNumber, egress, protocol)) + d.Set("egress", egress) + d.Set("network_acl_id", naclID) + d.Set("rule_number", ruleNumber) + + return []*schema.ResourceData{d}, nil +} + +const NetworkACLRuleImportIDSeparator = ":" + +func NetworkACLRuleCreateResourceID(naclID string, ruleNumber int, egress bool, protocol string) string { var buf bytes.Buffer - buf.WriteString(fmt.Sprintf("%s-", networkAclId)) + buf.WriteString(fmt.Sprintf("%s-", naclID)) buf.WriteString(fmt.Sprintf("%d-", ruleNumber)) buf.WriteString(fmt.Sprintf("%t-", egress)) buf.WriteString(fmt.Sprintf("%s-", protocol)) return fmt.Sprintf("nacl-%d", create.StringHashcode(buf.String())) } - -func ValidICMPArgumentValue(v interface{}, k string) (ws []string, errors []error) { - value := v.(string) - _, err := strconv.Atoi(value) - if len(value) == 0 || err != nil { - errors = append(errors, fmt.Errorf("%q must be an integer value: %q", k, value)) - } - return -} diff --git a/internal/service/ec2/network_acl_rule_test.go b/internal/service/ec2/network_acl_rule_test.go index 21feb838ea07..c361379a863d 100644 --- a/internal/service/ec2/network_acl_rule_test.go +++ b/internal/service/ec2/network_acl_rule_test.go @@ -4,10 +4,9 @@ import ( "fmt" "regexp" "strconv" + "strings" "testing" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -15,9 +14,15 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfec2 "github.com/hashicorp/terraform-provider-aws/internal/service/ec2" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccEC2NetworkACLRule_basic(t *testing.T) { + resource1Name := "aws_network_acl_rule.test1" + resource2Name := "aws_network_acl_rule.test2" + resource3Name := "aws_network_acl_rule.test3" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), @@ -25,29 +30,56 @@ func TestAccEC2NetworkACLRule_basic(t *testing.T) { CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleBasicConfig, - Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLRuleExists("aws_network_acl_rule.baz"), - testAccCheckNetworkACLRuleExists("aws_network_acl_rule.qux"), - testAccCheckNetworkACLRuleExists("aws_network_acl_rule.wibble"), + Config: testAccNetworkACLRuleConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLRuleExists(resource1Name), + testAccCheckNetworkACLRuleExists(resource2Name), + testAccCheckNetworkACLRuleExists(resource3Name), + + resource.TestCheckResourceAttr(resource1Name, "cidr_block", "0.0.0.0/0"), + resource.TestCheckResourceAttr(resource1Name, "egress", "false"), + resource.TestCheckResourceAttr(resource1Name, "from_port", "22"), + resource.TestCheckResourceAttr(resource1Name, "ipv6_cidr_block", ""), + resource.TestCheckResourceAttr(resource1Name, "protocol", "6"), + resource.TestCheckResourceAttr(resource1Name, "rule_action", "allow"), + resource.TestCheckResourceAttr(resource1Name, "rule_number", "200"), + resource.TestCheckResourceAttr(resource1Name, "to_port", "22"), + + resource.TestCheckResourceAttr(resource2Name, "cidr_block", "0.0.0.0/0"), + resource.TestCheckResourceAttr(resource2Name, "egress", "false"), + resource.TestCheckResourceAttr(resource2Name, "icmp_code", "-1"), + resource.TestCheckResourceAttr(resource2Name, "icmp_type", "0"), + resource.TestCheckResourceAttr(resource2Name, "ipv6_cidr_block", ""), + resource.TestCheckResourceAttr(resource2Name, "protocol", "1"), + resource.TestCheckResourceAttr(resource2Name, "rule_action", "allow"), + resource.TestCheckResourceAttr(resource2Name, "rule_number", "300"), + + resource.TestCheckResourceAttr(resource3Name, "cidr_block", "0.0.0.0/0"), + resource.TestCheckResourceAttr(resource3Name, "egress", "false"), + resource.TestCheckResourceAttr(resource3Name, "icmp_code", "-1"), + resource.TestCheckResourceAttr(resource3Name, "icmp_type", "-1"), + resource.TestCheckResourceAttr(resource3Name, "ipv6_cidr_block", ""), + resource.TestCheckResourceAttr(resource3Name, "protocol", "1"), + resource.TestCheckResourceAttr(resource3Name, "rule_action", "allow"), + resource.TestCheckResourceAttr(resource3Name, "rule_number", "400"), ), }, { - ResourceName: "aws_network_acl_rule.baz", + ResourceName: resource1Name, ImportState: true, - ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc("aws_network_acl_rule.baz", "tcp"), + ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc(resource1Name, "tcp"), ImportStateVerify: true, }, { - ResourceName: "aws_network_acl_rule.qux", + ResourceName: resource2Name, ImportState: true, - ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc("aws_network_acl_rule.qux", "icmp"), + ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc(resource2Name, "icmp"), ImportStateVerify: true, }, { - ResourceName: "aws_network_acl_rule.wibble", + ResourceName: resource3Name, ImportState: true, - ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc("aws_network_acl_rule.wibble", "icmp"), + ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc(resource3Name, "icmp"), ImportStateVerify: true, }, }, @@ -55,6 +87,9 @@ func TestAccEC2NetworkACLRule_basic(t *testing.T) { } func TestAccEC2NetworkACLRule_disappears(t *testing.T) { + resourceName := "aws_network_acl_rule.test1" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), @@ -62,10 +97,10 @@ func TestAccEC2NetworkACLRule_disappears(t *testing.T) { CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleBasicConfig, + Config: testAccNetworkACLRuleConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLRuleExists("aws_network_acl_rule.baz"), - testAccCheckNetworkACLRuleDelete("aws_network_acl_rule.baz"), + testAccCheckNetworkACLRuleExists(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACLRule(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -73,7 +108,10 @@ func TestAccEC2NetworkACLRule_disappears(t *testing.T) { }) } -func TestAccEC2NetworkACLRule_Disappears_ingressEgressSameNumber(t *testing.T) { +func TestAccEC2NetworkACLRule_Disappears_networkACL(t *testing.T) { + resourceName := "aws_network_acl_rule.test1" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), @@ -81,10 +119,10 @@ func TestAccEC2NetworkACLRule_Disappears_ingressEgressSameNumber(t *testing.T) { CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleIngressEgressSameNumberMissing, + Config: testAccNetworkACLRuleConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLRuleExists("aws_network_acl_rule.baz"), - testAccCheckNetworkACLRuleDelete("aws_network_acl_rule.baz"), + testAccCheckNetworkACLRuleExists(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACL(), "aws_network_acl.test"), ), ExpectNonEmptyPlan: true, }, @@ -92,9 +130,9 @@ func TestAccEC2NetworkACLRule_Disappears_ingressEgressSameNumber(t *testing.T) { }) } -func TestAccEC2NetworkACLRule_Disappears_networkACL(t *testing.T) { - var networkAcl ec2.NetworkAcl - resourceName := "aws_network_acl.bar" +func TestAccEC2NetworkACLRule_Disappears_ingressEgressSameNumber(t *testing.T) { + resourceName := "aws_network_acl_rule.test1" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -103,10 +141,10 @@ func TestAccEC2NetworkACLRule_Disappears_networkACL(t *testing.T) { CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleBasicConfig, + Config: testAccNetworkACLRuleIngressEgressSameNumberMissingConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACL(), resourceName), + testAccCheckNetworkACLRuleExists(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACLRule(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -114,22 +152,10 @@ func TestAccEC2NetworkACLRule_Disappears_networkACL(t *testing.T) { }) } -func TestAccEC2NetworkACLRule_missingParam(t *testing.T) { - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckNetworkACLRuleDestroy, - Steps: []resource.TestStep{ - { - Config: testAccNetworkACLRuleMissingParam, - ExpectError: regexp.MustCompile("Either `cidr_block` or `ipv6_cidr_block` must be defined"), - }, - }, - }) -} - func TestAccEC2NetworkACLRule_ipv6(t *testing.T) { + resourceName := "aws_network_acl_rule.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), @@ -137,15 +163,23 @@ func TestAccEC2NetworkACLRule_ipv6(t *testing.T) { CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleIPv6Config, - Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLRuleExists("aws_network_acl_rule.baz"), + Config: testAccNetworkACLRuleIPv6Config(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLRuleExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "cidr_block", ""), + resource.TestCheckResourceAttr(resourceName, "egress", "false"), + resource.TestCheckResourceAttr(resourceName, "from_port", "22"), + resource.TestCheckResourceAttr(resourceName, "ipv6_cidr_block", "::/0"), + resource.TestCheckResourceAttr(resourceName, "protocol", "6"), + resource.TestCheckResourceAttr(resourceName, "rule_action", "allow"), + resource.TestCheckResourceAttr(resourceName, "rule_number", "150"), + resource.TestCheckResourceAttr(resourceName, "to_port", "22"), ), }, { - ResourceName: "aws_network_acl_rule.baz", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc("aws_network_acl_rule.baz", "tcp"), + ImportStateIdFunc: testAccNetworkACLRuleImportStateIdFunc(resourceName, "tcp"), ImportStateVerify: true, }, }, @@ -164,8 +198,16 @@ func TestAccEC2NetworkACLRule_ipv6ICMP(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccNetworkACLRuleIPv6ICMPConfig(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckNetworkACLRuleExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "cidr_block", ""), + resource.TestCheckResourceAttr(resourceName, "egress", "false"), + resource.TestCheckResourceAttr(resourceName, "icmp_code", "-1"), + resource.TestCheckResourceAttr(resourceName, "icmp_type", "-1"), + resource.TestCheckResourceAttr(resourceName, "ipv6_cidr_block", "::/0"), + resource.TestCheckResourceAttr(resourceName, "protocol", "58"), + resource.TestCheckResourceAttr(resourceName, "rule_action", "allow"), + resource.TestCheckResourceAttr(resourceName, "rule_number", "150"), ), }, { @@ -180,7 +222,8 @@ func TestAccEC2NetworkACLRule_ipv6ICMP(t *testing.T) { // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/6710 func TestAccEC2NetworkACLRule_ipv6VPCAssignGeneratedIPv6CIDRBlockUpdate(t *testing.T) { - var vpc ec2.Vpc + var v ec2.Vpc + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) vpcResourceName := "aws_vpc.test" resourceName := "aws_network_acl_rule.test" @@ -191,20 +234,20 @@ func TestAccEC2NetworkACLRule_ipv6VPCAssignGeneratedIPv6CIDRBlockUpdate(t *testi CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleIPv6VPCNotAssignGeneratedIpv6CIDRBlockUpdateConfig(), + Config: testAccNetworkACLRuleIPv6VPCNotAssignGeneratedIpv6CIDRBlockUpdateConfig(rName), Check: resource.ComposeTestCheckFunc( - acctest.CheckVPCExists(vpcResourceName, &vpc), + acctest.CheckVPCExists(vpcResourceName, &v), resource.TestCheckResourceAttr(vpcResourceName, "assign_generated_ipv6_cidr_block", "false"), resource.TestCheckResourceAttr(vpcResourceName, "ipv6_cidr_block", ""), ), }, { - Config: testAccNetworkACLRuleIPv6VPCAssignGeneratedIpv6CIDRBlockUpdateConfig(), + Config: testAccNetworkACLRuleIPv6VPCAssignGeneratedIpv6CIDRBlockUpdateConfig(rName), Check: resource.ComposeTestCheckFunc( - acctest.CheckVPCExists(vpcResourceName, &vpc), + acctest.CheckVPCExists(vpcResourceName, &v), + testAccCheckNetworkACLRuleExists(resourceName), resource.TestCheckResourceAttr(vpcResourceName, "assign_generated_ipv6_cidr_block", "true"), resource.TestMatchResourceAttr(vpcResourceName, "ipv6_cidr_block", regexp.MustCompile(`/56$`)), - testAccCheckNetworkACLRuleExists(resourceName), ), }, { @@ -218,6 +261,9 @@ func TestAccEC2NetworkACLRule_ipv6VPCAssignGeneratedIPv6CIDRBlockUpdate(t *testi } func TestAccEC2NetworkACLRule_allProtocol(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_network_acl_rule.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), @@ -225,18 +271,31 @@ func TestAccEC2NetworkACLRule_allProtocol(t *testing.T) { CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleAllProtocolConfig, - ExpectNonEmptyPlan: false, + Config: testAccNetworkACLRuleAllProtocolConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLRuleExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "cidr_block", "0.0.0.0/0"), + resource.TestCheckResourceAttr(resourceName, "egress", "false"), + resource.TestCheckResourceAttr(resourceName, "from_port", "22"), + resource.TestCheckResourceAttr(resourceName, "ipv6_cidr_block", ""), + resource.TestCheckResourceAttr(resourceName, "protocol", "-1"), + resource.TestCheckResourceAttr(resourceName, "rule_action", "allow"), + resource.TestCheckResourceAttr(resourceName, "rule_number", "150"), + resource.TestCheckResourceAttr(resourceName, "to_port", "22"), + ), }, { - Config: testAccNetworkACLRuleAllProtocolNoRealUpdateConfig, - ExpectNonEmptyPlan: false, + Config: testAccNetworkACLRuleAllProtocolNoRealUpdateConfig(rName), + PlanOnly: true, }, }, }) } func TestAccEC2NetworkACLRule_tcpProtocol(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_network_acl_rule.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), @@ -244,96 +303,60 @@ func TestAccEC2NetworkACLRule_tcpProtocol(t *testing.T) { CheckDestroy: testAccCheckNetworkACLRuleDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLRuleTCPProtocolConfig, - ExpectNonEmptyPlan: false, + Config: testAccNetworkACLRuleTCPProtocolConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLRuleExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "cidr_block", "0.0.0.0/0"), + resource.TestCheckResourceAttr(resourceName, "egress", "true"), + resource.TestCheckResourceAttr(resourceName, "from_port", "22"), + resource.TestCheckResourceAttr(resourceName, "ipv6_cidr_block", ""), + resource.TestCheckResourceAttr(resourceName, "protocol", "6"), + resource.TestCheckResourceAttr(resourceName, "rule_action", "deny"), + resource.TestCheckResourceAttr(resourceName, "rule_number", "150"), + resource.TestCheckResourceAttr(resourceName, "to_port", "22"), + ), }, { - Config: testAccNetworkACLRuleTCPProtocolNoRealUpdateConfig, - ExpectNonEmptyPlan: false, + Config: testAccNetworkACLRuleTCPProtocolNoRealUpdateConfig(rName), + PlanOnly: true, }, }, }) } -func TestNetworkACLRule_validateICMPArgumentValue(t *testing.T) { - type testCases struct { - Value string - ErrCount int - } - - invalidCases := []testCases{ - { - Value: "", - ErrCount: 1, - }, - { - Value: "not-a-number", - ErrCount: 1, - }, - { - Value: "1.0", - ErrCount: 1, - }, - } +func testAccCheckNetworkACLRuleDestroy(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - for _, tc := range invalidCases { - _, errors := tfec2.ValidICMPArgumentValue(tc.Value, "icmp_type") - if len(errors) != tc.ErrCount { - t.Fatalf("Expected %q to trigger a validation error.", tc.Value) + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_network_acl_rule" { + continue } - } - validCases := []testCases{ - { - Value: "0", - ErrCount: 0, - }, - { - Value: "-1", - ErrCount: 0, - }, - { - Value: "1", - ErrCount: 0, - }, - } + egress, err := strconv.ParseBool(rs.Primary.Attributes["egress"]) - for _, tc := range validCases { - _, errors := tfec2.ValidICMPArgumentValue(tc.Value, "icmp_type") - if len(errors) != tc.ErrCount { - t.Fatalf("Expected %q not to trigger a validation error.", tc.Value) + if err != nil { + return err } - } -} + naclID := rs.Primary.Attributes["network_acl_id"] -func testAccCheckNetworkACLRuleDestroy(s *terraform.State) error { - for _, rs := range s.RootModule().Resources { - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - if rs.Type != "aws_network_acl_rule" { - continue - } + ruleNumber, err := strconv.Atoi(rs.Primary.Attributes["rule_number"]) - req := &ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(rs.Primary.ID)}, - } - resp, err := conn.DescribeNetworkAcls(req) - if err == nil { - if len(resp.NetworkAcls) > 0 && *resp.NetworkAcls[0].NetworkAclId == rs.Primary.ID { - networkAcl := resp.NetworkAcls[0] - if networkAcl.Entries != nil { - return fmt.Errorf("Network ACL Entries still exist") - } - } + if err != nil { + return err } - ec2err, ok := err.(awserr.Error) - if !ok { - return err + _, err = tfec2.FindNetworkACLEntryByThreePartKey(conn, naclID, egress, ruleNumber) + + if tfresource.NotFound(err) { + continue } - if ec2err.Code() != "InvalidNetworkAclID.NotFound" { + + if err != nil { return err } + + return fmt.Errorf("EC2 Network ACL Rule %s still exists", rs.Primary.ID) } return nil @@ -348,89 +371,53 @@ func testAccCheckNetworkACLRuleExists(n string) resource.TestCheckFunc { } if rs.Primary.ID == "" { - return fmt.Errorf("No Network ACL Rule Id is set") + return fmt.Errorf("No EC2 Network ACL Rule ID is set: %s", n) } - req := &ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(rs.Primary.Attributes["network_acl_id"])}, - } - resp, err := conn.DescribeNetworkAcls(req) - if err != nil { - return err - } - if len(resp.NetworkAcls) != 1 { - return fmt.Errorf("Network ACL not found") - } egress, err := strconv.ParseBool(rs.Primary.Attributes["egress"]) + if err != nil { return err } - ruleNo, err := strconv.ParseInt(rs.Primary.Attributes["rule_number"], 10, 64) - if err != nil { - return err - } - for _, e := range resp.NetworkAcls[0].Entries { - if *e.RuleNumber == ruleNo && *e.Egress == egress { - return nil - } - } - return fmt.Errorf("Entry not found: %s", resp.NetworkAcls[0]) - } -} -func testAccCheckNetworkACLRuleDelete(n string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[n] - if !ok { - return fmt.Errorf("Not found: %s", n) - } + naclID := rs.Primary.Attributes["network_acl_id"] - if rs.Primary.ID == "" { - return fmt.Errorf("No Network ACL Rule Id is set") - } + ruleNumber, err := strconv.Atoi(rs.Primary.Attributes["rule_number"]) - egress, err := strconv.ParseBool(rs.Primary.Attributes["egress"]) - if err != nil { - return err - } - ruleNo, err := strconv.ParseInt(rs.Primary.Attributes["rule_number"], 10, 64) if err != nil { return err } - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - _, err = conn.DeleteNetworkAclEntry(&ec2.DeleteNetworkAclEntryInput{ - NetworkAclId: aws.String(rs.Primary.Attributes["network_acl_id"]), - RuleNumber: aws.Int64(ruleNo), - Egress: aws.Bool(egress), - }) + _, err = tfec2.FindNetworkACLEntryByThreePartKey(conn, naclID, egress, ruleNumber) + if err != nil { - return fmt.Errorf("Error deleting Network ACL Rule (%s) in testAccCheckNetworkACLRuleDelete: %s", rs.Primary.ID, err) + return err } return nil } } -const testAccNetworkACLRuleBasicConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLRuleConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "terraform-testacc-network-acl-rule-basic" + Name = %[1]q } } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-acl-rule-basic" + Name = %[1]q } } -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test1" { + network_acl_id = aws_network_acl.test.id rule_number = 200 egress = false protocol = "tcp" @@ -440,8 +427,8 @@ resource "aws_network_acl_rule" "baz" { to_port = 22 } -resource "aws_network_acl_rule" "qux" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test2" { + network_acl_id = aws_network_acl.test.id rule_number = 300 protocol = "icmp" rule_action = "allow" @@ -450,8 +437,8 @@ resource "aws_network_acl_rule" "qux" { icmp_code = -1 } -resource "aws_network_acl_rule" "wibble" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test3" { + network_acl_id = aws_network_acl.test.id rule_number = 400 protocol = "icmp" rule_action = "allow" @@ -459,107 +446,91 @@ resource "aws_network_acl_rule" "wibble" { icmp_type = -1 icmp_code = -1 } -` +`, rName) +} -const testAccNetworkACLRuleMissingParam = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLRuleAllProtocolNoRealUpdateConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "terraform-testacc-network-acl-rule-missing-param" + Name = %[1]q } } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-acl-rule-missing-param" + Name = %[1]q } } -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id - rule_number = 200 +resource "aws_network_acl_rule" "test" { + network_acl_id = aws_network_acl.test.id + rule_number = 150 egress = false - protocol = "tcp" + protocol = "all" rule_action = "allow" + cidr_block = "0.0.0.0/0" from_port = 22 to_port = 22 } -` +`, rName) +} -const testAccNetworkACLRuleAllProtocolNoRealUpdateConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLRuleTCPProtocolNoRealUpdateConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "terraform-testacc-network-acl-rule-all-proto-no-real-upd" + Name = %[1]q } } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-acl-rule-no-real-update" + Name = %[1]q } } -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id - rule_number = 150 - egress = false - protocol = "all" - rule_action = "allow" - cidr_block = "0.0.0.0/0" - from_port = 22 - to_port = 22 -} -` - -const testAccNetworkACLRuleTCPProtocolNoRealUpdateConfig = ` -resource "aws_vpc" "foo" { - cidr_block = "10.3.0.0/16" - - tags = { - Name = "testAccNetworkACLRuleTCPProtocolNoRealUpdateConfig" - } -} -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id -} -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test" { + network_acl_id = aws_network_acl.test.id rule_number = 150 - egress = false + egress = true protocol = "tcp" - rule_action = "allow" + rule_action = "deny" cidr_block = "0.0.0.0/0" from_port = 22 to_port = 22 } -` +`, rName) +} -const testAccNetworkACLRuleAllProtocolConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLRuleAllProtocolConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "terraform-testacc-network-acl-rule-proto" + Name = %[1]q } } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-acl-rule-all-protocol" + Name = %[1]q } } -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test" { + network_acl_id = aws_network_acl.test.id rule_number = 150 egress = false protocol = "-1" @@ -568,50 +539,60 @@ resource "aws_network_acl_rule" "baz" { from_port = 22 to_port = 22 } -` +`, rName) +} -const testAccNetworkACLRuleTCPProtocolConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLRuleTCPProtocolConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "testAccNetworkACLRuleTCPProtocolConfig" + Name = %[1]q } } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id + +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id + + tags = { + Name = %[1]q + } } -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id + +resource "aws_network_acl_rule" "test" { + network_acl_id = aws_network_acl.test.id rule_number = 150 - egress = false + egress = true protocol = "6" - rule_action = "allow" + rule_action = "deny" cidr_block = "0.0.0.0/0" from_port = 22 to_port = 22 } -` +`, rName) +} -const testAccNetworkACLRuleIPv6Config = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLRuleIPv6Config(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "terraform-testacc-network-acl-rule-ipv6" + Name = %[1]q } } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-acl-rule-ipv6" + Name = %[1]q } } -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test" { + network_acl_id = aws_network_acl.test.id rule_number = 150 egress = false protocol = "tcp" @@ -620,27 +601,29 @@ resource "aws_network_acl_rule" "baz" { from_port = 22 to_port = 22 } -` +`, rName) +} -const testAccNetworkACLRuleIngressEgressSameNumberMissing = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLRuleIngressEgressSameNumberMissingConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "terraform-testacc-network-acl-rule-ingress-egress-same-number-missing" + Name = %[1]q } } -resource "aws_network_acl" "bar" { - vpc_id = aws_vpc.foo.id +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-acl-rule-basic" + Name = %[1]q } } -resource "aws_network_acl_rule" "baz" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test1" { + network_acl_id = aws_network_acl.test.id rule_number = 100 egress = false protocol = "tcp" @@ -650,8 +633,8 @@ resource "aws_network_acl_rule" "baz" { to_port = 22 } -resource "aws_network_acl_rule" "qux" { - network_acl_id = aws_network_acl.bar.id +resource "aws_network_acl_rule" "test2" { + network_acl_id = aws_network_acl.test.id rule_number = 100 egress = true protocol = "tcp" @@ -660,7 +643,8 @@ resource "aws_network_acl_rule" "qux" { from_port = 22 to_port = 22 } -` +`, rName) +} func testAccNetworkACLRuleIPv6ICMPConfig(rName string) string { return fmt.Sprintf(` @@ -668,7 +652,7 @@ resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = %q + Name = %[1]q } } @@ -676,7 +660,7 @@ resource "aws_network_acl" "test" { vpc_id = aws_vpc.test.id tags = { - Name = %q + Name = %[1]q } } @@ -692,14 +676,14 @@ resource "aws_network_acl_rule" "test" { `, rName, rName) } -func testAccNetworkACLRuleIPv6VPCAssignGeneratedIpv6CIDRBlockUpdateConfig() string { - return ` +func testAccNetworkACLRuleIPv6VPCAssignGeneratedIpv6CIDRBlockUpdateConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { assign_generated_ipv6_cidr_block = true cidr_block = "10.3.0.0/16" tags = { - Name = "tf-acc-test-network-acl-rule-ipv6-enabled" + Name = %[1]q } } @@ -707,7 +691,7 @@ resource "aws_network_acl" "test" { vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-test-network-acl-rule-ipv6-enabled" + Name = %[1]q } } @@ -720,17 +704,17 @@ resource "aws_network_acl_rule" "test" { rule_number = 150 to_port = 22 } -` +`, rName) } -func testAccNetworkACLRuleIPv6VPCNotAssignGeneratedIpv6CIDRBlockUpdateConfig() string { - return ` +func testAccNetworkACLRuleIPv6VPCNotAssignGeneratedIpv6CIDRBlockUpdateConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { assign_generated_ipv6_cidr_block = false cidr_block = "10.3.0.0/16" tags = { - Name = "tf-acc-test-network-acl-rule-ipv6-not-enabled" + Name = %[1]q } } @@ -738,18 +722,19 @@ resource "aws_network_acl" "test" { vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-test-network-acl-rule-ipv6-not-enabled" + Name = %[1]q } -}` +}`, rName) } func testAccNetworkACLRuleImportStateIdFunc(resourceName, resourceProtocol string) resource.ImportStateIdFunc { return func(s *terraform.State) (string, error) { rs, ok := s.RootModule().Resources[resourceName] if !ok { - return "", fmt.Errorf("not found: %s", resourceName) + return "", fmt.Errorf("Not found: %s", resourceName) } - networkAclId := rs.Primary.Attributes["network_acl_id"] + + naclID := rs.Primary.Attributes["network_acl_id"] ruleNumber := rs.Primary.Attributes["rule_number"] protocol := rs.Primary.Attributes["protocol"] // Ensure the resource's ID will be determined from the original protocol value set in the resource's config @@ -757,6 +742,7 @@ func testAccNetworkACLRuleImportStateIdFunc(resourceName, resourceProtocol strin protocol = resourceProtocol } egress := rs.Primary.Attributes["egress"] - return fmt.Sprintf("%s:%s:%s:%s", networkAclId, ruleNumber, protocol, egress), nil + + return strings.Join([]string{naclID, ruleNumber, protocol, egress}, tfec2.NetworkACLRuleImportIDSeparator), nil } } diff --git a/internal/service/ec2/network_acl_test.go b/internal/service/ec2/network_acl_test.go index 713a494e4214..30bf5d88a478 100644 --- a/internal/service/ec2/network_acl_test.go +++ b/internal/service/ec2/network_acl_test.go @@ -5,8 +5,6 @@ import ( "regexp" "testing" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -14,11 +12,14 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfec2 "github.com/hashicorp/terraform-provider-aws/internal/service/ec2" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccEC2NetworkACL_basic(t *testing.T) { + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" - var networkAcl ec2.NetworkAcl + vpcResourceName := "aws_vpc.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -27,11 +28,16 @@ func TestAccEC2NetworkACL_basic(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLEgressNIngressConfig, + Config: testAccNetworkACLConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`network-acl/acl-.+`)), + resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), + acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "0"), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttrPair(resourceName, "vpc_id", vpcResourceName, "id"), ), }, { @@ -43,10 +49,33 @@ func TestAccEC2NetworkACL_basic(t *testing.T) { }) } +func TestAccEC2NetworkACL_disappears(t *testing.T) { + var v ec2.NetworkAcl + resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckNetworkACLDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetworkACLConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckNetworkACLExists(resourceName, &v), + acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACL(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccEC2NetworkACL_tags(t *testing.T) { + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - var networkAcl ec2.NetworkAcl resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -57,7 +86,7 @@ func TestAccEC2NetworkACL_tags(t *testing.T) { { Config: testAccNetworkACLTags1Config(rName, "key1", "value1"), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), ), @@ -70,7 +99,7 @@ func TestAccEC2NetworkACL_tags(t *testing.T) { { Config: testAccNetworkACLTags2Config(rName, "key1", "value1updated", "key2", "value2"), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), @@ -79,7 +108,7 @@ func TestAccEC2NetworkACL_tags(t *testing.T) { { Config: testAccNetworkACLTags1Config(rName, "key2", "value2"), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), ), @@ -88,31 +117,10 @@ func TestAccEC2NetworkACL_tags(t *testing.T) { }) } -func TestAccEC2NetworkACL_disappears(t *testing.T) { - var networkAcl ec2.NetworkAcl - resourceName := "aws_network_acl.test" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckNetworkACLDestroy, - Steps: []resource.TestStep{ - { - Config: testAccNetworkACLEgressNIngressConfig, - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - acctest.CheckResourceDisappears(acctest.Provider, tfec2.ResourceNetworkACL(), resourceName), - ), - ExpectNonEmptyPlan: true, - }, - }, - }) -} - func TestAccEC2NetworkACL_Egress_mode(t *testing.T) { - var networkAcl1, networkAcl2, networkAcl3 ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -121,10 +129,9 @@ func TestAccEC2NetworkACL_Egress_mode(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLEgressModeBlocksConfig(), + Config: testAccNetworkACLEgressModeBlocksConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl1), - testAccCheckNetworkACLEgressRuleLength(&networkAcl1, 2), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "egress.#", "2"), ), }, @@ -134,10 +141,9 @@ func TestAccEC2NetworkACL_Egress_mode(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLEgressModeNoBlocksConfig(), + Config: testAccNetworkACLEgressModeNoBlocksConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl2), - testAccCheckNetworkACLEgressRuleLength(&networkAcl2, 2), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "egress.#", "2"), ), }, @@ -147,10 +153,9 @@ func TestAccEC2NetworkACL_Egress_mode(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLEgressModeZeroedConfig(), + Config: testAccNetworkACLEgressModeZeroedConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl3), - testAccCheckNetworkACLEgressRuleLength(&networkAcl3, 0), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "egress.#", "0"), ), }, @@ -164,8 +169,9 @@ func TestAccEC2NetworkACL_Egress_mode(t *testing.T) { } func TestAccEC2NetworkACL_Ingress_mode(t *testing.T) { - var networkAcl1, networkAcl2, networkAcl3 ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -174,10 +180,9 @@ func TestAccEC2NetworkACL_Ingress_mode(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLIngressModeBlocksConfig(), + Config: testAccNetworkACLIngressModeBlocksConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl1), - testIngressRuleLength(&networkAcl1, 2), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "ingress.#", "2"), ), }, @@ -187,10 +192,9 @@ func TestAccEC2NetworkACL_Ingress_mode(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLIngressModeNoBlocksConfig(), + Config: testAccNetworkACLIngressModeNoBlocksConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl2), - testIngressRuleLength(&networkAcl2, 2), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "ingress.#", "2"), ), }, @@ -200,10 +204,9 @@ func TestAccEC2NetworkACL_Ingress_mode(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLIngressModeZeroedConfig(), + Config: testAccNetworkACLIngressModeZeroedConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl3), - testIngressRuleLength(&networkAcl3, 0), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "ingress.#", "0"), ), }, @@ -217,8 +220,9 @@ func TestAccEC2NetworkACL_Ingress_mode(t *testing.T) { } func TestAccEC2NetworkACL_egressAndIngressRules(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -227,9 +231,9 @@ func TestAccEC2NetworkACL_egressAndIngressRules(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLEgressNIngressConfig, + Config: testAccNetworkACLEgressNIngressConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ "protocol": "6", "rule_no": "1", @@ -246,7 +250,6 @@ func TestAccEC2NetworkACL_egressAndIngressRules(t *testing.T) { "action": "allow", "cidr_block": "10.3.0.0/18", }), - acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), ), }, { @@ -259,8 +262,9 @@ func TestAccEC2NetworkACL_egressAndIngressRules(t *testing.T) { } func TestAccEC2NetworkACL_OnlyIngressRules_basic(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -269,9 +273,9 @@ func TestAccEC2NetworkACL_OnlyIngressRules_basic(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLIngressConfig, + Config: testAccNetworkACLIngressConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ "protocol": "6", "rule_no": "2", @@ -280,7 +284,6 @@ func TestAccEC2NetworkACL_OnlyIngressRules_basic(t *testing.T) { "action": "deny", "cidr_block": "10.2.0.0/18", }), - acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), ), }, { @@ -293,8 +296,9 @@ func TestAccEC2NetworkACL_OnlyIngressRules_basic(t *testing.T) { } func TestAccEC2NetworkACL_OnlyIngressRules_update(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -303,10 +307,10 @@ func TestAccEC2NetworkACL_OnlyIngressRules_update(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLIngressConfig, + Config: testAccNetworkACLIngressConfig(resourceName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testIngressRuleLength(&networkAcl, 2), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "2"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ "protocol": "6", "rule_no": "1", @@ -319,7 +323,6 @@ func TestAccEC2NetworkACL_OnlyIngressRules_update(t *testing.T) { "from_port": "443", "rule_no": "2", }), - acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), ), }, { @@ -328,10 +331,10 @@ func TestAccEC2NetworkACL_OnlyIngressRules_update(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLIngressChangeConfig, + Config: testAccNetworkACLIngressChangeConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testIngressRuleLength(&networkAcl, 1), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "1"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ "protocol": "6", "rule_no": "1", @@ -340,7 +343,6 @@ func TestAccEC2NetworkACL_OnlyIngressRules_update(t *testing.T) { "action": "deny", "cidr_block": "10.2.0.0/18", }), - acctest.CheckResourceAttrAccountID(resourceName, "owner_id"), ), }, }, @@ -348,8 +350,9 @@ func TestAccEC2NetworkACL_OnlyIngressRules_update(t *testing.T) { } func TestAccEC2NetworkACL_caseSensitivityNoChanges(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -358,9 +361,9 @@ func TestAccEC2NetworkACL_caseSensitivityNoChanges(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLCaseSensitiveConfig, + Config: testAccNetworkACLCaseSensitiveConfig(rName), Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), ), }, { @@ -373,8 +376,9 @@ func TestAccEC2NetworkACL_caseSensitivityNoChanges(t *testing.T) { } func TestAccEC2NetworkACL_onlyEgressRules(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -383,12 +387,9 @@ func TestAccEC2NetworkACL_onlyEgressRules(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLEgressConfig, + Config: testAccNetworkACLEgressConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), - resource.TestCheckResourceAttr(resourceName, "tags.Name", "tf-acc-acl-egress"), - resource.TestCheckResourceAttr(resourceName, "tags.foo", "bar"), + testAccCheckNetworkACLExists(resourceName, &v), ), }, { @@ -401,8 +402,9 @@ func TestAccEC2NetworkACL_onlyEgressRules(t *testing.T) { } func TestAccEC2NetworkACL_subnetChange(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -411,10 +413,11 @@ func TestAccEC2NetworkACL_subnetChange(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLSubnetConfig, + Config: testAccNetworkACLSubnetConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.old"), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "1"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test1", "id"), ), }, { @@ -423,11 +426,11 @@ func TestAccEC2NetworkACL_subnetChange(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLSubnetChangeConfig, + Config: testAccNetworkACLSubnetChangeConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testAccCheckSubnetIsNotAssociatedWithAcl(resourceName, "aws_subnet.old"), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.new"), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "1"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test2", "id"), ), }, }, @@ -436,18 +439,9 @@ func TestAccEC2NetworkACL_subnetChange(t *testing.T) { } func TestAccEC2NetworkACL_subnets(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" - - checkACLSubnets := func(acl *ec2.NetworkAcl, count int) resource.TestCheckFunc { - return func(*terraform.State) (err error) { - if count != len(acl.Associations) { - return fmt.Errorf("ACL association count does not match, expected %d, got %d", count, len(acl.Associations)) - } - - return nil - } - } + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -456,12 +450,12 @@ func TestAccEC2NetworkACL_subnets(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLSubnet_SubnetIDs, + Config: testAccNetworkACLSubnet_SubnetIDs(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.one"), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.two"), - checkACLSubnets(&networkAcl, 2), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "2"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test1", "id"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test2", "id"), ), }, { @@ -470,13 +464,13 @@ func TestAccEC2NetworkACL_subnets(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLSubnet_SubnetIdsUpdate, + Config: testAccNetworkACLSubnet_SubnetIDsUpdate(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.one"), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.three"), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.four"), - checkACLSubnets(&networkAcl, 3), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "3"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test1", "id"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test3", "id"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test4", "id"), ), }, }, @@ -484,18 +478,9 @@ func TestAccEC2NetworkACL_subnets(t *testing.T) { } func TestAccEC2NetworkACL_subnetsDelete(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" - - checkACLSubnets := func(acl *ec2.NetworkAcl, count int) resource.TestCheckFunc { - return func(*terraform.State) (err error) { - if count != len(acl.Associations) { - return fmt.Errorf("ACL association count does not match, expected %d, got %d", count, len(acl.Associations)) - } - - return nil - } - } + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -504,12 +489,12 @@ func TestAccEC2NetworkACL_subnetsDelete(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLSubnet_SubnetIDs, + Config: testAccNetworkACLSubnet_SubnetIDs(resourceName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.one"), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.two"), - checkACLSubnets(&networkAcl, 2), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "2"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test1", "id"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test2", "id"), ), }, { @@ -518,11 +503,11 @@ func TestAccEC2NetworkACL_subnetsDelete(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccNetworkACLSubnet_SubnetIdsDeleteOne, + Config: testAccNetworkACLSubnet_SubnetIDsDeleteOne(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - testAccCheckSubnetIsAssociatedWithAcl(resourceName, "aws_subnet.one"), - checkACLSubnets(&networkAcl, 1), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "subnet_ids.#", "1"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "subnet_ids.*", "aws_subnet.test1", "id"), ), }, }, @@ -530,8 +515,9 @@ func TestAccEC2NetworkACL_subnetsDelete(t *testing.T) { } func TestAccEC2NetworkACL_ipv6Rules(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -540,9 +526,9 @@ func TestAccEC2NetworkACL_ipv6Rules(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLIPv6Config, + Config: testAccNetworkACLIPv6Config(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), resource.TestCheckResourceAttr( resourceName, "ingress.#", "1"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ @@ -565,9 +551,9 @@ func TestAccEC2NetworkACL_ipv6Rules(t *testing.T) { } func TestAccEC2NetworkACL_ipv6ICMPRules(t *testing.T) { - var networkAcl ec2.NetworkAcl - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -578,7 +564,7 @@ func TestAccEC2NetworkACL_ipv6ICMPRules(t *testing.T) { { Config: testAccNetworkACLIPv6ICMPConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), ), }, }, @@ -586,8 +572,9 @@ func TestAccEC2NetworkACL_ipv6ICMPRules(t *testing.T) { } func TestAccEC2NetworkACL_ipv6VPCRules(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -596,11 +583,10 @@ func TestAccEC2NetworkACL_ipv6VPCRules(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLIPv6VPCConfig, + Config: testAccNetworkACLIPv6VPCConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), - resource.TestCheckResourceAttr( - resourceName, "ingress.#", "1"), + testAccCheckNetworkACLExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "ingress.#", "1"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ingress.*", map[string]string{ "ipv6_cidr_block": "2600:1f16:d1e:9a00::/56", }), @@ -616,8 +602,9 @@ func TestAccEC2NetworkACL_ipv6VPCRules(t *testing.T) { } func TestAccEC2NetworkACL_espProtocol(t *testing.T) { - var networkAcl ec2.NetworkAcl + var v ec2.NetworkAcl resourceName := "aws_network_acl.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -626,9 +613,9 @@ func TestAccEC2NetworkACL_espProtocol(t *testing.T) { CheckDestroy: testAccCheckNetworkACLDestroy, Steps: []resource.TestStep{ { - Config: testAccNetworkACLEsp, + Config: testAccNetworkACLEsp(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckNetworkACLExists(resourceName, &networkAcl), + testAccCheckNetworkACLExists(resourceName, &v), ), }, { @@ -644,36 +631,27 @@ func testAccCheckNetworkACLDestroy(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn for _, rs := range s.RootModule().Resources { - if rs.Type != "aws_network" { + if rs.Type != "aws_network_acl" { continue } - // Retrieve the network acl - resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(rs.Primary.ID)}, - }) - if err == nil { - if len(resp.NetworkAcls) > 0 && *resp.NetworkAcls[0].NetworkAclId == rs.Primary.ID { - return fmt.Errorf("Network Acl (%s) still exists.", rs.Primary.ID) - } + _, err := tfec2.FindNetworkACLByID(conn, rs.Primary.ID) - return nil + if tfresource.NotFound(err) { + continue } - ec2err, ok := err.(awserr.Error) - if !ok { - return err - } - // Confirm error code is what we want - if ec2err.Code() != "InvalidNetworkAclID.NotFound" { + if err != nil { return err } + + return fmt.Errorf("EC2 Network ACL %s still exists", rs.Primary.ID) } return nil } -func testAccCheckNetworkACLExists(n string, networkAcl *ec2.NetworkAcl) resource.TestCheckFunc { +func testAccCheckNetworkACLExists(n string, v *ec2.NetworkAcl) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -681,110 +659,37 @@ func testAccCheckNetworkACLExists(n string, networkAcl *ec2.NetworkAcl) resource } if rs.Primary.ID == "" { - return fmt.Errorf("No ID is set: %s", n) + return fmt.Errorf("No EC2 Network ACL ID is set: %s", n) } + conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(rs.Primary.ID)}, - }) + output, err := tfec2.FindNetworkACLByID(conn, rs.Primary.ID) + if err != nil { return err } - if len(resp.NetworkAcls) > 0 && *resp.NetworkAcls[0].NetworkAclId == rs.Primary.ID { - *networkAcl = *resp.NetworkAcls[0] - return nil - } + *v = *output - return fmt.Errorf("Network Acls not found") - } -} - -func testAccCheckNetworkACLEgressRuleLength(networkAcl *ec2.NetworkAcl, length int) resource.TestCheckFunc { - return func(s *terraform.State) error { - var entries []*ec2.NetworkAclEntry - for _, entry := range networkAcl.Entries { - if aws.BoolValue(entry.Egress) { - entries = append(entries, entry) - } - } - // There is always a default rule (ALL Traffic ... DENY) - // so we have to increase the length by 1 - if len(entries) != length+1 { - return fmt.Errorf("Invalid number of ingress entries found; count = %d", len(entries)) - } return nil } } -func testIngressRuleLength(networkAcl *ec2.NetworkAcl, length int) resource.TestCheckFunc { - return func(s *terraform.State) error { - var ingressEntries []*ec2.NetworkAclEntry - for _, e := range networkAcl.Entries { - if !*e.Egress { - ingressEntries = append(ingressEntries, e) - } - } - // There is always a default rule (ALL Traffic ... DENY) - // so we have to increase the length by 1 - if len(ingressEntries) != length+1 { - return fmt.Errorf("Invalid number of ingress entries found; count = %d", len(ingressEntries)) - } - return nil - } -} - -func testAccCheckSubnetIsAssociatedWithAcl(acl string, sub string) resource.TestCheckFunc { - return func(s *terraform.State) error { - networkAcl := s.RootModule().Resources[acl] - subnet := s.RootModule().Resources[sub] - - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(networkAcl.Primary.ID)}, - Filters: []*ec2.Filter{ - { - Name: aws.String("association.subnet-id"), - Values: []*string{aws.String(subnet.Primary.ID)}, - }, - }, - }) - if err != nil { - return err - } - if len(resp.NetworkAcls) > 0 { - return nil - } +func testAccNetworkACLConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.1.0.0/16" - return fmt.Errorf("Network Acl %s is not associated with subnet %s", acl, sub) - } + tags = { + Name = %[1]q + } } -func testAccCheckSubnetIsNotAssociatedWithAcl(acl string, subnet string) resource.TestCheckFunc { - return func(s *terraform.State) error { - networkAcl := s.RootModule().Resources[acl] - subnet := s.RootModule().Resources[subnet] - - conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn - resp, err := conn.DescribeNetworkAcls(&ec2.DescribeNetworkAclsInput{ - NetworkAclIds: []*string{aws.String(networkAcl.Primary.ID)}, - Filters: []*ec2.Filter{ - { - Name: aws.String("association.subnet-id"), - Values: []*string{aws.String(subnet.Primary.ID)}, - }, - }, - }) - - if err != nil { - return err - } - if len(resp.NetworkAcls) > 0 { - return fmt.Errorf("Network Acl %s is still associated with subnet %s", acl, subnet) - } - return nil - } +resource "aws_network_acl" "test" { + vpc_id = aws_vpc.test.id +} +`, rName) } func testAccNetworkACLIPv6ICMPConfig(rName string) string { @@ -793,7 +698,7 @@ resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = %q + Name = %[1]q } } @@ -812,33 +717,34 @@ resource "aws_network_acl" "test" { } tags = { - Name = %q + Name = %[1]q } } -`, rName, rName) +`, rName) } -const testAccNetworkACLIPv6Config = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLIPv6Config(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-ipv6" + Name = %[1]q } } -resource "aws_subnet" "blob" { +resource "aws_subnet" "test" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-ipv6" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id ingress { protocol = 6 @@ -849,26 +755,28 @@ resource "aws_network_acl" "test" { to_port = 22 } - subnet_ids = [aws_subnet.blob.id] + subnet_ids = [aws_subnet.test.id] tags = { - Name = "tf-acc-acl-ipv6" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLIPv6VPCConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLIPv6VPCConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" assign_generated_ipv6_cidr_block = true tags = { - Name = "terraform-testacc-network-acl-ipv6-vpc-rules" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id ingress { protocol = 6 @@ -880,32 +788,34 @@ resource "aws_network_acl" "test" { } tags = { - Name = "tf-acc-acl-ipv6" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLIngressConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLIngressConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-ingress" + Name = %[1]q } } -resource "aws_subnet" "blob" { +resource "aws_subnet" "test" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-ingress" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id ingress { protocol = 6 @@ -925,35 +835,37 @@ resource "aws_network_acl" "test" { to_port = 443 } - subnet_ids = [aws_subnet.blob.id] + subnet_ids = [aws_subnet.test.id] tags = { - Name = "tf-acc-acl-ingress" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLCaseSensitiveConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLCaseSensitiveConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-ingress" + Name = %[1]q } } -resource "aws_subnet" "blob" { +resource "aws_subnet" "test" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-ingress" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id ingress { protocol = 6 @@ -964,35 +876,37 @@ resource "aws_network_acl" "test" { to_port = 22 } - subnet_ids = [aws_subnet.blob.id] + subnet_ids = [aws_subnet.test.id] tags = { - Name = "tf-acc-acl-case-sensitive" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLIngressChangeConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLIngressChangeConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-ingress" + Name = %[1]q } } -resource "aws_subnet" "blob" { +resource "aws_subnet" "test" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-ingress" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id ingress { protocol = 6 @@ -1003,35 +917,37 @@ resource "aws_network_acl" "test" { to_port = 22 } - subnet_ids = [aws_subnet.blob.id] + subnet_ids = [aws_subnet.test.id] tags = { - Name = "tf-acc-acl-ingress" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLEgressConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLEgressConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.2.0.0/16" tags = { - Name = "terraform-testacc-network-acl-egress" + Name = %[1]q } } -resource "aws_subnet" "blob" { +resource "aws_subnet" "test" { cidr_block = "10.2.0.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-egress" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id egress { protocol = 6 @@ -1070,33 +986,34 @@ resource "aws_network_acl" "test" { } tags = { - foo = "bar" - Name = "tf-acc-acl-egress" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLEgressNIngressConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLEgressNIngressConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.3.0.0/16" tags = { - Name = "terraform-testacc-network-acl-egress-and-ingress" + Name = %[1]q } } -resource "aws_subnet" "blob" { +resource "aws_subnet" "test" { cidr_block = "10.3.0.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-egress-and-ingress" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id egress { protocol = 6 @@ -1115,231 +1032,247 @@ resource "aws_network_acl" "test" { from_port = 80 to_port = 80 } + + tags = { + Name = %[1]q + } +} +`, rName) } -` -const testAccNetworkACLSubnetConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLSubnetConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-subnet-change" + Name = %[1]q } } -resource "aws_subnet" "old" { +resource "aws_subnet" "test1" { cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-subnet-change-old" + Name = %[1]q } } -resource "aws_subnet" "new" { +resource "aws_subnet" "test2" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-subnet-change-new" + Name = %[1]q } } -resource "aws_network_acl" "roll" { - vpc_id = aws_vpc.foo.id - subnet_ids = [aws_subnet.new.id] +resource "aws_network_acl" "test1" { + vpc_id = aws_vpc.test.id + subnet_ids = [aws_subnet.test2.id] tags = { - Name = "tf-acc-acl-subnet-change-roll" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id - subnet_ids = [aws_subnet.old.id] + vpc_id = aws_vpc.test.id + subnet_ids = [aws_subnet.test1.id] tags = { - Name = "tf-acc-acl-subnet-change-test" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLSubnetChangeConfig = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLSubnetChangeConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-subnet-change" + Name = %[1]q } } -resource "aws_subnet" "old" { +resource "aws_subnet" "test1" { cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-subnet-change-old" + Name = %[1]q } } -resource "aws_subnet" "new" { +resource "aws_subnet" "test2" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id map_public_ip_on_launch = true tags = { - Name = "tf-acc-network-acl-subnet-change-new" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id - subnet_ids = [aws_subnet.new.id] + vpc_id = aws_vpc.test.id + subnet_ids = [aws_subnet.test2.id] tags = { - Name = "tf-acc-acl-subnet-change-test" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLSubnet_SubnetIDs = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLSubnet_SubnetIDs(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-subnet-ids" + Name = %[1]q } } -resource "aws_subnet" "one" { +resource "aws_subnet" "test1" { cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-network-acl-subnet-ids-one" + Name = %[1]q } } -resource "aws_subnet" "two" { +resource "aws_subnet" "test2" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-network-acl-subnet-ids-two" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id - subnet_ids = [aws_subnet.one.id, aws_subnet.two.id] + vpc_id = aws_vpc.test.id + subnet_ids = [aws_subnet.test1.id, aws_subnet.test2.id] tags = { - Name = "tf-acc-acl-subnet-ids" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLSubnet_SubnetIdsUpdate = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLSubnet_SubnetIDsUpdate(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-subnet-ids" + Name = %[1]q } } -resource "aws_subnet" "one" { +resource "aws_subnet" "test1" { cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-network-acl-subnet-ids-one" + Name = %[1]q } } -resource "aws_subnet" "two" { +resource "aws_subnet" "test2" { cidr_block = "10.1.1.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-network-acl-subnet-ids-two" + Name = %[1]q } } -resource "aws_subnet" "three" { +resource "aws_subnet" "test3" { cidr_block = "10.1.222.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-network-acl-subnet-ids-three" + Name = %[1]q } } -resource "aws_subnet" "four" { +resource "aws_subnet" "test4" { cidr_block = "10.1.4.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-network-acl-subnet-ids-four" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id subnet_ids = [ - aws_subnet.one.id, - aws_subnet.three.id, - aws_subnet.four.id, + aws_subnet.test1.id, + aws_subnet.test3.id, + aws_subnet.test4.id, ] tags = { - Name = "tf-acc-acl-subnet-ids" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLSubnet_SubnetIdsDeleteOne = ` -resource "aws_vpc" "foo" { +func testAccNetworkACLSubnet_SubnetIDsDeleteOne(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-subnet-ids" + Name = %[1]q } } -resource "aws_subnet" "one" { +resource "aws_subnet" "test1" { cidr_block = "10.1.111.0/24" - vpc_id = aws_vpc.foo.id + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-network-acl-subnet-ids-one" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.foo.id - subnet_ids = [aws_subnet.one.id] + vpc_id = aws_vpc.test.id + subnet_ids = [aws_subnet.test1.id] tags = { - Name = "tf-acc-acl-subnet-ids" + Name = %[1]q } } -` +`, rName) +} -const testAccNetworkACLEsp = ` -resource "aws_vpc" "testvpc" { +func testAccNetworkACLEsp(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { cidr_block = "10.1.0.0/16" tags = { - Name = "terraform-testacc-network-acl-esp" + Name = %[1]q } } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.testvpc.id + vpc_id = aws_vpc.test.id egress { protocol = "esp" @@ -1351,24 +1284,25 @@ resource "aws_network_acl" "test" { } tags = { - Name = "tf-acc-acl-esp" + Name = %[1]q } } -` +`, rName) +} -func testAccNetworkACLEgressModeBlocksConfig() string { - return ` +func testAccNetworkACLEgressModeBlocksConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-network-acl-egress-computed-attribute-mode" + Name = %[1]q } } resource "aws_network_acl" "test" { tags = { - Name = "terraform-testacc-network-acl-egress-computed-attribute-mode" + Name = %[1]q } vpc_id = aws_vpc.test.id @@ -1391,36 +1325,36 @@ resource "aws_network_acl" "test" { to_port = 0 } } -` +`, rName) } -func testAccNetworkACLEgressModeNoBlocksConfig() string { - return ` +func testAccNetworkACLEgressModeNoBlocksConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-network-acl-egress-computed-attribute-mode" + Name = %[1]q } } resource "aws_network_acl" "test" { tags = { - Name = "terraform-testacc-network-acl-egress-computed-attribute-mode" + Name = %[1]q } vpc_id = aws_vpc.test.id } -` +`, rName) } -func testAccNetworkACLEgressModeZeroedConfig() string { - return ` +func testAccNetworkACLEgressModeZeroedConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-network-acl-egress-computed-attribute-mode" + Name = %[1]q } } @@ -1428,27 +1362,27 @@ resource "aws_network_acl" "test" { egress = [] tags = { - Name = "terraform-testacc-network-acl-egress-computed-attribute-mode" + Name = %[1]q } vpc_id = aws_vpc.test.id } -` +`, rName) } -func testAccNetworkACLIngressModeBlocksConfig() string { - return ` +func testAccNetworkACLIngressModeBlocksConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-network-acl-ingress-computed-attribute-mode" + Name = %[1]q } } resource "aws_network_acl" "test" { tags = { - Name = "terraform-testacc-network-acl-ingress-computed-attribute-mode" + Name = %[1]q } vpc_id = aws_vpc.test.id @@ -1471,36 +1405,36 @@ resource "aws_network_acl" "test" { to_port = 0 } } -` +`, rName) } -func testAccNetworkACLIngressModeNoBlocksConfig() string { - return ` +func testAccNetworkACLIngressModeNoBlocksConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-network-acl-ingress-computed-attribute-mode" + Name = %[1]q } } resource "aws_network_acl" "test" { tags = { - Name = "terraform-testacc-network-acl-ingress-computed-attribute-mode" + Name = %[1]q } vpc_id = aws_vpc.test.id } -` +`, rName) } -func testAccNetworkACLIngressModeZeroedConfig() string { - return ` +func testAccNetworkACLIngressModeZeroedConfig(rName string) string { + return fmt.Sprintf(` resource "aws_vpc" "test" { cidr_block = "10.0.0.0/16" tags = { - Name = "terraform-testacc-network-acl-ingress-computed-attribute-mode" + Name = %[1]q } } @@ -1508,12 +1442,12 @@ resource "aws_network_acl" "test" { ingress = [] tags = { - Name = "terraform-testacc-network-acl-ingress-computed-attribute-mode" + Name = %[1]q } vpc_id = aws_vpc.test.id } -` +`, rName) } func testAccNetworkACLTags1Config(rName, tagKey1, tagValue1 string) string { @@ -1527,8 +1461,7 @@ resource "aws_vpc" "test" { } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.test.id - ingress = [] + vpc_id = aws_vpc.test.id tags = { %[2]q = %[3]q @@ -1548,8 +1481,7 @@ resource "aws_vpc" "test" { } resource "aws_network_acl" "test" { - vpc_id = aws_vpc.test.id - ingress = [] + vpc_id = aws_vpc.test.id tags = { %[2]q = %[3]q diff --git a/internal/service/ec2/sweep.go b/internal/service/ec2/sweep.go index a76066b0aaf6..7d8f8da6a7bc 100644 --- a/internal/service/ec2/sweep.go +++ b/internal/service/ec2/sweep.go @@ -1075,78 +1075,51 @@ func sweepNetworkACLs(region string) error { if err != nil { return fmt.Errorf("error getting client: %s", err) } + input := &ec2.DescribeNetworkAclsInput{} conn := client.(*conns.AWSClient).EC2Conn + sweepResources := make([]*sweep.SweepResource, 0) - req := &ec2.DescribeNetworkAclsInput{} - resp, err := conn.DescribeNetworkAcls(req) - if err != nil { - if sweep.SkipSweepError(err) { - log.Printf("[WARN] Skipping EC2 Network ACL sweep for %s: %s", region, err) - return nil + err = conn.DescribeNetworkAclsPages(input, func(page *ec2.DescribeNetworkAclsOutput, lastPage bool) bool { + if page == nil { + return !lastPage } - return fmt.Errorf("Error describing Network ACLs: %s", err) - } - - if len(resp.NetworkAcls) == 0 { - log.Print("[DEBUG] No Network ACLs to sweep") - return nil - } - for _, nacl := range resp.NetworkAcls { - // Delete rules first - for _, entry := range nacl.Entries { - // These are the rule numbers for IPv4 and IPv6 "ALL traffic" rules which cannot be deleted - if aws.Int64Value(entry.RuleNumber) == 32767 || aws.Int64Value(entry.RuleNumber) == 32768 { - log.Printf("[DEBUG] Skipping Network ACL rule: %q / %d", *nacl.NetworkAclId, *entry.RuleNumber) + for _, v := range page.NetworkAcls { + if aws.BoolValue(v.IsDefault) { continue } - log.Printf("[INFO] Deleting Network ACL rule: %q / %d", *nacl.NetworkAclId, *entry.RuleNumber) - _, err := conn.DeleteNetworkAclEntry(&ec2.DeleteNetworkAclEntryInput{ - NetworkAclId: nacl.NetworkAclId, - Egress: entry.Egress, - RuleNumber: entry.RuleNumber, - }) - if err != nil { - return fmt.Errorf( - "Error deleting Network ACL rule (%s / %d): %s", - *nacl.NetworkAclId, *entry.RuleNumber, err) - } - } + r := ResourceNetworkACL() + d := r.Data(nil) + d.SetId(aws.StringValue(v.NetworkAclId)) - // Disassociate subnets - log.Printf("[DEBUG] Found %d Network ACL associations for %q", len(nacl.Associations), *nacl.NetworkAclId) - for _, a := range nacl.Associations { - log.Printf("[DEBUG] Replacing subnet associations for Network ACL %q", *nacl.NetworkAclId) - defaultAcl, err := GetDefaultNetworkACL(*nacl.VpcId, conn) - if err != nil { - return fmt.Errorf("Failed to find default Network ACL for VPC %q", *nacl.VpcId) - } - _, err = conn.ReplaceNetworkAclAssociation(&ec2.ReplaceNetworkAclAssociationInput{ - NetworkAclId: defaultAcl.NetworkAclId, - AssociationId: a.NetworkAclAssociationId, - }) - if err != nil { - return fmt.Errorf("Failed to replace subnet association for Network ACL %q: %s", - *nacl.NetworkAclId, err) + var subnetIDs []string + for _, v := range v.Associations { + subnetIDs = append(subnetIDs, aws.StringValue(v.SubnetId)) } - } + d.Set("subnet_ids", subnetIDs) - // Default Network ACLs will be deleted along with VPC - if *nacl.IsDefault { - log.Printf("[DEBUG] Skipping default Network ACL: %q", *nacl.NetworkAclId) - continue - } + d.Set("vpc_id", v.VpcId) - log.Printf("[INFO] Deleting Network ACL: %q", *nacl.NetworkAclId) - _, err := conn.DeleteNetworkAcl(&ec2.DeleteNetworkAclInput{ - NetworkAclId: nacl.NetworkAclId, - }) - if err != nil { - return fmt.Errorf( - "Error deleting Network ACL (%s): %s", - *nacl.NetworkAclId, err) + sweepResources = append(sweepResources, sweep.NewSweepResource(r, d, client)) } + + return !lastPage + }) + + if sweep.SkipSweepError(err) { + log.Printf("[WARN] Skipping EC2 Network ACL sweep for %s: %s", region, err) + return nil + } + + if err != nil { + return fmt.Errorf("error listing EC2 Network ACLs (%s): %w", region, err) + } + + err = sweep.SweepOrchestrator(sweepResources) + + if err != nil { + return fmt.Errorf("error sweeping EC2 Network ACLs (%s): %w", region, err) } return nil diff --git a/internal/service/ec2/vpc.go b/internal/service/ec2/vpc.go index 3036e72f1243..c3c4cd57f03d 100644 --- a/internal/service/ec2/vpc.go +++ b/internal/service/ec2/vpc.go @@ -491,6 +491,15 @@ func resourceVPCImport(d *schema.ResourceData, meta interface{}) ([]*schema.Reso } func resourceVPCCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, v interface{}) error { + if diff.HasChange("assign_generated_ipv6_cidr_block") { + if err := diff.SetNewComputed("ipv6_association_id"); err != nil { + return fmt.Errorf("error setting ipv6_association_id to computed: %s", err) + } + if err := diff.SetNewComputed("ipv6_cidr_block"); err != nil { + return fmt.Errorf("error setting ipv6_cidr_block to computed: %s", err) + } + } + if diff.HasChange("instance_tenancy") { old, new := diff.GetChange("instance_tenancy") if old.(string) != ec2.TenancyDedicated || new.(string) != ec2.TenancyDefault { diff --git a/internal/service/ec2/wait.go b/internal/service/ec2/wait.go index e09605eb183e..5db5542980c8 100644 --- a/internal/service/ec2/wait.go +++ b/internal/service/ec2/wait.go @@ -291,7 +291,6 @@ func WaitInstanceIAMInstanceProfileUpdated(conn *ec2.EC2, instanceID string, exp const ManagedPrefixListEntryCreateTimeout = 5 * time.Minute const ( - NetworkACLPropagationTimeout = 2 * time.Minute NetworkACLEntryPropagationTimeout = 5 * time.Minute ) diff --git a/website/docs/r/network_acl.html.markdown b/website/docs/r/network_acl.html.markdown index db83cc4727f0..fe88d0ca2fab 100644 --- a/website/docs/r/network_acl.html.markdown +++ b/website/docs/r/network_acl.html.markdown @@ -17,6 +17,10 @@ defined in-line. At this time you cannot use a Network ACL with in-line rules in conjunction with any Network ACL Rule resources. Doing so will cause a conflict of rule settings and will overwrite rules. +~> **NOTE on Network ACLs and Network ACL Associations:** Terraform provides both a standalone [network ACL association](network_acl_association.html) +resource and a network ACL resource with a `subnet_ids` attribute. Do not use the same subnet ID in both a network ACL +resource and a network ACL association resource. Doing so will cause a conflict of associations and will overwrite the association. + ## Example Usage ```terraform diff --git a/website/docs/r/network_acl_association.html.markdown b/website/docs/r/network_acl_association.html.markdown new file mode 100644 index 000000000000..7c674e470fb0 --- /dev/null +++ b/website/docs/r/network_acl_association.html.markdown @@ -0,0 +1,37 @@ +--- +subcategory: "VPC" +layout: "aws" +page_title: "AWS: aws_network_acl_association" +description: |- + Provides an network ACL association resource. +--- + +# Resource: aws_network_acl_association + +Provides an network ACL association resource which allows you to associate your network ACL with any subnet(s). + +~> **NOTE on Network ACLs and Network ACL Associations:** Terraform provides both a standalone network ACL association resource +and a [network ACL](network_acl.html) resource with a `subnet_ids` attribute. Do not use the same subnet ID in both a network ACL +resource and a network ACL association resource. Doing so will cause a conflict of associations and will overwrite the association. + +## Example Usage + +```terraform +resource "aws_network_acl_association" "main" { + network_acl_id = aws_network_acl.main.id + subnet_id = aws_subnet.main.id +} +``` + +## Argument Reference + +The following arguments are supported: + +* `network_acl_id` - (Required) The ID of the network ACL. +* `subnet_id` - (Required) The ID of the associated Subnet. + +## Attributes Reference + +In addition to all arguments above, the following attributes are exported: + +* `id` - The ID of the network ACL association \ No newline at end of file