Fix issues in Cloudwatch Log Group tag

[johnthedev97]

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

[stack72]

Hi @johnthedev97

Thanks for the work here - please can you include an acceptance test to demonstrate the issue has been fixeD?

Thanks

Paul

[johnthedev97]

https://gist.github.com/johnthedev97/17c22380ea3dc0110579cbba6c145c32

Is that enough? Oe are you looking for something different?

[johnthedev97]

If what I have provided is not enough, please guide me on what needs to be done to get this merged?

[stack72]

Hi @johnthedev97

Thanks for showing me the output from your tests. What I mean is, there will be a file called resource_aws_cloudwatch_log_group_test.go

In that file, you will see acceptance tests that show different scenarios we run through nightly or on feature changes. Ideally, it would be really nice to run those tests (at the minimum) to include the output in your PR description. Even better would be to show that the work you have done actually fixes the bug you found

to run the tests, you can then run the command:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchLogGroup_basic'

Hope this helps

Paul

[johnthedev97]

Ok, Thanks for the info. Sorry this was my first commit.
I have updated the test cases as the current ones didn't cover the scenarios.
Here is the output

---

terraform dev$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchLogGroup_tagging'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/06/01 09:37:02 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchLogGroup_tagging -timeout 120m
=== RUN TestAccAWSCloudWatchLogGroup_tagging
--- PASS: TestAccAWSCloudWatchLogGroup_tagging (59.54s)
PASS
ok github.com/johnthedev97/terraform/builtin/providers/aws 59.577s
terraform dev$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchLogGroup_basic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/06/01 09:38:54 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchLogGroup_basic -timeout 120m
=== RUN TestAccAWSCloudWatchLogGroup_basic
--- PASS: TestAccAWSCloudWatchLogGroup_basic (16.58s)
PASS
ok github.com/johnthedev97/terraform/builtin/providers/aws 16.613s

[johnthedev97]

@stack72 hope the above details are up to your expectations

[stack72]

Hi @johnthedev97

This LGTM! Thanks for this :)

Paul

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.