Merge pull request #11687 from DrFaust92/r/cloudwatch-dest

r/cloudwatch_log_destination: add plan time validation to `role_arn` and `target_arn`

.changelog/11687.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/aws_cloudwatch_log_destination: Add plan time validation to `role_arn`, `name` and `target_arn`.
+```

aws/resource_aws_cloudwatch_log_destination.go

@@ -2,12 +2,14 @@ package aws
 
 import (
 	"fmt"
+	"regexp"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/cloudwatchlogs"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
 
 func resourceAwsCloudWatchLogDestination() *schema.Resource {
@@ -29,16 +31,22 @@ func resourceAwsCloudWatchLogDestination() *schema.Resource {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
+				ValidateFunc: validation.Any(
+					validation.StringLenBetween(1, 512),
+					validation.StringMatch(regexp.MustCompile(`[^:*]*`), ""),
+				),
 			},
 
 			"role_arn": {
-				Type:     schema.TypeString,
-				Required: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ValidateFunc: validateArn,
 			},
 
 			"target_arn": {
-				Type:     schema.TypeString,
-				Required: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ValidateFunc: validateArn,
 			},
 
 			"arn": {
@@ -53,21 +61,20 @@ func resourceAwsCloudWatchLogDestinationPut(d *schema.ResourceData, meta interfa
 	conn := meta.(*AWSClient).cloudwatchlogsconn
 
 	name := d.Get("name").(string)
-	role_arn := d.Get("role_arn").(string)
-	target_arn := d.Get("target_arn").(string)
+	roleArn := d.Get("role_arn").(string)
+	targetArn := d.Get("target_arn").(string)
 
 	params := &cloudwatchlogs.PutDestinationInput{
 		DestinationName: aws.String(name),
-		RoleArn:         aws.String(role_arn),
-		TargetArn:       aws.String(target_arn),
+		RoleArn:         aws.String(roleArn),
+		TargetArn:       aws.String(targetArn),
 	}
 
-	var resp *cloudwatchlogs.PutDestinationOutput
 	var err error
 	err = resource.Retry(3*time.Minute, func() *resource.RetryError {
-		resp, err = conn.PutDestination(params)
+		_, err = conn.PutDestination(params)
 
-		if isAWSErr(err, cloudwatchlogs.ErrCodeInvalidParameterException, "Could not deliver test message to specified") {
+		if isAWSErr(err, cloudwatchlogs.ErrCodeInvalidParameterException, "") {
 			return resource.RetryableError(err)
 		}
 		if err != nil {
@@ -76,20 +83,20 @@ func resourceAwsCloudWatchLogDestinationPut(d *schema.ResourceData, meta interfa
 		return nil
 	})
 	if isResourceTimeoutError(err) {
-		resp, err = conn.PutDestination(params)
+		_, err = conn.PutDestination(params)
 	}
 	if err != nil {
 		return fmt.Errorf("Error putting cloudwatch log destination: %s", err)
 	}
 	d.SetId(name)
-	d.Set("arn", resp.Destination.Arn)
-	return nil
+
+	return resourceAwsCloudWatchLogDestinationRead(d, meta)
 }
 
 func resourceAwsCloudWatchLogDestinationRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).cloudwatchlogsconn
-	name := d.Get("name").(string)
-	destination, exists, err := lookupCloudWatchLogDestination(conn, name, nil)
+
+	destination, exists, err := lookupCloudWatchLogDestination(conn, d.Id(), nil)
 	if err != nil {
 		return err
 	}
@@ -99,7 +106,6 @@ func resourceAwsCloudWatchLogDestinationRead(d *schema.ResourceData, meta interf
 		return nil
 	}
 
-	d.SetId(name)
 	d.Set("arn", destination.Arn)
 	d.Set("role_arn", destination.RoleArn)
 	d.Set("target_arn", destination.TargetArn)
@@ -110,14 +116,12 @@ func resourceAwsCloudWatchLogDestinationRead(d *schema.ResourceData, meta interf
 func resourceAwsCloudWatchLogDestinationDelete(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).cloudwatchlogsconn
 
-	name := d.Get("name").(string)
-
 	params := &cloudwatchlogs.DeleteDestinationInput{
-		DestinationName: aws.String(name),
+		DestinationName: aws.String(d.Id()),
 	}
 	_, err := conn.DeleteDestination(params)
 	if err != nil {
-		return fmt.Errorf("Error deleting Destination with name %s", name)
+		return fmt.Errorf("Error deleting Destination with name %s", d.Id())
 	}
 
 	return nil

aws/resource_aws_cloudwatch_log_destination_test.go

@@ -2,6 +2,7 @@ package aws
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/service/cloudwatchlogs"
@@ -13,6 +14,8 @@ import (
 func TestAccAWSCloudwatchLogDestination_basic(t *testing.T) {
 	var destination cloudwatchlogs.Destination
 	resourceName := "aws_cloudwatch_log_destination.test"
+	streamResourceName := "aws_kinesis_stream.test"
+	roleResourceName := "aws_iam_role.test"
 	rstring := acctest.RandString(5)
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -24,6 +27,9 @@ func TestAccAWSCloudwatchLogDestination_basic(t *testing.T) {
 				Config: testAccAWSCloudwatchLogDestinationConfig(rstring),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSCloudwatchLogDestinationExists(resourceName, &destination),
+					resource.TestCheckResourceAttrPair(resourceName, "target_arn", streamResourceName, "arn"),
+					resource.TestCheckResourceAttrPair(resourceName, "role_arn", roleResourceName, "arn"),
+					testAccMatchResourceAttrRegionalARN(resourceName, "arn", "logs", regexp.MustCompile(`destination:.+`)),
 				),
 			},
 			{
@@ -35,6 +41,29 @@ func TestAccAWSCloudwatchLogDestination_basic(t *testing.T) {
 	})
 }
 
+func TestAccAWSCloudwatchLogDestination_disappears(t *testing.T) {
+	var destination cloudwatchlogs.Destination
+	resourceName := "aws_cloudwatch_log_destination.test"
+
+	rstring := acctest.RandString(5)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSCloudwatchLogDestinationDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSCloudwatchLogDestinationConfig(rstring),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSCloudwatchLogDestinationExists(resourceName, &destination),
+					testAccCheckResourceDisappears(testAccProvider, resourceAwsCloudWatchLogDestination(), resourceName),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
 func testAccCheckAWSCloudwatchLogDestinationDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).cloudwatchlogsconn
 
@@ -81,7 +110,7 @@ func testAccCheckAWSCloudwatchLogDestinationExists(n string, d *cloudwatchlogs.D
 func testAccAWSCloudwatchLogDestinationConfig(rstring string) string {
 	return fmt.Sprintf(`
 resource "aws_kinesis_stream" "test" {
-  name        = "RootAccess_%s"
+  name        = "RootAccess_%[1]s"
   shard_count = 1
 }
 
