Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/aws_cloudwatch_event_target: Include state upgrade for new default event bus name in Read path and only include EventBusName when non-empty in API requests #16075

Merged

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Nov 6, 2020

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #16069

Release note for CHANGELOG:

* resource/aws_cloudwatch_event_target: Prevent regression from version 3.14.0 with `ListTargetsByRuleInput.EventBusName` error

Until we sort out the details of setting up the acceptance testing framework to handle state upgrades (likely via Terraform CLI 0.13+ and TestCase.ExternalProviders), manually verified with the following:

terraform {
  required_providers {
    aws = "3.13.0"
  }
  required_version = "0.12.29"
}

provider "aws" {
  region = "us-east-2"
}

resource "aws_cloudwatch_event_rule" "test" {
  name                = "16069-test"
  schedule_expression = "rate(1 hour)"
}

resource "aws_cloudwatch_event_target" "test" {
  rule      = aws_cloudwatch_event_rule.test.name
  target_id = "16069-test"
  arn       = aws_sns_topic.test.arn
}

resource "aws_sns_topic" "test" {
  name = "16069-test"
}
$ terraform init
$ terraform apply
# update version to 3.14.0
$ terraform init
$ terraform apply
aws_cloudwatch_event_rule.test: Refreshing state... [id=16069-test]
aws_sns_topic.test: Refreshing state... [id=arn:aws:sns:us-east-2:--OMITTED--:16069-test]
aws_cloudwatch_event_target.test: Refreshing state... [id=16069-test-16069-test]

Error: InvalidParameter: 1 validation error(s) found.
- minimum field size of 1, ListTargetsByRuleInput.EventBusName.

After updating local plugin:

$ terraform apply
...
Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Output from acceptance testing in AWS Commercial:

