resource/aws_launch_template: Prevent encrypted flag cannot be specified error with block_device_mappings ebs argument

aws/diff_suppress_funcs.go

@@ -20,6 +20,19 @@ func suppressEquivalentAwsPolicyDiffs(k, old, new string, d *schema.ResourceData
 	return equivalent
 }
 
+// suppressEquivalentTypeStringBoolean provides custom difference suppression for TypeString booleans
+// Some arguments require three values: true, false, and "" (unspecified), but
+// confusing behavior exists when converting bare true/false values with state.
+func suppressEquivalentTypeStringBoolean(k, old, new string, d *schema.ResourceData) bool {
+	if old == "false" && new == "0" {
+		return true
+	}
+	if old == "true" && new == "1" {
+		return true
+	}
+	return false
+}
+
 // Suppresses minor version changes to the db_instance engine_version attribute
 func suppressAwsDbEngineVersionDiffs(k, old, new string, d *schema.ResourceData) bool {
 	// First check if the old/new values are nil.

aws/diff_suppress_funcs_test.go

@@ -29,3 +29,44 @@ func TestSuppressEquivalentJsonDiffsWhitespaceAndNoWhitespace(t *testing.T) {
 		t.Errorf("Expected suppressEquivalentJsonDiffs to return false for %s == %s", noWhitespaceDiff, whitespaceDiff)
 	}
 }
+
+func TestSuppressEquivalentTypeStringBoolean(t *testing.T) {
+	testCases := []struct {
+		old        string
+		new        string
+		equivalent bool
+	}{
+		{
+			old:        "false",
+			new:        "0",
+			equivalent: true,
+		},
+		{
+			old:        "true",
+			new:        "1",
+			equivalent: true,
+		},
+		{
+			old:        "",
+			new:        "0",
+			equivalent: false,
+		},
+		{
+			old:        "",
+			new:        "1",
+			equivalent: false,
+		},
+	}
+
+	for i, tc := range testCases {
+		value := suppressEquivalentTypeStringBoolean("test_property", tc.old, tc.new, nil)
+
+		if tc.equivalent && !value {
+			t.Fatalf("expected test case %d to be equivalent", i)
+		}
+
+		if !tc.equivalent && value {
+			t.Fatalf("expected test case %d to not be equivalent", i)
+		}
+	}
+}

aws/resource_aws_launch_template.go

