From 219aa3e7887f47e056a52072a7f886d682510deb Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Tue, 3 Feb 2015 19:48:25 -0600 Subject: [PATCH 1/2] helper/schema: fix DiffFieldReader map handling An `InstanceDiff` will include `ResourceAttrDiff` entries for the "length" / `#` field of maps. This makes sense, since for something like `terraform plan` it's useful to see when counts are changing. The `DiffFieldReader` was not taking these entries into account when reading maps out, and was therefore incorrectly returning maps that included an extra `'#'` field, which was causing all sorts of havoc for providers (extra tags on AWS instances, broken google compute instance launch, possibly others). * fixes #914 - extra tags on AWS instances * fixes #883 - general core issue sprouted from #757 * removes the hack+TODO from #757 --- .../aws/resource_aws_instance_test.go | 2 + helper/schema/field_reader_diff.go | 5 +++ helper/schema/field_reader_diff_test.go | 45 +++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index 73da0197f..7275dde92 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -180,6 +180,8 @@ func TestAccInstance_tags(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckInstanceExists("aws_instance.foo", &v), testAccCheckTags(&v.Tags, "foo", "bar"), + // Guard against regression of https://github.com/hashicorp/terraform/issues/914 + testAccCheckTags(&v.Tags, "#", ""), ), }, diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go index aaacd5d68..378909fd3 100644 --- a/helper/schema/field_reader_diff.go +++ b/helper/schema/field_reader_diff.go @@ -82,6 +82,11 @@ func (r *DiffFieldReader) readMap( if !strings.HasPrefix(k, prefix) { continue } + if strings.HasPrefix(k, prefix+"#") { + // Ignore the count field + continue + } + resultSet = true k = k[len(prefix):] diff --git a/helper/schema/field_reader_diff_test.go b/helper/schema/field_reader_diff_test.go index cc07bc013..fbb10fcaf 100644 --- a/helper/schema/field_reader_diff_test.go +++ b/helper/schema/field_reader_diff_test.go @@ -11,6 +11,51 @@ func TestDiffFieldReader_impl(t *testing.T) { var _ FieldReader = new(DiffFieldReader) } +// https://github.com/hashicorp/terraform/issues/914 +func TestDiffFieldReader_MapHandling(t *testing.T) { + schema := map[string]*Schema{ + "tags": &Schema{ + Type: TypeMap, + }, + } + r := &DiffFieldReader{ + Schema: schema, + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "tags.#": &terraform.ResourceAttrDiff{ + Old: "1", + New: "2", + }, + "tags.baz": &terraform.ResourceAttrDiff{ + Old: "", + New: "qux", + }, + }, + }, + Source: &MapFieldReader{ + Schema: schema, + Map: BasicMapReader(map[string]string{ + "tags.#": "1", + "tags.foo": "bar", + }), + }, + } + + result, err := r.ReadField([]string{"tags"}) + if err != nil { + t.Fatalf("ReadField failed: %#v", err) + } + + expected := map[string]interface{}{ + "foo": "bar", + "baz": "qux", + } + + if !reflect.DeepEqual(expected, result.Value) { + t.Fatalf("bad: DiffHandling\n\nexpected: %#v\n\ngot: %#v\n\n", expected, result.Value) + } +} + func TestDiffFieldReader_extra(t *testing.T) { schema := map[string]*Schema{ "stringComputed": &Schema{Type: TypeString}, From 4e8e3dad863fc18fc15a56f9f1a2756970048ca0 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 4 Feb 2015 09:25:45 -0600 Subject: [PATCH 2/2] DiffFieldReader: filter all '#' fields from sets Now that readMap filters out '#' fields, when maps are nested in sets, we exposed a related bug where a set was iterating over nested maps and expected the '#' key to be present in those nested maps. By skipping _all_ count fields when iterating over set keys, all is right with the world again. --- helper/schema/field_reader_diff.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go index 378909fd3..ec875421b 100644 --- a/helper/schema/field_reader_diff.go +++ b/helper/schema/field_reader_diff.go @@ -153,8 +153,8 @@ func (r *DiffFieldReader) readSet( if !strings.HasPrefix(k, prefix) { continue } - if strings.HasPrefix(k, prefix+"#") { - // Ignore the count field + if strings.HasSuffix(k, "#") { + // Ignore any count field continue }