From 4f23c48dd50e59b42fb661c091190b658ba94b12 Mon Sep 17 00:00:00 2001 From: rick-masters Date: Fri, 26 Feb 2021 17:34:41 +0000 Subject: [PATCH 01/14] Add support for private_ip_list, private_ip_list_enabled, ipv6_address_list, and ipv6_address_list_enabled. --- aws/resource_aws_network_interface.go | 340 +++++++++++++++++++-- aws/resource_aws_network_interface_test.go | 333 +++++++++++++++++--- website/docs/r/network_interface.markdown | 26 +- 3 files changed, 637 insertions(+), 62 deletions(-) diff --git a/aws/resource_aws_network_interface.go b/aws/resource_aws_network_interface.go index 949b71014c5..547554efd41 100644 --- a/aws/resource_aws_network_interface.go +++ b/aws/resource_aws_network_interface.go @@ -2,6 +2,7 @@ package aws import ( "bytes" + "context" "fmt" "log" "math" @@ -10,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "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" @@ -52,16 +54,32 @@ func resourceAwsNetworkInterface() *schema.Resource { }, "private_ips": { - Type: schema.TypeSet, - Optional: true, - Computed: true, - Elem: &schema.Schema{Type: schema.TypeString}, + Type: schema.TypeSet, + Optional: true, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + ConflictsWith: []string{"private_ip_list"}, }, "private_ips_count": { - Type: schema.TypeInt, + Type: schema.TypeInt, + Optional: true, + Computed: true, + ConflictsWith: []string{"private_ip_list"}, + }, + + "private_ip_list": { + Type: schema.TypeList, + Optional: true, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + ConflictsWith: []string{"private_ips", "private_ips_count"}, + }, + + "private_ip_list_enabled": { + Type: schema.TypeBool, Optional: true, - Computed: true, + Default: false, }, "security_groups": { @@ -115,7 +133,7 @@ func resourceAwsNetworkInterface() *schema.Resource { Type: schema.TypeInt, Optional: true, Computed: true, - ConflictsWith: []string{"ipv6_addresses"}, + ConflictsWith: []string{"ipv6_addresses", "ipv6_address_list"}, }, "ipv6_addresses": { Type: schema.TypeSet, @@ -125,9 +143,143 @@ func resourceAwsNetworkInterface() *schema.Resource { Type: schema.TypeString, ValidateFunc: validation.IsIPv6Address, }, - ConflictsWith: []string{"ipv6_address_count"}, + ConflictsWith: []string{"ipv6_address_count", "ipv6_address_list"}, + }, + "ipv6_address_list": { + Type: schema.TypeList, + Optional: true, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + ConflictsWith: []string{"ipv6_addresses", "ipv6_address_count"}, + }, + "ipv6_address_list_enabled": { + Type: schema.TypeBool, + Optional: true, + Default: false, }, }, + CustomizeDiff: customdiff.Sequence( + customdiff.ForceNewIf("private_ips", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + private_ip_list_enabled := d.Get("private_ip_list_enabled").(bool) + if private_ip_list_enabled { + return false + } + _, new := d.GetChange("private_ips") + if new != nil { + old_primary_ip := "" + if v, ok := d.GetOk("private_ip_list"); ok { + for _, ip := range v.([]interface{}) { + old_primary_ip = ip.(string) + break + } + } + for _, ip := range new.(*schema.Set).List() { + // no need for new resource if we still have the primary ip + if old_primary_ip == ip.(string) { + return false + } + } + // new primary ip requires a new resource + return true + } else { + return false + } + }), + customdiff.ForceNewIf("private_ip_list", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool { + private_ip_list_enabled := d.Get("private_ip_list_enabled").(bool) + if !private_ip_list_enabled { + return false + } + old, new := d.GetChange("private_ip_list") + if old != nil && new != nil { + old_primary_ip := "" + new_primary_ip := "" + for _, ip := range old.([]interface{}) { + old_primary_ip = ip.(string) + break + } + for _, ip := range new.([]interface{}) { + new_primary_ip = ip.(string) + break + } + + // change in primary private ip requires a new resource + return old_primary_ip != new_primary_ip + } else { + return false + } + }), + customdiff.ComputedIf("private_ips", func(_ context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + if !diff.Get("private_ip_list_enabled").(bool) { + // it is not computed if we are actively updating it + if diff.HasChange("private_ips") { + return false + } else { + return diff.HasChange("private_ips_count") + } + } else { + return diff.HasChange("private_ip_list") + } + }), + customdiff.ComputedIf("private_ips_count", func(_ context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + if !diff.Get("private_ip_list_enabled").(bool) { + // it is not computed if we are actively updating it + if diff.HasChange("private_ips_count") { + return false + } else { + // compute the new count if private_ips change + return diff.HasChange("private_ips") + } + } else { + // compute the new count if private_ip_list changes + return diff.HasChange("private_ip_list") + } + }), + customdiff.ComputedIf("private_ip_list", func(_ context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + if diff.Get("private_ip_list_enabled").(bool) { + // if the list is controlling it does not need to be computed + return false + } else { + // list is not controlling so compute new list if private_ips or private_ips_count changes + return diff.HasChange("private_ips") || diff.HasChange("private_ips_count") || diff.HasChange("private_ip_list") + } + }), + customdiff.ComputedIf("ipv6_addresses", func(_ context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + if !diff.Get("ipv6_address_list_enabled").(bool) { + // it is not computed if we are actively updating it + if diff.HasChange("private_ips") { + return false + } else { + return diff.HasChange("ipv6_address_count") + } + } else { + return diff.HasChange("ipv6_address_list") + } + }), + customdiff.ComputedIf("ipv6_address_count", func(_ context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + if !diff.Get("ipv6_address_list_enabled").(bool) { + // it is not computed if we are actively updating it + if diff.HasChange("ipv6_address_count") { + return false + } else { + // compute the new count if ipv6_addresses change + return diff.HasChange("ipv6_addresses") + } + } else { + // compute the new count if ipv6_address_list changes + return diff.HasChange("ipv6_address_list") + } + }), + customdiff.ComputedIf("ipv6_address_list", func(_ context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + if diff.Get("ipv6_address_list_enabled").(bool) { + // if the list is controlling it does not need to be computed + return false + } else { + // list is not controlling so compute new list if anything changes + return diff.HasChange("ipv6_addresses") || diff.HasChange("ipv6_address_count") || diff.HasChange("ipv6_address_list") + } + }), + ), } } @@ -144,18 +296,42 @@ func resourceAwsNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) request.Groups = expandStringSet(v.(*schema.Set)) } - if v, ok := d.GetOk("private_ips"); ok && v.(*schema.Set).Len() > 0 { - request.PrivateIpAddresses = expandPrivateIPAddresses(v.(*schema.Set).List()) + if v, ok := d.GetOk("private_ip_list_enabled"); ok && v.(bool) { + if v, ok := d.GetOk("private_ip_list"); ok && len(v.([]interface{})) > 0 { + request.PrivateIpAddresses = expandPrivateIPAddresses(v.([]interface{})) + } + } else { + if v, ok := d.GetOk("private_ips"); ok && v.(*schema.Set).Len() > 0 { + private_ips := v.(*schema.Set).List() + // total includes the primary + total_private_ips := len(private_ips) + // private_ips_count is for secondaries + if v, ok := d.GetOk("private_ips_count"); ok { + // reduce total count if necessary + if v.(int)+1 < total_private_ips { + total_private_ips = v.(int) + 1 + } + } + // truncate the list + count_limited_ips := make([]interface{}, total_private_ips) + for i, ip := range private_ips { + count_limited_ips[i] = ip.(string) + if i == total_private_ips-1 { + break + } + } + request.PrivateIpAddresses = expandPrivateIPAddresses(count_limited_ips) + } else { + if v, ok := d.GetOk("private_ips_count"); ok { + request.SecondaryPrivateIpAddressCount = aws.Int64(int64(v.(int))) + } + } } if v, ok := d.GetOk("description"); ok { request.Description = aws.String(v.(string)) } - if v, ok := d.GetOk("private_ips_count"); ok { - request.SecondaryPrivateIpAddressCount = aws.Int64(int64(v.(int))) - } - if v, ok := d.GetOk("ipv6_address_count"); ok { request.Ipv6AddressCount = aws.Int64(int64(v.(int))) } @@ -176,6 +352,25 @@ func resourceAwsNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) return fmt.Errorf("error waiting for Network Interface (%s) creation: %s", d.Id(), err) } + if v, ok := d.GetOk("private_ip_list_enabled"); ok && !v.(bool) { + // add more ips to match the count + if v, ok := d.GetOk("private_ips"); ok && v.(*schema.Set).Len() > 0 { + total_private_ips := v.(*schema.Set).Len() + if private_ips_count, ok := d.GetOk("private_ips_count"); ok { + if private_ips_count.(int)+1 > total_private_ips { + input := &ec2.AssignPrivateIpAddressesInput{ + NetworkInterfaceId: aws.String(d.Id()), + SecondaryPrivateIpAddressCount: aws.Int64(int64(private_ips_count.(int) + 1 - total_private_ips)), + } + _, err := conn.AssignPrivateIpAddresses(input) + if err != nil { + return fmt.Errorf("Failure to assign Private IPs: %s", err) + } + } + } + } + } + //Default value is enabled if !d.Get("source_dest_check").(bool) { request := &ec2.ModifyNetworkInterfaceAttributeInput{ @@ -253,6 +448,10 @@ func resourceAwsNetworkInterfaceRead(d *schema.ResourceData, meta interface{}) e d.Set("private_ips_count", len(eni.PrivateIpAddresses)-1) + if err := d.Set("private_ip_list", flattenNetworkInterfacesPrivateIPAddresses(eni.PrivateIpAddresses)); err != nil { + return fmt.Errorf("error setting private_ip_list: %s", err) + } + if err := d.Set("security_groups", flattenGroupIdentifiers(eni.Groups)); err != nil { return fmt.Errorf("error setting security_groups: %s", err) } @@ -265,6 +464,10 @@ func resourceAwsNetworkInterfaceRead(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("error setting ipv6 addresses: %s", err) } + if err := d.Set("ipv6_address_list", flattenEc2NetworkInterfaceIpv6Address(eni.Ipv6Addresses)); err != nil { + return fmt.Errorf("error setting ipv6 address list: %s", err) + } + if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(eni.TagSet).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { return fmt.Errorf("error setting tags: %s", err) } @@ -326,6 +529,7 @@ func resourceAwsNetworkInterfaceDetach(oa *schema.Set, meta interface{}, eniId s func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn + private_ips_net_change := 0 if d.HasChange("attachment") { oa, na := d.GetChange("attachment") @@ -351,7 +555,7 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) } } - if d.HasChange("private_ips") { + if d.HasChange("private_ips") && !d.Get("private_ip_list_enabled").(bool) { o, n := d.GetChange("private_ips") if o == nil { o = new(schema.Set) @@ -374,6 +578,7 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) if err != nil { return fmt.Errorf("Failure to unassign Private IPs: %s", err) } + private_ips_net_change -= unassignIps.Len() } // Assign new IP addresses @@ -387,10 +592,63 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) if err != nil { return fmt.Errorf("Failure to assign Private IPs: %s", err) } + private_ips_net_change += assignIps.Len() + } + } + + if d.HasChange("private_ip_list") && d.Get("private_ip_list_enabled").(bool) { + o, n := d.GetChange("private_ip_list") + if o == nil { + o = make([]string, 0) + } + if n == nil { + n = make([]string, 0) + } + if len(o.([]interface{}))-1 > 0 { + privateIpsToUnassign := make([]interface{}, len(o.([]interface{}))-1) + idx := 0 + for i, ip := range o.([]interface{}) { + // skip primary private ip address + if i == 0 { + continue + } + privateIpsToUnassign[idx] = ip + log.Printf("[INFO] Unassigning private ip %s", ip) + idx += 1 + } + + // Unassign the secondary IP addresses + input := &ec2.UnassignPrivateIpAddressesInput{ + NetworkInterfaceId: aws.String(d.Id()), + PrivateIpAddresses: expandStringList(privateIpsToUnassign), + } + _, err := conn.UnassignPrivateIpAddresses(input) + if err != nil { + return fmt.Errorf("Failure to unassign Private IPs: %s", err) + } + } + + // Assign each ip one-by-one in order to retain order + for i, ip := range n.([]interface{}) { + // skip primary private ip address + if i == 0 { + continue + } + privateIpToAssign := []interface{}{ip} + log.Printf("[INFO] Assigning private ip %s", ip) + + input := &ec2.AssignPrivateIpAddressesInput{ + NetworkInterfaceId: aws.String(d.Id()), + PrivateIpAddresses: expandStringList(privateIpToAssign), + } + _, err := conn.AssignPrivateIpAddresses(input) + if err != nil { + return fmt.Errorf("Failure to assign Private IPs: %s", err) + } } } - if d.HasChange("ipv6_addresses") { + if d.HasChange("ipv6_addresses") && !d.Get("ipv6_address_list_enabled").(bool) { o, n := d.GetChange("ipv6_addresses") if o == nil { o = new(schema.Set) @@ -429,7 +687,7 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) } } - if d.HasChange("ipv6_address_count") { + if d.HasChange("ipv6_address_count") && !d.Get("ipv6_address_list_enabled").(bool) { o, n := d.GetChange("ipv6_address_count") ipv6Addresses := d.Get("ipv6_addresses").(*schema.Set).List() @@ -462,6 +720,50 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) } } + if d.HasChange("ipv6_address_list") && d.Get("ipv6_address_list_enabled").(bool) { + o, n := d.GetChange("ipv6_address_list") + if o == nil { + o = make([]string, 0) + } + if n == nil { + n = make([]string, 0) + } + + // Unassign old IPV6 addresses + if len(o.([]interface{})) > 0 { + unassignIps := make([]interface{}, len(o.([]interface{}))) + for i, ip := range o.([]interface{}) { + unassignIps[i] = ip + log.Printf("[INFO] Unassigning ipv6 address %s", ip) + } + + log.Printf("[INFO] Unassigning ipv6 addresses") + input := &ec2.UnassignIpv6AddressesInput{ + NetworkInterfaceId: aws.String(d.Id()), + Ipv6Addresses: expandStringList(unassignIps), + } + _, err := conn.UnassignIpv6Addresses(input) + if err != nil { + return fmt.Errorf("failure to unassign IPV6 Addresses: %s", err) + } + } + + // Assign each ip one-by-one in order to retain order + for _, ip := range n.([]interface{}) { + privateIpToAssign := []interface{}{ip} + log.Printf("[INFO] Assigning ipv6 address %s", ip) + + input := &ec2.AssignIpv6AddressesInput{ + NetworkInterfaceId: aws.String(d.Id()), + Ipv6Addresses: expandStringList(privateIpToAssign), + } + _, err := conn.AssignIpv6Addresses(input) + if err != nil { + return fmt.Errorf("Failure to assign IPV6 Addresses: %s", err) + } + } + } + if d.HasChange("source_dest_check") { request := &ec2.ModifyNetworkInterfaceAttributeInput{ NetworkInterfaceId: aws.String(d.Id()), @@ -474,7 +776,7 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) } } - if d.HasChange("private_ips_count") { + if d.HasChange("private_ips_count") && !d.Get("private_ip_list_enabled").(bool) { o, n := d.GetChange("private_ips_count") privateIPs := d.Get("private_ips").(*schema.Set).List() privateIPsFiltered := privateIPs[:0] @@ -488,7 +790,7 @@ func resourceAwsNetworkInterfaceUpdate(d *schema.ResourceData, meta interface{}) if o != nil && n != nil && n != len(privateIPsFiltered) { - diff := n.(int) - o.(int) + diff := n.(int) - o.(int) - private_ips_net_change // Surplus of IPs, add the diff if diff > 0 { diff --git a/aws/resource_aws_network_interface_test.go b/aws/resource_aws_network_interface_test.go index 9c9d240eccc..a4600e9eca0 100644 --- a/aws/resource_aws_network_interface_test.go +++ b/aws/resource_aws_network_interface_test.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "strings" "testing" "github.com/aws/aws-sdk-go/aws" @@ -92,9 +93,10 @@ func TestAccAWSENI_basic(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, }, }) @@ -121,9 +123,10 @@ func TestAccAWSENI_ipv6(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENIIPV6MultipleConfig(rName), @@ -167,9 +170,10 @@ func TestAccAWSENI_tags(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENITagsConfig2(rName, "key1", "value1updated", "key2", "value2"), @@ -211,9 +215,10 @@ func TestAccAWSENI_ipv6_count(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENIIPV6CountConfig(2, rName), @@ -279,9 +284,10 @@ func TestAccAWSENI_updatedDescription(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENIConfigUpdatedDescription(), @@ -314,9 +320,10 @@ func TestAccAWSENI_attached(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, }, }) @@ -342,9 +349,10 @@ func TestAccAWSENI_ignoreExternalAttachment(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, }, }) @@ -368,9 +376,10 @@ func TestAccAWSENI_sourceDestCheck(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENIConfigWithSourceDestCheck(true), @@ -408,9 +417,10 @@ func TestAccAWSENI_computedIPs(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, }, }) @@ -433,9 +443,10 @@ func TestAccAWSENI_PrivateIpsCount(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENIConfigPrivateIpsCount(2), @@ -445,9 +456,10 @@ func TestAccAWSENI_PrivateIpsCount(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENIConfigPrivateIpsCount(0), @@ -457,9 +469,10 @@ func TestAccAWSENI_PrivateIpsCount(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, { Config: testAccAWSENIConfigPrivateIpsCount(1), @@ -469,14 +482,169 @@ func TestAccAWSENI_PrivateIpsCount(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, }, }, }) } +type privateIpListTestConfigData struct { + private_ips []string + private_ips_count string + private_ip_list_enabled string + private_ip_list []string + replacesInterface bool +} + +func TestAccAWSENI_PrivateIpsSet(t *testing.T) { + var networkInterface, lastInterface ec2.NetworkInterface + resourceName := "aws_network_interface.test" + + testConfigs := []privateIpListTestConfigData{ + + {[]string{"44", "59", "123"}, "", "", []string{}, false}, // Configuration with three private_ips + {[]string{"123", "44", "59"}, "", "", []string{}, false}, // Change order of private_ips + {[]string{"123", "12", "59", "44"}, "", "", []string{}, false}, // Add secondaries to private_ips + {[]string{"123", "59", "44"}, "", "", []string{}, false}, // Remove secondaries from private_ips + {[]string{"123", "59", "57"}, "", "", []string{}, true}, // Remove primary + {[]string{}, "4", "", []string{}, false}, // Use count to add IPs + {[]string{"44", "57"}, "", "", []string{}, false}, // Change list, retain primary + {[]string{"44", "57", "123", "12"}, "", "", []string{}, false}, // Add to secondaries + {[]string{"17"}, "", "", []string{}, true}, // New list + {[]string{"17", "45", "89"}, "", "", []string{}, false}, // Add secondaries + } + + testSteps := make([]resource.TestStep, len(testConfigs)*2) + testSteps[0] = resource.TestStep{ + Config: testAccAWSENIConfigPrivateIpList(testConfigs[0], resourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSENIExists(resourceName, &networkInterface), + testAccCheckAWSENIPrivateIpList(testConfigs[0], &networkInterface), + testAccCheckAWSENIExists(resourceName, &lastInterface), + ), + } + testSteps[1] = resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, + } + + for i, testConfig := range testConfigs { + if i == 0 { + continue + } + if testConfig.replacesInterface { + testSteps[i*2] = resource.TestStep{ + Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSENIExists(resourceName, &networkInterface), + testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), + testAccCheckAWSENIDifferent(&lastInterface, &networkInterface), // different + testAccCheckAWSENIExists(resourceName, &lastInterface), + ), + } + } else { + testSteps[i*2] = resource.TestStep{ + Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSENIExists(resourceName, &networkInterface), + testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), + testAccCheckAWSENISame(&lastInterface, &networkInterface), // same + testAccCheckAWSENIExists(resourceName, &lastInterface), + ), + } + } + // import check + testSteps[i*2+1] = testSteps[1] + } + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSENIDestroy, + Steps: testSteps, + }) +} + +func TestAccAWSENI_PrivateIpList(t *testing.T) { + var networkInterface, lastInterface ec2.NetworkInterface + resourceName := "aws_network_interface.test" + + // private_ips, private_ips_count, private_ip_list_enabed, private_ip_list, replacesInterface + testConfigs := []privateIpListTestConfigData{ + {[]string{"17"}, "", "", []string{}, true}, // Build a set incrementally in order + {[]string{"17", "45"}, "", "", []string{}, false}, // Add to set + {[]string{"17", "45", "89"}, "", "", []string{}, false}, // Add to set + {[]string{"17", "45", "89", "122"}, "", "", []string{}, false}, // Add to set + {[]string{}, "", "true", []string{"17", "45", "89", "122"}, false}, // Change from set to list using same order + {[]string{}, "", "true", []string{"17", "89", "45", "122"}, false}, // Change order of private_ip_list + {[]string{}, "", "true", []string{"17", "89", "45"}, false}, // Remove secondaries from end + {[]string{}, "", "true", []string{"17", "89", "45", "123"}, false}, // Add secondaries to end + {[]string{}, "", "true", []string{"17", "89", "77", "45", "123"}, false}, // Add secondaries to middle + {[]string{}, "", "true", []string{"17", "89", "123"}, false}, // Remove secondaries from middle + {[]string{}, "4", "", []string{}, false}, // Use count to add IPs + {[]string{}, "", "true", []string{"59", "123", "44"}, true}, // Change to specific list - forces new + {[]string{}, "", "true", []string{"123", "59", "44"}, true}, // Change first of private_ip_list - forces new + {[]string{"123", "59", "44"}, "", "", []string{}, false}, // Change from list to set using same set + } + + testSteps := make([]resource.TestStep, len(testConfigs)*2) + testSteps[0] = resource.TestStep{ + Config: testAccAWSENIConfigPrivateIpList(testConfigs[0], resourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSENIExists(resourceName, &networkInterface), + testAccCheckAWSENIPrivateIpList(testConfigs[0], &networkInterface), + testAccCheckAWSENIExists(resourceName, &lastInterface), + ), + } + testSteps[1] = resource.TestStep{ + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, + } + + for i, testConfig := range testConfigs { + if i == 0 { + continue + } + if testConfig.replacesInterface { + testSteps[i*2] = resource.TestStep{ + Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSENIExists(resourceName, &networkInterface), + testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), + testAccCheckAWSENIDifferent(&lastInterface, &networkInterface), // different + testAccCheckAWSENIExists(resourceName, &lastInterface), + ), + } + } else { + testSteps[i*2] = resource.TestStep{ + Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSENIExists(resourceName, &networkInterface), + testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), + testAccCheckAWSENISame(&lastInterface, &networkInterface), // same + testAccCheckAWSENIExists(resourceName, &lastInterface), + ), + } + } + // import check + testSteps[i*2+1] = testSteps[1] + } + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSENIDestroy, + Steps: testSteps, + }) +} + func testAccCheckAWSENIExists(n string, res *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -626,6 +794,54 @@ func testAccCheckAWSENIMakeExternalAttachment(n string, conf *ec2.NetworkInterfa } } +func testAccCheckAWSENIPrivateIpList(testConfig privateIpListTestConfigData, iface *ec2.NetworkInterface) resource.TestCheckFunc { + return func(s *terraform.State) error { + havePrivateIps := flattenNetworkInterfacesPrivateIPAddresses(iface.PrivateIpAddresses) + PRIVATE_IPS_LOOP: + // every IP from private_ips should be present on the interface + for _, needIp := range testConfig.private_ips { + for _, haveIp := range havePrivateIps { + if haveIp == "172.16.10."+needIp { + continue PRIVATE_IPS_LOOP + } + } + return fmt.Errorf("expected ip 172.16.10.%s to be in interface set %s", needIp, strings.Join(havePrivateIps, ",")) + } + // every configured IP should be present on the interface + for needIdx, needIp := range testConfig.private_ip_list { + if len(havePrivateIps) <= needIdx || "172.16.10."+needIp != havePrivateIps[needIdx] { + return fmt.Errorf("expected ip 172.16.10.%s to be at %d in the list %s", needIp, needIdx, strings.Join(havePrivateIps, ",")) + } + } + // number of ips configured should match interface + if len(testConfig.private_ips) > 0 && len(testConfig.private_ips) != len(havePrivateIps) { + return fmt.Errorf("expected %s got %s", strings.Join(testConfig.private_ips, ","), strings.Join(havePrivateIps, ",")) + } + if len(testConfig.private_ip_list) > 0 && len(testConfig.private_ip_list) != len(havePrivateIps) { + return fmt.Errorf("expected %s got %s", strings.Join(testConfig.private_ip_list, ","), strings.Join(havePrivateIps, ",")) + } + return nil + } +} + +func testAccCheckAWSENISame(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { + return func(s *terraform.State) error { + if aws.StringValue(iface1.NetworkInterfaceId) != aws.StringValue(iface2.NetworkInterfaceId) { + return fmt.Errorf("Interface %s should not have been replaced with %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) + } + return nil + } +} + +func testAccCheckAWSENIDifferent(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { + return func(s *terraform.State) error { + if aws.StringValue(iface1.NetworkInterfaceId) == aws.StringValue(iface2.NetworkInterfaceId) { + return fmt.Errorf("Interface %s should have been replaced, have %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) + } + return nil + } +} + func testAccAWSENIConfig() string { return composeConfig(testAccAvailableAZsNoOptInConfig(), ` resource "aws_vpc" "test" { @@ -750,6 +966,45 @@ resource "aws_network_interface" "test" { `, ipCount) } +func testAccAWSENIConfigPrivateIpList(testConfig privateIpListTestConfigData, rName string) string { + var config strings.Builder + + config.WriteString(` +resource "aws_network_interface" "test" { + subnet_id = aws_subnet.test.id + security_groups = [aws_security_group.test.id] + description = "Managed by Terraform" +`) + + if len(testConfig.private_ips) > 0 { + config.WriteString(" private_ips = [\n") + for _, ip := range testConfig.private_ips { + config.WriteString(fmt.Sprintf(" \"172.16.10.%s\",\n", ip)) + } + config.WriteString("]\n") + } + + if testConfig.private_ips_count != "" { + config.WriteString(fmt.Sprintf(" private_ips_count = %s\n", testConfig.private_ips_count)) + } + + if testConfig.private_ip_list_enabled != "" { + config.WriteString(fmt.Sprintf(" private_ip_list_enabled = %s\n", testConfig.private_ip_list_enabled)) + } + config.WriteString(" ipv6_address_list_enabled = false\n") + + if len(testConfig.private_ip_list) > 0 { + config.WriteString(" private_ip_list = [\n") + for _, ip := range testConfig.private_ip_list { + config.WriteString(fmt.Sprintf(" \"172.16.10.%s\",\n", ip)) + } + config.WriteString("]\n") + } + + config.WriteString("}\n") + return testAccAWSENIIPV6ConfigBase(rName) + config.String() +} + func testAccAWSENIConfigUpdatedDescription() string { return composeConfig(testAccAvailableAZsNoOptInConfig(), ` resource "aws_vpc" "test" { diff --git a/website/docs/r/network_interface.markdown b/website/docs/r/network_interface.markdown index df298d2c741..081a5c26529 100644 --- a/website/docs/r/network_interface.markdown +++ b/website/docs/r/network_interface.markdown @@ -31,10 +31,14 @@ The following arguments are supported: * `subnet_id` - (Required) Subnet ID to create the ENI in. * `description` - (Optional) A description for the network interface. -* `private_ips` - (Optional) List of private IPs to assign to the ENI. -* `private_ips_count` - (Optional) Number of secondary private IPs to assign to the ENI. The total number of private IPs will be 1 + private_ips_count, as a primary private IP will be assiged to an ENI by default. -* `ipv6_addresses` - (Optional) One or more specific IPv6 addresses from the IPv6 CIDR block range of your subnet. You can't use this option if you're specifying `ipv6_address_count`. +* `private_ips` - (Optional) List of private IPs to assign to the ENI without regard to order. +* `private_ips_count` - (Optional) Number of secondary private IPs to assign to the ENI. The total number of private IPs will be 1 + `private_ips_count`, as a primary private IP will be assiged to an ENI by default. +* `private_ip_list` - (Optional) List of private IPs to assign to the ENI in sequential order. Requires setting `private_ip_list_enable` to `true`. +* `private_ip_list_enable` - (Optional) Whether `private_ip_list` is allowed and controls the IPs to assign to the ENI and `private_ips` and `private_ips_count` become read-only. Default false. +* `ipv6_addresses` - (Optional) One or more specific IPv6 addresses from the IPv6 CIDR block range of your subnet. Addresses are assigned without regard to order. You can't use this option if you're specifying `ipv6_address_count`. * `ipv6_address_count` - (Optional) The number of IPv6 addresses to assign to a network interface. You can't use this option if specifying specific `ipv6_addresses`. If your subnet has the AssignIpv6AddressOnCreation attribute set to `true`, you can specify `0` to override this setting. +* `ipv6_addresses_list` - (Optional) List of private IPs to assign to the ENI in sequential order. +* `ipv6_addresses_list_enable` - (Optional) Whether `ipv6_addreses_list` is allowed and controls the IPs to assign to the ENI and `ipv6_addresses` and `ipv6_addresses_count` become read-only. Default false. * `security_groups` - (Optional) List of security group IDs to assign to the ENI. * `attachment` - (Optional) Block to define the attachment of the ENI. Documented below. * `source_dest_check` - (Optional) Whether to enable source destination checking for the ENI. Default true. @@ -54,13 +58,27 @@ In addition to all arguments above, the following attributes are exported: * `mac_address` - The MAC address of the network interface. * `private_dns_name` - The private DNS name of the network interface (IPv4). * `description` - A description for the network interface. -* `private_ips` - List of private IPs assigned to the ENI. +* `private_ips` - Set of private IPs assigned to the ENI. * `security_groups` - List of security groups attached to the ENI. * `attachment` - Block defining the attachment of the ENI. * `source_dest_check` - Whether source destination checking is enabled * `tags` - Tags assigned to the ENI. +## Managing Multiple IPs on a Network Interface +By default, private IPs are managed through the `private_ips` and `private_ips_count` arguments which manage IPs as a set of IPs that are configured without regard to order. For a new network interface, the same primary IP address is consistently selected from a given a set of addresses, regardless of the order provided. However, modifications of the set of addresses of an existing interface will not alter the current primary IP address unless it has been removed from the set. + +In order to manage the private Ips as a sequentially ordered list instead, configure `private_ip_list_enabled` to `true` and use `private_ip_list` to manage the IPs. This will disable the `private_ips` and `private_ips_count` settings, which must be removed from the config file but are still exported. Note that changing the first address of `private_ip_list`, which is the primary, always requires a new interface. + +If you are managing a specific set or list of Ips, instead of just using `private_ips_count`, here is a workflow for also leveraging `private_ips_count` to have AWS automatically assign additional IP addresses: +* Comment out any settings for `private_ips`, `private_ip_list`, and `private_ip_list_enabled` +* Increase to the desired `private_ips_count`. Note that this count is for the number of secondaries. The primary is not included in this count. +* Apply to assign the extra Ips +* Remove `private_ips_count` and restore your settings from the first step +* Add the new Ips to your current settings +* Apply again to update the stored state + +This process can also be used to remove IP addresses in addition to the option of manually removing them. Adding IP addresses in a manual fashion is more difficult because it requires knowledge of which addresses are available. ## Import From a5352af626f649b7853f20b6f3defcfd724cbb39 Mon Sep 17 00:00:00 2001 From: rick-masters Date: Fri, 26 Feb 2021 18:08:33 +0000 Subject: [PATCH 02/14] Update CHANGELOG.md for hashicorp#17846 --- .changelog/17846.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/17846.txt diff --git a/.changelog/17846.txt b/.changelog/17846.txt new file mode 100644 index 00000000000..3e8d0501f63 --- /dev/null +++ b/.changelog/17846.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_network_interface: Add `private_ip_list`, `private_ip_list_enabled`, `ipv6_addresses_list`, and `ipv6_addresses_list_enabled` attributes +``` From 46dacf82b78f8d86ab1fae8f336316fc7b4ec55f Mon Sep 17 00:00:00 2001 From: rick-masters Date: Fri, 26 Feb 2021 19:31:26 +0000 Subject: [PATCH 03/14] Use Get for values with defaults, and other lint issues. --- aws/resource_aws_network_interface.go | 4 ++-- aws/resource_aws_network_interface_test.go | 6 +++--- website/docs/r/network_interface.markdown | 7 +++++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/aws/resource_aws_network_interface.go b/aws/resource_aws_network_interface.go index 547554efd41..020b9af9b5b 100644 --- a/aws/resource_aws_network_interface.go +++ b/aws/resource_aws_network_interface.go @@ -296,7 +296,7 @@ func resourceAwsNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) request.Groups = expandStringSet(v.(*schema.Set)) } - if v, ok := d.GetOk("private_ip_list_enabled"); ok && v.(bool) { + if d.Get("private_ip_list_enabled").(bool) { if v, ok := d.GetOk("private_ip_list"); ok && len(v.([]interface{})) > 0 { request.PrivateIpAddresses = expandPrivateIPAddresses(v.([]interface{})) } @@ -352,7 +352,7 @@ func resourceAwsNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) return fmt.Errorf("error waiting for Network Interface (%s) creation: %s", d.Id(), err) } - if v, ok := d.GetOk("private_ip_list_enabled"); ok && !v.(bool) { + if !d.Get("private_ip_list_enabled").(bool) { // add more ips to match the count if v, ok := d.GetOk("private_ips"); ok && v.(*schema.Set).Len() > 0 { total_private_ips := v.(*schema.Set).Len() diff --git a/aws/resource_aws_network_interface_test.go b/aws/resource_aws_network_interface_test.go index a4600e9eca0..57818e48b7a 100644 --- a/aws/resource_aws_network_interface_test.go +++ b/aws/resource_aws_network_interface_test.go @@ -969,12 +969,12 @@ resource "aws_network_interface" "test" { func testAccAWSENIConfigPrivateIpList(testConfig privateIpListTestConfigData, rName string) string { var config strings.Builder - config.WriteString(` -resource "aws_network_interface" "test" { + config.WriteString(fmt.Sprintf(` +%s "aws_network_interface" "test" { subnet_id = aws_subnet.test.id security_groups = [aws_security_group.test.id] description = "Managed by Terraform" -`) +`, "resource")) if len(testConfig.private_ips) > 0 { config.WriteString(" private_ips = [\n") diff --git a/website/docs/r/network_interface.markdown b/website/docs/r/network_interface.markdown index 081a5c26529..d07831d33ad 100644 --- a/website/docs/r/network_interface.markdown +++ b/website/docs/r/network_interface.markdown @@ -71,8 +71,11 @@ By default, private IPs are managed through the `private_ips` and `private_ips_c In order to manage the private Ips as a sequentially ordered list instead, configure `private_ip_list_enabled` to `true` and use `private_ip_list` to manage the IPs. This will disable the `private_ips` and `private_ips_count` settings, which must be removed from the config file but are still exported. Note that changing the first address of `private_ip_list`, which is the primary, always requires a new interface. If you are managing a specific set or list of Ips, instead of just using `private_ips_count`, here is a workflow for also leveraging `private_ips_count` to have AWS automatically assign additional IP addresses: -* Comment out any settings for `private_ips`, `private_ip_list`, and `private_ip_list_enabled` -* Increase to the desired `private_ips_count`. Note that this count is for the number of secondaries. The primary is not included in this count. +* Comment out these settings: + * `private_ips` + * `private_ip_list` + * `private_ip_list_enabled` +* Set the desired `private_ips_count`. Note that this count is for the number of secondaries. The primary is not included in this count. * Apply to assign the extra Ips * Remove `private_ips_count` and restore your settings from the first step * Add the new Ips to your current settings From e187d5205d8a29a7b65de0b4c0af12c8e3be8b48 Mon Sep 17 00:00:00 2001 From: rick-masters Date: Fri, 26 Feb 2021 19:37:56 +0000 Subject: [PATCH 04/14] Add spacing around lists --- website/docs/r/network_interface.markdown | 2 ++ 1 file changed, 2 insertions(+) diff --git a/website/docs/r/network_interface.markdown b/website/docs/r/network_interface.markdown index d07831d33ad..d804da1c777 100644 --- a/website/docs/r/network_interface.markdown +++ b/website/docs/r/network_interface.markdown @@ -72,9 +72,11 @@ In order to manage the private Ips as a sequentially ordered list instead, confi If you are managing a specific set or list of Ips, instead of just using `private_ips_count`, here is a workflow for also leveraging `private_ips_count` to have AWS automatically assign additional IP addresses: * Comment out these settings: + * `private_ips` * `private_ip_list` * `private_ip_list_enabled` + * Set the desired `private_ips_count`. Note that this count is for the number of secondaries. The primary is not included in this count. * Apply to assign the extra Ips * Remove `private_ips_count` and restore your settings from the first step From 6862c0d02b5359d85ed6a4bde17215392d65e5fb Mon Sep 17 00:00:00 2001 From: rick-masters Date: Fri, 26 Feb 2021 19:43:52 +0000 Subject: [PATCH 05/14] More spacing around lists --- website/docs/r/network_interface.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/r/network_interface.markdown b/website/docs/r/network_interface.markdown index d804da1c777..edcf9375243 100644 --- a/website/docs/r/network_interface.markdown +++ b/website/docs/r/network_interface.markdown @@ -71,6 +71,7 @@ By default, private IPs are managed through the `private_ips` and `private_ips_c In order to manage the private Ips as a sequentially ordered list instead, configure `private_ip_list_enabled` to `true` and use `private_ip_list` to manage the IPs. This will disable the `private_ips` and `private_ips_count` settings, which must be removed from the config file but are still exported. Note that changing the first address of `private_ip_list`, which is the primary, always requires a new interface. If you are managing a specific set or list of Ips, instead of just using `private_ips_count`, here is a workflow for also leveraging `private_ips_count` to have AWS automatically assign additional IP addresses: + * Comment out these settings: * `private_ips` From 2628926bc420af45f4b725a7552bf869b31cf44f Mon Sep 17 00:00:00 2001 From: rick-masters Date: Fri, 26 Feb 2021 20:51:11 +0000 Subject: [PATCH 06/14] Correct attribute names in documentation --- .changelog/17846.txt | 2 +- website/docs/r/network_interface.markdown | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.changelog/17846.txt b/.changelog/17846.txt index 3e8d0501f63..b192c857d70 100644 --- a/.changelog/17846.txt +++ b/.changelog/17846.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -resource/aws_network_interface: Add `private_ip_list`, `private_ip_list_enabled`, `ipv6_addresses_list`, and `ipv6_addresses_list_enabled` attributes +resource/aws_network_interface: Add `private_ip_list`, `private_ip_list_enabled`, `ipv6_address_list`, and `ipv6_address_list_enabled` attributes ``` diff --git a/website/docs/r/network_interface.markdown b/website/docs/r/network_interface.markdown index edcf9375243..b48f54bbb4b 100644 --- a/website/docs/r/network_interface.markdown +++ b/website/docs/r/network_interface.markdown @@ -37,8 +37,8 @@ The following arguments are supported: * `private_ip_list_enable` - (Optional) Whether `private_ip_list` is allowed and controls the IPs to assign to the ENI and `private_ips` and `private_ips_count` become read-only. Default false. * `ipv6_addresses` - (Optional) One or more specific IPv6 addresses from the IPv6 CIDR block range of your subnet. Addresses are assigned without regard to order. You can't use this option if you're specifying `ipv6_address_count`. * `ipv6_address_count` - (Optional) The number of IPv6 addresses to assign to a network interface. You can't use this option if specifying specific `ipv6_addresses`. If your subnet has the AssignIpv6AddressOnCreation attribute set to `true`, you can specify `0` to override this setting. -* `ipv6_addresses_list` - (Optional) List of private IPs to assign to the ENI in sequential order. -* `ipv6_addresses_list_enable` - (Optional) Whether `ipv6_addreses_list` is allowed and controls the IPs to assign to the ENI and `ipv6_addresses` and `ipv6_addresses_count` become read-only. Default false. +* `ipv6_address_list` - (Optional) List of private IPs to assign to the ENI in sequential order. +* `ipv6_address_list_enable` - (Optional) Whether `ipv6_addreses_list` is allowed and controls the IPs to assign to the ENI and `ipv6_addresses` and `ipv6_addresses_count` become read-only. Default false. * `security_groups` - (Optional) List of security group IDs to assign to the ENI. * `attachment` - (Optional) Block to define the attachment of the ENI. Documented below. * `source_dest_check` - (Optional) Whether to enable source destination checking for the ENI. Default true. From 4f4e39047b72ac1a9212f88396faaefdf8f88a09 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 15:51:46 -0500 Subject: [PATCH 07/14] Clean up docs --- website/docs/r/network_interface.markdown | 93 ++++++++++------------- 1 file changed, 42 insertions(+), 51 deletions(-) diff --git a/website/docs/r/network_interface.markdown b/website/docs/r/network_interface.markdown index b2d614a1c8d..4738f37beb2 100644 --- a/website/docs/r/network_interface.markdown +++ b/website/docs/r/network_interface.markdown @@ -25,34 +25,53 @@ resource "aws_network_interface" "test" { } ``` +### Example of Managing Multiple IPs on a Network Interface + +By default, private IPs are managed through the `private_ips` and `private_ips_count` arguments which manage IPs as a set of IPs that are configured without regard to order. For a new network interface, the same primary IP address is consistently selected from a given set of addresses, regardless of the order provided. However, modifications of the set of addresses of an existing interface will not alter the current primary IP address unless it has been removed from the set. + +In order to manage the private IPs as a sequentially ordered list, configure `private_ip_list_enabled` to `true` and use `private_ip_list` to manage the IPs. This will disable the `private_ips` and `private_ips_count` settings, which must be removed from the config file but are still exported. Note that changing the first address of `private_ip_list`, which is the primary, always requires a new interface. + +If you are managing a specific set or list of IPs, instead of just using `private_ips_count`, this is a potential workflow for also leveraging `private_ips_count` to have AWS automatically assign additional IP addresses: + +1. Comment out `private_ips`, `private_ip_list`, `private_ip_list_enabled` in your configuration +2. Set the desired `private_ips_count` (count of the number of secondaries, the primary is not included) +3. Apply to assign the extra IPs +4. Remove `private_ips_count` and restore your settings from the first step +5. Add the new IPs to your current settings +6. Apply again to update the stored state + +This process can also be used to remove IP addresses in addition to the option of manually removing them. Adding IP addresses in a manually is more difficult because it requires knowledge of which addresses are available. + ## Argument Reference -The following arguments are supported: +The following arguments are required: * `subnet_id` - (Required) Subnet ID to create the ENI in. -* `description` - (Optional) A description for the network interface. -* `private_ips` - (Optional) List of private IPs to assign to the ENI without regard to order. -* `private_ips_count` - (Optional) Number of secondary private IPs to assign to the ENI. The total number of private IPs will be 1 + `private_ips_count`, as a primary private IP will be assiged to an ENI by default. + +The following arguments are optional: + +* `attachment` - (Optional) Configuration block to define the attachment of the ENI. See below. +* `description` - (Optional) Description for the network interface. +* `interface_type` - (Optional) Type of network interface to create. Set to `efa` for Elastic Fabric Adapter. +* `ipv4_prefix_count` - (Optional) Number of IPv4 prefixes that AWS automatically assigns to the network interface. +* `ipv4_prefixes` - (Optional) One or more IPv4 prefixes assigned to the network interface. +* `ipv6_address_count` - (Optional) Number of IPv6 addresses to assign to a network interface. You can't use this option if specifying specific `ipv6_addresses`. If your subnet has the AssignIpv6AddressOnCreation attribute set to `true`, you can specify `0` to override this setting. +* `ipv6_address_list_enable` - (Optional) Whether `ipv6_addreses_list` is allowed and controls the IPs to assign to the ENI and `ipv6_addresses` and `ipv6_addresses_count` become read-only. Default false. +* `ipv6_address_list` - (Optional) List of private IPs to assign to the ENI in sequential order. +* `ipv6_addresses` - (Optional) One or more specific IPv6 addresses from the IPv6 CIDR block range of your subnet. Addresses are assigned without regard to order. You can't use this option if you're specifying `ipv6_address_count`. +* `ipv6_prefix_count` - (Optional) Number of IPv6 prefixes that AWS automatically assigns to the network interface. +* `ipv6_prefixes` - (Optional) One or more IPv6 prefixes assigned to the network interface. * `private_ip_list` - (Optional) List of private IPs to assign to the ENI in sequential order. Requires setting `private_ip_list_enable` to `true`. * `private_ip_list_enable` - (Optional) Whether `private_ip_list` is allowed and controls the IPs to assign to the ENI and `private_ips` and `private_ips_count` become read-only. Default false. -* `ipv6_addresses` - (Optional) One or more specific IPv6 addresses from the IPv6 CIDR block range of your subnet. Addresses are assigned without regard to order. You can't use this option if you're specifying `ipv6_address_count`. -* `ipv6_address_count` - (Optional) The number of IPv6 addresses to assign to a network interface. You can't use this option if specifying specific `ipv6_addresses`. If your subnet has the AssignIpv6AddressOnCreation attribute set to `true`, you can specify `0` to override this setting. -* `ipv6_address_list` - (Optional) List of private IPs to assign to the ENI in sequential order. -* `ipv6_address_list_enable` - (Optional) Whether `ipv6_addreses_list` is allowed and controls the IPs to assign to the ENI and `ipv6_addresses` and `ipv6_addresses_count` become read-only. Default false. +* `private_ips` - (Optional) List of private IPs to assign to the ENI without regard to order. +* `private_ips_count` - (Optional) Number of secondary private IPs to assign to the ENI. The total number of private IPs will be 1 + `private_ips_count`, as a primary private IP will be assiged to an ENI by default. * `security_groups` - (Optional) List of security group IDs to assign to the ENI. -* `attachment` - (Optional) Block to define the attachment of the ENI. Documented below. * `source_dest_check` - (Optional) Whether to enable source destination checking for the ENI. Default true. -* `ipv4_prefixes` - (Optional) One or more IPv4 prefixes assigned to the network interface. -* `ipv4_prefix_count` - (Optional) The number of IPv4 prefixes that AWS automatically assigns to the network interface. -* `ipv6_prefixes` - (Optional) One or more IPv6 prefixes assigned to the network interface. -* `ipv6_prefix_count` - (Optional) The number of IPv6 prefixes that AWS automatically assigns to the network interface. +* `tags` - (Optional) Map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. -> **NOTE:** Changing `interface_type` will cause the resource to be destroyed and re-created. -* `interface_type` - (Optional) Type of network interface to create. Set to `efa` for Elastic Fabric Adapter. -* `tags` - (Optional) A map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. - -The `attachment` block supports: +### `attachment` * `instance` - (Required) ID of the instance to attach to. * `device_index` - (Required) Integer to define the devices index. @@ -61,40 +80,12 @@ The `attachment` block supports: In addition to all arguments above, the following attributes are exported: -* `arn` - The ARN of the network interface. -* `id` - The ID of the network interface. -* `mac_address` - The MAC address of the network interface. -* `owner_id` - The AWS account ID of the owner of the network interface. -* `private_dns_name` - The private DNS name of the network interface (IPv4). -* `description` - A description for the network interface. -* `private_ips` - Set of private IPs assigned to the ENI. -* `security_groups` - List of security groups attached to the ENI. -* `attachment` - Block defining the attachment of the ENI. -* `source_dest_check` - Whether source destination checking is enabled -* `tags` - Tags assigned to the ENI. - -## Managing Multiple IPs on a Network Interface - -By default, private IPs are managed through the `private_ips` and `private_ips_count` arguments which manage IPs as a set of IPs that are configured without regard to order. For a new network interface, the same primary IP address is consistently selected from a given a set of addresses, regardless of the order provided. However, modifications of the set of addresses of an existing interface will not alter the current primary IP address unless it has been removed from the set. - -In order to manage the private Ips as a sequentially ordered list instead, configure `private_ip_list_enabled` to `true` and use `private_ip_list` to manage the IPs. This will disable the `private_ips` and `private_ips_count` settings, which must be removed from the config file but are still exported. Note that changing the first address of `private_ip_list`, which is the primary, always requires a new interface. - -If you are managing a specific set or list of Ips, instead of just using `private_ips_count`, here is a workflow for also leveraging `private_ips_count` to have AWS automatically assign additional IP addresses: - -* Comment out these settings: - - * `private_ips` - * `private_ip_list` - * `private_ip_list_enabled` - -* Set the desired `private_ips_count`. Note that this count is for the number of secondaries. The primary is not included in this count. -* Apply to assign the extra Ips -* Remove `private_ips_count` and restore your settings from the first step -* Add the new Ips to your current settings -* Apply again to update the stored state - -This process can also be used to remove IP addresses in addition to the option of manually removing them. Adding IP addresses in a manual fashion is more difficult because it requires knowledge of which addresses are available. -* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). +* `arn` - ARN of the network interface. +* `id` - ID of the network interface. +* `mac_address` - MAC address of the network interface. +* `owner_id` - AWS account ID of the owner of the network interface. +* `private_dns_name` - Private DNS name of the network interface (IPv4). +* `tags_all` - Map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). ## Import From 2f1d052225820e07a91dddf20248e97f2fe224f5 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 15:58:43 -0500 Subject: [PATCH 08/14] Modernize tests --- .../service/ec2/network_interface_test.go | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/internal/service/ec2/network_interface_test.go b/internal/service/ec2/network_interface_test.go index 1b927eadd4c..89c4a77584d 100644 --- a/internal/service/ec2/network_interface_test.go +++ b/internal/service/ec2/network_interface_test.go @@ -689,7 +689,7 @@ type privateIpListTestConfigData struct { replacesInterface bool } -func TestAccAWSENI_PrivateIpsSet(t *testing.T) { +func TestAccEC2NetworkInterface_PrivateIpsSet(t *testing.T) { var networkInterface, lastInterface ec2.NetworkInterface resourceName := "aws_network_interface.test" @@ -709,10 +709,10 @@ func TestAccAWSENI_PrivateIpsSet(t *testing.T) { testSteps := make([]resource.TestStep, len(testConfigs)*2) testSteps[0] = resource.TestStep{ - Config: testAccAWSENIConfigPrivateIpList(testConfigs[0], resourceName), + Config: testAccENIConfig_privateIPList(testConfigs[0], resourceName), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckAWSENIPrivateIpList(testConfigs[0], &networkInterface), + testAccCheckENIPrivateIPList(testConfigs[0], &networkInterface), testAccCheckENIExists(resourceName, &lastInterface), ), } @@ -729,21 +729,21 @@ func TestAccAWSENI_PrivateIpsSet(t *testing.T) { } if testConfig.replacesInterface { testSteps[i*2] = resource.TestStep{ - Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), - testAccCheckAWSENIDifferent(&lastInterface, &networkInterface), // different + testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), + testAccCheckENIDifferent(&lastInterface, &networkInterface), // different testAccCheckENIExists(resourceName, &lastInterface), ), } } else { testSteps[i*2] = resource.TestStep{ - Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), - testAccCheckAWSENISame(&lastInterface, &networkInterface), // same + testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same testAccCheckENIExists(resourceName, &lastInterface), ), } @@ -761,7 +761,7 @@ func TestAccAWSENI_PrivateIpsSet(t *testing.T) { }) } -func TestAccAWSENI_PrivateIpList(t *testing.T) { +func TestAccEC2NetworkInterface_PrivateIpList(t *testing.T) { var networkInterface, lastInterface ec2.NetworkInterface resourceName := "aws_network_interface.test" @@ -785,10 +785,10 @@ func TestAccAWSENI_PrivateIpList(t *testing.T) { testSteps := make([]resource.TestStep, len(testConfigs)*2) testSteps[0] = resource.TestStep{ - Config: testAccAWSENIConfigPrivateIpList(testConfigs[0], resourceName), + Config: testAccENIConfig_privateIPList(testConfigs[0], resourceName), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckAWSENIPrivateIpList(testConfigs[0], &networkInterface), + testAccCheckENIPrivateIPList(testConfigs[0], &networkInterface), testAccCheckENIExists(resourceName, &lastInterface), ), } @@ -805,21 +805,21 @@ func TestAccAWSENI_PrivateIpList(t *testing.T) { } if testConfig.replacesInterface { testSteps[i*2] = resource.TestStep{ - Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), - testAccCheckAWSENIDifferent(&lastInterface, &networkInterface), // different + testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), + testAccCheckENIDifferent(&lastInterface, &networkInterface), // different testAccCheckENIExists(resourceName, &lastInterface), ), } } else { testSteps[i*2] = resource.TestStep{ - Config: testAccAWSENIConfigPrivateIpList(testConfigs[i], resourceName), + Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckAWSENIPrivateIpList(testConfigs[i], &networkInterface), - testAccCheckAWSENISame(&lastInterface, &networkInterface), // same + testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same testAccCheckENIExists(resourceName, &lastInterface), ), } @@ -931,7 +931,7 @@ func testAccCheckENIMakeExternalAttachment(n string, conf *ec2.NetworkInterface) } } -func testAccCheckAWSENIPrivateIpList(testConfig privateIpListTestConfigData, iface *ec2.NetworkInterface) resource.TestCheckFunc { +func testAccCheckENIPrivateIPList(testConfig privateIpListTestConfigData, iface *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { havePrivateIps := tfec2.FlattenNetworkInterfacePrivateIpAddresses(iface.PrivateIpAddresses) PRIVATE_IPS_LOOP: @@ -961,7 +961,7 @@ func testAccCheckAWSENIPrivateIpList(testConfig privateIpListTestConfigData, ifa } } -func testAccCheckAWSENISame(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { +func testAccCheckENISame(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { if aws.StringValue(iface1.NetworkInterfaceId) != aws.StringValue(iface2.NetworkInterfaceId) { return fmt.Errorf("Interface %s should not have been replaced with %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) @@ -970,7 +970,7 @@ func testAccCheckAWSENISame(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInt } } -func testAccCheckAWSENIDifferent(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { +func testAccCheckENIDifferent(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { if aws.StringValue(iface1.NetworkInterfaceId) == aws.StringValue(iface2.NetworkInterfaceId) { return fmt.Errorf("Interface %s should have been replaced, have %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) @@ -1370,7 +1370,7 @@ resource "aws_network_interface" "test" { `, rName, ipv6PrefixCount)) } -func testAccAWSENIConfigPrivateIpList(testConfig privateIpListTestConfigData, rName string) string { +func testAccENIConfig_privateIPList(testConfig privateIpListTestConfigData, rName string) string { var config strings.Builder config.WriteString(fmt.Sprintf(` From 50daa812766df9c517bcd39292145cff5a34bf2b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 16:20:42 -0500 Subject: [PATCH 09/14] Clean up --- internal/service/ec2/network_interface.go | 60 +++++++++---------- .../ec2/network_interface_data_source.go | 2 +- .../service/ec2/network_interface_test.go | 16 ++--- 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/internal/service/ec2/network_interface.go b/internal/service/ec2/network_interface.go index 1e44090fda6..6d6080a0d27 100644 --- a/internal/service/ec2/network_interface.go +++ b/internal/service/ec2/network_interface.go @@ -341,7 +341,7 @@ func resourceNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) er if v, ok := d.GetOk("ipv4_prefixes"); ok && v.(*schema.Set).Len() > 0 { ipv4PrefixesSpecified = true - input.Ipv4Prefixes = expandIpv4PrefixSpecificationRequests(v.(*schema.Set).List()) + input.Ipv4Prefixes = expandIPv4PrefixSpecificationRequests(v.(*schema.Set).List()) } if v, ok := d.GetOk("ipv4_prefix_count"); ok { @@ -353,12 +353,12 @@ func resourceNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) er } if v, ok := d.GetOk("ipv6_addresses"); ok && v.(*schema.Set).Len() > 0 { - input.Ipv6Addresses = expandInstanceIpv6Addresses(v.(*schema.Set).List()) + input.Ipv6Addresses = expandInstanceIPv6Addresses(v.(*schema.Set).List()) } if v, ok := d.GetOk("ipv6_prefixes"); ok && v.(*schema.Set).Len() > 0 { ipv6PrefixesSpecified = true - input.Ipv6Prefixes = expandIpv6PrefixSpecificationRequests(v.(*schema.Set).List()) + input.Ipv6Prefixes = expandIPv6PrefixSpecificationRequests(v.(*schema.Set).List()) } if v, ok := d.GetOk("ipv6_prefix_count"); ok { @@ -367,7 +367,7 @@ func resourceNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) er if d.Get("private_ip_list_enabled").(bool) { if v, ok := d.GetOk("private_ip_list"); ok && len(v.([]interface{})) > 0 { - input.PrivateIpAddresses = expandPrivateIpAddressSpecifications(v.([]interface{})) + input.PrivateIpAddresses = expandPrivateIPAddressSpecifications(v.([]interface{})) } } else { if v, ok := d.GetOk("private_ips"); ok && v.(*schema.Set).Len() > 0 { @@ -389,7 +389,7 @@ func resourceNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) er break } } - input.PrivateIpAddresses = expandPrivateIpAddressSpecifications(count_limited_ips) + input.PrivateIpAddresses = expandPrivateIPAddressSpecifications(count_limited_ips) } else { if v, ok := d.GetOk("private_ips_count"); ok { input.SecondaryPrivateIpAddressCount = aws.Int64(int64(v.(int))) @@ -515,7 +515,7 @@ func resourceNetworkInterfaceRead(d *schema.ResourceData, meta interface{}) erro d.Set("description", eni.Description) d.Set("interface_type", eni.InterfaceType) - if err := d.Set("ipv4_prefixes", flattenIpv4PrefixSpecifications(eni.Ipv4Prefixes)); err != nil { + if err := d.Set("ipv4_prefixes", flattenIPv4PrefixSpecifications(eni.Ipv4Prefixes)); err != nil { return fmt.Errorf("error setting ipv4_prefixes: %w", err) } @@ -531,7 +531,7 @@ func resourceNetworkInterfaceRead(d *schema.ResourceData, meta interface{}) erro return fmt.Errorf("error setting ipv6_addresses: %w", err) } - if err := d.Set("ipv6_prefixes", flattenIpv6PrefixSpecifications(eni.Ipv6Prefixes)); err != nil { + if err := d.Set("ipv6_prefixes", flattenIPv6PrefixSpecifications(eni.Ipv6Prefixes)); err != nil { return fmt.Errorf("error setting ipv6_prefixes: %w", err) } @@ -543,13 +543,13 @@ func resourceNetworkInterfaceRead(d *schema.ResourceData, meta interface{}) erro d.Set("private_dns_name", eni.PrivateDnsName) d.Set("private_ip", eni.PrivateIpAddress) - if err := d.Set("private_ips", FlattenNetworkInterfacePrivateIpAddresses(eni.PrivateIpAddresses)); err != nil { + if err := d.Set("private_ips", FlattenNetworkInterfacePrivateIPAddresses(eni.PrivateIpAddresses)); err != nil { return fmt.Errorf("error setting private_ips: %w", err) } d.Set("private_ips_count", len(eni.PrivateIpAddresses)-1) - if err := d.Set("private_ip_list", FlattenNetworkInterfacePrivateIpAddresses(eni.PrivateIpAddresses)); err != nil { + if err := d.Set("private_ip_list", FlattenNetworkInterfacePrivateIPAddresses(eni.PrivateIpAddresses)); err != nil { return fmt.Errorf("error setting private_ip_list: %s", err) } @@ -1222,7 +1222,7 @@ func flattenNetworkInterfaceAttachment(apiObject *ec2.NetworkInterfaceAttachment return tfMap } -func expandPrivateIpAddressSpecification(tfString string) *ec2.PrivateIpAddressSpecification { +func expandPrivateIPAddressSpecification(tfString string) *ec2.PrivateIpAddressSpecification { if tfString == "" { return nil } @@ -1234,7 +1234,7 @@ func expandPrivateIpAddressSpecification(tfString string) *ec2.PrivateIpAddressS return apiObject } -func expandPrivateIpAddressSpecifications(tfList []interface{}) []*ec2.PrivateIpAddressSpecification { +func expandPrivateIPAddressSpecifications(tfList []interface{}) []*ec2.PrivateIpAddressSpecification { if len(tfList) == 0 { return nil } @@ -1248,7 +1248,7 @@ func expandPrivateIpAddressSpecifications(tfList []interface{}) []*ec2.PrivateIp continue } - apiObject := expandPrivateIpAddressSpecification(tfString) + apiObject := expandPrivateIPAddressSpecification(tfString) if apiObject == nil { continue @@ -1264,7 +1264,7 @@ func expandPrivateIpAddressSpecifications(tfList []interface{}) []*ec2.PrivateIp return apiObjects } -func expandInstanceIpv6Address(tfString string) *ec2.InstanceIpv6Address { +func expandInstanceIPv6Address(tfString string) *ec2.InstanceIpv6Address { if tfString == "" { return nil } @@ -1276,7 +1276,7 @@ func expandInstanceIpv6Address(tfString string) *ec2.InstanceIpv6Address { return apiObject } -func expandInstanceIpv6Addresses(tfList []interface{}) []*ec2.InstanceIpv6Address { +func expandInstanceIPv6Addresses(tfList []interface{}) []*ec2.InstanceIpv6Address { if len(tfList) == 0 { return nil } @@ -1290,7 +1290,7 @@ func expandInstanceIpv6Addresses(tfList []interface{}) []*ec2.InstanceIpv6Addres continue } - apiObject := expandInstanceIpv6Address(tfString) + apiObject := expandInstanceIPv6Address(tfString) if apiObject == nil { continue @@ -1302,7 +1302,7 @@ func expandInstanceIpv6Addresses(tfList []interface{}) []*ec2.InstanceIpv6Addres return apiObjects } -func flattenNetworkInterfacePrivateIpAddress(apiObject *ec2.NetworkInterfacePrivateIpAddress) string { +func flattenNetworkInterfacePrivateIPAddress(apiObject *ec2.NetworkInterfacePrivateIpAddress) string { if apiObject == nil { return "" } @@ -1316,7 +1316,7 @@ func flattenNetworkInterfacePrivateIpAddress(apiObject *ec2.NetworkInterfacePriv return tfString } -func FlattenNetworkInterfacePrivateIpAddresses(apiObjects []*ec2.NetworkInterfacePrivateIpAddress) []string { +func FlattenNetworkInterfacePrivateIPAddresses(apiObjects []*ec2.NetworkInterfacePrivateIpAddress) []string { if len(apiObjects) == 0 { return nil } @@ -1328,7 +1328,7 @@ func FlattenNetworkInterfacePrivateIpAddresses(apiObjects []*ec2.NetworkInterfac continue } - tfList = append(tfList, flattenNetworkInterfacePrivateIpAddress(apiObject)) + tfList = append(tfList, flattenNetworkInterfacePrivateIPAddress(apiObject)) } return tfList @@ -1366,7 +1366,7 @@ func flattenNetworkInterfaceIPv6Addresses(apiObjects []*ec2.NetworkInterfaceIpv6 return tfList } -func expandIpv4PrefixSpecificationRequest(tfString string) *ec2.Ipv4PrefixSpecificationRequest { +func expandIPv4PrefixSpecificationRequest(tfString string) *ec2.Ipv4PrefixSpecificationRequest { if tfString == "" { return nil } @@ -1378,7 +1378,7 @@ func expandIpv4PrefixSpecificationRequest(tfString string) *ec2.Ipv4PrefixSpecif return apiObject } -func expandIpv4PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv4PrefixSpecificationRequest { +func expandIPv4PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv4PrefixSpecificationRequest { if len(tfList) == 0 { return nil } @@ -1392,7 +1392,7 @@ func expandIpv4PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv4Pref continue } - apiObject := expandIpv4PrefixSpecificationRequest(tfString) + apiObject := expandIPv4PrefixSpecificationRequest(tfString) if apiObject == nil { continue @@ -1404,7 +1404,7 @@ func expandIpv4PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv4Pref return apiObjects } -func expandIpv6PrefixSpecificationRequest(tfString string) *ec2.Ipv6PrefixSpecificationRequest { +func expandIPv6PrefixSpecificationRequest(tfString string) *ec2.Ipv6PrefixSpecificationRequest { if tfString == "" { return nil } @@ -1416,7 +1416,7 @@ func expandIpv6PrefixSpecificationRequest(tfString string) *ec2.Ipv6PrefixSpecif return apiObject } -func expandIpv6PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv6PrefixSpecificationRequest { +func expandIPv6PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv6PrefixSpecificationRequest { if len(tfList) == 0 { return nil } @@ -1430,7 +1430,7 @@ func expandIpv6PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv6Pref continue } - apiObject := expandIpv6PrefixSpecificationRequest(tfString) + apiObject := expandIPv6PrefixSpecificationRequest(tfString) if apiObject == nil { continue @@ -1442,7 +1442,7 @@ func expandIpv6PrefixSpecificationRequests(tfList []interface{}) []*ec2.Ipv6Pref return apiObjects } -func flattenIpv4PrefixSpecification(apiObject *ec2.Ipv4PrefixSpecification) string { +func flattenIPv4PrefixSpecification(apiObject *ec2.Ipv4PrefixSpecification) string { if apiObject == nil { return "" } @@ -1456,7 +1456,7 @@ func flattenIpv4PrefixSpecification(apiObject *ec2.Ipv4PrefixSpecification) stri return tfString } -func flattenIpv4PrefixSpecifications(apiObjects []*ec2.Ipv4PrefixSpecification) []string { +func flattenIPv4PrefixSpecifications(apiObjects []*ec2.Ipv4PrefixSpecification) []string { if len(apiObjects) == 0 { return nil } @@ -1468,13 +1468,13 @@ func flattenIpv4PrefixSpecifications(apiObjects []*ec2.Ipv4PrefixSpecification) continue } - tfList = append(tfList, flattenIpv4PrefixSpecification(apiObject)) + tfList = append(tfList, flattenIPv4PrefixSpecification(apiObject)) } return tfList } -func flattenIpv6PrefixSpecification(apiObject *ec2.Ipv6PrefixSpecification) string { +func flattenIPv6PrefixSpecification(apiObject *ec2.Ipv6PrefixSpecification) string { if apiObject == nil { return "" } @@ -1488,7 +1488,7 @@ func flattenIpv6PrefixSpecification(apiObject *ec2.Ipv6PrefixSpecification) stri return tfString } -func flattenIpv6PrefixSpecifications(apiObjects []*ec2.Ipv6PrefixSpecification) []string { +func flattenIPv6PrefixSpecifications(apiObjects []*ec2.Ipv6PrefixSpecification) []string { if len(apiObjects) == 0 { return nil } @@ -1500,7 +1500,7 @@ func flattenIpv6PrefixSpecifications(apiObjects []*ec2.Ipv6PrefixSpecification) continue } - tfList = append(tfList, flattenIpv6PrefixSpecification(apiObject)) + tfList = append(tfList, flattenIPv6PrefixSpecification(apiObject)) } return tfList diff --git a/internal/service/ec2/network_interface_data_source.go b/internal/service/ec2/network_interface_data_source.go index c1dd6481dea..5562ac3a4f7 100644 --- a/internal/service/ec2/network_interface_data_source.go +++ b/internal/service/ec2/network_interface_data_source.go @@ -203,7 +203,7 @@ func dataSourceNetworkInterfaceRead(d *schema.ResourceData, meta interface{}) er d.Set("owner_id", ownerID) d.Set("private_dns_name", eni.PrivateDnsName) d.Set("private_ip", eni.PrivateIpAddress) - d.Set("private_ips", FlattenNetworkInterfacePrivateIpAddresses(eni.PrivateIpAddresses)) + d.Set("private_ips", FlattenNetworkInterfacePrivateIPAddresses(eni.PrivateIpAddresses)) d.Set("requester_id", eni.RequesterId) d.Set("subnet_id", eni.SubnetId) d.Set("vpc_id", eni.VpcId) diff --git a/internal/service/ec2/network_interface_test.go b/internal/service/ec2/network_interface_test.go index 89c4a77584d..6c2826095fa 100644 --- a/internal/service/ec2/network_interface_test.go +++ b/internal/service/ec2/network_interface_test.go @@ -681,7 +681,7 @@ func TestAccEC2NetworkInterface_ENI_ipv6PrefixCount(t *testing.T) { }) } -type privateIpListTestConfigData struct { +type privateIPListTestConfigData struct { private_ips []string private_ips_count string private_ip_list_enabled string @@ -689,11 +689,11 @@ type privateIpListTestConfigData struct { replacesInterface bool } -func TestAccEC2NetworkInterface_PrivateIpsSet(t *testing.T) { +func TestAccEC2NetworkInterface_privateIPsSet(t *testing.T) { var networkInterface, lastInterface ec2.NetworkInterface resourceName := "aws_network_interface.test" - testConfigs := []privateIpListTestConfigData{ + testConfigs := []privateIPListTestConfigData{ {[]string{"44", "59", "123"}, "", "", []string{}, false}, // Configuration with three private_ips {[]string{"123", "44", "59"}, "", "", []string{}, false}, // Change order of private_ips @@ -761,12 +761,12 @@ func TestAccEC2NetworkInterface_PrivateIpsSet(t *testing.T) { }) } -func TestAccEC2NetworkInterface_PrivateIpList(t *testing.T) { +func TestAccEC2NetworkInterface_privateIPList(t *testing.T) { var networkInterface, lastInterface ec2.NetworkInterface resourceName := "aws_network_interface.test" // private_ips, private_ips_count, private_ip_list_enabed, private_ip_list, replacesInterface - testConfigs := []privateIpListTestConfigData{ + testConfigs := []privateIPListTestConfigData{ {[]string{"17"}, "", "", []string{}, true}, // Build a set incrementally in order {[]string{"17", "45"}, "", "", []string{}, false}, // Add to set {[]string{"17", "45", "89"}, "", "", []string{}, false}, // Add to set @@ -931,9 +931,9 @@ func testAccCheckENIMakeExternalAttachment(n string, conf *ec2.NetworkInterface) } } -func testAccCheckENIPrivateIPList(testConfig privateIpListTestConfigData, iface *ec2.NetworkInterface) resource.TestCheckFunc { +func testAccCheckENIPrivateIPList(testConfig privateIPListTestConfigData, iface *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { - havePrivateIps := tfec2.FlattenNetworkInterfacePrivateIpAddresses(iface.PrivateIpAddresses) + havePrivateIps := tfec2.FlattenNetworkInterfacePrivateIPAddresses(iface.PrivateIpAddresses) PRIVATE_IPS_LOOP: // every IP from private_ips should be present on the interface for _, needIp := range testConfig.private_ips { @@ -1370,7 +1370,7 @@ resource "aws_network_interface" "test" { `, rName, ipv6PrefixCount)) } -func testAccENIConfig_privateIPList(testConfig privateIpListTestConfigData, rName string) string { +func testAccENIConfig_privateIPList(testConfig privateIPListTestConfigData, rName string) string { var config strings.Builder config.WriteString(fmt.Sprintf(` From e8e79bba7fee58865c75bb3e579083d02e9611df Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 18:12:20 -0500 Subject: [PATCH 10/14] Make tests more readable --- .../service/ec2/network_interface_test.go | 471 +++++++++++------- 1 file changed, 284 insertions(+), 187 deletions(-) diff --git a/internal/service/ec2/network_interface_test.go b/internal/service/ec2/network_interface_test.go index 6c2826095fa..07ff7bbed1d 100644 --- a/internal/service/ec2/network_interface_test.go +++ b/internal/service/ec2/network_interface_test.go @@ -2,7 +2,9 @@ package ec2_test import ( "fmt" + "reflect" "regexp" + "sort" "strings" "testing" @@ -295,6 +297,10 @@ func TestAccEC2NetworkInterface_description(t *testing.T) { } func TestAccEC2NetworkInterface_attachment(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + var conf ec2.NetworkInterface resourceName := "aws_network_interface.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -681,159 +687,239 @@ func TestAccEC2NetworkInterface_ENI_ipv6PrefixCount(t *testing.T) { }) } -type privateIPListTestConfigData struct { - private_ips []string - private_ips_count string - private_ip_list_enabled string - private_ip_list []string - replacesInterface bool -} - -func TestAccEC2NetworkInterface_privateIPsSet(t *testing.T) { +func TestAccEC2NetworkInterface_privateIPSet(t *testing.T) { var networkInterface, lastInterface ec2.NetworkInterface resourceName := "aws_network_interface.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - testConfigs := []privateIPListTestConfigData{ - - {[]string{"44", "59", "123"}, "", "", []string{}, false}, // Configuration with three private_ips - {[]string{"123", "44", "59"}, "", "", []string{}, false}, // Change order of private_ips - {[]string{"123", "12", "59", "44"}, "", "", []string{}, false}, // Add secondaries to private_ips - {[]string{"123", "59", "44"}, "", "", []string{}, false}, // Remove secondaries from private_ips - {[]string{"123", "59", "57"}, "", "", []string{}, true}, // Remove primary - {[]string{}, "4", "", []string{}, false}, // Use count to add IPs - {[]string{"44", "57"}, "", "", []string{}, false}, // Change list, retain primary - {[]string{"44", "57", "123", "12"}, "", "", []string{}, false}, // Add to secondaries - {[]string{"17"}, "", "", []string{}, true}, // New list - {[]string{"17", "45", "89"}, "", "", []string{}, false}, // Add secondaries - } - - testSteps := make([]resource.TestStep, len(testConfigs)*2) - testSteps[0] = resource.TestStep{ - Config: testAccENIConfig_privateIPList(testConfigs[0], resourceName), - Check: resource.ComposeTestCheckFunc( - testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckENIPrivateIPList(testConfigs[0], &networkInterface), - testAccCheckENIExists(resourceName, &lastInterface), - ), - } - testSteps[1] = resource.TestStep{ - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, - } - - for i, testConfig := range testConfigs { - if i == 0 { - continue - } - if testConfig.replacesInterface { - testSteps[i*2] = resource.TestStep{ - Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckENIDestroy, + Steps: []resource.TestStep{ + { // Configuration with three private_ips + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.44", "172.16.10.59", "172.16.10.123"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.44", "172.16.10.59", "172.16.10.123"}, &networkInterface), + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, + }, + { // Change order of private_ips + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.123", "172.16.10.44", "172.16.10.59"}), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.44", "172.16.10.59", "172.16.10.123"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Add secondaries to private_ips + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.123", "172.16.10.12", "172.16.10.44", "172.16.10.59"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.44", "172.16.10.12", "172.16.10.59", "172.16.10.123"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Remove secondary to private_ips + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.123", "172.16.10.44", "172.16.10.59"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.44", "172.16.10.59", "172.16.10.123"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Remove primary + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.123", "172.16.10.59", "172.16.10.57"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.57", "172.16.10.59", "172.16.10.123"}, &networkInterface), testAccCheckENIDifferent(&lastInterface, &networkInterface), // different testAccCheckENIExists(resourceName, &lastInterface), ), - } - } else { - testSteps[i*2] = resource.TestStep{ - Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), + }, + { // Use count to add IPs + Config: testAccENIConfig_privateIPSetCount(rName, 4), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), testAccCheckENISame(&lastInterface, &networkInterface), // same testAccCheckENIExists(resourceName, &lastInterface), ), - } - } - // import check - testSteps[i*2+1] = testSteps[1] - } - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckENIDestroy, - Steps: testSteps, + }, + { // Change list, retain primary + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.44", "172.16.10.57"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.44", "172.16.10.57"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // New list + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.17"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.17"}, &networkInterface), + testAccCheckENIDifferent(&lastInterface, &networkInterface), // different + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + }, }) } func TestAccEC2NetworkInterface_privateIPList(t *testing.T) { var networkInterface, lastInterface ec2.NetworkInterface resourceName := "aws_network_interface.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - // private_ips, private_ips_count, private_ip_list_enabed, private_ip_list, replacesInterface - testConfigs := []privateIPListTestConfigData{ - {[]string{"17"}, "", "", []string{}, true}, // Build a set incrementally in order - {[]string{"17", "45"}, "", "", []string{}, false}, // Add to set - {[]string{"17", "45", "89"}, "", "", []string{}, false}, // Add to set - {[]string{"17", "45", "89", "122"}, "", "", []string{}, false}, // Add to set - {[]string{}, "", "true", []string{"17", "45", "89", "122"}, false}, // Change from set to list using same order - {[]string{}, "", "true", []string{"17", "89", "45", "122"}, false}, // Change order of private_ip_list - {[]string{}, "", "true", []string{"17", "89", "45"}, false}, // Remove secondaries from end - {[]string{}, "", "true", []string{"17", "89", "45", "123"}, false}, // Add secondaries to end - {[]string{}, "", "true", []string{"17", "89", "77", "45", "123"}, false}, // Add secondaries to middle - {[]string{}, "", "true", []string{"17", "89", "123"}, false}, // Remove secondaries from middle - {[]string{}, "4", "", []string{}, false}, // Use count to add IPs - {[]string{}, "", "true", []string{"59", "123", "44"}, true}, // Change to specific list - forces new - {[]string{}, "", "true", []string{"123", "59", "44"}, true}, // Change first of private_ip_list - forces new - {[]string{"123", "59", "44"}, "", "", []string{}, false}, // Change from list to set using same set - } - - testSteps := make([]resource.TestStep, len(testConfigs)*2) - testSteps[0] = resource.TestStep{ - Config: testAccENIConfig_privateIPList(testConfigs[0], resourceName), - Check: resource.ComposeTestCheckFunc( - testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckENIPrivateIPList(testConfigs[0], &networkInterface), - testAccCheckENIExists(resourceName, &lastInterface), - ), - } - testSteps[1] = resource.TestStep{ - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, - } - - for i, testConfig := range testConfigs { - if i == 0 { - continue - } - if testConfig.replacesInterface { - testSteps[i*2] = resource.TestStep{ - Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckENIDestroy, + Steps: []resource.TestStep{ + { // Build a set incrementally in order + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.17"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.17"}, &networkInterface), + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"private_ip_list_enabled", "ipv6_address_list_enabled"}, + }, + { // Add to set + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.17", "172.16.10.45"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.17", "172.16.10.45"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Add to set + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.17", "172.16.10.45", "172.16.10.89"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.17", "172.16.10.45", "172.16.10.89"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Add to set + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.17", "172.16.10.45", "172.16.10.89", "172.16.10.122"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.17", "172.16.10.45", "172.16.10.89", "172.16.10.122"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Change from set to list using same order + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.17", "172.16.10.45", "172.16.10.89", "172.16.10.122"}), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.17", "172.16.10.45", "172.16.10.89", "172.16.10.122"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Change order of private_ip_list + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.17", "172.16.10.89", "172.16.10.45", "172.16.10.122"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.17", "172.16.10.89", "172.16.10.45", "172.16.10.122"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Remove secondaries from end + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.17", "172.16.10.89", "172.16.10.45"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.17", "172.16.10.89", "172.16.10.45"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Add secondaries to end + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.17", "172.16.10.89", "172.16.10.45", "172.16.10.123"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.17", "172.16.10.89", "172.16.10.45", "172.16.10.123"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Add secondaries to middle + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.17", "172.16.10.89", "172.16.10.77", "172.16.10.45", "172.16.10.123"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.17", "172.16.10.89", "172.16.10.77", "172.16.10.45", "172.16.10.123"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Remove secondaries from middle + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.17", "172.16.10.89", "172.16.10.123"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.17", "172.16.10.89", "172.16.10.123"}, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Use count to add IPs + Config: testAccENIConfig_privateIPSetCount(rName, 4), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENISame(&lastInterface, &networkInterface), // same + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Change to specific list - forces new + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.59", "172.16.10.123", "172.16.10.38"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.59", "172.16.10.123", "172.16.10.38"}, &networkInterface), + testAccCheckENIDifferent(&lastInterface, &networkInterface), // different + testAccCheckENIExists(resourceName, &lastInterface), + ), + }, + { // Change first of private_ip_list - forces new + Config: testAccENIConfig_privateIPList(rName, []string{"172.16.10.123", "172.16.10.59", "172.16.10.38"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckENIExists(resourceName, &networkInterface), + testAccCheckENIPrivateIPList([]string{"172.16.10.123", "172.16.10.59", "172.16.10.38"}, &networkInterface), testAccCheckENIDifferent(&lastInterface, &networkInterface), // different testAccCheckENIExists(resourceName, &lastInterface), ), - } - } else { - testSteps[i*2] = resource.TestStep{ - Config: testAccENIConfig_privateIPList(testConfigs[i], resourceName), + }, + { // Change from list to set using same set + Config: testAccENIConfig_privateIPSet(rName, []string{"172.16.10.123", "172.16.10.59", "172.16.10.38"}), Check: resource.ComposeTestCheckFunc( testAccCheckENIExists(resourceName, &networkInterface), - testAccCheckENIPrivateIPList(testConfigs[i], &networkInterface), + testAccCheckENIPrivateIPSet([]string{"172.16.10.123", "172.16.10.59", "172.16.10.38"}, &networkInterface), testAccCheckENISame(&lastInterface, &networkInterface), // same testAccCheckENIExists(resourceName, &lastInterface), ), - } - } - // import check - testSteps[i*2+1] = testSteps[1] - } - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckENIDestroy, - Steps: testSteps, + }, + }, }) } @@ -931,40 +1017,53 @@ func testAccCheckENIMakeExternalAttachment(n string, conf *ec2.NetworkInterface) } } -func testAccCheckENIPrivateIPList(testConfig privateIPListTestConfigData, iface *ec2.NetworkInterface) resource.TestCheckFunc { +func testAccCheckENIPrivateIPSet(ips []string, iface *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { - havePrivateIps := tfec2.FlattenNetworkInterfacePrivateIPAddresses(iface.PrivateIpAddresses) - PRIVATE_IPS_LOOP: - // every IP from private_ips should be present on the interface - for _, needIp := range testConfig.private_ips { - for _, haveIp := range havePrivateIps { - if haveIp == "172.16.10."+needIp { - continue PRIVATE_IPS_LOOP - } - } - return fmt.Errorf("expected ip 172.16.10.%s to be in interface set %s", needIp, strings.Join(havePrivateIps, ",")) - } - // every configured IP should be present on the interface - for needIdx, needIp := range testConfig.private_ip_list { - if len(havePrivateIps) <= needIdx || "172.16.10."+needIp != havePrivateIps[needIdx] { - return fmt.Errorf("expected ip 172.16.10.%s to be at %d in the list %s", needIp, needIdx, strings.Join(havePrivateIps, ",")) - } - } - // number of ips configured should match interface - if len(testConfig.private_ips) > 0 && len(testConfig.private_ips) != len(havePrivateIps) { - return fmt.Errorf("expected %s got %s", strings.Join(testConfig.private_ips, ","), strings.Join(havePrivateIps, ",")) + iIPs := tfec2.FlattenNetworkInterfacePrivateIPAddresses(iface.PrivateIpAddresses) + + if !stringSlicesEqualIgnoreOrder(ips, iIPs) { + return fmt.Errorf("expected private IP set %s, got %s", strings.Join(ips, ","), strings.Join(iIPs, ",")) } - if len(testConfig.private_ip_list) > 0 && len(testConfig.private_ip_list) != len(havePrivateIps) { - return fmt.Errorf("expected %s got %s", strings.Join(testConfig.private_ip_list, ","), strings.Join(havePrivateIps, ",")) + + return nil + } +} + +func testAccCheckENIPrivateIPList(ips []string, iface *ec2.NetworkInterface) resource.TestCheckFunc { + return func(s *terraform.State) error { + iIPs := tfec2.FlattenNetworkInterfacePrivateIPAddresses(iface.PrivateIpAddresses) + + if !stringSlicesEqual(ips, iIPs) { + return fmt.Errorf("expected private IP set %s, got %s", strings.Join(ips, ","), strings.Join(iIPs, ",")) } + return nil } } +func stringSlicesEqualIgnoreOrder(s1, s2 []string) bool { + if len(s1) != len(s2) { + return false + } + + sort.Strings(s1) + sort.Strings(s2) + + return reflect.DeepEqual(s1, s2) +} + +func stringSlicesEqual(s1, s2 []string) bool { + if len(s1) != len(s2) { + return false + } + + return reflect.DeepEqual(s1, s2) +} + func testAccCheckENISame(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { if aws.StringValue(iface1.NetworkInterfaceId) != aws.StringValue(iface2.NetworkInterfaceId) { - return fmt.Errorf("Interface %s should not have been replaced with %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) + return fmt.Errorf("interface %s should not have been replaced with %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) } return nil } @@ -973,7 +1072,7 @@ func testAccCheckENISame(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterf func testAccCheckENIDifferent(iface1 *ec2.NetworkInterface, iface2 *ec2.NetworkInterface) resource.TestCheckFunc { return func(s *terraform.State) error { if aws.StringValue(iface1.NetworkInterfaceId) == aws.StringValue(iface2.NetworkInterfaceId) { - return fmt.Errorf("Interface %s should have been replaced, have %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) + return fmt.Errorf("interface %s should have been replaced, have %s", aws.StringValue(iface1.NetworkInterfaceId), aws.StringValue(iface2.NetworkInterfaceId)) } return nil } @@ -1098,7 +1197,7 @@ resource "aws_network_interface" "test" { } func testAccENIIPV6CountConfig(rName string, ipv6Count int) string { - return acctest.ConfigCompose(testAccENIIPV6BaseConfig(rName) + fmt.Sprintf(` + return acctest.ConfigCompose(testAccENIIPV6BaseConfig(rName), fmt.Sprintf(` resource "aws_network_interface" "test" { subnet_id = aws_subnet.test.id private_ips = ["172.16.10.100"] @@ -1128,7 +1227,7 @@ resource "aws_network_interface" "test" { } func testAccENISourceDestCheckConfig(rName string, sourceDestCheck bool) string { - return acctest.ConfigCompose(testAccENIIPV6BaseConfig(rName) + fmt.Sprintf(` + return acctest.ConfigCompose(testAccENIIPV6BaseConfig(rName), fmt.Sprintf(` resource "aws_network_interface" "test" { subnet_id = aws_subnet.test.id source_dest_check = %[2]t @@ -1312,7 +1411,7 @@ resource "aws_network_interface" "test" { } func testAccENIIPV4PrefixCountConfig(rName string, ipv4PrefixCount int) string { - return acctest.ConfigCompose(testAccENIIPV4BaseConfig(rName) + fmt.Sprintf(` + return acctest.ConfigCompose(testAccENIIPV4BaseConfig(rName), fmt.Sprintf(` resource "aws_network_interface" "test" { subnet_id = aws_subnet.test.id ipv4_prefix_count = %[2]d @@ -1356,7 +1455,7 @@ resource "aws_network_interface" "test" { } func testAccENIIPV6PrefixCountConfig(rName string, ipv6PrefixCount int) string { - return acctest.ConfigCompose(testAccENIIPV6BaseConfig(rName) + fmt.Sprintf(` + return acctest.ConfigCompose(testAccENIIPV6BaseConfig(rName), fmt.Sprintf(` resource "aws_network_interface" "test" { subnet_id = aws_subnet.test.id private_ips = ["172.16.10.100"] @@ -1370,41 +1469,39 @@ resource "aws_network_interface" "test" { `, rName, ipv6PrefixCount)) } -func testAccENIConfig_privateIPList(testConfig privateIPListTestConfigData, rName string) string { - var config strings.Builder - - config.WriteString(fmt.Sprintf(` -%s "aws_network_interface" "test" { - subnet_id = aws_subnet.test.id - security_groups = [aws_security_group.test.id] - description = "Managed by Terraform" -`, "resource")) - - if len(testConfig.private_ips) > 0 { - config.WriteString(" private_ips = [\n") - for _, ip := range testConfig.private_ips { - config.WriteString(fmt.Sprintf(" \"172.16.10.%s\",\n", ip)) - } - config.WriteString("]\n") - } - - if testConfig.private_ips_count != "" { - config.WriteString(fmt.Sprintf(" private_ips_count = %s\n", testConfig.private_ips_count)) - } - - if testConfig.private_ip_list_enabled != "" { - config.WriteString(fmt.Sprintf(" private_ip_list_enabled = %s\n", testConfig.private_ip_list_enabled)) - } - config.WriteString(" ipv6_address_list_enabled = false\n") +func testAccENIConfig_privateIPSet(rName string, privateIPs []string) string { + return acctest.ConfigCompose( + testAccENIIPV6BaseConfig(rName), + fmt.Sprintf(` +resource "aws_network_interface" "test" { + subnet_id = aws_subnet.test.id + security_groups = [aws_security_group.test.id] + private_ips = ["%[1]s"] +} +`, strings.Join(privateIPs, `", "`))) +} - if len(testConfig.private_ip_list) > 0 { - config.WriteString(" private_ip_list = [\n") - for _, ip := range testConfig.private_ip_list { - config.WriteString(fmt.Sprintf(" \"172.16.10.%s\",\n", ip)) - } - config.WriteString("]\n") - } +func testAccENIConfig_privateIPSetCount(rName string, count int) string { + return acctest.ConfigCompose( + testAccENIIPV6BaseConfig(rName), + fmt.Sprintf(` +resource "aws_network_interface" "test" { + subnet_id = aws_subnet.test.id + security_groups = [aws_security_group.test.id] + private_ips_count = %[1]d +} +`, count)) +} - config.WriteString("}\n") - return testAccENIIPV6BaseConfig(rName) + config.String() +func testAccENIConfig_privateIPList(rName string, privateIPs []string) string { + return acctest.ConfigCompose( + testAccENIIPV6BaseConfig(rName), + fmt.Sprintf(` +resource "aws_network_interface" "test" { + subnet_id = aws_subnet.test.id + security_groups = [aws_security_group.test.id] + private_ip_list_enabled = true + private_ip_list = ["%[1]s"] +} +`, strings.Join(privateIPs, `", "`))) } From 8eddc107fe76f86f5d72fbeeff3596ccb7b9810f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 18:14:35 -0500 Subject: [PATCH 11/14] Variable naming --- internal/service/ec2/network_interface.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/ec2/network_interface.go b/internal/service/ec2/network_interface.go index 6d6080a0d27..28a1b3124e9 100644 --- a/internal/service/ec2/network_interface.go +++ b/internal/service/ec2/network_interface.go @@ -382,14 +382,14 @@ func resourceNetworkInterfaceCreate(d *schema.ResourceData, meta interface{}) er } } // truncate the list - count_limited_ips := make([]interface{}, totalPrivateIPs) + countLimitedIPs := make([]interface{}, totalPrivateIPs) for i, ip := range privateIPs { - count_limited_ips[i] = ip.(string) + countLimitedIPs[i] = ip.(string) if i == totalPrivateIPs-1 { break } } - input.PrivateIpAddresses = expandPrivateIPAddressSpecifications(count_limited_ips) + input.PrivateIpAddresses = expandPrivateIPAddressSpecifications(countLimitedIPs) } else { if v, ok := d.GetOk("private_ips_count"); ok { input.SecondaryPrivateIpAddressCount = aws.Int64(int64(v.(int))) From 1887b916fd814a22eb299d687c6fc9bfd5fb29ed Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 18:16:25 -0500 Subject: [PATCH 12/14] Meet the Lintstones --- internal/service/ec2/network_interface_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/ec2/network_interface_test.go b/internal/service/ec2/network_interface_test.go index 07ff7bbed1d..2dfbc9fb1e0 100644 --- a/internal/service/ec2/network_interface_test.go +++ b/internal/service/ec2/network_interface_test.go @@ -1501,7 +1501,7 @@ resource "aws_network_interface" "test" { subnet_id = aws_subnet.test.id security_groups = [aws_security_group.test.id] private_ip_list_enabled = true - private_ip_list = ["%[1]s"] + private_ip_list = ["%[1]s"] } `, strings.Join(privateIPs, `", "`))) } From 899a09a635b2351a7405b2c2e0d4f5a8b6ed8ea7 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 18:19:19 -0500 Subject: [PATCH 13/14] Doc fix --- website/docs/r/network_interface.markdown | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/website/docs/r/network_interface.markdown b/website/docs/r/network_interface.markdown index 4738f37beb2..89799f2f0a7 100644 --- a/website/docs/r/network_interface.markdown +++ b/website/docs/r/network_interface.markdown @@ -52,7 +52,7 @@ The following arguments are optional: * `attachment` - (Optional) Configuration block to define the attachment of the ENI. See below. * `description` - (Optional) Description for the network interface. -* `interface_type` - (Optional) Type of network interface to create. Set to `efa` for Elastic Fabric Adapter. +* `interface_type` - (Optional) Type of network interface to create. Set to `efa` for Elastic Fabric Adapter. Changing `interface_type` will cause the resource to be destroyed and re-created. * `ipv4_prefix_count` - (Optional) Number of IPv4 prefixes that AWS automatically assigns to the network interface. * `ipv4_prefixes` - (Optional) One or more IPv4 prefixes assigned to the network interface. * `ipv6_address_count` - (Optional) Number of IPv6 addresses to assign to a network interface. You can't use this option if specifying specific `ipv6_addresses`. If your subnet has the AssignIpv6AddressOnCreation attribute set to `true`, you can specify `0` to override this setting. @@ -69,8 +69,6 @@ The following arguments are optional: * `source_dest_check` - (Optional) Whether to enable source destination checking for the ENI. Default true. * `tags` - (Optional) Map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. --> **NOTE:** Changing `interface_type` will cause the resource to be destroyed and re-created. - ### `attachment` * `instance` - (Required) ID of the instance to attach to. From 4d2d62805ceb7a91682630af5de0dbca856c09e8 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 21 Jan 2022 18:21:49 -0500 Subject: [PATCH 14/14] Mark long-running test --- internal/service/ec2/network_interface_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/service/ec2/network_interface_test.go b/internal/service/ec2/network_interface_test.go index 2dfbc9fb1e0..d286e1a10a9 100644 --- a/internal/service/ec2/network_interface_test.go +++ b/internal/service/ec2/network_interface_test.go @@ -779,6 +779,10 @@ func TestAccEC2NetworkInterface_privateIPSet(t *testing.T) { } func TestAccEC2NetworkInterface_privateIPList(t *testing.T) { + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + var networkInterface, lastInterface ec2.NetworkInterface resourceName := "aws_network_interface.test" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)