providers/aws: should delete tags that are changing in-place

builtin/providers/aws/tags.go

@@ -56,7 +56,8 @@ func diffTags(oldTags, newTags []ec2.Tag) ([]ec2.Tag, []ec2.Tag) {
 	// Build the list of what to remove
 	var remove []ec2.Tag
 	for _, t := range oldTags {
-		if _, ok := create[t.Key]; !ok {
+		old, ok := create[t.Key]
+		if !ok || old != t.Value {
 			// Delete it!
 			remove = append(remove, t)
 		}

builtin/providers/aws/tags_test.go

@@ -2,12 +2,65 @@ package aws
 
 import (
 	"fmt"
+	"reflect"
+	"testing"
 
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/goamz/ec2"
 )
 
+func TestDiffTags(t *testing.T) {
+	cases := []struct {
+		Old, New       map[string]interface{}
+		Create, Remove map[string]string
+	}{
+		// Basic add/remove
+		{
+			Old: map[string]interface{}{
+				"foo": "bar",
+			},
+			New: map[string]interface{}{
+				"bar": "baz",
+			},
+			Create: map[string]string{
+				"bar": "baz",
+			},
+			Remove: map[string]string{
+				"foo": "bar",
+			},
+		},
+
+		// Modify
+		{
+			Old: map[string]interface{}{
+				"foo": "bar",
+			},
+			New: map[string]interface{}{
+				"foo": "baz",
+			},
+			Create: map[string]string{
+				"foo": "baz",
+			},
+			Remove: map[string]string{
+				"foo": "bar",
+			},
+		},
+	}
+
+	for i, tc := range cases {
+		c, r := diffTags(tagsFromMap(tc.Old), tagsFromMap(tc.New))
+		cm := tagsToMap(c)
+		rm := tagsToMap(r)
+		if !reflect.DeepEqual(cm, tc.Create) {
+			t.Fatalf("%i: bad create: %#v", i, cm)
+		}
+		if !reflect.DeepEqual(rm, tc.Remove) {
+			t.Fatalf("%i: bad remove: %#v", i, rm)
+		}
+	}
+}
+
 // testAccCheckTags can be used to check the tags on a resource.
 func testAccCheckTags(
 	ts *[]ec2.Tag, key string, value string) resource.TestCheckFunc {