@@ -89,12 +89,24 @@ func resourceAwsLaunchTemplate() *schema.Resource {
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									"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,
 									},
 									"encrypted": {
-										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,
 									},
 									"iops": {
 										Type:     schema.TypeInt,
@@ -638,14 +650,18 @@ func getBlockDeviceMappings(m []*ec2.LaunchTemplateBlockDeviceMapping) []interfa
 			"virtual_name": aws.StringValue(v.VirtualName),
 		}
 		if v.NoDevice != nil {
-			mapping["no_device"] = *v.NoDevice
+			mapping["no_device"] = aws.StringValue(v.NoDevice)
 		}
 		if v.Ebs != nil {
 			ebs := map[string]interface{}{
-				"delete_on_termination": aws.BoolValue(v.Ebs.DeleteOnTermination),
-				"encrypted":             aws.BoolValue(v.Ebs.Encrypted),
-				"volume_size":           int(aws.Int64Value(v.Ebs.VolumeSize)),
-				"volume_type":           aws.StringValue(v.Ebs.VolumeType),
+				"volume_size": int(aws.Int64Value(v.Ebs.VolumeSize)),
+				"volume_type": aws.StringValue(v.Ebs.VolumeType),
+			}
+			if v.Ebs.DeleteOnTermination != nil {
+				ebs["delete_on_termination"] = strconv.FormatBool(aws.BoolValue(v.Ebs.DeleteOnTermination))
+			}
+			if v.Ebs.Encrypted != nil {
+				ebs["encrypted"] = strconv.FormatBool(aws.BoolValue(v.Ebs.Encrypted))
 			}
 			if v.Ebs.Iops != nil {
 				ebs["iops"] = aws.Int64Value(v.Ebs.Iops)
@@ -857,7 +873,11 @@ func buildLaunchTemplateData(d *schema.ResourceData, meta interface{}) (*ec2.Req
 		bdms := v.([]interface{})
 
 		for _, bdm := range bdms {
-			blockDeviceMappings = append(blockDeviceMappings, readBlockDeviceMappingFromConfig(bdm.(map[string]interface{})))
+			blockDeviceMapping, err := readBlockDeviceMappingFromConfig(bdm.(map[string]interface{}))
+			if err != nil {
+				return nil, err
+			}
+			blockDeviceMappings = append(blockDeviceMappings, blockDeviceMapping)
 		}
 		opts.BlockDeviceMappings = blockDeviceMappings
 	}
@@ -950,7 +970,7 @@ func buildLaunchTemplateData(d *schema.ResourceData, meta interface{}) (*ec2.Req
 	return opts, nil
 }
 
-func readBlockDeviceMappingFromConfig(bdm map[string]interface{}) *ec2.LaunchTemplateBlockDeviceMappingRequest {
+func readBlockDeviceMappingFromConfig(bdm map[string]interface{}) (*ec2.LaunchTemplateBlockDeviceMappingRequest, error) {
 	blockDeviceMapping := &ec2.LaunchTemplateBlockDeviceMappingRequest{}
 
 	if v := bdm["device_name"].(string); v != "" {
@@ -967,24 +987,36 @@ func readBlockDeviceMappingFromConfig(bdm map[string]interface{}) *ec2.LaunchTem
 
 	if v := bdm["ebs"]; len(v.([]interface{})) > 0 {
 		ebs := v.([]interface{})
-		if len(ebs) > 0 {
-			ebsData := ebs[0]
-			blockDeviceMapping.Ebs = readEbsBlockDeviceFromConfig(ebsData.(map[string]interface{}))
+		if len(ebs) > 0 && ebs[0] != nil {
+			ebsData := ebs[0].(map[string]interface{})
+			launchTemplateEbsBlockDeviceRequest, err := readEbsBlockDeviceFromConfig(ebsData)
+			if err != nil {
+				return nil, err
+			}
+			blockDeviceMapping.Ebs = launchTemplateEbsBlockDeviceRequest
 		}
 	}
 
-	return blockDeviceMapping
+	return blockDeviceMapping, nil
 }
 
-func readEbsBlockDeviceFromConfig(ebs map[string]interface{}) *ec2.LaunchTemplateEbsBlockDeviceRequest {
+func readEbsBlockDeviceFromConfig(ebs map[string]interface{}) (*ec2.LaunchTemplateEbsBlockDeviceRequest, error) {
 	ebsDevice := &ec2.LaunchTemplateEbsBlockDeviceRequest{}
 
-	if v := ebs["delete_on_termination"]; v != nil {
-		ebsDevice.DeleteOnTermination = aws.Bool(v.(bool))
+	if v, ok := ebs["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)
+		}
+		ebsDevice.DeleteOnTermination = aws.Bool(vBool)
 	}
 
-	if v := ebs["encrypted"]; v != nil {
-		ebsDevice.Encrypted = aws.Bool(v.(bool))
+	if v, ok := ebs["encrypted"]; ok && v.(string) != "" {
+		vBool, err := strconv.ParseBool(v.(string))
+		if err != nil {
+			return nil, fmt.Errorf("error converting encrypted %q from string to boolean: %s", v.(string), err)
+		}
+		ebsDevice.Encrypted = aws.Bool(vBool)
 	}
 
 	if v := ebs["iops"].(int); v > 0 {
@@ -1007,7 +1039,7 @@ func readEbsBlockDeviceFromConfig(ebs map[string]interface{}) *ec2.LaunchTemplat
 		ebsDevice.VolumeType = aws.String(v)
 	}
 
-	return ebsDevice
+	return ebsDevice, nil
 }
 
 func readNetworkInterfacesFromConfig(ni map[string]interface{}) *ec2.LaunchTemplateInstanceNetworkInterfaceSpecificationRequest {

aws/resource_aws_launch_template_test.go

@@ -64,6 +64,47 @@ func TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS(t *testing.T) {
 	})
 }
 
+func TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS_DeleteOnTermination(t *testing.T) {
+	var template ec2.LaunchTemplate
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+	resourceName := "aws_launch_template.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSLaunchTemplateDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSLaunchTemplateConfig_BlockDeviceMappings_EBS_DeleteOnTermination(rName, true),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSLaunchTemplateExists(resourceName, &template),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.device_name", "/dev/sda1"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.ebs.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.ebs.0.delete_on_termination", "true"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.ebs.0.volume_size", "15"),
+				),
+			},
+			{
+				Config: testAccAWSLaunchTemplateConfig_BlockDeviceMappings_EBS_DeleteOnTermination(rName, false),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSLaunchTemplateExists(resourceName, &template),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.device_name", "/dev/sda1"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.ebs.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.ebs.0.delete_on_termination", "false"),
+					resource.TestCheckResourceAttr(resourceName, "block_device_mappings.0.ebs.0.volume_size", "15"),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccAWSLaunchTemplate_data(t *testing.T) {
 	var template ec2.LaunchTemplate
 	resName := "aws_launch_template.foo"
@@ -293,9 +334,11 @@ data "aws_ami" "test" {
   }
 }
 
+data "aws_availability_zones" "available" {}
+
 resource "aws_launch_template" "test" {
   image_id = "${data.aws_ami.test.id}"
-  name     = "%s"
+  name     = %q
 
   block_device_mappings {
     device_name = "/dev/sda1"
@@ -305,7 +348,72 @@ resource "aws_launch_template" "test" {
     }
   }
 }
-`, rName)
+
+# Creating an AutoScaling Group verifies the launch template
+# ValidationError: You must use a valid fully-formed launch template. the encrypted flag cannot be specified since device /dev/sda1 has a snapshot specified.
+resource "aws_autoscaling_group" "test" {
+  availability_zones = ["${data.aws_availability_zones.available.names[0]}"]
+  desired_capacity = 0
+  max_size         = 0
+  min_size         = 0
+  name             = %q
+
+  launch_template {
+    id      = "${aws_launch_template.test.id}"
+    version = "${aws_launch_template.test.default_version}"
+  }
+}
+`, rName, rName)
+}
+
+func testAccAWSLaunchTemplateConfig_BlockDeviceMappings_EBS_DeleteOnTermination(rName string, deleteOnTermination bool) string {
+	return fmt.Sprintf(`
+data "aws_ami" "test" {
+  most_recent = true
+  owners      = ["099720109477"] # Canonical
+
+  filter {
+    name   = "name"
+    values = ["ubuntu/images/hvm-ssd/ubuntu-xenial-16.04-amd64-server-*"]
+  }
+
+  filter {
+    name   = "virtualization-type"
+    values = ["hvm"]
+  }
+}
+
+data "aws_availability_zones" "available" {}
+
+resource "aws_launch_template" "test" {
+  image_id = "${data.aws_ami.test.id}"
+  name     = %q
+
+  block_device_mappings {
+    device_name = "/dev/sda1"
+
+    ebs {
+      delete_on_termination = %t
+      volume_size           = 15
+    }
+  }
+}
+
+# Creating an AutoScaling Group verifies the launch template
+# ValidationError: You must use a valid fully-formed launch template. the encrypted flag cannot be specified since device /dev/sda1 has a snapshot specified.
+resource "aws_autoscaling_group" "test" {
+  availability_zones = ["${data.aws_availability_zones.available.names[0]}"]
+  desired_capacity = 0
+  max_size         = 0
+  min_size         = 0
+  name             = %q
+
+  launch_template {
+    id      = "${aws_launch_template.test.id}"
+    version = "${aws_launch_template.test.default_version}"
+  }
+}
+`, rName, deleteOnTermination, rName)
 }
 
 func testAccAWSLaunchTemplateConfig_data(rInt int) string {

aws/validators.go

@@ -32,6 +32,28 @@ func validateRFC3339TimeString(v interface{}, k string) (ws []string, errors []e
 	return
 }
 
+// validateTypeStringNullableBoolean provides custom error messaging for TypeString booleans
+// Some arguments require three values: true, false, and "" (unspecified).
+// This ValidateFunc returns a custom message since the message with
+// validation.StringInSlice([]string{"", "false", "true"}, false) is confusing:
+// to be one of [ false true], got 1
+func validateTypeStringNullableBoolean(v interface{}, k string) (ws []string, es []error) {
+	value, ok := v.(string)
+	if !ok {
+		es = append(es, fmt.Errorf("expected type of %s to be string", k))
+		return
+	}
+
+	for _, str := range []string{"", "0", "1"} {
+		if value == str {
+			return
+		}
+	}
+
+	es = append(es, fmt.Errorf("expected %s to be one of [\"\", false, true], got %s", k, value))
+	return
+}
+
 func validateRdsIdentifier(v interface{}, k string) (ws []string, errors []error) {
 	value := v.(string)
 	if !regexp.MustCompile(`^[0-9a-z-]+$`).MatchString(value) {