Feature/add support resource tags for aws cloudwatch event rule resource

[teraken0509]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #8047

Changes proposed in this pull request:

• Add resource tags support
• Add tags attribute for aws_cloudwatch_event_rule resource

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCloudWatchEventRule_tags'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSCloudWatchEventRule_tags -timeout 120m
=== RUN   TestAccAWSCloudWatchEventRule_tags
=== PAUSE TestAccAWSCloudWatchEventRule_tags
=== CONT  TestAccAWSCloudWatchEventRule_tags
--- PASS: TestAccAWSCloudWatchEventRule_tags (57.41s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	57.489s

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCloudWatchEventRule_full'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSCloudWatchEventRule_full -timeout 120m
=== RUN   TestAccAWSCloudWatchEventRule_full
=== PAUSE TestAccAWSCloudWatchEventRule_full
=== CONT  TestAccAWSCloudWatchEventRule_full
--- PASS: TestAccAWSCloudWatchEventRule_full (51.71s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	51.780s

[bflad]

This implementation seems off, which likely can be verified with error checking when using d.Set(). With aggregate types (TypeList, TypeSet, TypeMap), we should perform error checking to prevent issues where the code is not properly able to set the Terraform state. e.g.

if err := d.Set("tags", ...); err != nil {
  return fmt.Errorf("error setting tags: %s", err)
}

However saveTagsCloudWatchEvents is returning an error type itself. The value in the second argument for d.Set() with a TypeMap should be a map[string]interface{}. In general, its preferred to keep tag API calls in the Read function (so you can return errors with context) then just set the tags attribute like the others.

[bflad]

Thanks for the updates, @kterada0509! 🚀

--- PASS: TestAccAWSCloudWatchEventRule_prefix (12.66s)
--- PASS: TestAccAWSCloudWatchEventRule_full (13.87s)
--- PASS: TestAccAWSCloudWatchEventRule_importBasic (13.98s)
--- PASS: TestAccAWSCloudWatchEventRule_basic (21.56s)
--- PASS: TestAccAWSCloudWatchEventRule_enable (30.87s)
--- PASS: TestAccAWSCloudWatchEventRule_tags (33.59s)

[bflad]

For future enhancement, it appears PutRule supports a Tags parameter directly: https://docs.aws.amazon.com/sdk-for-go/api/service/cloudwatchevents/#PutRuleInput

Instead of setting the Tags via TagResource we may be able to simplify with something like:

Tags: tagsFromMapCloudWatchEvents(d.Get("tags").(map[string]interface{})),

Which (at least on creation) may be a requirement in some environments with IAM-based restrictions based on tags.

The current implementation is passing acceptance testing though, so for now we can get this in as-is.

[bflad]

This has been released in version 2.4.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[sernst]

Just updated to provider version 2.4.0 and am getting error output that appears to be what was introduced in this PR:

Error: Error applying plan:
aws_cloudwatch_event_rule.rule: error setting tags: Error retreiving tags for ARN: arn:aws:events:us-east-1:######:rule/my-cloudwatch-rule

and then after the failed apply it's not possible to refresh state from the same error:

Error: Error refreshing state: 1 error(s) occurred:
aws_cloudwatch_event_rule.rule: error setting tags: Error retreiving tags for ARN: arn:aws:events:us-east-1:######:rule/my-cloudwatch-rule

Downgrading to 2.3.0 resolves the issue. The resource definition is fairly simple:

resource "aws_cloudwatch_event_rule" "rule" {
  name = "${var.name}-cloudwatch-rule"
  schedule_expression = "cron(2 * * * ? *)"

  depends_on = ["aws_lambda_function.lambda"]
}

Release notes for 2.4.0 don't show this as a breaking change. Am I missing something about how this has been implemented, or should this be made into an issue?

[jgelens]

2.4.0 introduced the issue above for me as well.

[bflad]

Hi @sernst and @jgelens 👋 Version 2.4.0 of the Terraform AWS Provider began fetching (and configuring if desired) the new support for CloudWatch Event Rule resource tags, but was omitting the actual error message as you found. Apologies about that and its been rectified in #8190 which will release in version 2.5.0 likely later today.

If you are limiting IAM permissions, it is likely that you'll need to add events:ListTagsForResource to any IAM policies so the Terraform resource can at least read the information from the API.

In general though, the maintainers here will always recommend creating a new top level GitHub issue over commenting on a closed issue or pull request as these can be lost/forgotten in a project of this size.

[sernst]

@bflad Good to know and thanks for the fast turnaround on it. Will do in the future.

[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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!