Merge pull request #21661 from DrFaust92/datasync_s3_validation

r/datasync_s3_location - add validation `agent_arns`, `s3_bucket_arn` and `s3_config. bucket_access_role_arn`
hashicorp · Nov 8, 2021 · 98e5a72 · 98e5a72
2 parents 2eb46b9 + b74234a
commit 98e5a72
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 30 deletions.
diff --git a/.changelog/21661.txt b/.changelog/21661.txt
@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/aws_datasync_s3_location: Add validation to `agent_arns`, `s3_bucket_arn` and `s3_config.bucket_access_role_arn` arguments
+```
diff --git a/internal/service/datasync/location_s3.go b/internal/service/datasync/location_s3.go
@@ -38,13 +38,16 @@ func ResourceLocationS3() *schema.Resource {
 				Type:     schema.TypeSet,
 				Optional: true,
 				ForceNew: true,
-				Elem:     &schema.Schema{Type: schema.TypeString},
+				Elem: &schema.Schema{
+					Type:         schema.TypeString,
+					ValidateFunc: verify.ValidARN,
+				},
 			},
 			"s3_bucket_arn": {
 				Type:         schema.TypeString,
 				Required:     true,
 				ForceNew:     true,
-				ValidateFunc: validation.NoZeroValues,
+				ValidateFunc: verify.ValidARN,
 			},
 			"s3_config": {
 				Type:     schema.TypeList,
@@ -57,7 +60,7 @@ func ResourceLocationS3() *schema.Resource {
 							Type:         schema.TypeString,
 							Required:     true,
 							ForceNew:     true,
-							ValidateFunc: validation.NoZeroValues,
+							ValidateFunc: verify.ValidARN,
 						},
 					},
 				},

diff --git a/internal/service/datasync/location_s3_test.go b/internal/service/datasync/location_s3_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 	"github.com/hashicorp/terraform-provider-aws/internal/acctest"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
+	tfdatasync "github.com/hashicorp/terraform-provider-aws/internal/service/datasync"
 )
 
 func TestAccDataSyncLocationS3_basic(t *testing.T) {
@@ -105,8 +106,8 @@ func TestAccDataSyncLocationS3_disappears(t *testing.T) {
 				Config: testAccLocationS3Config(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckLocationS3Exists(resourceName, &locationS31),
-					testAccCheckLocationS3Disappears(&locationS31),
-				),
+					acctest.CheckResourceDisappears(acctest.Provider, tfdatasync.ResourceLocationS3(), resourceName),
+					acctest.CheckResourceDisappears(acctest.Provider, tfdatasync.ResourceLocationS3(), resourceName)),
 				ExpectNonEmptyPlan: true,
 			},
 		},
@@ -215,20 +216,6 @@ func testAccCheckLocationS3Exists(resourceName string, locationS3 *datasync.Desc
 	}
 }
 
-func testAccCheckLocationS3Disappears(location *datasync.DescribeLocationS3Output) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		conn := acctest.Provider.Meta().(*conns.AWSClient).DataSyncConn
-
-		input := &datasync.DeleteLocationInput{
-			LocationArn: location.LocationArn,
-		}
-
-		_, err := conn.DeleteLocation(input)
-
-		return err
-	}
-}
-
 func testAccCheckLocationS3NotRecreated(i, j *datasync.DescribeLocationS3Output) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		if !aws.TimeValue(i.CreationTime).Equal(aws.TimeValue(j.CreationTime)) {
@@ -242,7 +229,7 @@ func testAccCheckLocationS3NotRecreated(i, j *datasync.DescribeLocationS3Output)
 func testAccLocationS3BaseConfig(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_iam_role" "test" {
-  name = %q
+  name = %[1]q
 
   assume_role_policy = <<POLICY
 {
@@ -280,10 +267,10 @@ POLICY
 }
 
 resource "aws_s3_bucket" "test" {
-  bucket        = %q
+  bucket        = %[1]q
   force_destroy = true
 }
-`, rName, rName)
+`, rName)
 }
 
 func testAccLocationS3Config(rName string) string {
@@ -295,8 +282,6 @@ resource "aws_datasync_location_s3" "test" {
   s3_config {
     bucket_access_role_arn = aws_iam_role.test.arn
   }
-
-  depends_on = [aws_iam_role_policy.test]
 }
 `
 }
@@ -311,8 +296,6 @@ resource "aws_datasync_location_s3" "test" {
   s3_config {
     bucket_access_role_arn = aws_iam_role.test.arn
   }
-
-  depends_on = [aws_iam_role_policy.test]
 }
 `
 }
@@ -330,8 +313,6 @@ resource "aws_datasync_location_s3" "test" {
   tags = {
     %q = %q
   }
-
-  depends_on = [aws_iam_role_policy.test]
 }
 `, key1, value1)
 }
@@ -350,8 +331,6 @@ resource "aws_datasync_location_s3" "test" {
     %q = %q
     %q = %q
   }
-
-  depends_on = [aws_iam_role_policy.test]
 }
 `, key1, value1, key2, value2)
 }