Skip to content

Commit

Permalink
helper/schema: fix DiffFieldReader map handling
Browse files Browse the repository at this point in the history
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
  • Loading branch information
phinze committed Feb 4, 2015
1 parent 7df0932 commit 219aa3e
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 0 deletions.
2 changes: 2 additions & 0 deletions builtin/providers/aws/resource_aws_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, "#", ""),
),
},

Expand Down
5 changes: 5 additions & 0 deletions helper/schema/field_reader_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):]
Expand Down
45 changes: 45 additions & 0 deletions helper/schema/field_reader_diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down

0 comments on commit 219aa3e

Please sign in to comment.