From c69f6424731b44c4f3abb0a82f79fe78e21d93f7 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 1 Aug 2019 13:00:31 -0400 Subject: [PATCH] Fix adjust issues --- aws/data_source_aws_s3_bucket_objects.go | 109 ++++++------------ aws/data_source_aws_s3_bucket_objects_test.go | 64 ++++++---- .../docs/d/s3_bucket_objects.html.markdown | 8 +- 3 files changed, 84 insertions(+), 97 deletions(-) diff --git a/aws/data_source_aws_s3_bucket_objects.go b/aws/data_source_aws_s3_bucket_objects.go index a3ef69ff050..e3fd777b072 100644 --- a/aws/data_source_aws_s3_bucket_objects.go +++ b/aws/data_source_aws_s3_bucket_objects.go @@ -2,11 +2,10 @@ package aws import ( "fmt" - "log" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" ) @@ -46,10 +45,6 @@ func dataSourceAwsS3BucketObjects() *schema.Resource { Type: schema.TypeBool, Optional: true, }, - "arn": { - Type: schema.TypeString, - Computed: true, - }, "keys": { Type: schema.TypeList, Computed: true, @@ -75,25 +70,7 @@ func dataSourceAwsS3BucketObjectsRead(d *schema.ResourceData, meta interface{}) bucket := d.Get("bucket").(string) prefix := d.Get("prefix").(string) - input := &s3.HeadBucketInput{ - Bucket: aws.String(bucket), - } - - log.Printf("[DEBUG] Reading S3 bucket: %s", input) - _, err := conn.HeadBucket(input) - - if err != nil { - return fmt.Errorf("Failed listing S3 bucket object keys: %s Bucket: %q", err, bucket) - } - - d.SetId(fmt.Sprintf("%s_%s", bucket, prefix)) - - arn := arn.ARN{ - Partition: meta.(*AWSClient).partition, - Service: "s3", - Resource: bucket, - }.String() - d.Set("arn", arn) + d.SetId(resource.UniqueId()) listInput := s3.ListObjectsV2Input{ Bucket: aws.String(bucket), @@ -114,14 +91,9 @@ func dataSourceAwsS3BucketObjectsRead(d *schema.ResourceData, meta interface{}) // "listInput.MaxKeys" refers to max keys returned in a single request // (i.e., page size), not the total number of keys returned if you page // through the results. "maxKeys" does refer to total keys returned. - maxKeys := -1 - if max, ok := d.GetOk("max_keys"); ok { - maxKeys = max.(int) - if maxKeys > keyRequestPageSize { - listInput.MaxKeys = aws.Int64(int64(keyRequestPageSize)) - } else { - listInput.MaxKeys = aws.Int64(int64(maxKeys)) - } + maxKeys := int64(d.Get("max_keys").(int)) + if maxKeys <= keyRequestPageSize { + listInput.MaxKeys = aws.Int64(maxKeys) } if s, ok := d.GetOk("start_after"); ok { @@ -132,52 +104,47 @@ func dataSourceAwsS3BucketObjectsRead(d *schema.ResourceData, meta interface{}) listInput.FetchOwner = aws.Bool(b.(bool)) } - keys, prefixes, owners, err := listS3Objects(conn, listInput, maxKeys) - if err != nil { - return err - } - d.Set("keys", keys) - d.Set("common_prefixes", prefixes) - d.Set("owners", owners) - - return nil -} + var commonPrefixes []string + var keys []string + var owners []string -func listS3Objects(conn *s3.S3, input s3.ListObjectsV2Input, maxKeys int) ([]string, []string, []string, error) { - var objectList []string - var commonPrefixList []string - var ownerList []string - var continueToken *string - for { - //page through keys - input.ContinuationToken = continueToken - - log.Printf("[DEBUG] Requesting page of S3 bucket (%s) object keys", *input.Bucket) - listOutput, err := conn.ListObjectsV2(&input) - if err != nil { - return nil, nil, nil, fmt.Errorf("Failed listing S3 bucket object keys: %s Bucket: %q", err, *input.Bucket) + err := conn.ListObjectsV2Pages(&listInput, func(page *s3.ListObjectsV2Output, lastPage bool) bool { + for _, commonPrefix := range page.CommonPrefixes { + commonPrefixes = append(commonPrefixes, aws.StringValue(commonPrefix.Prefix)) } - for _, content := range listOutput.Contents { - objectList = append(objectList, *content.Key) - if input.FetchOwner != nil && *input.FetchOwner { - ownerList = append(ownerList, *content.Owner.ID) - } - if maxKeys > -1 && len(objectList) >= maxKeys { - break + for _, object := range page.Contents { + keys = append(keys, aws.StringValue(object.Key)) + + if object.Owner != nil { + owners = append(owners, aws.StringValue(object.Owner.ID)) } } - for _, commonPrefix := range listOutput.CommonPrefixes { - commonPrefixList = append(commonPrefixList, *commonPrefix.Prefix) - } + maxKeys = maxKeys - aws.Int64Value(page.KeyCount) - // stop requesting if no more results OR all wanted keys done - if !*listOutput.IsTruncated || (maxKeys > -1 && len(objectList) >= maxKeys) { - break + if maxKeys <= keyRequestPageSize { + listInput.MaxKeys = aws.Int64(maxKeys) } - continueToken = listOutput.NextContinuationToken + + return !lastPage + }) + + if err != nil { + return fmt.Errorf("error listing S3 Bucket (%s) Objects: %s", bucket, err) } - return objectList, commonPrefixList, ownerList, nil + if err := d.Set("common_prefixes", commonPrefixes); err != nil { + return fmt.Errorf("error setting common_prefixes: %s", err) + } + + if err := d.Set("keys", keys); err != nil { + return fmt.Errorf("error setting keys: %s", err) + } + + if err := d.Set("owners", owners); err != nil { + return fmt.Errorf("error setting owners: %s", err) + } + + return nil } diff --git a/aws/data_source_aws_s3_bucket_objects_test.go b/aws/data_source_aws_s3_bucket_objects_test.go index 21b5fae8cf8..6f575c68714 100644 --- a/aws/data_source_aws_s3_bucket_objects_test.go +++ b/aws/data_source_aws_s3_bucket_objects_test.go @@ -11,7 +11,6 @@ import ( func TestAccDataSourceAWSS3BucketObjects_basic(t *testing.T) { rInt := acctest.RandInt() - basic := testAccAWSDataSourceS3ObjectsConfigBasic(rInt) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -19,14 +18,13 @@ func TestAccDataSourceAWSS3BucketObjects_basic(t *testing.T) { PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ { - Config: basic, + Config: testAccAWSDataSourceS3ObjectsConfigResources(rInt), // NOTE: contains no data source + // Does not need Check + }, + { + Config: testAccAWSDataSourceS3ObjectsConfigBasic(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsS3ObjectsDataSourceExists("data.aws_s3_bucket_objects.yesh"), - resource.TestCheckResourceAttr( - "data.aws_s3_bucket_objects.yesh", - "arn", - fmt.Sprintf("arn:aws:s3:::tf-objects-test-bucket-%d", rInt), - ), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.#", "2"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.0", "arch/navajo/north_window"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.1", "arch/navajo/sand_dune"), @@ -38,7 +36,6 @@ func TestAccDataSourceAWSS3BucketObjects_basic(t *testing.T) { func TestAccDataSourceAWSS3BucketObjects_all(t *testing.T) { rInt := acctest.RandInt() - basic := testAccAWSDataSourceS3ObjectsConfigAll(rInt) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -46,7 +43,11 @@ func TestAccDataSourceAWSS3BucketObjects_all(t *testing.T) { PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ { - Config: basic, + Config: testAccAWSDataSourceS3ObjectsConfigResources(rInt), // NOTE: contains no data source + // Does not need Check + }, + { + Config: testAccAWSDataSourceS3ObjectsConfigAll(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsS3ObjectsDataSourceExists("data.aws_s3_bucket_objects.yesh"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.#", "7"), @@ -65,7 +66,6 @@ func TestAccDataSourceAWSS3BucketObjects_all(t *testing.T) { func TestAccDataSourceAWSS3BucketObjects_prefixes(t *testing.T) { rInt := acctest.RandInt() - basic := testAccAWSDataSourceS3ObjectsConfigPrefixes(rInt) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -73,7 +73,11 @@ func TestAccDataSourceAWSS3BucketObjects_prefixes(t *testing.T) { PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ { - Config: basic, + Config: testAccAWSDataSourceS3ObjectsConfigResources(rInt), // NOTE: contains no data source + // Does not need Check + }, + { + Config: testAccAWSDataSourceS3ObjectsConfigPrefixes(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsS3ObjectsDataSourceExists("data.aws_s3_bucket_objects.yesh"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.#", "1"), @@ -91,7 +95,6 @@ func TestAccDataSourceAWSS3BucketObjects_prefixes(t *testing.T) { func TestAccDataSourceAWSS3BucketObjects_encoded(t *testing.T) { rInt := acctest.RandInt() - basic := testAccAWSDataSourceS3ObjectsConfigEncoded(rInt) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -99,7 +102,11 @@ func TestAccDataSourceAWSS3BucketObjects_encoded(t *testing.T) { PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ { - Config: basic, + Config: testAccAWSDataSourceS3ObjectsConfigExtraResource(rInt), // NOTE: contains no data source + // Does not need Check + }, + { + Config: testAccAWSDataSourceS3ObjectsConfigEncoded(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsS3ObjectsDataSourceExists("data.aws_s3_bucket_objects.yesh"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.#", "2"), @@ -113,7 +120,6 @@ func TestAccDataSourceAWSS3BucketObjects_encoded(t *testing.T) { func TestAccDataSourceAWSS3BucketObjects_maxKeys(t *testing.T) { rInt := acctest.RandInt() - basic := testAccAWSDataSourceS3ObjectsConfigMaxKeys(rInt) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -121,7 +127,11 @@ func TestAccDataSourceAWSS3BucketObjects_maxKeys(t *testing.T) { PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ { - Config: basic, + Config: testAccAWSDataSourceS3ObjectsConfigResources(rInt), // NOTE: contains no data source + // Does not need Check + }, + { + Config: testAccAWSDataSourceS3ObjectsConfigMaxKeys(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsS3ObjectsDataSourceExists("data.aws_s3_bucket_objects.yesh"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.#", "2"), @@ -135,7 +145,6 @@ func TestAccDataSourceAWSS3BucketObjects_maxKeys(t *testing.T) { func TestAccDataSourceAWSS3BucketObjects_startAfter(t *testing.T) { rInt := acctest.RandInt() - basic := testAccAWSDataSourceS3ObjectsConfigStartAfter(rInt) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -143,7 +152,11 @@ func TestAccDataSourceAWSS3BucketObjects_startAfter(t *testing.T) { PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ { - Config: basic, + Config: testAccAWSDataSourceS3ObjectsConfigResources(rInt), // NOTE: contains no data source + // Does not need Check + }, + { + Config: testAccAWSDataSourceS3ObjectsConfigStartAfter(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsS3ObjectsDataSourceExists("data.aws_s3_bucket_objects.yesh"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.#", "1"), @@ -156,7 +169,6 @@ func TestAccDataSourceAWSS3BucketObjects_startAfter(t *testing.T) { func TestAccDataSourceAWSS3BucketObjects_fetchOwner(t *testing.T) { rInt := acctest.RandInt() - basic := testAccAWSDataSourceS3ObjectsConfigOwners(rInt) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -164,7 +176,11 @@ func TestAccDataSourceAWSS3BucketObjects_fetchOwner(t *testing.T) { PreventPostDestroyRefresh: true, Steps: []resource.TestStep{ { - Config: basic, + Config: testAccAWSDataSourceS3ObjectsConfigResources(rInt), // NOTE: contains no data source + // Does not need Check + }, + { + Config: testAccAWSDataSourceS3ObjectsConfigOwners(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAwsS3ObjectsDataSourceExists("data.aws_s3_bucket_objects.yesh"), resource.TestCheckResourceAttr("data.aws_s3_bucket_objects.yesh", "keys.#", "2"), @@ -274,7 +290,7 @@ data "aws_s3_bucket_objects" "yesh" { `, testAccAWSDataSourceS3ObjectsConfigResources(randInt)) } -func testAccAWSDataSourceS3ObjectsConfigEncoded(randInt int) string { +func testAccAWSDataSourceS3ObjectsConfigExtraResource(randInt int) string { return fmt.Sprintf(` %s @@ -283,13 +299,19 @@ resource "aws_s3_bucket_object" "object8" { key = "arch/ru b ic on" content = "Goose Island" } +`, testAccAWSDataSourceS3ObjectsConfigResources(randInt)) +} + +func testAccAWSDataSourceS3ObjectsConfigEncoded(randInt int) string { + return fmt.Sprintf(` +%s data "aws_s3_bucket_objects" "yesh" { bucket = "${aws_s3_bucket.objects_bucket.id}" encoding_type = "url" prefix = "arch/ru" } -`, testAccAWSDataSourceS3ObjectsConfigResources(randInt)) +`, testAccAWSDataSourceS3ObjectsConfigExtraResource(randInt)) } func testAccAWSDataSourceS3ObjectsConfigMaxKeys(randInt int) string { diff --git a/website/docs/d/s3_bucket_objects.html.markdown b/website/docs/d/s3_bucket_objects.html.markdown index 5699e876733..cf11b0842d1 100644 --- a/website/docs/d/s3_bucket_objects.html.markdown +++ b/website/docs/d/s3_bucket_objects.html.markdown @@ -8,6 +8,8 @@ description: |- # Data Source: aws_s3_bucket_objects +~> **NOTE on `max_keys`:** Retrieving very large numbers of keys can adversely affect Terraform's performance. + The bucket-objects data source returns keys (i.e., file names) and other metadata about objects in an S3 bucket. ## Example Usage @@ -34,10 +36,7 @@ The following arguments are supported: * `prefix` - (Optional) Limits results to object keys with this prefix (Default: none) * `delimiter` - (Optional) A character used to group keys (Default: none) * `encoding_type` - (Optional) Encodes keys using this method (Default: none; besides none, only "url" can be used) -* `max_keys` - (Optional) Maximum object keys to return or `-1` to retrieve all keys (Default: 1000) - -~> **NOTE on `max_keys`:** Retrieving very large numbers of keys can adversely affect Terraform's performance. - +* `max_keys` - (Optional) Maximum object keys to return (Default: 1000) * `start_after` - (Optional) Returns key names lexicographically after a specific object key in your bucket (Default: none; S3 lists object keys in UTF-8 character encoding in lexicographical order) * `fetch_owner` - (Optional) Boolean specifying whether to populate the owner list (Default: false) @@ -45,7 +44,6 @@ The following arguments are supported: In addition to all arguments above, the following attributes are exported: -* `arn` - ARN of the bucket in the format `arn:aws:s3:::bucketname` * `keys` - List of strings representing object keys * `common_prefixes` - List of any keys between `prefix` and the next occurrence of `delimiter` (i.e., similar to subdirectories of the `prefix` "directory"); the list is only returned when you specify `delimiter` * `owners` - List of strings representing object owner IDs (see `fetch_owner` above)