Fix issues in Cloudwatch Log Group tag (#14886)

* Fix issues in Cloudwatch Log Group tag
    1) Removing tags from terraform doesn’t actually get removed in AWS
    2) Trying to update a tag with empty value (“”) to a non-empty value
causes terraform to loop forever

The issue was caused by a mixup of using tag values where tag name
should have used and is corrected in this patch.
This patch also removes the comparison of old and new tag values,
because AWS api takes care of updates by itself and there is no need to
perform an unnecessary UnTag API to update an existing tag value

* Updated the test cases to cover the removal and empty update scenarios
This commit is contained in:
johnthedev97 2017-06-02 15:18:27 -04:00 committed by Paul Stack
parent 47461f2286
commit de78838cd4
2 changed files with 62 additions and 7 deletions

View File

@ -213,10 +213,10 @@ func diffCloudWatchTags(oldTags map[string]interface{}, newTags map[string]inter
}
var remove []*string
for _, t := range oldTags {
old, ok := create[t.(string)]
if !ok || *old != t.(string) {
remove = append(remove, aws.String(t.(string)))
for t, _ := range oldTags {
_, ok := create[t]
if !ok {
remove = append(remove, aws.String(t))
}
}

View File

@ -153,20 +153,44 @@ func TestAccAWSCloudWatchLogGroup_tagging(t *testing.T) {
Config: testAccAWSCloudWatchLogGroupConfigWithTags(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogGroupExists("aws_cloudwatch_log_group.foobar", &lg),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.%", "2"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.%", "3"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Environment", "Production"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Foo", "Bar"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Empty", ""),
),
},
{
Config: testAccAWSCloudWatchLogGroupConfigWithTagsAdded(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogGroupExists("aws_cloudwatch_log_group.foobar", &lg),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.%", "4"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Environment", "Development"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Foo", "Bar"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Empty", ""),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Bar", "baz"),
),
},
{
Config: testAccAWSCloudWatchLogGroupConfigWithTagsUpdated(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogGroupExists("aws_cloudwatch_log_group.foobar", &lg),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.%", "3"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.%", "4"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Environment", "Development"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Empty", "NotEmpty"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Foo", "UpdatedBar"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Bar", "baz"),
),
},
{
Config: testAccAWSCloudWatchLogGroupConfigWithTags(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckCloudWatchLogGroupExists("aws_cloudwatch_log_group.foobar", &lg),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.%", "3"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Environment", "Production"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Foo", "Bar"),
resource.TestCheckResourceAttr("aws_cloudwatch_log_group.foobar", "tags.Empty", ""),
),
},
},
})
}
@ -243,6 +267,22 @@ resource "aws_cloudwatch_log_group" "foobar" {
tags {
Environment = "Production"
Foo = "Bar"
Empty = ""
}
}
`, rInt)
}
func testAccAWSCloudWatchLogGroupConfigWithTagsAdded(rInt int) string {
return fmt.Sprintf(`
resource "aws_cloudwatch_log_group" "foobar" {
name = "foo-bar-%d"
tags {
Environment = "Development"
Foo = "Bar"
Empty = ""
Bar = "baz"
}
}
`, rInt)
@ -255,13 +295,28 @@ resource "aws_cloudwatch_log_group" "foobar" {
tags {
Environment = "Development"
Foo = "Bar"
Foo = "UpdatedBar"
Empty = "NotEmpty"
Bar = "baz"
}
}
`, rInt)
}
func testAccAWSCloudWatchLogGroupConfigWithTagsRemoval(rInt int) string {
return fmt.Sprintf(`
resource "aws_cloudwatch_log_group" "foobar" {
name = "foo-bar-%d"
tags {
Environment = "Production"
Foo = "Bar"
Empty = ""
}
}
`, rInt)
}
func testAccAWSCloudWatchLogGroupConfig_withRetention(rInt int) string {
return fmt.Sprintf(`
resource "aws_cloudwatch_log_group" "foobar" {