--- PASS: TestAccAWSCloudWatchEventTarget_basic (41.56s)
--- PASS: TestAccAWSCloudWatchEventTarget_batch (146.89s)
--- PASS: TestAccAWSCloudWatchEventTarget_disappears (15.51s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecs (34.45s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecsWithBlankTaskCount (34.90s)
--- PASS: TestAccAWSCloudWatchEventTarget_EventBusName (36.74s)
--- PASS: TestAccAWSCloudWatchEventTarget_full (62.30s)
--- PASS: TestAccAWSCloudWatchEventTarget_GeneratedTargetId (17.97s)
--- PASS: TestAccAWSCloudWatchEventTarget_input_transformer (43.71s)
--- PASS: TestAccAWSCloudWatchEventTarget_inputTransformerJsonString (48.88s)
--- PASS: TestAccAWSCloudWatchEventTarget_kinesis (62.16s)
--- PASS: TestAccAWSCloudWatchEventTarget_sqs (18.22s)
--- PASS: TestAccAWSCloudWatchEventTarget_ssmDocument (21.40s)

Output from acceptance testing in AWS GovCloud (US):

--- PASS: TestAccAWSCloudWatchEventTarget_basic (51.37s)
--- PASS: TestAccAWSCloudWatchEventTarget_batch (153.85s)
--- PASS: TestAccAWSCloudWatchEventTarget_disappears (19.90s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecs (40.23s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecsWithBlankTaskCount (40.10s)
--- PASS: TestAccAWSCloudWatchEventTarget_EventBusName (43.57s)
--- PASS: TestAccAWSCloudWatchEventTarget_full (67.95s)
--- PASS: TestAccAWSCloudWatchEventTarget_GeneratedTargetId (22.57s)
--- PASS: TestAccAWSCloudWatchEventTarget_input_transformer (49.06s)
--- PASS: TestAccAWSCloudWatchEventTarget_inputTransformerJsonString (39.35s)
--- PASS: TestAccAWSCloudWatchEventTarget_kinesis (66.38s)
--- PASS: TestAccAWSCloudWatchEventTarget_sqs (22.96s)
--- PASS: TestAccAWSCloudWatchEventTarget_ssmDocument (25.39s)

…efault event bus name in Read path and only include EventBusName when non-empty in API requests

Reference: #16069

Until we sort out the details of setting up the acceptance testing framework to handle state upgrades (likely via Terraform CLI 0.13+ and TestCase.ExternalProviders), manually verified with the following:

```terraform
terraform {
  required_providers {
    aws = "3.13.0"
  }
  required_version = "0.12.29"
}

provider "aws" {
  region = "us-east-2"
}

resource "aws_cloudwatch_event_rule" "test" {
  name                = "16069-test"
  schedule_expression = "rate(1 hour)"
}

resource "aws_cloudwatch_event_target" "test" {
  rule      = aws_cloudwatch_event_rule.test.name
  target_id = "16069-test"
  arn       = aws_sns_topic.test.arn
}

resource "aws_sns_topic" "test" {
  name = "16069-test"
}
```

```console
$ terraform init
$ terraform apply
# update version to 3.14.0
$ terraform init
$ terraform apply
aws_cloudwatch_event_rule.test: Refreshing state... [id=16069-test]
aws_sns_topic.test: Refreshing state... [id=arn:aws:sns:us-east-2:--OMITTED--:16069-test]
aws_cloudwatch_event_target.test: Refreshing state... [id=16069-test-16069-test]

Error: InvalidParameter: 1 validation error(s) found.
- minimum field size of 1, ListTargetsByRuleInput.EventBusName.
```

After updating local plugin:

```console
$ terraform apply
...
Apply complete! Resources: 0 added, 0 changed, 0 destroyed.
```

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSCloudWatchEventTarget_basic (41.56s)
--- PASS: TestAccAWSCloudWatchEventTarget_batch (146.89s)
--- PASS: TestAccAWSCloudWatchEventTarget_disappears (15.51s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecs (34.45s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecsWithBlankTaskCount (34.90s)
--- PASS: TestAccAWSCloudWatchEventTarget_EventBusName (36.74s)
--- PASS: TestAccAWSCloudWatchEventTarget_full (62.30s)
--- PASS: TestAccAWSCloudWatchEventTarget_GeneratedTargetId (17.97s)
--- PASS: TestAccAWSCloudWatchEventTarget_input_transformer (43.71s)
--- PASS: TestAccAWSCloudWatchEventTarget_inputTransformerJsonString (48.88s)
--- PASS: TestAccAWSCloudWatchEventTarget_kinesis (62.16s)
--- PASS: TestAccAWSCloudWatchEventTarget_sqs (18.22s)
--- PASS: TestAccAWSCloudWatchEventTarget_ssmDocument (21.40s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- PASS: TestAccAWSCloudWatchEventTarget_basic (51.37s)
--- PASS: TestAccAWSCloudWatchEventTarget_batch (153.85s)
--- PASS: TestAccAWSCloudWatchEventTarget_disappears (19.90s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecs (40.23s)
--- PASS: TestAccAWSCloudWatchEventTarget_ecsWithBlankTaskCount (40.10s)
--- PASS: TestAccAWSCloudWatchEventTarget_EventBusName (43.57s)
--- PASS: TestAccAWSCloudWatchEventTarget_full (67.95s)
--- PASS: TestAccAWSCloudWatchEventTarget_GeneratedTargetId (22.57s)
--- PASS: TestAccAWSCloudWatchEventTarget_input_transformer (49.06s)
--- PASS: TestAccAWSCloudWatchEventTarget_inputTransformerJsonString (39.35s)
--- PASS: TestAccAWSCloudWatchEventTarget_kinesis (66.38s)
--- PASS: TestAccAWSCloudWatchEventTarget_sqs (22.96s)
--- PASS: TestAccAWSCloudWatchEventTarget_ssmDocument (25.39s)
```
@bflad bflad added bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. labels Nov 6, 2020
@bflad bflad added this to the v3.15.0 milestone Nov 6, 2020
@bflad bflad requested a review from a team as a code owner November 6, 2020 15:19
@ghost ghost added size/L Managed by automation to categorize the size of a PR. service/cloudwatchevents tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Nov 6, 2020
@Pimenta14
Copy link

Any estimation for when this will be released? Just to see if its worth it to release a patch on our side to force 3.13.0.

@bflad bflad modified the milestones: v3.15.0, v3.14.1 Nov 6, 2020
@anGie44 anGie44 self-requested a review November 6, 2020 18:08
Copy link
Contributor

@anGie44 anGie44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the state upgrader 👍

verified locally with config provided:

(1) w/provider v3.14.0 😿 :

 $ terraform plan -out=plan.out
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

aws_cloudwatch_event_rule.test: Refreshing state... [id=xxxx-test]
aws_sns_topic.test: Refreshing state... [id=arn:aws:sns:us-east-2:xxxxxxxxx:xxxxx-test]
aws_cloudwatch_event_target.test: Refreshing state... [id=xxxxx-test-xxxxx-test]

Error: InvalidParameter: 1 validation error(s) found.
- minimum field size of 1, ListTargetsByRuleInput.EventBusName.

(2) w/provider built from this branch 😄 :

 $ terraform plan -out=plan.out
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

aws_cloudwatch_event_rule.test: Refreshing state... [id=xxxxx-test]
aws_sns_topic.test: Refreshing state... [id=arn:aws:sns:us-east-2:xxxxxxxxx:xxxxx-test]
aws_cloudwatch_event_target.test: Refreshing state... [id=xxxxx-test-xxxxx-test]

------------------------------------------------------------------------

No changes. Infrastructure is up-to-date.

@bflad bflad merged commit 3b79f2d into master Nov 6, 2020
@bflad bflad deleted the b-aws_cloudwatch_event_target-event_bus_name-regression branch November 6, 2020 18:23
bflad added a commit that referenced this pull request Nov 6, 2020
@ghost
Copy link

ghost commented Nov 6, 2020

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Dec 7, 2020

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!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
3 participants