@@ -107,7 +136,7 @@ data "aws_iam_policy_document" "role" {
 }
 
 resource "aws_iam_role" "test" {
-  name               = "CWLtoKinesisRole_%s"
+  name               = "CWLtoKinesisRole_%[1]s"
   assume_role_policy = data.aws_iam_policy_document.role.json
 }
 
@@ -138,13 +167,13 @@ data "aws_iam_policy_document" "policy" {
 }
 
 resource "aws_iam_role_policy" "test" {
-  name   = "Permissions-Policy-For-CWL_%s"
+  name   = "Permissions-Policy-For-CWL_%[1]s"
   role   = aws_iam_role.test.id
   policy = data.aws_iam_policy_document.policy.json
 }
 
 resource "aws_cloudwatch_log_destination" "test" {
-  name       = "testDestination_%s"
+  name       = "testDestination_%[1]s"
   target_arn = aws_kinesis_stream.test.arn
   role_arn   = aws_iam_role.test.arn
   depends_on = [aws_iam_role_policy.test]
@@ -176,5 +205,5 @@ resource "aws_cloudwatch_log_destination_policy" "test" {
   destination_name = aws_cloudwatch_log_destination.test.name
   access_policy    = data.aws_iam_policy_document.access.json
 }
-`, rstring, rstring, rstring, rstring)
+`, rstring)
 }