From 7405c676bcc6d195506f96c904f3f1b5d985f225 Mon Sep 17 00:00:00 2001 From: yuonoda Date: Sun, 21 Feb 2021 15:28:50 +0900 Subject: [PATCH 01/23] added a basic form of data source --- aws/data_source_aws_s3_bucket_policy.go | 42 +++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 aws/data_source_aws_s3_bucket_policy.go diff --git a/aws/data_source_aws_s3_bucket_policy.go b/aws/data_source_aws_s3_bucket_policy.go new file mode 100644 index 00000000000..c92ab4ca8ee --- /dev/null +++ b/aws/data_source_aws_s3_bucket_policy.go @@ -0,0 +1,42 @@ +package aws + +import ( + "fmt" + "log" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func dataSourceAwsS3BucketPolicy() *schema.Resource { + return &schema.Resource{ + Read: dataSourceAwsS3BucketPolicyRead, + + Schema: map[string]*schema.Schema{ + "bucket": { + Type: schema.TypeString, + Computed: true, + }, + }, + } +} + +func dataSourceAwsS3BucketPolicyRead(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).s3conn + + bucketName := d.Get("bucket").(string) + input := &s3.GetBucketPolicyInput{ + Bucket: aws.String(bucketName), + } + + log.Printf("[DEBUG] Reading S3 bucket: %s", input) + output, err := conn.GetBucketPolicy(input) + if err != nil { + return fmt.Errorf("Failed getting S3 bucket (%s): %w", bucketName, err) + } + + d.SetId(bucketName) + d.Set("policy", output.Policy) + return nil +} From 9d647c5bf857164a89de0bf9035b198b3abb0f3d Mon Sep 17 00:00:00 2001 From: yuonoda Date: Sun, 21 Feb 2021 17:45:56 +0900 Subject: [PATCH 02/23] added a basic test --- aws/data_source_aws_s3_bucket_policy.go | 14 ++- aws/data_source_aws_s3_bucket_policy_test.go | 114 +++++++++++++++++++ aws/provider.go | 1 + 3 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 aws/data_source_aws_s3_bucket_policy_test.go diff --git a/aws/data_source_aws_s3_bucket_policy.go b/aws/data_source_aws_s3_bucket_policy.go index c92ab4ca8ee..90f75980e29 100644 --- a/aws/data_source_aws_s3_bucket_policy.go +++ b/aws/data_source_aws_s3_bucket_policy.go @@ -2,11 +2,10 @@ package aws import ( "fmt" - "log" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "log" ) func dataSourceAwsS3BucketPolicy() *schema.Resource { @@ -16,7 +15,7 @@ func dataSourceAwsS3BucketPolicy() *schema.Resource { Schema: map[string]*schema.Schema{ "bucket": { Type: schema.TypeString, - Computed: true, + Required: true, }, }, } @@ -26,17 +25,20 @@ func dataSourceAwsS3BucketPolicyRead(d *schema.ResourceData, meta interface{}) e conn := meta.(*AWSClient).s3conn bucketName := d.Get("bucket").(string) + fmt.Println("bucketName:", bucketName) input := &s3.GetBucketPolicyInput{ Bucket: aws.String(bucketName), } - log.Printf("[DEBUG] Reading S3 bucket: %s", input) + log.Printf("[DEBUG] Reading S3 bucket policy: %s", input) output, err := conn.GetBucketPolicy(input) + fmt.Printf("output:%v\n", output) if err != nil { - return fmt.Errorf("Failed getting S3 bucket (%s): %w", bucketName, err) + return fmt.Errorf("failed getting S3 bucket policy (%s): %w", bucketName, err) } + fmt.Println("policy:", *output.Policy) d.SetId(bucketName) - d.Set("policy", output.Policy) + d.Set("policy", *output.Policy) return nil } diff --git a/aws/data_source_aws_s3_bucket_policy_test.go b/aws/data_source_aws_s3_bucket_policy_test.go new file mode 100644 index 00000000000..423c920423e --- /dev/null +++ b/aws/data_source_aws_s3_bucket_policy_test.go @@ -0,0 +1,114 @@ +package aws + +import ( + "fmt" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { + bucketName := acctest.RandomWithPrefix("tf-test-bucket") + //region := testAccGetRegion() + //hostedZoneID, _ := HostedZoneIDForRegion(region) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketPolicyExists("data.aws_s3_bucket_policy.policy"), + resource.TestCheckResourceAttrPair("data.aws_s3_bucket_policy.policy", "policy", "aws_s3_bucket_policy.policy", "policy"), + //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "region", region), + //testAccCheckS3BucketDomainName("data.aws_s3_bucket.bucket", "bucket_domain_name", bucketName), + //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "bucket_regional_domain_name", testAccBucketRegionalDomainName(bucketName, region)), + //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "hosted_zone_id", hostedZoneID), + //resource.TestCheckNoResourceAttr("data.aws_s3_bucket.bucket", "website_endpoint"), + ), + }, + }, + }) +} + +func testAccCheckAWSS3BucketPolicyExists(n string) resource.TestCheckFunc { + return testAccCheckAWSS3BucketPolicyExistsWithProvider(n, func() *schema.Provider { return testAccProvider }) +} + +func testAccCheckAWSS3BucketPolicyExistsWithProvider(n string, providerF func() *schema.Provider) resource.TestCheckFunc { + return func(s *terraform.State) error { + fmt.Println("s.RootModule().Resources:", s.RootModule().Resources) + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("no ID is set") + } + + provider := providerF() + + conn := provider.Meta().(*AWSClient).s3conn + _, err := conn.GetBucketPolicy(&s3.GetBucketPolicyInput{ + Bucket: aws.String(rs.Primary.ID), + }) + + if err != nil { + if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") { + return fmt.Errorf("s3 bucket not found") + } + return err + } + return nil + + } +} + +func testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "bucket" { + bucket = "%s" + + tags = { + TestName = "TestAccAWSS3BucketPolicy_basic" + } +} + +resource "aws_s3_bucket_policy" "bucket" { + bucket = aws_s3_bucket.bucket.id + policy = jsonencode({ + Version = "2012-10-17" + Id = "MYBUCKETPOLICY" + Statement = [ + { + Sid = "IPAllow" + Effect = "Deny" + Principal = "*" + Action = "s3:*" + Resource = [ + aws_s3_bucket.bucket.arn, + "${aws_s3_bucket.bucket.arn}/*", + ] + Condition = { + IpAddress = { + "aws:SourceIp" = "8.8.8.8/32" + } + } + }, + ] + }) +} + +data "aws_s3_bucket_policy" "policy" { + bucket = aws_s3_bucket.bucket.bucket +} + +`, bucketName) +} diff --git a/aws/provider.go b/aws/provider.go index d234981ddbd..51a4c9d5a0e 100644 --- a/aws/provider.go +++ b/aws/provider.go @@ -347,6 +347,7 @@ func Provider() *schema.Provider { "aws_s3_bucket": dataSourceAwsS3Bucket(), "aws_s3_bucket_object": dataSourceAwsS3BucketObject(), "aws_s3_bucket_objects": dataSourceAwsS3BucketObjects(), + "aws_s3_bucket_policy": dataSourceAwsS3BucketPolicy(), "aws_sagemaker_prebuilt_ecr_image": dataSourceAwsSageMakerPrebuiltECRImage(), "aws_secretsmanager_secret": dataSourceAwsSecretsManagerSecret(), "aws_secretsmanager_secret_rotation": dataSourceAwsSecretsManagerSecretRotation(), From 921b6b7cf375419b94ef151340f6c2e6f4ef3c94 Mon Sep 17 00:00:00 2001 From: yuonoda Date: Sun, 21 Feb 2021 18:39:55 +0900 Subject: [PATCH 03/23] check if there is a bucket --- aws/data_source_aws_s3_bucket_policy.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/aws/data_source_aws_s3_bucket_policy.go b/aws/data_source_aws_s3_bucket_policy.go index 90f75980e29..a4c4fb84a7d 100644 --- a/aws/data_source_aws_s3_bucket_policy.go +++ b/aws/data_source_aws_s3_bucket_policy.go @@ -17,6 +17,10 @@ func dataSourceAwsS3BucketPolicy() *schema.Resource { Type: schema.TypeString, Required: true, }, + "policy": { + Type: schema.TypeString, + Computed: true, + }, }, } } @@ -25,11 +29,19 @@ func dataSourceAwsS3BucketPolicyRead(d *schema.ResourceData, meta interface{}) e conn := meta.(*AWSClient).s3conn bucketName := d.Get("bucket").(string) + + // fails to get policy without this part + _, err := conn.HeadBucket(&s3.HeadBucketInput{ + Bucket: aws.String(bucketName), + }) + if err != nil { + return fmt.Errorf("bucket not found: %s", err) + } + fmt.Println("bucketName:", bucketName) input := &s3.GetBucketPolicyInput{ Bucket: aws.String(bucketName), } - log.Printf("[DEBUG] Reading S3 bucket policy: %s", input) output, err := conn.GetBucketPolicy(input) fmt.Printf("output:%v\n", output) From 7f551f13c5b85610d4a51d40bf3f595ae1c90bb2 Mon Sep 17 00:00:00 2001 From: yuonoda Date: Sun, 21 Feb 2021 18:40:45 +0900 Subject: [PATCH 04/23] changed test config --- aws/data_source_aws_s3_bucket_policy_test.go | 44 ++++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/aws/data_source_aws_s3_bucket_policy_test.go b/aws/data_source_aws_s3_bucket_policy_test.go index 423c920423e..db37a0ccfe6 100644 --- a/aws/data_source_aws_s3_bucket_policy_test.go +++ b/aws/data_source_aws_s3_bucket_policy_test.go @@ -25,7 +25,7 @@ func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { Config: testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketPolicyExists("data.aws_s3_bucket_policy.policy"), - resource.TestCheckResourceAttrPair("data.aws_s3_bucket_policy.policy", "policy", "aws_s3_bucket_policy.policy", "policy"), + resource.TestCheckResourceAttrPair("data.aws_s3_bucket_policy.policy", "policy", "aws_s3_bucket_policy.bucket", "policy"), //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "region", region), //testAccCheckS3BucketDomainName("data.aws_s3_bucket.bucket", "bucket_domain_name", bucketName), //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "bucket_regional_domain_name", testAccBucketRegionalDomainName(bucketName, region)), @@ -82,28 +82,28 @@ resource "aws_s3_bucket" "bucket" { } resource "aws_s3_bucket_policy" "bucket" { - bucket = aws_s3_bucket.bucket.id - policy = jsonencode({ - Version = "2012-10-17" - Id = "MYBUCKETPOLICY" - Statement = [ - { - Sid = "IPAllow" - Effect = "Deny" - Principal = "*" - Action = "s3:*" - Resource = [ - aws_s3_bucket.bucket.arn, - "${aws_s3_bucket.bucket.arn}/*", - ] - Condition = { - IpAddress = { - "aws:SourceIp" = "8.8.8.8/32" - } - } - }, + bucket = aws_s3_bucket.bucket.bucket + policy = data.aws_iam_policy_document.policy.json +} + +data "aws_iam_policy_document" "policy" { + statement { + effect = "Allow" + + actions = [ + "s3:*", ] - }) + + resources = [ + aws_s3_bucket.bucket.arn, + "${aws_s3_bucket.bucket.arn}/*", + ] + + principals { + type = "AWS" + identifiers = ["*"] + } + } } data "aws_s3_bucket_policy" "policy" { From 4fb564df0948edd1b50e4263ac8d47e48adf999a Mon Sep 17 00:00:00 2001 From: yuonoda Date: Sun, 21 Feb 2021 19:03:30 +0900 Subject: [PATCH 05/23] added a function to compare policy --- aws/data_source_aws_s3_bucket_policy_test.go | 42 +++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/aws/data_source_aws_s3_bucket_policy_test.go b/aws/data_source_aws_s3_bucket_policy_test.go index db37a0ccfe6..34058cc724b 100644 --- a/aws/data_source_aws_s3_bucket_policy_test.go +++ b/aws/data_source_aws_s3_bucket_policy_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/service/s3" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + awspolicy "github.com/jen20/awspolicyequivalence" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" @@ -25,7 +26,7 @@ func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { Config: testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketPolicyExists("data.aws_s3_bucket_policy.policy"), - resource.TestCheckResourceAttrPair("data.aws_s3_bucket_policy.policy", "policy", "aws_s3_bucket_policy.bucket", "policy"), + testAccCheckAWSS3BucketPolicyPolicyMatch("data.aws_s3_bucket_policy.policy", "policy", "aws_s3_bucket_policy.bucket", "policy"), //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "region", region), //testAccCheckS3BucketDomainName("data.aws_s3_bucket.bucket", "bucket_domain_name", bucketName), //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "bucket_regional_domain_name", testAccBucketRegionalDomainName(bucketName, region)), @@ -41,6 +42,45 @@ func testAccCheckAWSS3BucketPolicyExists(n string) resource.TestCheckFunc { return testAccCheckAWSS3BucketPolicyExistsWithProvider(n, func() *schema.Provider { return testAccProvider }) } +func testAccCheckAWSS3BucketPolicyPolicyMatch(resource1, attr1, resource2, attr2 string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resource1] + if !ok { + return fmt.Errorf("Not found: %s", resource1) + } + if rs.Primary.ID == "" { + return fmt.Errorf("No ID is set") + } + policy1, ok := rs.Primary.Attributes[attr1] + if !ok { + return fmt.Errorf("Attribute %q not found for %q", attr1, resource1) + } + + rs, ok = s.RootModule().Resources[resource2] + if !ok { + return fmt.Errorf("Not found: %s", resource2) + } + if rs.Primary.ID == "" { + return fmt.Errorf("No ID is set") + } + policy2, ok := rs.Primary.Attributes[attr2] + if !ok { + return fmt.Errorf("Attribute %q not found for %q", attr2, resource2) + } + + areEquivalent, err := awspolicy.PoliciesAreEquivalent(policy1, policy2) + if err != nil { + return fmt.Errorf("Comparing AWS Policies failed: %s", err) + } + + if !areEquivalent { + return fmt.Errorf("AWS policies differ.\npolicy1: %s\npolicy2: %s", policy1, policy2) + } + + return nil + } +} + func testAccCheckAWSS3BucketPolicyExistsWithProvider(n string, providerF func() *schema.Provider) resource.TestCheckFunc { return func(s *terraform.State) error { fmt.Println("s.RootModule().Resources:", s.RootModule().Resources) From 9d975d8d6dba3d2bfcae47918b695c96a0ac088c Mon Sep 17 00:00:00 2001 From: yuonoda Date: Tue, 23 Feb 2021 10:21:11 +0900 Subject: [PATCH 06/23] set policy without pointer type --- aws/data_source_aws_s3_bucket_policy.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aws/data_source_aws_s3_bucket_policy.go b/aws/data_source_aws_s3_bucket_policy.go index a4c4fb84a7d..8d01ace9864 100644 --- a/aws/data_source_aws_s3_bucket_policy.go +++ b/aws/data_source_aws_s3_bucket_policy.go @@ -49,8 +49,9 @@ func dataSourceAwsS3BucketPolicyRead(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("failed getting S3 bucket policy (%s): %w", bucketName, err) } - fmt.Println("policy:", *output.Policy) + policy := *output.Policy + fmt.Println("policy:", policy) d.SetId(bucketName) - d.Set("policy", *output.Policy) + d.Set("policy", policy) return nil } From 07290d0ae7748173867ce0d27e2f23f8bfb043c3 Mon Sep 17 00:00:00 2001 From: yuonoda Date: Tue, 23 Feb 2021 10:23:40 +0900 Subject: [PATCH 07/23] Tidied up imports --- aws/data_source_aws_s3_bucket_policy.go | 3 ++- aws/data_source_aws_s3_bucket_policy_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/aws/data_source_aws_s3_bucket_policy.go b/aws/data_source_aws_s3_bucket_policy.go index 8d01ace9864..0500ea79a14 100644 --- a/aws/data_source_aws_s3_bucket_policy.go +++ b/aws/data_source_aws_s3_bucket_policy.go @@ -2,10 +2,11 @@ package aws import ( "fmt" + "log" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "log" ) func dataSourceAwsS3BucketPolicy() *schema.Resource { diff --git a/aws/data_source_aws_s3_bucket_policy_test.go b/aws/data_source_aws_s3_bucket_policy_test.go index 34058cc724b..1fa755e554b 100644 --- a/aws/data_source_aws_s3_bucket_policy_test.go +++ b/aws/data_source_aws_s3_bucket_policy_test.go @@ -2,15 +2,15 @@ package aws import ( "fmt" + "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "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/terraform" awspolicy "github.com/jen20/awspolicyequivalence" - "testing" - - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" ) func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { From 12b2456042f71e2bb741b526a7ce83b5d191e774 Mon Sep 17 00:00:00 2001 From: yuonoda Date: Tue, 23 Feb 2021 10:58:28 +0900 Subject: [PATCH 08/23] made it prepare resources in test --- aws/data_source_aws_s3_bucket_policy.go | 12 ------------ aws/data_source_aws_s3_bucket_policy_test.go | 19 ++++++++++++------- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/aws/data_source_aws_s3_bucket_policy.go b/aws/data_source_aws_s3_bucket_policy.go index 0500ea79a14..a3fdb218cc8 100644 --- a/aws/data_source_aws_s3_bucket_policy.go +++ b/aws/data_source_aws_s3_bucket_policy.go @@ -30,28 +30,16 @@ func dataSourceAwsS3BucketPolicyRead(d *schema.ResourceData, meta interface{}) e conn := meta.(*AWSClient).s3conn bucketName := d.Get("bucket").(string) - - // fails to get policy without this part - _, err := conn.HeadBucket(&s3.HeadBucketInput{ - Bucket: aws.String(bucketName), - }) - if err != nil { - return fmt.Errorf("bucket not found: %s", err) - } - - fmt.Println("bucketName:", bucketName) input := &s3.GetBucketPolicyInput{ Bucket: aws.String(bucketName), } log.Printf("[DEBUG] Reading S3 bucket policy: %s", input) output, err := conn.GetBucketPolicy(input) - fmt.Printf("output:%v\n", output) if err != nil { return fmt.Errorf("failed getting S3 bucket policy (%s): %w", bucketName, err) } policy := *output.Policy - fmt.Println("policy:", policy) d.SetId(bucketName) d.Set("policy", policy) return nil diff --git a/aws/data_source_aws_s3_bucket_policy_test.go b/aws/data_source_aws_s3_bucket_policy_test.go index 1fa755e554b..54aae488c11 100644 --- a/aws/data_source_aws_s3_bucket_policy_test.go +++ b/aws/data_source_aws_s3_bucket_policy_test.go @@ -15,13 +15,14 @@ import ( func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { bucketName := acctest.RandomWithPrefix("tf-test-bucket") - //region := testAccGetRegion() - //hostedZoneID, _ := HostedZoneIDForRegion(region) - resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ + { + // prepare resources which wil be fetched with data source + Config: testAccAWSDataSourceS3BucketPolicyConfigResources(bucketName), + }, { Config: testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName), Check: resource.ComposeTestCheckFunc( @@ -83,7 +84,6 @@ func testAccCheckAWSS3BucketPolicyPolicyMatch(resource1, attr1, resource2, attr2 func testAccCheckAWSS3BucketPolicyExistsWithProvider(n string, providerF func() *schema.Provider) resource.TestCheckFunc { return func(s *terraform.State) error { - fmt.Println("s.RootModule().Resources:", s.RootModule().Resources) rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("not found: %s", n) @@ -111,7 +111,7 @@ func testAccCheckAWSS3BucketPolicyExistsWithProvider(n string, providerF func() } } -func testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName string) string { +func testAccAWSDataSourceS3BucketPolicyConfigResources(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "bucket" { bucket = "%s" @@ -145,10 +145,15 @@ data "aws_iam_policy_document" "policy" { } } } +`, bucketName) +} + +func testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName string) string { + return fmt.Sprintf(` +%s data "aws_s3_bucket_policy" "policy" { bucket = aws_s3_bucket.bucket.bucket } - -`, bucketName) +`, testAccAWSDataSourceS3BucketPolicyConfigResources(bucketName)) } From 353836d5104105e44d516c8212c7d8f19b1169c9 Mon Sep 17 00:00:00 2001 From: yuonoda Date: Tue, 23 Feb 2021 11:36:37 +0900 Subject: [PATCH 09/23] Added a document --- website/docs/d/s3_bucket_policy.html.markdown | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 website/docs/d/s3_bucket_policy.html.markdown diff --git a/website/docs/d/s3_bucket_policy.html.markdown b/website/docs/d/s3_bucket_policy.html.markdown new file mode 100644 index 00000000000..a6ed0b8ed02 --- /dev/null +++ b/website/docs/d/s3_bucket_policy.html.markdown @@ -0,0 +1,35 @@ +--- +subcategory: "S3" +layout: "aws" +page_title: "AWS: aws_s3_bucket_policy" +description: |- + Provides IAM policy of an S3 bucket +--- + +# Data Source: aws_s3_bucket_policy +The bucket-policy data source returns IAM policy of an S3 bucket. + +## Example Usage + +The following example retrieves IAM policy of a specified S3 bucket. + +```hcl +data "aws_s3_bucket_policy" "policy" { + bucket = "example-bucket-name" +} + +output "foo" { + value = data.aws_s3_bucket_policy.policy +} +``` + +## Argument Reference + +The following arguments are supported: + +* `bucket` - (Required) The name of the bucket to read the policy from. + +## Attributes Reference + +In addition to all arguments above, the following attributes are exported: +* `policy` - IAM policy attached to the S3 bucket From c5e25af553f4916674969a89ace066ccc2deb0e7 Mon Sep 17 00:00:00 2001 From: yuonoda Date: Tue, 23 Feb 2021 11:44:18 +0900 Subject: [PATCH 10/23] formated document --- website/docs/d/s3_bucket_policy.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/d/s3_bucket_policy.html.markdown b/website/docs/d/s3_bucket_policy.html.markdown index a6ed0b8ed02..f516452f3eb 100644 --- a/website/docs/d/s3_bucket_policy.html.markdown +++ b/website/docs/d/s3_bucket_policy.html.markdown @@ -32,4 +32,5 @@ The following arguments are supported: ## Attributes Reference In addition to all arguments above, the following attributes are exported: + * `policy` - IAM policy attached to the S3 bucket From d01639aaf2509c47c5188fef3057c31e67527b8b Mon Sep 17 00:00:00 2001 From: yuonoda Date: Tue, 23 Feb 2021 12:53:14 +0900 Subject: [PATCH 11/23] removed comments --- aws/data_source_aws_s3_bucket_policy_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/aws/data_source_aws_s3_bucket_policy_test.go b/aws/data_source_aws_s3_bucket_policy_test.go index 54aae488c11..a0559087a43 100644 --- a/aws/data_source_aws_s3_bucket_policy_test.go +++ b/aws/data_source_aws_s3_bucket_policy_test.go @@ -28,11 +28,6 @@ func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketPolicyExists("data.aws_s3_bucket_policy.policy"), testAccCheckAWSS3BucketPolicyPolicyMatch("data.aws_s3_bucket_policy.policy", "policy", "aws_s3_bucket_policy.bucket", "policy"), - //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "region", region), - //testAccCheckS3BucketDomainName("data.aws_s3_bucket.bucket", "bucket_domain_name", bucketName), - //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "bucket_regional_domain_name", testAccBucketRegionalDomainName(bucketName, region)), - //resource.TestCheckResourceAttr("data.aws_s3_bucket.bucket", "hosted_zone_id", hostedZoneID), - //resource.TestCheckNoResourceAttr("data.aws_s3_bucket.bucket", "website_endpoint"), ), }, }, From 11ad666434f0795bc769d80ea6b93a3522b80e9e Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 19 Apr 2022 15:21:03 -0500 Subject: [PATCH 12/23] d/aws_s3_bucket_policy: acctest updates --- .../s3/data_source_aws_s3_bucket_policy.go | 10 ++++++---- .../data_source_aws_s3_bucket_policy_test.go | 19 +++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/internal/service/s3/data_source_aws_s3_bucket_policy.go b/internal/service/s3/data_source_aws_s3_bucket_policy.go index f5fd53f1784..80d20cec595 100644 --- a/internal/service/s3/data_source_aws_s3_bucket_policy.go +++ b/internal/service/s3/data_source_aws_s3_bucket_policy.go @@ -1,18 +1,19 @@ package s3 import ( - "fmt" + "context" "log" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" ) func DataSourceAwsS3BucketPolicy() *schema.Resource { return &schema.Resource{ - Read: dataSourceAwsS3BucketPolicyRead, + ReadWithoutTimeout: dataSourceAwsS3BucketPolicyRead, Schema: map[string]*schema.Schema{ "bucket": { @@ -27,7 +28,7 @@ func DataSourceAwsS3BucketPolicy() *schema.Resource { } } -func dataSourceAwsS3BucketPolicyRead(d *schema.ResourceData, meta interface{}) error { +func dataSourceAwsS3BucketPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn bucketName := d.Get("bucket").(string) @@ -37,11 +38,12 @@ func dataSourceAwsS3BucketPolicyRead(d *schema.ResourceData, meta interface{}) e log.Printf("[DEBUG] Reading S3 bucket policy: %s", input) output, err := conn.GetBucketPolicy(input) if err != nil { - return fmt.Errorf("failed getting S3 bucket policy (%s): %w", bucketName, err) + return diag.Errorf("failed getting S3 bucket policy (%s): %w", bucketName, err) } policy := *output.Policy d.SetId(bucketName) d.Set("policy", policy) + return nil } diff --git a/internal/service/s3/data_source_aws_s3_bucket_policy_test.go b/internal/service/s3/data_source_aws_s3_bucket_policy_test.go index 8cc6edc6a1b..625a679e05a 100644 --- a/internal/service/s3/data_source_aws_s3_bucket_policy_test.go +++ b/internal/service/s3/data_source_aws_s3_bucket_policy_test.go @@ -6,18 +6,20 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + awspolicy "github.com/hashicorp/awspolicyequivalence" + sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "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/terraform" - awspolicy "github.com/jen20/awspolicyequivalence" + "github.com/hashicorp/terraform-provider-aws/internal/acctest" + "github.com/hashicorp/terraform-provider-aws/internal/conns" ) func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { - bucketName := acctest.RandomWithPrefix("tf-test-bucket") + bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, + PreCheck: func() { acctest.PreCheck(t) }, + Providers: acctest.Providers, Steps: []resource.TestStep{ { // prepare resources which wil be fetched with data source @@ -35,7 +37,7 @@ func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { } func testAccCheckAWSS3BucketPolicyExists(n string) resource.TestCheckFunc { - return testAccCheckAWSS3BucketPolicyExistsWithProvider(n, func() *schema.Provider { return testAccProvider }) + return testAccCheckAWSS3BucketPolicyExistsWithProvider(n, func() *schema.Provider { return acctest.Provider }) } func testAccCheckAWSS3BucketPolicyPolicyMatch(resource1, attr1, resource2, attr2 string) resource.TestCheckFunc { @@ -90,15 +92,12 @@ func testAccCheckAWSS3BucketPolicyExistsWithProvider(n string, providerF func() provider := providerF() - conn := provider.Meta().(*AWSClient).s3conn + conn := provider.Meta().(*conns.AWSClient).S3Conn _, err := conn.GetBucketPolicy(&s3.GetBucketPolicyInput{ Bucket: aws.String(rs.Primary.ID), }) if err != nil { - if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") { - return fmt.Errorf("s3 bucket not found") - } return err } return nil From 04cbf3e3171edbe0d811cab2e811f5a6da915f77 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Tue, 19 Apr 2022 15:23:00 -0500 Subject: [PATCH 13/23] rename files --- ...ource_aws_s3_bucket_policy.go => bucket_policy_data_source.go} | 0 ...s3_bucket_policy_test.go => bucket_policy_data_source_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename internal/service/s3/{data_source_aws_s3_bucket_policy.go => bucket_policy_data_source.go} (100%) rename internal/service/s3/{data_source_aws_s3_bucket_policy_test.go => bucket_policy_data_source_test.go} (100%) diff --git a/internal/service/s3/data_source_aws_s3_bucket_policy.go b/internal/service/s3/bucket_policy_data_source.go similarity index 100% rename from internal/service/s3/data_source_aws_s3_bucket_policy.go rename to internal/service/s3/bucket_policy_data_source.go diff --git a/internal/service/s3/data_source_aws_s3_bucket_policy_test.go b/internal/service/s3/bucket_policy_data_source_test.go similarity index 100% rename from internal/service/s3/data_source_aws_s3_bucket_policy_test.go rename to internal/service/s3/bucket_policy_data_source_test.go From ef9950f4f02d017c17c0c7222de5bdcf7a057341 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 12:50:19 -0500 Subject: [PATCH 14/23] d/aws_s3_bucket_policy: handle errors in find --- internal/provider/provider.go | 2 +- .../service/s3/bucket_policy_data_source.go | 53 +++++++++++++++---- .../s3/bucket_policy_data_source_test.go | 2 +- 3 files changed, 44 insertions(+), 13 deletions(-) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 75019171bc3..efdd8dab1f1 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -801,7 +801,7 @@ func Provider() *schema.Provider { "aws_s3_objects": s3.DataSourceObjects(), "aws_s3_bucket_object": s3.DataSourceBucketObject(), // DEPRECATED: use aws_s3_object instead "aws_s3_bucket_objects": s3.DataSourceBucketObjects(), // DEPRECATED: use aws_s3_objects instead - "aws_s3_bucket_policy": s3.DataSourceAwsS3BucketPolicy(), + "aws_s3_bucket_policy": s3.DataSourceBucketPolicy(), "aws_sagemaker_prebuilt_ecr_image": sagemaker.DataSourcePrebuiltECRImage(), diff --git a/internal/service/s3/bucket_policy_data_source.go b/internal/service/s3/bucket_policy_data_source.go index 80d20cec595..888fc033887 100644 --- a/internal/service/s3/bucket_policy_data_source.go +++ b/internal/service/s3/bucket_policy_data_source.go @@ -6,14 +6,18 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "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/structure" "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -func DataSourceAwsS3BucketPolicy() *schema.Resource { +func DataSourceBucketPolicy() *schema.Resource { return &schema.Resource{ - ReadWithoutTimeout: dataSourceAwsS3BucketPolicyRead, + ReadWithoutTimeout: dataSourceBucketPolicyRead, Schema: map[string]*schema.Schema{ "bucket": { @@ -28,22 +32,49 @@ func DataSourceAwsS3BucketPolicy() *schema.Resource { } } -func dataSourceAwsS3BucketPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { +func dataSourceBucketPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn - bucketName := d.Get("bucket").(string) - input := &s3.GetBucketPolicyInput{ - Bucket: aws.String(bucketName), + name := d.Get("bucket").(string) + + out, err := FindBucketPolicy(ctx, conn, name) + if err != nil { + return diag.Errorf("failed getting S3 bucket policy (%s): %s", name, err) } - log.Printf("[DEBUG] Reading S3 bucket policy: %s", input) - output, err := conn.GetBucketPolicy(input) + + policy, err := structure.NormalizeJsonString(aws.StringValue(out.Policy)) if err != nil { - return diag.Errorf("failed getting S3 bucket policy (%s): %w", bucketName, err) + return diag.Errorf("policy (%s) is an invalid JSON: %s", policy, err) } - policy := *output.Policy - d.SetId(bucketName) + d.SetId(name) d.Set("policy", policy) return nil } + +func FindBucketPolicy(ctx context.Context, conn *s3.S3, name string) (*s3.GetBucketPolicyOutput, error) { + in := &s3.GetBucketPolicyInput{ + Bucket: aws.String(name), + } + log.Printf("[DEBUG] Reading S3 bucket policy: %s", in) + + out, err := conn.GetBucketPolicyWithContext(ctx, in) + + if tfawserr.ErrCodeEquals(err, ErrCodeNoSuchBucketPolicy) { + return nil, &resource.NotFoundError{ + LastError: err, + LastRequest: in, + } + } + + if err != nil { + return nil, err + } + + if out == nil { + return nil, tfresource.NewEmptyResultError(in) + } + + return out, nil +} diff --git a/internal/service/s3/bucket_policy_data_source_test.go b/internal/service/s3/bucket_policy_data_source_test.go index 625a679e05a..72a838bd5fb 100644 --- a/internal/service/s3/bucket_policy_data_source_test.go +++ b/internal/service/s3/bucket_policy_data_source_test.go @@ -1,4 +1,4 @@ -package s3 +package s3_test import ( "fmt" From 80e1f6b4dbc22fb3efc305943b6184b8663c7e38 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 14:20:35 -0500 Subject: [PATCH 15/23] d/aws_s3_bucket_policy: acctest errcheck --- internal/service/s3/bucket_policy_data_source_test.go | 5 +++-- website/docs/d/s3_bucket_policy.html.markdown | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/service/s3/bucket_policy_data_source_test.go b/internal/service/s3/bucket_policy_data_source_test.go index 72a838bd5fb..674837d0f5a 100644 --- a/internal/service/s3/bucket_policy_data_source_test.go +++ b/internal/service/s3/bucket_policy_data_source_test.go @@ -18,8 +18,9 @@ import ( func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - Providers: acctest.Providers, + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + Providers: acctest.Providers, Steps: []resource.TestStep{ { // prepare resources which wil be fetched with data source diff --git a/website/docs/d/s3_bucket_policy.html.markdown b/website/docs/d/s3_bucket_policy.html.markdown index f516452f3eb..e8fd567c72b 100644 --- a/website/docs/d/s3_bucket_policy.html.markdown +++ b/website/docs/d/s3_bucket_policy.html.markdown @@ -7,13 +7,14 @@ description: |- --- # Data Source: aws_s3_bucket_policy + The bucket-policy data source returns IAM policy of an S3 bucket. ## Example Usage The following example retrieves IAM policy of a specified S3 bucket. -```hcl +```terraform data "aws_s3_bucket_policy" "policy" { bucket = "example-bucket-name" } From 29f9d0a90fa58576bf10538506efca991e986043 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 14:49:45 -0500 Subject: [PATCH 16/23] docs: fix docs --- website/docs/d/s3_bucket_policy.html.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/d/s3_bucket_policy.html.markdown b/website/docs/d/s3_bucket_policy.html.markdown index e8fd567c72b..a0f5815d967 100644 --- a/website/docs/d/s3_bucket_policy.html.markdown +++ b/website/docs/d/s3_bucket_policy.html.markdown @@ -1,5 +1,5 @@ --- -subcategory: "S3" +subcategory: "S3 (Simple Storage)" layout: "aws" page_title: "AWS: aws_s3_bucket_policy" description: |- @@ -28,10 +28,10 @@ output "foo" { The following arguments are supported: -* `bucket` - (Required) The name of the bucket to read the policy from. +* `bucket` - (Required) The bucket name. ## Attributes Reference In addition to all arguments above, the following attributes are exported: -* `policy` - IAM policy attached to the S3 bucket +* `policy` - IAM bucket policy. From 7ad1383136e1d185d6529cfcc7234d53f9ff717c Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 16:26:28 -0500 Subject: [PATCH 17/23] docs: fix docs --- website/docs/r/kms_replica_external_key.html.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/kms_replica_external_key.html.markdown b/website/docs/r/kms_replica_external_key.html.markdown index dac329bf6cf..0fd274c3246 100644 --- a/website/docs/r/kms_replica_external_key.html.markdown +++ b/website/docs/r/kms_replica_external_key.html.markdown @@ -59,7 +59,7 @@ If you specify a value, it must be between `7` and `30`, inclusive. If you do no * `policy` - (Optional) The key policy to attach to the KMS key. If you do not specify a key policy, AWS KMS attaches the [default key policy](https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default) to the KMS key. For more information about building policy documents with Terraform, see the [AWS IAM Policy Document Guide](https://learn.hashicorp.com/terraform/aws/iam-policy). * `primary_key_arn` - (Required) The ARN of the multi-Region primary key to replicate. The primary key must be in a different AWS Region of the same AWS Partition. You can create only one replica of a given primary key in each AWS Region. -* `tags` - (Optional) A map of tags to assign to the replica key. If configured with a provider [`default_tags` configuration block](https://www.terraform.io/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. +* `tags` - (Optional) A map of tags to assign to the replica key. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. * `valid_to` - (Optional) Time at which the imported key material expires. When the key material expires, AWS KMS deletes the key material and the key becomes unusable. If not specified, key material does not expire. Valid values: [RFC3339 time string](https://tools.ietf.org/html/rfc3339#section-5.8) (`YYYY-MM-DDTHH:MM:SSZ`) ## Attributes Reference @@ -71,7 +71,7 @@ In addition to all arguments above, the following attributes are exported: * `key_id` - The key ID of the replica key. Related multi-Region keys have the same key ID. * `key_state` - The state of the replica key. * `key_usage` - The [cryptographic operations](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#cryptographic-operations) for which you can use the KMS key. This is a shared property of multi-Region keys. -* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://www.terraform.io/docs/providers/aws/index.html#default_tags-configuration-block). +* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). ## Import From bb19d243617d173dcc7323a7afbcb569697234de Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 16:33:17 -0500 Subject: [PATCH 18/23] docs: fix docs --- website/docs/r/kms_replica_external_key.html.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/kms_replica_external_key.html.markdown b/website/docs/r/kms_replica_external_key.html.markdown index 0fd274c3246..dac329bf6cf 100644 --- a/website/docs/r/kms_replica_external_key.html.markdown +++ b/website/docs/r/kms_replica_external_key.html.markdown @@ -59,7 +59,7 @@ If you specify a value, it must be between `7` and `30`, inclusive. If you do no * `policy` - (Optional) The key policy to attach to the KMS key. If you do not specify a key policy, AWS KMS attaches the [default key policy](https://docs.aws.amazon.com/kms/latest/developerguide/key-policies.html#key-policy-default) to the KMS key. For more information about building policy documents with Terraform, see the [AWS IAM Policy Document Guide](https://learn.hashicorp.com/terraform/aws/iam-policy). * `primary_key_arn` - (Required) The ARN of the multi-Region primary key to replicate. The primary key must be in a different AWS Region of the same AWS Partition. You can create only one replica of a given primary key in each AWS Region. -* `tags` - (Optional) A map of tags to assign to the replica key. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. +* `tags` - (Optional) A map of tags to assign to the replica key. If configured with a provider [`default_tags` configuration block](https://www.terraform.io/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. * `valid_to` - (Optional) Time at which the imported key material expires. When the key material expires, AWS KMS deletes the key material and the key becomes unusable. If not specified, key material does not expire. Valid values: [RFC3339 time string](https://tools.ietf.org/html/rfc3339#section-5.8) (`YYYY-MM-DDTHH:MM:SSZ`) ## Attributes Reference @@ -71,7 +71,7 @@ In addition to all arguments above, the following attributes are exported: * `key_id` - The key ID of the replica key. Related multi-Region keys have the same key ID. * `key_state` - The state of the replica key. * `key_usage` - The [cryptographic operations](https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#cryptographic-operations) for which you can use the KMS key. This is a shared property of multi-Region keys. -* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). +* `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://www.terraform.io/docs/providers/aws/index.html#default_tags-configuration-block). ## Import From 1bec9f2a76fd5ab5f860fb12eed2e50af427f636 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 17:44:35 -0500 Subject: [PATCH 19/23] d/aws_s3_bucket_policy: acctest updates --- .../s3/bucket_policy_data_source_test.go | 102 +++++++++--------- 1 file changed, 48 insertions(+), 54 deletions(-) diff --git a/internal/service/s3/bucket_policy_data_source_test.go b/internal/service/s3/bucket_policy_data_source_test.go index 674837d0f5a..b8312d666b6 100644 --- a/internal/service/s3/bucket_policy_data_source_test.go +++ b/internal/service/s3/bucket_policy_data_source_test.go @@ -1,86 +1,81 @@ package s3_test import ( + "context" "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" awspolicy "github.com/hashicorp/awspolicyequivalence" sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "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/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfs3 "github.com/hashicorp/terraform-provider-aws/internal/service/s3" ) -func TestAccDataSourceS3BucketPolicy_basic(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") +func TestAccDataSourceBucketPolicy_basic(t *testing.T) { + var conf s3.GetBucketPolicyOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + dataSourceName := "data.aws_s3_bucket_policy.test" + resourceName := "aws_s3_bucket_policy.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), Providers: acctest.Providers, Steps: []resource.TestStep{ { - // prepare resources which wil be fetched with data source - Config: testAccAWSDataSourceS3BucketPolicyConfigResources(bucketName), - }, - { - Config: testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSS3BucketPolicyExists("data.aws_s3_bucket_policy.policy"), - testAccCheckAWSS3BucketPolicyPolicyMatch("data.aws_s3_bucket_policy.policy", "policy", "aws_s3_bucket_policy.bucket", "policy"), + Config: testAccDataSourceBucketPolicyConfigBasicConfig(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketPolicyExists(resourceName, &conf), + testAccCheckBucketPolicyMatch(dataSourceName, "policy", resourceName, "policy"), ), }, }, }) } -func testAccCheckAWSS3BucketPolicyExists(n string) resource.TestCheckFunc { - return testAccCheckAWSS3BucketPolicyExistsWithProvider(n, func() *schema.Provider { return acctest.Provider }) -} - -func testAccCheckAWSS3BucketPolicyPolicyMatch(resource1, attr1, resource2, attr2 string) resource.TestCheckFunc { +func testAccCheckBucketPolicyMatch(resource1, attr1, resource2, attr2 string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resource1] if !ok { - return fmt.Errorf("Not found: %s", resource1) + return fmt.Errorf("not found: %s", resource1) } if rs.Primary.ID == "" { - return fmt.Errorf("No ID is set") + return fmt.Errorf("no ID is set") } policy1, ok := rs.Primary.Attributes[attr1] if !ok { - return fmt.Errorf("Attribute %q not found for %q", attr1, resource1) + return fmt.Errorf("attribute %q not found for %q", attr1, resource1) } rs, ok = s.RootModule().Resources[resource2] if !ok { - return fmt.Errorf("Not found: %s", resource2) + return fmt.Errorf("not found: %s", resource2) } if rs.Primary.ID == "" { - return fmt.Errorf("No ID is set") + return fmt.Errorf("mo ID is set") } policy2, ok := rs.Primary.Attributes[attr2] if !ok { - return fmt.Errorf("Attribute %q not found for %q", attr2, resource2) + return fmt.Errorf("attribute %q not found for %q", attr2, resource2) } areEquivalent, err := awspolicy.PoliciesAreEquivalent(policy1, policy2) if err != nil { - return fmt.Errorf("Comparing AWS Policies failed: %s", err) + return fmt.Errorf("comparing IAM Policies failed: %s", err) } if !areEquivalent { - return fmt.Errorf("AWS policies differ.\npolicy1: %s\npolicy2: %s", policy1, policy2) + return fmt.Errorf("S3 bucket policies differ.\npolicy1: %s\npolicy2: %s", policy1, policy2) } return nil } } -func testAccCheckAWSS3BucketPolicyExistsWithProvider(n string, providerF func() *schema.Provider) resource.TestCheckFunc { +func testAccCheckBucketPolicyExists(n string, ci *s3.GetBucketPolicyOutput) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -88,67 +83,66 @@ func testAccCheckAWSS3BucketPolicyExistsWithProvider(n string, providerF func() } if rs.Primary.ID == "" { - return fmt.Errorf("no ID is set") + return fmt.Errorf("no S3 Bucket Policy ID is set") } - provider := providerF() - - conn := provider.Meta().(*conns.AWSClient).S3Conn - _, err := conn.GetBucketPolicy(&s3.GetBucketPolicyInput{ - Bucket: aws.String(rs.Primary.ID), - }) + conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn + output, err := tfs3.FindBucketPolicy(context.Background(), conn, rs.Primary.ID) if err != nil { return err } - return nil + *ci = *output + + return nil } } -func testAccAWSDataSourceS3BucketPolicyConfigResources(bucketName string) string { +func testAccDataSourceBucketPolicyBaseConfig(rName string) string { return fmt.Sprintf(` -resource "aws_s3_bucket" "bucket" { - bucket = "%s" +resource "aws_s3_bucket" "test" { + bucket = %[1]q tags = { - TestName = "TestAccAWSS3BucketPolicy_basic" + Name = %[1]q } } -resource "aws_s3_bucket_policy" "bucket" { - bucket = aws_s3_bucket.bucket.bucket - policy = data.aws_iam_policy_document.policy.json +resource "aws_s3_bucket_policy" "test" { + bucket = aws_s3_bucket.test.id + policy = data.aws_iam_policy_document.test.json } -data "aws_iam_policy_document" "policy" { +data "aws_iam_policy_document" "test" { statement { effect = "Allow" actions = [ - "s3:*", + "s3:GetObject", + "s3:ListBucket", ] resources = [ - aws_s3_bucket.bucket.arn, - "${aws_s3_bucket.bucket.arn}/*", + aws_s3_bucket.test.arn, + "${aws_s3_bucket.test.arn}/*", ] principals { - type = "AWS" - identifiers = ["*"] + type = "Service" + identifiers = ["lambda.amazonaws.com"] } } } -`, bucketName) +`, rName) } -func testAccAWSDataSourceS3BucketPolicyConfig_basic(bucketName string) string { - return fmt.Sprintf(` -%s +func testAccDataSourceBucketPolicyConfigBasicConfig(rName string) string { + return acctest.ConfigCompose(testAccDataSourceBucketPolicyBaseConfig(rName), fmt.Sprintf(` +data "aws_s3_bucket_policy" "test" { + bucket = aws_s3_bucket.test.id -data "aws_s3_bucket_policy" "policy" { - bucket = aws_s3_bucket.bucket.bucket +depends_on = [aws_s3_bucket_policy.test] } -`, testAccAWSDataSourceS3BucketPolicyConfigResources(bucketName)) +`)) } From ad797839456cd74e837fde4d0769f1057c16ed13 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 17:49:05 -0500 Subject: [PATCH 20/23] update CHANGELOG for #17738 --- .changelog/17738.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/17738.txt diff --git a/.changelog/17738.txt b/.changelog/17738.txt new file mode 100644 index 00000000000..9f5f6dfd41b --- /dev/null +++ b/.changelog/17738.txt @@ -0,0 +1,3 @@ +```release-note:new-data-source + aws_s3_bucket_policy + ``` \ No newline at end of file From b6dce67fb4ee34194daa2da94f39ee5dd1762d9b Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 17:51:23 -0500 Subject: [PATCH 21/23] d/aws_s3_bucket_policy: fmt --- internal/service/s3/bucket_policy_data_source_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/s3/bucket_policy_data_source_test.go b/internal/service/s3/bucket_policy_data_source_test.go index b8312d666b6..b34ac67b18c 100644 --- a/internal/service/s3/bucket_policy_data_source_test.go +++ b/internal/service/s3/bucket_policy_data_source_test.go @@ -142,7 +142,7 @@ func testAccDataSourceBucketPolicyConfigBasicConfig(rName string) string { data "aws_s3_bucket_policy" "test" { bucket = aws_s3_bucket.test.id -depends_on = [aws_s3_bucket_policy.test] + depends_on = [aws_s3_bucket_policy.test] } `)) } From 89143a465e64033a889f802215bb19bb09b74b1d Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 17:55:34 -0500 Subject: [PATCH 22/23] docs: minor updates --- website/docs/d/s3_bucket_policy.html.markdown | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/website/docs/d/s3_bucket_policy.html.markdown b/website/docs/d/s3_bucket_policy.html.markdown index a0f5815d967..6a298d59cba 100644 --- a/website/docs/d/s3_bucket_policy.html.markdown +++ b/website/docs/d/s3_bucket_policy.html.markdown @@ -8,19 +8,19 @@ description: |- # Data Source: aws_s3_bucket_policy -The bucket-policy data source returns IAM policy of an S3 bucket. +The bucket policy data source returns IAM policy of an S3 bucket. ## Example Usage The following example retrieves IAM policy of a specified S3 bucket. ```terraform -data "aws_s3_bucket_policy" "policy" { +data "aws_s3_bucket_policy" "example" { bucket = "example-bucket-name" } output "foo" { - value = data.aws_s3_bucket_policy.policy + value = data.aws_s3_bucket_policy.example.policy } ``` From 4d7b9a404b24bc766ce2f687163c41b8c45635b2 Mon Sep 17 00:00:00 2001 From: Adrian Johnson Date: Wed, 20 Apr 2022 18:22:40 -0500 Subject: [PATCH 23/23] d/aws_s3_bucket_policy: fmt --- internal/service/s3/bucket_policy_data_source_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/s3/bucket_policy_data_source_test.go b/internal/service/s3/bucket_policy_data_source_test.go index b34ac67b18c..6deb68dc9cf 100644 --- a/internal/service/s3/bucket_policy_data_source_test.go +++ b/internal/service/s3/bucket_policy_data_source_test.go @@ -138,11 +138,11 @@ data "aws_iam_policy_document" "test" { } func testAccDataSourceBucketPolicyConfigBasicConfig(rName string) string { - return acctest.ConfigCompose(testAccDataSourceBucketPolicyBaseConfig(rName), fmt.Sprintf(` + return acctest.ConfigCompose(testAccDataSourceBucketPolicyBaseConfig(rName), ` data "aws_s3_bucket_policy" "test" { bucket = aws_s3_bucket.test.id depends_on = [aws_s3_bucket_policy.test] } -`)) +`) }