Skip to content

Commit

Permalink
Merge pull request #32989 from bakeemawaytoys/b-aws_s3_bucket_logging…
Browse files Browse the repository at this point in the history 
…-fix-expected_bucket_owner-behavior

Fix perpetual diff on aws_s3_bucket_logging resource when expected_bucket_owner contains a value
  • Loading branch information
@ewbankkit
ewbankkit authored Aug 14, 2023
2 parents a6ced2d + f5482ec commit b7f1b0e
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 294 deletions.
3 changes: 3 additions & 0 deletions .changelog/32989.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_s3_bucket_logging: Fix perpetual drift when `expected_bucket_owner` is configured
```
141 changes: 59 additions & 82 deletions internal/service/s3/bucket_logging.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package s3

import (
"context"
"errors"
"log"
"time"

Expand All @@ -29,6 +28,7 @@ func ResourceBucketLogging() *schema.Resource {
ReadWithoutTimeout: resourceBucketLoggingRead,
UpdateWithoutTimeout: resourceBucketLoggingUpdate,
DeleteWithoutTimeout: resourceBucketLoggingDelete,

Importer: &schema.ResourceImporter{
StateContext: schema.ImportStatePassthroughContext,
},
Expand Down Expand Up @@ -107,23 +107,20 @@ func resourceBucketLoggingCreate(ctx context.Context, d *schema.ResourceData, me

bucket := d.Get("bucket").(string)
expectedBucketOwner := d.Get("expected_bucket_owner").(string)

loggingEnabled := &s3.LoggingEnabled{
TargetBucket: aws.String(d.Get("target_bucket").(string)),
TargetPrefix: aws.String(d.Get("target_prefix").(string)),
}

if v, ok := d.GetOk("target_grant"); ok && v.(*schema.Set).Len() > 0 {
loggingEnabled.TargetGrants = expandBucketLoggingTargetGrants(v.(*schema.Set).List())
}

input := &s3.PutBucketLoggingInput{
Bucket: aws.String(bucket),
BucketLoggingStatus: &s3.BucketLoggingStatus{
LoggingEnabled: loggingEnabled,
LoggingEnabled: &s3.LoggingEnabled{
TargetBucket: aws.String(d.Get("target_bucket").(string)),
TargetPrefix: aws.String(d.Get("target_prefix").(string)),
},
},
}

if v, ok := d.GetOk("target_grant"); ok && v.(*schema.Set).Len() > 0 {
input.BucketLoggingStatus.LoggingEnabled.TargetGrants = expandBucketLoggingTargetGrants(v.(*schema.Set).List())
}

if expectedBucketOwner != "" {
input.ExpectedBucketOwner = aws.String(expectedBucketOwner)
}
Expand All @@ -138,6 +135,14 @@ func resourceBucketLoggingCreate(ctx context.Context, d *schema.ResourceData, me

d.SetId(CreateResourceID(bucket, expectedBucketOwner))

_, err = tfresource.RetryWhenNotFound(ctx, propagationTimeout, func() (interface{}, error) {
return FindBucketLogging(ctx, conn, bucket, expectedBucketOwner)
})

if err != nil {
return diag.Errorf("waiting for S3 Bucket Logging (%s) create: %s", d.Id(), err)
}

return resourceBucketLoggingRead(ctx, d, meta)
}

Expand All @@ -150,20 +155,7 @@ func resourceBucketLoggingRead(ctx context.Context, d *schema.ResourceData, meta
return sdkdiag.AppendFromErr(diags, err)
}

outputRaw, err := tfresource.RetryWhen(ctx, 2*time.Minute, func() (interface{}, error) {
return FindBucketLoggingByID(ctx, conn, bucket, expectedBucketOwner)
},
func(err error) (bool, error) {
if tfresource.NotFound(err) {
return true, err
}

if errors.Is(err, tfresource.ErrEmptyResult) {
return true, err
}

return false, err
})
loggingEnabled, err := FindBucketLogging(ctx, conn, bucket, expectedBucketOwner)

if !d.IsNewResource() && tfresource.NotFound(err) {
log.Printf("[WARN] S3 Bucket Logging (%s) not found, removing from state", d.Id())
Expand All @@ -172,28 +164,16 @@ func resourceBucketLoggingRead(ctx context.Context, d *schema.ResourceData, meta
}

if err != nil {
return sdkdiag.AppendErrorf(diags, "reading S3 Bucket Logging for bucket (%s): %s", d.Id(), err)
}

if errors.Is(err, tfresource.ErrEmptyResult) {
if d.IsNewResource() {
return sdkdiag.AppendErrorf(diags, "reading S3 Bucket (%s) Logging: empty output", d.Id())
}
log.Printf("[WARN] S3 Bucket Logging (%s) not found, removing from state", d.Id())
d.SetId("")
return diags
return sdkdiag.AppendErrorf(diags, "reading S3 Bucket Logging (%s): %s", d.Id(), err)
}

loggingEnabled := outputRaw.(*s3.GetBucketLoggingOutput).LoggingEnabled

d.Set("bucket", d.Id())
d.Set("bucket", bucket)
d.Set("expected_bucket_owner", expectedBucketOwner)
d.Set("target_bucket", loggingEnabled.TargetBucket)
d.Set("target_prefix", loggingEnabled.TargetPrefix)

if err := d.Set("target_grant", flattenBucketLoggingTargetGrants(loggingEnabled.TargetGrants)); err != nil {
return sdkdiag.AppendErrorf(diags, "setting target_grant: %s", err)
}
d.Set("target_prefix", loggingEnabled.TargetPrefix)

return diags
}
Expand All @@ -207,22 +187,20 @@ func resourceBucketLoggingUpdate(ctx context.Context, d *schema.ResourceData, me
return sdkdiag.AppendFromErr(diags, err)
}

loggingEnabled := &s3.LoggingEnabled{
TargetBucket: aws.String(d.Get("target_bucket").(string)),
TargetPrefix: aws.String(d.Get("target_prefix").(string)),
}

if v, ok := d.GetOk("target_grant"); ok && v.(*schema.Set).Len() > 0 {
loggingEnabled.TargetGrants = expandBucketLoggingTargetGrants(v.(*schema.Set).List())
}

input := &s3.PutBucketLoggingInput{
Bucket: aws.String(bucket),
BucketLoggingStatus: &s3.BucketLoggingStatus{
LoggingEnabled: loggingEnabled,
LoggingEnabled: &s3.LoggingEnabled{
TargetBucket: aws.String(d.Get("target_bucket").(string)),
TargetPrefix: aws.String(d.Get("target_prefix").(string)),
},
},
}

if v, ok := d.GetOk("target_grant"); ok && v.(*schema.Set).Len() > 0 {
input.BucketLoggingStatus.LoggingEnabled.TargetGrants = expandBucketLoggingTargetGrants(v.(*schema.Set).List())
}

if expectedBucketOwner != "" {
input.ExpectedBucketOwner = aws.String(expectedBucketOwner)
}
Expand All @@ -232,7 +210,7 @@ func resourceBucketLoggingUpdate(ctx context.Context, d *schema.ResourceData, me
}, s3.ErrCodeNoSuchBucket)

if err != nil {
return sdkdiag.AppendErrorf(diags, "updating S3 bucket (%s) logging: %s", d.Id(), err)
return sdkdiag.AppendErrorf(diags, "updating S3 Bucket Logging (%s): %s", d.Id(), err)
}

return resourceBucketLoggingRead(ctx, d, meta)
Expand Down Expand Up @@ -263,12 +241,40 @@ func resourceBucketLoggingDelete(ctx context.Context, d *schema.ResourceData, me
}

if err != nil {
return sdkdiag.AppendErrorf(diags, "deleting S3 Bucket (%s) Logging: %s", d.Id(), err)
return sdkdiag.AppendErrorf(diags, "deleting S3 Bucket Logging (%s): %s", d.Id(), err)
}

return nil
}

func FindBucketLogging(ctx context.Context, conn *s3.S3, bucketName, expectedBucketOwner string) (*s3.LoggingEnabled, error) {
input := &s3.GetBucketLoggingInput{
Bucket: aws.String(bucketName),
}
if expectedBucketOwner != "" {
input.ExpectedBucketOwner = aws.String(expectedBucketOwner)
}

output, err := conn.GetBucketLoggingWithContext(ctx, input)

if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) {
return nil, &retry.NotFoundError{
LastError: err,
LastRequest: input,
}
}

if err != nil {
return nil, err
}

if output == nil || output.LoggingEnabled == nil {
return nil, tfresource.NewEmptyResultError(input)
}

return output.LoggingEnabled, nil
}

func expandBucketLoggingTargetGrants(l []interface{}) []*s3.TargetGrant {
var grants []*s3.TargetGrant

Expand Down Expand Up @@ -382,32 +388,3 @@ func flattenBucketLoggingTargetGrantGrantee(g *s3.Grantee) []interface{} {

return []interface{}{m}
}

func FindBucketLoggingByID(ctx context.Context, conn *s3.S3, id, expectedBucketOwner string) (*s3.GetBucketLoggingOutput, error) {
in := &s3.GetBucketLoggingInput{
Bucket: aws.String(id),
}

if expectedBucketOwner != "" {
in.ExpectedBucketOwner = aws.String(expectedBucketOwner)
}

out, err := conn.GetBucketLoggingWithContext(ctx, in)

if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) {
return nil, &retry.NotFoundError{
LastError: err,
LastRequest: in,
}
}

if err != nil {
return nil, err
}

if out == nil || out.LoggingEnabled == nil {
return nil, tfresource.NewEmptyResultError(in)
}

return out, nil
}
Loading

0 comments on commit b7f1b0e

Please sign in to comment.