Skip to content

Commit

Permalink
resource/aws_launch_template: Handle bare true/false values correctly…
Browse files Browse the repository at this point in the history
… with TypeString boolean attributes

Previously:

```
--- FAIL: TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS_DeleteOnTermination (1.27s)
    testing.go:527: Step 0 error: config is invalid: aws_launch_template.test: expected block_device_mappings.0.ebs.0.delete_on_termination to be one of [ false true], got 1
```

Now:

```
make testacc TEST=./aws TESTARGS='-run=TestAccAWSLaunchTemplate_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLaunchTemplate_ -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_importBasic
--- PASS: TestAccAWSLaunchTemplate_importBasic (13.71s)
=== RUN   TestAccAWSLaunchTemplate_importData
--- PASS: TestAccAWSLaunchTemplate_importData (11.56s)
=== RUN   TestAccAWSLaunchTemplate_basic
--- PASS: TestAccAWSLaunchTemplate_basic (12.18s)
=== RUN   TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS
--- PASS: TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS (48.61s)
=== RUN   TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS_DeleteOnTermination
--- PASS: TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS_DeleteOnTermination (48.98s)
=== RUN   TestAccAWSLaunchTemplate_data
--- PASS: TestAccAWSLaunchTemplate_data (11.61s)
=== RUN   TestAccAWSLaunchTemplate_update
--- PASS: TestAccAWSLaunchTemplate_update (54.26s)
=== RUN   TestAccAWSLaunchTemplate_tags
--- PASS: TestAccAWSLaunchTemplate_tags (20.58s)
=== RUN   TestAccAWSLaunchTemplate_nonBurstable
--- PASS: TestAccAWSLaunchTemplate_nonBurstable (11.58s)
=== RUN   TestAccAWSLaunchTemplate_networkInterface
--- PASS: TestAccAWSLaunchTemplate_networkInterface (30.64s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	264.442s
```
  • Loading branch information
bflad committed Aug 30, 2018
1 parent 5b7fbaa commit 2e8a2b3
Show file tree
Hide file tree
Showing 6 changed files with 223 additions and 14 deletions.
13 changes: 13 additions & 0 deletions aws/diff_suppress_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
41 changes: 41 additions & 0 deletions aws/diff_suppress_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
22 changes: 8 additions & 14 deletions aws/resource_aws_launch_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,26 +93,20 @@ func resourceAwsLaunchTemplate() *schema.Resource {
// 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,
ValidateFunc: validation.StringInSlice([]string{
"",
"false",
"true",
}, false),
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: suppressEquivalentTypeStringBoolean,
ValidateFunc: validateTypeStringNullableBoolean,
},
"encrypted": {
// 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,
ValidateFunc: validation.StringInSlice([]string{
"",
"false",
"true",
}, false),
Type: schema.TypeString,
Optional: true,
DiffSuppressFunc: suppressEquivalentTypeStringBoolean,
ValidateFunc: validateTypeStringNullableBoolean,
},
"iops": {
Type: schema.TypeInt,
Expand Down
91 changes: 91 additions & 0 deletions aws/resource_aws_launch_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -325,6 +366,56 @@ resource "aws_autoscaling_group" "test" {
`, 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 {
return fmt.Sprintf(`
resource "aws_launch_template" "foo" {
Expand Down
22 changes: 22 additions & 0 deletions aws/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
48 changes: 48 additions & 0 deletions aws/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,54 @@ func TestValidateRFC3339TimeString(t *testing.T) {
}
}

func TestValidateTypeStringNullableBoolean(t *testing.T) {
testCases := []struct {
val interface{}
expectedErr *regexp.Regexp
}{
{
val: "",
},
{
val: "0",
},
{
val: "1",
},
{
val: "invalid",
expectedErr: regexp.MustCompile(`to be one of \["", false, true\]`),
},
}

matchErr := func(errs []error, r *regexp.Regexp) bool {
// err must match one provided
for _, err := range errs {
if r.MatchString(err.Error()) {
return true
}
}

return false
}

for i, tc := range testCases {
_, errs := validateTypeStringNullableBoolean(tc.val, "test_property")

if len(errs) == 0 && tc.expectedErr == nil {
continue
}

if len(errs) != 0 && tc.expectedErr == nil {
t.Fatalf("expected test case %d to produce no errors, got %v", i, errs)
}

if !matchErr(errs, tc.expectedErr) {
t.Fatalf("expected test case %d to produce error matching \"%s\", got %v", i, tc.expectedErr, errs)
}
}
}

func TestValidateEcrRepositoryName(t *testing.T) {
validNames := []string{
"nginx-web-app",
Expand Down

0 comments on commit 2e8a2b3

Please sign in to comment.