From 6b450302fd98b82b5858b31c9149128ce6774620 Mon Sep 17 00:00:00 2001 From: Simon Effenberg Date: Mon, 13 May 2019 10:54:16 +0200 Subject: [PATCH 1/4] delete_on_termination on ENI has to be optional like the EBS delete_on_termination this can be optional and cannot be treated like a real bool but has to be treated as a string which can be empty or a bool representation --- aws/resource_aws_launch_template.go | 37 +++++++++++------ aws/resource_aws_launch_template_test.go | 53 ++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/aws/resource_aws_launch_template.go b/aws/resource_aws_launch_template.go index 457d2b4a3968..125352389e87 100644 --- a/aws/resource_aws_launch_template.go +++ b/aws/resource_aws_launch_template.go @@ -381,8 +381,14 @@ func resourceAwsLaunchTemplate() *schema.Resource { ValidateFunc: validateTypeStringNullableBoolean, }, "delete_on_termination": { - Type: schema.TypeBool, - Optional: true, + // Use TypeString to allow an "unspecified" value, + // since TypeBool only has true/false with false default. + // The conversion from bare true/false values in + // configurations to TypeString value is currently safe. + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: suppressEquivalentTypeStringBoolean, + ValidateFunc: validateTypeStringNullableBoolean, }, "description": { Type: schema.TypeString, @@ -956,19 +962,22 @@ func getNetworkInterfaces(n []*ec2.LaunchTemplateInstanceNetworkInterfaceSpecifi var ipv4Addresses []string networkInterface := map[string]interface{}{ - "delete_on_termination": aws.BoolValue(v.DeleteOnTermination), - "description": aws.StringValue(v.Description), - "device_index": aws.Int64Value(v.DeviceIndex), - "ipv4_address_count": aws.Int64Value(v.SecondaryPrivateIpAddressCount), - "ipv6_address_count": aws.Int64Value(v.Ipv6AddressCount), - "network_interface_id": aws.StringValue(v.NetworkInterfaceId), - "private_ip_address": aws.StringValue(v.PrivateIpAddress), - "subnet_id": aws.StringValue(v.SubnetId), + "description": aws.StringValue(v.Description), + "device_index": aws.Int64Value(v.DeviceIndex), + "ipv4_address_count": aws.Int64Value(v.SecondaryPrivateIpAddressCount), + "ipv6_address_count": aws.Int64Value(v.Ipv6AddressCount), + "network_interface_id": aws.StringValue(v.NetworkInterfaceId), + "private_ip_address": aws.StringValue(v.PrivateIpAddress), + "subnet_id": aws.StringValue(v.SubnetId), } if v.AssociatePublicIpAddress != nil { networkInterface["associate_public_ip_address"] = strconv.FormatBool(aws.BoolValue(v.AssociatePublicIpAddress)) } + if v.DeleteOnTermination != nil { + networkInterface["delete_on_termination"] = strconv.FormatBool(aws.BoolValue(v.DeleteOnTermination)) + } + if len(v.Ipv6Addresses) > 0 { raw, ok := networkInterface["ipv6_addresses"] if !ok { @@ -1310,8 +1319,12 @@ func readNetworkInterfacesFromConfig(ni map[string]interface{}) (*ec2.LaunchTemp var privateIpAddress string networkInterface := &ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest{} - if v, ok := ni["delete_on_termination"]; ok { - networkInterface.DeleteOnTermination = aws.Bool(v.(bool)) + if v, ok := ni["delete_on_termination"]; ok && v.(string) != "" { + vBool, err := strconv.ParseBool(v.(string)) + if err != nil { + return nil, fmt.Errorf("error converting delete_on_termination %q from string to boolean: %s", v.(string), err) + } + networkInterface.DeleteOnTermination = aws.Bool(vBool) } if v, ok := ni["description"].(string); ok && v != "" { diff --git a/aws/resource_aws_launch_template_test.go b/aws/resource_aws_launch_template_test.go index 109fc6813a88..3a7e235c07ff 100644 --- a/aws/resource_aws_launch_template_test.go +++ b/aws/resource_aws_launch_template_test.go @@ -284,6 +284,43 @@ func TestAccAWSLaunchTemplate_ElasticInferenceAccelerator(t *testing.T) { }) } +func TestAccAWSLaunchTemplate_NetworkInterfaces_DeleteOnTermination(t *testing.T) { + var template ec2.LaunchTemplate + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_launch_template.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSLaunchTemplateDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSLaunchTemplateExists(resourceName, &template), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.security_groups.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.delete_on_termination", "true"), + ), + }, + { + Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSLaunchTemplateExists(resourceName, &template), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.security_groups.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.delete_on_termination", "false"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccAWSLaunchTemplate_data(t *testing.T) { var template ec2.LaunchTemplate resourceName := "aws_launch_template.test" @@ -312,6 +349,7 @@ func TestAccAWSLaunchTemplate_data(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "monitoring.#", "1"), resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.security_groups.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.delete_on_termination", ""), resource.TestCheckResourceAttr(resourceName, "placement.#", "1"), resource.TestCheckResourceAttrSet(resourceName, "ram_disk_id"), resource.TestCheckResourceAttr(resourceName, "vpc_security_group_ids.#", "1"), @@ -624,6 +662,7 @@ func TestAccAWSLaunchTemplate_networkInterface(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), resource.TestCheckResourceAttrSet(resourceName, "network_interfaces.0.network_interface_id"), resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.associate_public_ip_address", ""), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.delete_on_termination", ""), resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.ipv4_address_count", "2"), ), }, @@ -1033,6 +1072,20 @@ resource "aws_autoscaling_group" "test" { `, rName, deleteOnTermination, rName) } +func testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName string, deleteOnTermination bool) string { + return fmt.Sprintf(` +resource "aws_launch_template" "test" { + name = %q + + network_interfaces { + network_interface_id = "eni-123456ab" + security_groups = ["sg-1a23bc45"] + delete_on_termination = %t + } +} +`, rName, deleteOnTermination) +} + func testAccAWSLaunchTemplateConfig_EbsOptimized(rName, ebsOptimized string) string { return fmt.Sprintf(` resource "aws_launch_template" "test" { From d047c7fe3466dc019e67272e190136830c7a4a5b Mon Sep 17 00:00:00 2001 From: Simon Effenberg Date: Tue, 14 Jul 2020 17:44:20 +0200 Subject: [PATCH 2/4] testing all possible inputs now testing as well `delete_on_termination = ""` and `delete_on_termination = null` which both should not set the value to anything. --- aws/resource_aws_launch_template.go | 2 +- aws/resource_aws_launch_template_test.go | 26 ++++++++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/aws/resource_aws_launch_template.go b/aws/resource_aws_launch_template.go index 125352389e87..4fc1577e1137 100644 --- a/aws/resource_aws_launch_template.go +++ b/aws/resource_aws_launch_template.go @@ -1322,7 +1322,7 @@ func readNetworkInterfacesFromConfig(ni map[string]interface{}) (*ec2.LaunchTemp if v, ok := ni["delete_on_termination"]; ok && v.(string) != "" { vBool, err := strconv.ParseBool(v.(string)) if err != nil { - return nil, fmt.Errorf("error converting delete_on_termination %q from string to boolean: %s", v.(string), err) + return nil, fmt.Errorf("error converting delete_on_termination %q from string to boolean: %w", v.(string), err) } networkInterface.DeleteOnTermination = aws.Bool(vBool) } diff --git a/aws/resource_aws_launch_template_test.go b/aws/resource_aws_launch_template_test.go index 3a7e235c07ff..a9e1e2900830 100644 --- a/aws/resource_aws_launch_template_test.go +++ b/aws/resource_aws_launch_template_test.go @@ -295,7 +295,7 @@ func TestAccAWSLaunchTemplate_NetworkInterfaces_DeleteOnTermination(t *testing.T CheckDestroy: testAccCheckAWSLaunchTemplateDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, true), + Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, "true"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSLaunchTemplateExists(resourceName, &template), resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), @@ -304,7 +304,7 @@ func TestAccAWSLaunchTemplate_NetworkInterfaces_DeleteOnTermination(t *testing.T ), }, { - Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, false), + Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, "false"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSLaunchTemplateExists(resourceName, &template), resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), @@ -312,6 +312,24 @@ func TestAccAWSLaunchTemplate_NetworkInterfaces_DeleteOnTermination(t *testing.T resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.delete_on_termination", "false"), ), }, + { + Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, "\"\""), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSLaunchTemplateExists(resourceName, &template), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.security_groups.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.delete_on_termination", ""), + ), + }, + { + Config: testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName, "null"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSLaunchTemplateExists(resourceName, &template), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.security_groups.#", "1"), + resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.delete_on_termination", ""), + ), + }, { ResourceName: resourceName, ImportState: true, @@ -1072,7 +1090,7 @@ resource "aws_autoscaling_group" "test" { `, rName, deleteOnTermination, rName) } -func testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName string, deleteOnTermination bool) string { +func testAccAWSLaunchTemplateConfig_NetworkInterfaces_DeleteOnTermination(rName string, deleteOnTermination string) string { return fmt.Sprintf(` resource "aws_launch_template" "test" { name = %q @@ -1080,7 +1098,7 @@ resource "aws_launch_template" "test" { network_interfaces { network_interface_id = "eni-123456ab" security_groups = ["sg-1a23bc45"] - delete_on_termination = %t + delete_on_termination = %s } } `, rName, deleteOnTermination) From 8eaa664127064d77da501a5b8f6e433f9935a16b Mon Sep 17 00:00:00 2001 From: Simon Effenberg Date: Wed, 15 Jul 2020 08:18:02 +0200 Subject: [PATCH 3/4] adding upgrade instructions --- website/docs/guides/version-3-upgrade.html.md | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/website/docs/guides/version-3-upgrade.html.md b/website/docs/guides/version-3-upgrade.html.md index bb91f6590287..a2fe9c4a6cdc 100644 --- a/website/docs/guides/version-3-upgrade.html.md +++ b/website/docs/guides/version-3-upgrade.html.md @@ -185,6 +185,41 @@ resource "aws_emr_cluster" "example" { } ``` +## Resource: aws_launch_template + +### network_interfaces.delete_on_termination Argument type change + + +Enforce `delete_on_termination` to `false` if it was not set previously to keep the (incorrect) behavior to treat no value as `false`. + +For example, given this previous configuration: + +```hcl +resource "aws_launch_template" "example" { + # ... other configuration ... + + network_interfaces { + # ... other configuration ... + + delete_on_termination = null + } +} +``` + +An updated configuration: + +```hcl +resource "aws_launch_template" "example" { + # ... other configuration ... + + network_interfaces { + # ... other configuration ... + + delete_on_termination = false + } +} +``` + ## Resource: aws_lb_listener_rule ### condition.field and condition.values Arguments Removal From f2d80fbbedeb6e7e27bd47c47b37aec0e8c61c3a Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Wed, 15 Jul 2020 16:12:34 -0400 Subject: [PATCH 4/4] version 3 upgrade details --- website/docs/guides/version-3-upgrade.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/guides/version-3-upgrade.html.md b/website/docs/guides/version-3-upgrade.html.md index 479c82eb23d9..12324dc4cef4 100644 --- a/website/docs/guides/version-3-upgrade.html.md +++ b/website/docs/guides/version-3-upgrade.html.md @@ -27,6 +27,7 @@ Upgrade topics: - [Resource: aws_dx_gateway](#resource-aws_dx_gateway) - [Resource: aws_elastic_transcoder_preset](#resource-aws_elastic_transcoder_preset) - [Resource: aws_emr_cluster](#resource-aws_emr_cluster) +- [Resource: aws_launch_template](#resource-aws_launch_template) - [Resource: aws_lb_listener_rule](#resource-aws_lb_listener_rule) - [Resource: aws_msk_cluster](#resource-aws_msk_cluster) - [Resource: aws_s3_bucket](#resource-aws_s3_bucket) @@ -380,8 +381,7 @@ resource "aws_emr_cluster" "example" { ### network_interfaces.delete_on_termination Argument type change - -Enforce `delete_on_termination` to `false` if it was not set previously to keep the (incorrect) behavior to treat no value as `false`. +The `network_interfaces.delete_on_termination` argument is now of type `string`, allowing an unspecified value for the argument since the previous `bool` type only allowed for `true/false` and defaulted to `false` when no value was set. Now to enforce `delete_on_termination` to `false`, the string `"false"` or bare `false` value must be used. For example, given this previous configuration: