From b10f0748e42e09a693ce2862dc3ad640cdf2a3ec Mon Sep 17 00:00:00 2001
From: Fabian Breckle <fabian.breckle@breuninger.de>
Date: Tue, 6 Nov 2018 11:22:34 +0100
Subject: [PATCH 1/3] Fix diffTags to actually create a diff instead of
 unnecessarily recreating existing tags.

---
 aws/tags.go | 47 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/aws/tags.go b/aws/tags.go
index b2293ba9e179..852fa0f3155a 100644
--- a/aws/tags.go
+++ b/aws/tags.go
@@ -187,22 +187,47 @@ func setTags(conn *ec2.EC2, d *schema.ResourceData) error {
 // the set of tags that must be created, and the set of tags that must
 // be destroyed.
 func diffTags(oldTags, newTags []*ec2.Tag) ([]*ec2.Tag, []*ec2.Tag) {
-	// First, we're creating everything we have
-	create := make(map[string]interface{})
-	for _, t := range newTags {
-		create[*t.Key] = *t.Value
+	oldTagsMap := tagsToMap(oldTags)
+	newTagsMap := tagsToMap(newTags)
+
+	// These maps will hold the Tags to create and remove, respectively
+	var create = make(map[string]interface{})
+	var remove = make(map[string]interface{})
+
+	// For each new Tag, we check if an identical Tag (in key and value) already exists.
+	// If yes, we don't have to do anything with this Tag.
+	// Else, if the key was found with a different value, remove it and recreate it with the new value
+	// If the key was not found in the in the old Tags, just create it.
+	// Then, if one of the old Tags is no longer in the new Tags, delete the old Tag.
+	for key, newval := range newTagsMap {
+		oldval, found := oldTagsMap[key]
+		if found {
+			// New key exists in the old Tags already
+			// Check if the value is the same, too
+			if newval != oldval {
+				// If not, recreate the Tag with the new value
+				remove[key] = oldval
+				create[key] = newval
+			} else {
+				// If the key and value are the same, ignore the Tag
+			}
+		} else {
+			// If new Tag key is not in old Tags, create the new Tag
+			create[key] = newval
+		}
 	}
 
-	// Build the list of what to remove
-	var remove []*ec2.Tag
-	for _, t := range oldTags {
-		old, ok := create[*t.Key]
-		if !ok || old != *t.Value {
-			remove = append(remove, t)
+	// Now we handled the new Tags and the Tags that need to be updated
+	// We still have to handle the Tags that have to be deleted
+	for key, oldval := range oldTagsMap {
+		_, found := newTagsMap[key]
+		// Delete old Tag if it is not found in the new Tags
+		if !found {
+			remove[key] = oldval
 		}
 	}
 
-	return tagsFromMap(create), remove
+	return tagsFromMap(create), tagsFromMap(remove)
 }
 
 // tagsFromMap returns the tags for the given map of data.

From 981b785ea82fbe6e76eafc083ec52f49a9b3ee0d Mon Sep 17 00:00:00 2001
From: Fabian Breckle <fabian.breckle@breuninger.de>
Date: Wed, 7 Nov 2018 12:18:11 +0100
Subject: [PATCH 2/3] Fix TestDiffTags to test for duplicate Tag creation.

---
 aws/tags_test.go | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/aws/tags_test.go b/aws/tags_test.go
index bfe4d35d898e..5ebb9fe4f63f 100644
--- a/aws/tags_test.go
+++ b/aws/tags_test.go
@@ -16,20 +16,19 @@ func TestDiffTags(t *testing.T) {
 		Old, New       map[string]interface{}
 		Create, Remove map[string]string
 	}{
-		// Basic add/remove
+		// Add
 		{
 			Old: map[string]interface{}{
 				"foo": "bar",
 			},
 			New: map[string]interface{}{
+				"foo": "bar",
 				"bar": "baz",
 			},
 			Create: map[string]string{
 				"bar": "baz",
 			},
-			Remove: map[string]string{
-				"foo": "bar",
-			},
+			Remove: map[string]string{},
 		},
 
 		// Modify
@@ -47,12 +46,28 @@ func TestDiffTags(t *testing.T) {
 				"foo": "bar",
 			},
 		},
+		// Remove
+		{
+			Old: map[string]interface{}{
+				"foo": "bar",
+				"bar": "baz",
+			},
+			New: map[string]interface{}{
+				"foo": "bar",
+			},
+			Create: map[string]string{},
+			Remove: map[string]string{
+				"bar": "baz",
+			},
+		},
 	}
 
 	for i, tc := range cases {
 		c, r := diffTags(tagsFromMap(tc.Old), tagsFromMap(tc.New))
+		t.Logf("create: %v remove: %v", c, r)
 		cm := tagsToMap(c)
 		rm := tagsToMap(r)
+		t.Logf("maps: create: %v remove: %v", cm, rm)
 		if !reflect.DeepEqual(cm, tc.Create) {
 			t.Fatalf("%d: bad create: %#v", i, cm)
 		}

From de20326bcaf4502cf329f52c71dbb3ae0be3874f Mon Sep 17 00:00:00 2001
From: Fabian Breckle <fabian.breckle@breuninger.de>
Date: Wed, 7 Nov 2018 12:20:11 +0100
Subject: [PATCH 3/3] Remove logging calls.

---
 aws/tags_test.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/aws/tags_test.go b/aws/tags_test.go
index 5ebb9fe4f63f..3e769cbe8060 100644
--- a/aws/tags_test.go
+++ b/aws/tags_test.go
@@ -64,10 +64,8 @@ func TestDiffTags(t *testing.T) {
 
 	for i, tc := range cases {
 		c, r := diffTags(tagsFromMap(tc.Old), tagsFromMap(tc.New))
-		t.Logf("create: %v remove: %v", c, r)
 		cm := tagsToMap(c)
 		rm := tagsToMap(r)
-		t.Logf("maps: create: %v remove: %v", cm, rm)
 		if !reflect.DeepEqual(cm, tc.Create) {
 			t.Fatalf("%d: bad create: %#v", i, cm)
 		}