diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index 9b1e3b8e7..11df73a8c 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -513,6 +513,41 @@ func TestAccAWSInstance_rootBlockDeviceMismatch(t *testing.T) { }) } +// This test reproduces the bug here: +// https://github.com/hashicorp/terraform/issues/1752 +// +// I wish there were a way to exercise resources built with helper.Schema in a +// unit context, in which case this test could be moved there, but for now this +// will cover the bugfix. +// +// The following triggers "diffs didn't match during apply" without the fix in to +// set NewRemoved on the .# field when it changes to 0. +func TestAccAWSInstance_forceNewAndTagsDrift(t *testing.T) { + var v ec2.Instance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccInstanceConfigForceNewAndTagsDrift, + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists("aws_instance.foo", &v), + driftTags(&v), + ), + ExpectNonEmptyPlan: true, + }, + resource.TestStep{ + Config: testAccInstanceConfigForceNewAndTagsDrift_Update, + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists("aws_instance.foo", &v), + ), + }, + }, + }) +} + func testAccCheckInstanceDestroy(s *terraform.State) error { return testAccCheckInstanceDestroyWithProvider(s, testAccProvider) } @@ -622,6 +657,22 @@ func TestInstanceTenancySchema(t *testing.T) { } } +func driftTags(instance *ec2.Instance) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).ec2conn + _, err := conn.CreateTags(&ec2.CreateTagsInput{ + Resources: []*string{instance.InstanceId}, + Tags: []*ec2.Tag{ + &ec2.Tag{ + Key: aws.String("Drift"), + Value: aws.String("Happens"), + }, + }, + }) + return err + } +} + const testAccInstanceConfig_pre = ` resource "aws_security_group" "tf_test_foo" { name = "tf_test_foo" @@ -988,3 +1039,37 @@ resource "aws_instance" "foo" { } } ` + +const testAccInstanceConfigForceNewAndTagsDrift = ` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" +} + +resource "aws_subnet" "foo" { + cidr_block = "10.1.1.0/24" + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_instance" "foo" { + ami = "ami-22b9a343" + instance_type = "t2.nano" + subnet_id = "${aws_subnet.foo.id}" +} +` + +const testAccInstanceConfigForceNewAndTagsDrift_Update = ` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" +} + +resource "aws_subnet" "foo" { + cidr_block = "10.1.1.0/24" + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_instance" "foo" { + ami = "ami-22b9a343" + instance_type = "t2.micro" + subnet_id = "${aws_subnet.foo.id}" +} +` diff --git a/terraform/diff.go b/terraform/diff.go index 87ae84ccf..f1a41efb2 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -466,6 +466,13 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) { ok = true } + // Similarly, in a RequiresNew scenario, a list that shows up in the plan + // diff can disappear from the apply diff, which is calculated from an + // empty state. + if d.RequiresNew() && strings.HasSuffix(k, ".#") { + ok = true + } + if !ok { return false, fmt.Sprintf("attribute mismatch: %s", k) } diff --git a/terraform/diff_test.go b/terraform/diff_test.go index 4eeb8d387..6dbdd505e 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -553,6 +553,43 @@ func TestInstanceDiffSame(t *testing.T) { true, "", }, + + // Another thing that can occur in DESTROY/CREATE scenarios is that list + // values that are going to zero have diffs that show up at plan time but + // are gone at apply time. The NewRemoved handling catches the fields and + // treats them as OK, but it also needs to treat the .# field itself as + // okay to be present in the old diff but not in the new one. + { + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "reqnew": &ResourceAttrDiff{ + Old: "old", + New: "new", + RequiresNew: true, + }, + "somemap.#": &ResourceAttrDiff{ + Old: "1", + New: "0", + }, + "somemap.oldkey": &ResourceAttrDiff{ + Old: "long ago", + New: "", + NewRemoved: true, + }, + }, + }, + &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "reqnew": &ResourceAttrDiff{ + Old: "", + New: "new", + RequiresNew: true, + }, + }, + }, + true, + "", + }, } for i, tc := range cases {