ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional #5787

iceycake · 2016-03-22T18:57:00Z

According to the AWS documentation, the target_id is being used to guarantee the uniqueness of the CloudWatch rule target. According to the discussion in #5702, it makes developer life eaiser by having target_id be optional and let terraform to generate the unique ID.

stack72 · 2016-03-22T18:58:06Z

builtin/providers/aws/resource_aws_cloudwatch_event_target.go

@@ -29,7 +30,7 @@ func resourceAwsCloudWatchEventTarget() *schema.Resource {

 			"target_id": &schema.Schema{
 				Type:         schema.TypeString,
-				Required:     true,
+				Optional:     true,
 				ForceNew:     true,


Hi @iceycake do you still want to force a new resource if the target_id changes?

I think it should be a new resource if the target_id changed (in order to guarantee the uniqueness of the rule target)

stack72 · 2016-03-22T19:06:12Z

Hi @iceycake

I just ran the tests and they don't work I'm afraid

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchEventTarget_' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchEventTarget_ -timeout 120m
=== RUN   TestAccAWSCloudWatchEventTarget_basic
--- PASS: TestAccAWSCloudWatchEventTarget_basic (28.19s)
=== RUN   TestAccAWSCloudWatchEventTarget_missingTargetId
--- FAIL: TestAccAWSCloudWatchEventTarget_missingTargetId (9.27s)
    testing.go:154: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_cloudwatch_event_target.moobar: Creating CloudWatch Event Target failed: InvalidParameter: 1 validation errors:
        - field too short, minimum length 1: Targets[0].Id
=== RUN   TestAccAWSCloudWatchEventTarget_full
--- PASS: TestAccAWSCloudWatchEventTarget_full (100.29s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    137.766s

iceycake · 2016-03-22T22:20:58Z

There was a bug and it seems I get it fixed. Please try again.

stack72 · 2016-03-22T22:34:02Z

Hi @iceycake

just ran the tests again and go the following (twice!)

terraform [iceycake-ISSUE-5702] % TF_LOG=1 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchEventTarget_' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchEventTarget_ -timeout 120m
=== RUN   TestAccAWSCloudWatchEventTarget_basic
--- PASS: TestAccAWSCloudWatchEventTarget_basic (28.51s)
=== RUN   TestAccAWSCloudWatchEventTarget_missingTargetId
--- FAIL: TestAccAWSCloudWatchEventTarget_missingTargetId (10.29s)
    testing.go:154: Step 0 error: Check failed: Check 1/3 error: Not found: aws_cloudwatch_event_target.moobar
    testing.go:175: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error(s) occurred:

        * aws_cloudwatch_event_rule.foo: Error deleting CloudWatch Event Rule: ValidationException: Rule can't be deleted since it has targets.
            status code: 400, request id: 349053be-f07d-11e5-b2e5-e7e20c9002c5

        State: aws_cloudwatch_event_rule.foo:
          ID = tf-acc-cw-event-rule-missing-target-id
          arn = arn:aws:events:us-west-2:881237884953:rule/tf-acc-cw-event-rule-missing-target-id
          description =
          is_enabled = true
          name = tf-acc-cw-event-rule-missing-target-id
          role_arn =
          schedule_expression = rate(1 hour)
=== RUN   TestAccAWSCloudWatchEventTarget_full
--- PASS: TestAccAWSCloudWatchEventTarget_full (97.07s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    135.892s



terraform [iceycake-ISSUE-5702] % TF_LOG=1 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchEventTarget_' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchEventTarget_ -timeout 120m
=== RUN   TestAccAWSCloudWatchEventTarget_basic
--- PASS: TestAccAWSCloudWatchEventTarget_basic (28.13s)
=== RUN   TestAccAWSCloudWatchEventTarget_missingTargetId
--- FAIL: TestAccAWSCloudWatchEventTarget_missingTargetId (10.50s)
    testing.go:154: Step 0 error: Check failed: Check 1/3 error: Not found: aws_cloudwatch_event_target.moobar
    testing.go:175: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error(s) occurred:

        * aws_cloudwatch_event_rule.foo: Error deleting CloudWatch Event Rule: ValidationException: Rule can't be deleted since it has targets.
            status code: 400, request id: 988f62b9-f07d-11e5-a13e-f3aea3f0a9cc

        State: aws_cloudwatch_event_rule.foo:
          ID = tf-acc-cw-event-rule-missing-target-id
          arn = arn:aws:events:us-west-2:881237884953:rule/tf-acc-cw-event-rule-missing-target-id
          description =
          is_enabled = true
          name = tf-acc-cw-event-rule-missing-target-id
          role_arn =
          schedule_expression = rate(1 hour)
=== RUN   TestAccAWSCloudWatchEventTarget_full
--- PASS: TestAccAWSCloudWatchEventTarget_full (96.98s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    135.635s

iceycake · 2016-03-23T00:36:26Z

ok, it's far more difficult than i originally thought. The target_id indeed is the only unique key to identify the rule target. Don't know why i completely missing that (my test passed when i did the previous commit).

I need to rethink again how to deal with this.

stack72 · 2016-03-23T11:53:14Z

Nps - Thanks for all of the effort on this!

…ange with target_id.

iceycake · 2016-03-23T18:16:12Z

I think I got it fixed and didn't see error again using the same cli

USHOLACH01-ML:terraform andy.chan$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchEventTarget_' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
/Users/andy.chan/Projects/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchEventTarget_ -timeout 120m
=== RUN   TestAccAWSCloudWatchEventTarget_basic
--- PASS: TestAccAWSCloudWatchEventTarget_basic (17.18s)
=== RUN   TestAccAWSCloudWatchEventTarget_missingTargetId
--- PASS: TestAccAWSCloudWatchEventTarget_missingTargetId (9.95s)
=== RUN   TestAccAWSCloudWatchEventTarget_full
--- PASS: TestAccAWSCloudWatchEventTarget_full (100.05s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    127.192s

Please review it again.

stack72 · 2016-03-23T22:00:39Z

Hi @iceycake

I still can't get this to work I'm afraid

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchEventTarget_' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchEventTarget_ -timeout 120m
=== RUN   TestAccAWSCloudWatchEventTarget_basic
--- PASS: TestAccAWSCloudWatchEventTarget_basic (28.66s)
=== RUN   TestAccAWSCloudWatchEventTarget_missingTargetId
--- FAIL: TestAccAWSCloudWatchEventTarget_missingTargetId (15.79s)
    testing.go:175: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error(s) occurred:

        * aws_cloudwatch_event_rule.foo: Error deleting CloudWatch Event Rule: ValidationException: Rule can't be deleted since it has targets.
            status code: 400, request id: 2c07007f-f142-11e5-8f8b-01b1c0a8bc1a

        State: aws_cloudwatch_event_rule.foo:
          ID = tf-acc-cw-event-rule-missing-target-id
          arn = arn:aws:events:us-west-2:881237884953:rule/tf-acc-cw-event-rule-missing-target-id
          description =
          is_enabled = true
          name = tf-acc-cw-event-rule-missing-target-id
          role_arn =
          schedule_expression = rate(1 hour)
=== RUN   TestAccAWSCloudWatchEventTarget_full
--- PASS: TestAccAWSCloudWatchEventTarget_full (100.16s)
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    144.620s

iceycake · 2016-03-23T22:28:54Z

I ran into the same problem too. After I cleanup all the aws resources that were created from the previous bad code, I can pass the test.

I just ran again:

USHOLACH01-ML:terraform andy.chan$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudWatchEventTarget_' 2>~/tf.log==> Checking that code complies with gofmt requirements...
/Users/andy.chan/Projects/go/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudWatchEventTarget_ -timeout 120m
=== RUN   TestAccAWSCloudWatchEventTarget_basic
--- PASS: TestAccAWSCloudWatchEventTarget_basic (10.72s)
=== RUN   TestAccAWSCloudWatchEventTarget_missingTargetId
--- PASS: TestAccAWSCloudWatchEventTarget_missingTargetId (8.22s)
=== RUN   TestAccAWSCloudWatchEventTarget_full
--- PASS: TestAccAWSCloudWatchEventTarget_full (89.60s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    108.557s

stack72 · 2016-03-31T00:17:15Z

Hi @iceycake

You were indeed correct - this was due to cleanup. I will merge this as is and try to make the tests a little more reliable by using different ids :)

Thanks for all the hard work on this PR

Paul

ghost · 2020-04-27T01:53:11Z

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.

stack72 reviewed Mar 22, 2016
View reviewed changes

stack72 added enhancement provider/aws labels Mar 22, 2016

stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Mar 22, 2016

iceycake changed the title ~~ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional~~ [WIP] ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional Mar 23, 2016

Andy Chan added 6 commits March 23, 2016 10:25

ISSUE-5702: Making the CloudWatch Event Rule Target ID optional

7dc5a27

ISSUE-5702: Updating the documentation to reflex the corresponding ch…

7008bb6

…ange with target_id.

Fixed the test case to check if the targetId is created

92590d1

ISSUE-5702: Fixed a bug on target_id generation and related test case

0eee165

ISSUE-5702: 2nd attempt to impl the target_id be optional

76adbef

ISSUE-5702: Fixed the testing

4115249

iceycake changed the title ~~[WIP] ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional~~ ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional Mar 23, 2016

stack72 merged commit 3dbedc6 into hashicorp:master Mar 31, 2016

stack72 mentioned this pull request Mar 31, 2016

Required target_id param in aws_cloudwatch_event_target #5702

Closed

ghost locked and limited conversation to collaborators Apr 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional #5787

ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional #5787

iceycake commented Mar 22, 2016

stack72 Mar 22, 2016

iceycake Mar 22, 2016

stack72 Mar 22, 2016

stack72 commented Mar 22, 2016

iceycake commented Mar 22, 2016

stack72 commented Mar 22, 2016

iceycake commented Mar 23, 2016

stack72 commented Mar 23, 2016

iceycake commented Mar 23, 2016

stack72 commented Mar 23, 2016

iceycake commented Mar 23, 2016

stack72 commented Mar 31, 2016

ghost commented Apr 27, 2020

ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional #5787

ISSUE-5702: Making the Cloudwatch Event Rule Target target_id optional #5787

Conversation

iceycake commented Mar 22, 2016

stack72 Mar 22, 2016

Choose a reason for hiding this comment

iceycake Mar 22, 2016

Choose a reason for hiding this comment

stack72 Mar 22, 2016

Choose a reason for hiding this comment

stack72 commented Mar 22, 2016

iceycake commented Mar 22, 2016

stack72 commented Mar 22, 2016

iceycake commented Mar 23, 2016

stack72 commented Mar 23, 2016

iceycake commented Mar 23, 2016

stack72 commented Mar 23, 2016

iceycake commented Mar 23, 2016

stack72 commented Mar 31, 2016

ghost commented Apr 27, 2020