From 54db46ef1b6f17871777795e64001d3310323f27 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 12 Dec 2014 15:24:29 +0100 Subject: [PATCH] Fixing a small logic bug in diffList MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s not enough to only check if no new value is set. It can also be that a new value is set, but contains a variable that cannot be interpolated until a depending resource is created during the apply fase. I actually found this one as one of the acceptance tests for the AWS ELB resource was failing. It failed with the following error: ``` --- FAIL: TestAccAWSELB_InstanceAttaching (177.83 seconds) testing.go:121: Step 1 error: Error applying: aws_elb.bar: diffs didn't match during apply. This is a bug with the resource provider, please report a bug. FAIL exit status 1 FAIL github.com/hashicorp/terraform/builtin/providers/aws 177.882s ``` After a quick look I noticed it was actually a bug in core TF so added the test and made sure all unit tests and AWS acceptance tests are now running successfully. --- helper/schema/schema.go | 7 ++--- helper/schema/schema_test.go | 51 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/helper/schema/schema.go b/helper/schema/schema.go index be5a56aa3..336292227 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -459,9 +459,10 @@ func (m schemaMap) diffList( o, n, _, computedList := d.diffChange(k) nSet := n != nil - // If we have an old value, but no new value set but we're computed, - // then nothing has changed. - if o != nil && n == nil && schema.Computed { + // If we have an old value and no new value is set or will be + // computed once all variables can be interpolated and we're + // computed, then nothing has changed. + if o != nil && n == nil && !computedList && schema.Computed { return nil } diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 5351f61d7..b55ef7dba 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -1466,6 +1466,57 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + + { + Schema: map[string]*Schema{ + "internal": &Schema{ + Type: TypeBool, + Required: true, + }, + + "instances": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeString}, + Optional: true, + Computed: true, + Set: func(v interface{}) int { + return len(v) + }, + }, + }, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "internal": "false", + "instances.#": "0", + }, + }, + + Config: map[string]interface{}{ + "internal": true, + "instances": []interface{}{"${var.foo}"}, + }, + + ConfigVariables: map[string]string{ + "var.foo": config.UnknownVariableValue, + }, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "internal": &terraform.ResourceAttrDiff{ + Old: "0", + New: "1", + }, + + "instances.#": &terraform.ResourceAttrDiff{ + Old: "0", + NewComputed: true, + }, + }, + }, + + Err: false, + }, } for i, tc := range cases {