From 97b9f3ffd73e1a37c71c8ccdcc5fc7d3f661ae8f Mon Sep 17 00:00:00 2001 From: DrFaust92 Date: Tue, 21 Jan 2020 11:19:50 +0200 Subject: [PATCH] refactor error add plan time validation to `arn`, `role_arn`, `launch_type`, `task_definition_arn` refactor tests --- aws/resource_aws_cloudwatch_event_target.go | 45 ++-- ...source_aws_cloudwatch_event_target_test.go | 252 +++++++++--------- 2 files changed, 153 insertions(+), 144 deletions(-) diff --git a/aws/resource_aws_cloudwatch_event_target.go b/aws/resource_aws_cloudwatch_event_target.go index 9e559012e15..1390279295c 100644 --- a/aws/resource_aws_cloudwatch_event_target.go +++ b/aws/resource_aws_cloudwatch_event_target.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" events "github.com/aws/aws-sdk-go/service/cloudwatchevents" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" ) @@ -44,8 +43,9 @@ func resourceAwsCloudWatchEventTarget() *schema.Resource { }, "arn": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + ValidateFunc: validateArn, }, "input": { @@ -63,8 +63,9 @@ func resourceAwsCloudWatchEventTarget() *schema.Resource { }, "role_arn": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Optional: true, + ValidateFunc: validateArn, }, "run_command_targets": { @@ -101,7 +102,11 @@ func resourceAwsCloudWatchEventTarget() *schema.Resource { "launch_type": { Type: schema.TypeString, Optional: true, - Default: "EC2", + Default: events.LaunchTypeEc2, + ValidateFunc: validation.StringInSlice([]string{ + events.LaunchTypeEc2, + events.LaunchTypeFargate, + }, true), }, "network_configuration": { Type: schema.TypeList, @@ -143,7 +148,7 @@ func resourceAwsCloudWatchEventTarget() *schema.Resource { "task_definition_arn": { Type: schema.TypeString, Required: true, - ValidateFunc: validation.StringLenBetween(1, 1600), + ValidateFunc: validateArn, }, }, }, @@ -275,22 +280,20 @@ func resourceAwsCloudWatchEventTargetRead(d *schema.ResourceData, meta interface d.SetId("") return nil } - if awsErr, ok := err.(awserr.Error); ok { - // This should never happen, but it's useful - // for recovering from https://github.com/hashicorp/terraform/issues/5389 - if awsErr.Code() == "ValidationException" { - log.Printf("[WARN] Removing CloudWatch Event Target %q because it never existed.", d.Id()) - d.SetId("") - return nil - } - - if awsErr.Code() == "ResourceNotFoundException" { - log.Printf("[WARN] CloudWatch Event Target (%q) not found. Removing it from state.", d.Id()) - d.SetId("") - return nil - } + // This should never happen, but it's useful + // for recovering from https://github.com/hashicorp/terraform/issues/5389 + if isAWSErr(err, "ValidationException", "") { + log.Printf("[WARN] Removing CloudWatch Event Target %q because it never existed.", d.Id()) + d.SetId("") + return nil + } + if isAWSErr(err, events.ErrCodeResourceNotFoundException, "") { + log.Printf("[WARN] CloudWatch Event Target (%q) not found. Removing it from state.", d.Id()) + d.SetId("") + return nil } + return err } log.Printf("[DEBUG] Found Event Target: %s", t) diff --git a/aws/resource_aws_cloudwatch_event_target_test.go b/aws/resource_aws_cloudwatch_event_target_test.go index 61a4d9d203b..f506c63a468 100644 --- a/aws/resource_aws_cloudwatch_event_target_test.go +++ b/aws/resource_aws_cloudwatch_event_target_test.go @@ -90,13 +90,12 @@ func testSweepCloudWatchEventTargets(region string) error { func TestAccAWSCloudWatchEventTarget_basic(t *testing.T) { var target events.Target - rName1 := acctest.RandString(5) - rName2 := acctest.RandString(5) - ruleName := fmt.Sprintf("tf-acc-cw-event-rule-basic-%s", rName1) - snsTopicName1 := fmt.Sprintf("tf-acc-%s", rName1) - snsTopicName2 := fmt.Sprintf("tf-acc-%s", rName2) - targetID1 := fmt.Sprintf("tf-acc-cw-target-%s", rName1) - targetID2 := fmt.Sprintf("tf-acc-cw-target-%s", rName2) + resourceName := "aws_cloudwatch_event_target.test" + ruleName := acctest.RandomWithPrefix("tf-acc-cw-event-rule-basic") + snsTopicName1 := acctest.RandomWithPrefix("tf-acc") + snsTopicName2 := acctest.RandomWithPrefix("tf-acc") + targetID1 := acctest.RandomWithPrefix("tf-acc-cw-target") + targetID2 := acctest.RandomWithPrefix("tf-acc-cw-target") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -106,27 +105,27 @@ func TestAccAWSCloudWatchEventTarget_basic(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfig(ruleName, snsTopicName1, targetID1), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.moobar", &target), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.moobar", "rule", ruleName), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.moobar", "target_id", targetID1), - resource.TestMatchResourceAttr("aws_cloudwatch_event_target.moobar", "arn", + testAccCheckCloudWatchEventTargetExists(resourceName, &target), + resource.TestCheckResourceAttr(resourceName, "rule", ruleName), + resource.TestCheckResourceAttr(resourceName, "target_id", targetID1), + resource.TestMatchResourceAttr(resourceName, "arn", regexp.MustCompile(fmt.Sprintf(":%s$", snsTopicName1))), ), }, { Config: testAccAWSCloudWatchEventTargetConfig(ruleName, snsTopicName2, targetID2), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.moobar", &target), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.moobar", "rule", ruleName), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.moobar", "target_id", targetID2), - resource.TestMatchResourceAttr("aws_cloudwatch_event_target.moobar", "arn", + testAccCheckCloudWatchEventTargetExists(resourceName, &target), + resource.TestCheckResourceAttr(resourceName, "rule", ruleName), + resource.TestCheckResourceAttr(resourceName, "target_id", targetID2), + resource.TestMatchResourceAttr(resourceName, "arn", regexp.MustCompile(fmt.Sprintf(":%s$", snsTopicName2))), ), }, { - ResourceName: "aws_cloudwatch_event_target.moobar", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.moobar"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -135,9 +134,9 @@ func TestAccAWSCloudWatchEventTarget_basic(t *testing.T) { func TestAccAWSCloudWatchEventTarget_missingTargetId(t *testing.T) { var target events.Target - rName := acctest.RandString(5) - ruleName := fmt.Sprintf("tf-acc-cw-event-rule-missing-target-id-%s", rName) - snsTopicName := fmt.Sprintf("tf-acc-%s", rName) + resourceName := "aws_cloudwatch_event_target.test" + ruleName := acctest.RandomWithPrefix("tf-acc-cw-event-rule-missing-target-id") + snsTopicName := acctest.RandomWithPrefix("tf-acc") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -147,16 +146,16 @@ func TestAccAWSCloudWatchEventTarget_missingTargetId(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigMissingTargetId(ruleName, snsTopicName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.moobar", &target), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.moobar", "rule", ruleName), - resource.TestMatchResourceAttr("aws_cloudwatch_event_target.moobar", "arn", + testAccCheckCloudWatchEventTargetExists(resourceName, &target), + resource.TestCheckResourceAttr(resourceName, "rule", ruleName), + resource.TestMatchResourceAttr(resourceName, "arn", regexp.MustCompile(fmt.Sprintf(":%s$", snsTopicName))), ), }, { - ResourceName: "aws_cloudwatch_event_target.moobar", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.moobar"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -165,10 +164,10 @@ func TestAccAWSCloudWatchEventTarget_missingTargetId(t *testing.T) { func TestAccAWSCloudWatchEventTarget_full(t *testing.T) { var target events.Target - rName := acctest.RandString(5) - ruleName := fmt.Sprintf("tf-acc-cw-event-rule-full-%s", rName) + resourceName := "aws_cloudwatch_event_target.test" + ruleName := acctest.RandomWithPrefix("tf-acc-cw-event-rule-full") ssmDocumentName := acctest.RandomWithPrefix("tf_ssm_Document") - targetID := fmt.Sprintf("tf-acc-cw-target-full-%s", rName) + targetID := acctest.RandomWithPrefix("tf-acc-cw-target-full") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -178,19 +177,19 @@ func TestAccAWSCloudWatchEventTarget_full(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfig_full(ruleName, targetID, ssmDocumentName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.foobar", &target), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.foobar", "rule", ruleName), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.foobar", "target_id", targetID), - resource.TestMatchResourceAttr("aws_cloudwatch_event_target.foobar", "arn", + testAccCheckCloudWatchEventTargetExists(resourceName, &target), + resource.TestCheckResourceAttr(resourceName, "rule", ruleName), + resource.TestCheckResourceAttr(resourceName, "target_id", targetID), + resource.TestMatchResourceAttr(resourceName, "arn", regexp.MustCompile("^arn:aws:kinesis:.*:stream/tf_ssm_Document")), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.foobar", "input", "{ \"source\": [\"aws.cloudtrail\"] }\n"), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.foobar", "input_path", ""), + resource.TestCheckResourceAttr(resourceName, "input", "{ \"source\": [\"aws.cloudtrail\"] }\n"), + resource.TestCheckResourceAttr(resourceName, "input_path", ""), ), }, { - ResourceName: "aws_cloudwatch_event_target.foobar", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.foobar"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -199,6 +198,7 @@ func TestAccAWSCloudWatchEventTarget_full(t *testing.T) { func TestAccAWSCloudWatchEventTarget_ssmDocument(t *testing.T) { var target events.Target + resourceName := "aws_cloudwatch_event_target.test" rName := acctest.RandomWithPrefix("tf_ssm_Document") resource.ParallelTest(t, resource.TestCase{ @@ -209,13 +209,13 @@ func TestAccAWSCloudWatchEventTarget_ssmDocument(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigSsmDocument(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.test", &target), + testAccCheckCloudWatchEventTargetExists(resourceName, &target), ), }, { - ResourceName: "aws_cloudwatch_event_target.test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.test"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -224,6 +224,7 @@ func TestAccAWSCloudWatchEventTarget_ssmDocument(t *testing.T) { func TestAccAWSCloudWatchEventTarget_ecs(t *testing.T) { var target events.Target + resourceName := "aws_cloudwatch_event_target.test" rName := acctest.RandomWithPrefix("tf_ecs_target") resource.ParallelTest(t, resource.TestCase{ @@ -234,13 +235,13 @@ func TestAccAWSCloudWatchEventTarget_ecs(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigEcs(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.test", &target), + testAccCheckCloudWatchEventTargetExists(resourceName, &target), ), }, { - ResourceName: "aws_cloudwatch_event_target.test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.test"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -249,6 +250,7 @@ func TestAccAWSCloudWatchEventTarget_ecs(t *testing.T) { func TestAccAWSCloudWatchEventTarget_ecsWithBlankTaskCount(t *testing.T) { var target events.Target + resourceName := "aws_cloudwatch_event_target.test" rName := acctest.RandomWithPrefix("tf_ecs_target") resource.ParallelTest(t, resource.TestCase{ @@ -259,14 +261,14 @@ func TestAccAWSCloudWatchEventTarget_ecsWithBlankTaskCount(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigEcsWithBlankTaskCount(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.test", &target), - resource.TestCheckResourceAttr("aws_cloudwatch_event_target.test", "ecs_target.0.task_count", "1"), + testAccCheckCloudWatchEventTargetExists(resourceName, &target), + resource.TestCheckResourceAttr(resourceName, "ecs_target.0.task_count", "1"), ), }, { - ResourceName: "aws_cloudwatch_event_target.test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.test"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -275,6 +277,7 @@ func TestAccAWSCloudWatchEventTarget_ecsWithBlankTaskCount(t *testing.T) { func TestAccAWSCloudWatchEventTarget_batch(t *testing.T) { var target events.Target + resourceName := "aws_cloudwatch_event_target.test" rName := acctest.RandomWithPrefix("tf_batch_target") resource.ParallelTest(t, resource.TestCase{ @@ -285,13 +288,13 @@ func TestAccAWSCloudWatchEventTarget_batch(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigBatch(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.test", &target), + testAccCheckCloudWatchEventTargetExists(resourceName, &target), ), }, { - ResourceName: "aws_cloudwatch_event_target.test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.test"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -300,6 +303,7 @@ func TestAccAWSCloudWatchEventTarget_batch(t *testing.T) { func TestAccAWSCloudWatchEventTarget_kinesis(t *testing.T) { var target events.Target + resourceName := "aws_cloudwatch_event_target.test" rName := acctest.RandomWithPrefix("tf_kinesis_target") resource.ParallelTest(t, resource.TestCase{ @@ -310,13 +314,13 @@ func TestAccAWSCloudWatchEventTarget_kinesis(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigKinesis(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.test", &target), + testAccCheckCloudWatchEventTargetExists(resourceName, &target), ), }, { - ResourceName: "aws_cloudwatch_event_target.test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.test"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -325,6 +329,7 @@ func TestAccAWSCloudWatchEventTarget_kinesis(t *testing.T) { func TestAccAWSCloudWatchEventTarget_sqs(t *testing.T) { var target events.Target + resourceName := "aws_cloudwatch_event_target.test" rName := acctest.RandomWithPrefix("tf_sqs_target") resource.ParallelTest(t, resource.TestCase{ @@ -335,13 +340,13 @@ func TestAccAWSCloudWatchEventTarget_sqs(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigSqs(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.test", &target), + testAccCheckCloudWatchEventTargetExists(resourceName, &target), ), }, { - ResourceName: "aws_cloudwatch_event_target.test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.test"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -350,6 +355,7 @@ func TestAccAWSCloudWatchEventTarget_sqs(t *testing.T) { func TestAccAWSCloudWatchEventTarget_input_transformer(t *testing.T) { var target events.Target + resourceName := "aws_cloudwatch_event_target.test" rName := acctest.RandomWithPrefix("tf_input_transformer") resource.ParallelTest(t, resource.TestCase{ @@ -360,13 +366,13 @@ func TestAccAWSCloudWatchEventTarget_input_transformer(t *testing.T) { { Config: testAccAWSCloudWatchEventTargetConfigInputTransformer(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckCloudWatchEventTargetExists("aws_cloudwatch_event_target.test", &target), + testAccCheckCloudWatchEventTargetExists(resourceName, &target), ), }, { - ResourceName: "aws_cloudwatch_event_target.test", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc("aws_cloudwatch_event_target.test"), + ImportStateIdFunc: testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName), ImportStateVerify: true, }, }, @@ -425,18 +431,18 @@ func testAccAWSCloudWatchEventTargetImportStateIdFunc(resourceName string) resou func testAccAWSCloudWatchEventTargetConfig(ruleName, snsTopicName, targetID string) string { return fmt.Sprintf(` -resource "aws_cloudwatch_event_rule" "foo" { +resource "aws_cloudwatch_event_rule" "test" { name = "%s" schedule_expression = "rate(1 hour)" } -resource "aws_cloudwatch_event_target" "moobar" { - rule = "${aws_cloudwatch_event_rule.foo.name}" +resource "aws_cloudwatch_event_target" "test" { + rule = "${aws_cloudwatch_event_rule.test.name}" target_id = "%s" - arn = "${aws_sns_topic.moon.arn}" + arn = "${aws_sns_topic.test.arn}" } -resource "aws_sns_topic" "moon" { +resource "aws_sns_topic" "test" { name = "%s" } `, ruleName, targetID, snsTopicName) @@ -444,17 +450,17 @@ resource "aws_sns_topic" "moon" { func testAccAWSCloudWatchEventTargetConfigMissingTargetId(ruleName, snsTopicName string) string { return fmt.Sprintf(` -resource "aws_cloudwatch_event_rule" "foo" { +resource "aws_cloudwatch_event_rule" "test" { name = "%s" schedule_expression = "rate(1 hour)" } -resource "aws_cloudwatch_event_target" "moobar" { - rule = "${aws_cloudwatch_event_rule.foo.name}" - arn = "${aws_sns_topic.moon.arn}" +resource "aws_cloudwatch_event_target" "test" { + rule = "${aws_cloudwatch_event_rule.test.name}" + arn = "${aws_sns_topic.test.arn}" } -resource "aws_sns_topic" "moon" { +resource "aws_sns_topic" "test" { name = "%s" } `, ruleName, snsTopicName) @@ -462,13 +468,13 @@ resource "aws_sns_topic" "moon" { func testAccAWSCloudWatchEventTargetConfig_full(ruleName, targetName, rName string) string { return fmt.Sprintf(` -resource "aws_cloudwatch_event_rule" "foo" { +resource "aws_cloudwatch_event_rule" "test" { name = "%s" schedule_expression = "rate(1 hour)" - role_arn = "${aws_iam_role.role.arn}" + role_arn = "${aws_iam_role.test.arn}" } -resource "aws_iam_role" "role" { +resource "aws_iam_role" "test" { name = "%s" assume_role_policy = <