From 7b6462512befdb8332465ae7969a4375d87a7ade Mon Sep 17 00:00:00 2001
From: Erik Kristensen <erik@erikkristensen.com>
Date: Mon, 23 Dec 2024 15:39:35 -0700
Subject: [PATCH 1/6] refactor(s3-bucket): query on region vs querying region
 separate

---
 resources/s3-bucket-helpers.go    | 172 ++++++++++++++++++++++
 resources/s3-bucket.go            | 237 ++++--------------------------
 resources/s3-bucket_test.go       |  29 ++--
 resources/s3-multipart-uploads.go |   3 +-
 resources/s3-objects.go           |   3 +-
 5 files changed, 221 insertions(+), 223 deletions(-)
 create mode 100644 resources/s3-bucket-helpers.go

diff --git a/resources/s3-bucket-helpers.go b/resources/s3-bucket-helpers.go
new file mode 100644
index 00000000..6e3b4fcf
--- /dev/null
+++ b/resources/s3-bucket-helpers.go
@@ -0,0 +1,172 @@
+package resources
+
+import (
+	"context"
+	"time"
+
+	"github.com/gotidy/ptr"
+	"github.com/sirupsen/logrus"
+
+	"github.com/aws/aws-sdk-go-v2/service/s3"
+	s3types "github.com/aws/aws-sdk-go-v2/service/s3/types"
+
+	"github.com/ekristen/aws-nuke/v3/pkg/awsmod"
+)
+
+func bypassGovernanceRetention(input *s3.DeleteObjectsInput) {
+	input.BypassGovernanceRetention = ptr.Bool(true)
+}
+
+type s3DeleteVersionListIterator struct {
+	Bucket                    *string
+	Paginator                 *s3.ListObjectVersionsPaginator
+	objects                   []s3types.ObjectVersion
+	lastNotify                time.Time
+	BypassGovernanceRetention *bool
+	err                       error
+}
+
+func newS3DeleteVersionListIterator(
+	svc *s3.Client,
+	input *s3.ListObjectVersionsInput,
+	bypass bool,
+	opts ...func(*s3DeleteVersionListIterator)) awsmod.BatchDeleteIterator {
+	iter := &s3DeleteVersionListIterator{
+		Bucket:                    input.Bucket,
+		Paginator:                 s3.NewListObjectVersionsPaginator(svc, input),
+		BypassGovernanceRetention: ptr.Bool(bypass),
+	}
+
+	for _, opt := range opts {
+		opt(iter)
+	}
+
+	return iter
+}
+
+// Next will use the S3API client to iterate through a list of objects.
+func (iter *s3DeleteVersionListIterator) Next() bool {
+	if len(iter.objects) > 0 {
+		iter.objects = iter.objects[1:]
+		if len(iter.objects) > 0 {
+			return true
+		}
+	}
+
+	if !iter.Paginator.HasMorePages() {
+		return false
+	}
+
+	page, err := iter.Paginator.NextPage(context.TODO())
+	if err != nil {
+		iter.err = err
+		return false
+	}
+
+	iter.objects = page.Versions
+	for _, entry := range page.DeleteMarkers {
+		iter.objects = append(iter.objects, s3types.ObjectVersion{
+			Key:       entry.Key,
+			VersionId: entry.VersionId,
+		})
+	}
+
+	if len(iter.objects) > 500 && (iter.lastNotify.IsZero() || time.Since(iter.lastNotify) > 120*time.Second) {
+		logrus.Infof(
+			"S3Bucket: %s - empty bucket operation in progress, this could take a while, please be patient",
+			*iter.Bucket)
+		iter.lastNotify = time.Now().UTC()
+	}
+
+	return len(iter.objects) > 0
+}
+
+// Err will return the last known error from Next.
+func (iter *s3DeleteVersionListIterator) Err() error {
+	return iter.err
+}
+
+// DeleteObject will return the current object to be deleted.
+func (iter *s3DeleteVersionListIterator) DeleteObject() awsmod.BatchDeleteObject {
+	return awsmod.BatchDeleteObject{
+		Object: &s3.DeleteObjectInput{
+			Bucket:                    iter.Bucket,
+			Key:                       iter.objects[0].Key,
+			VersionId:                 iter.objects[0].VersionId,
+			BypassGovernanceRetention: iter.BypassGovernanceRetention,
+		},
+	}
+}
+
+type s3ObjectDeleteListIterator struct {
+	Bucket                    *string
+	Paginator                 *s3.ListObjectsV2Paginator
+	objects                   []s3types.Object
+	lastNotify                time.Time
+	BypassGovernanceRetention bool
+	err                       error
+}
+
+func newS3ObjectDeleteListIterator(
+	svc *s3.Client,
+	input *s3.ListObjectsV2Input,
+	bypass bool,
+	opts ...func(*s3ObjectDeleteListIterator)) awsmod.BatchDeleteIterator {
+	iter := &s3ObjectDeleteListIterator{
+		Bucket:                    input.Bucket,
+		Paginator:                 s3.NewListObjectsV2Paginator(svc, input),
+		BypassGovernanceRetention: bypass,
+	}
+
+	for _, opt := range opts {
+		opt(iter)
+	}
+	return iter
+}
+
+// Next will use the S3API client to iterate through a list of objects.
+func (iter *s3ObjectDeleteListIterator) Next() bool {
+	if len(iter.objects) > 0 {
+		iter.objects = iter.objects[1:]
+		if len(iter.objects) > 0 {
+			return true
+		}
+	}
+
+	if !iter.Paginator.HasMorePages() {
+		return false
+	}
+
+	page, err := iter.Paginator.NextPage(context.TODO())
+	if err != nil {
+		iter.err = err
+		return false
+	}
+
+	iter.objects = page.Contents
+
+	if len(iter.objects) > 500 && (iter.lastNotify.IsZero() || time.Since(iter.lastNotify) > 120*time.Second) {
+		logrus.Infof(
+			"S3Bucket: %s - empty bucket operation in progress, this could take a while, please be patient",
+			*iter.Bucket)
+		iter.lastNotify = time.Now().UTC()
+	}
+
+	return len(iter.objects) > 0
+}
+
+// Err will return the last known error from Next.
+func (iter *s3ObjectDeleteListIterator) Err() error {
+	return iter.err
+}
+
+// DeleteObject will return the current object to be deleted.
+func (iter *s3ObjectDeleteListIterator) DeleteObject() awsmod.BatchDeleteObject {
+	return awsmod.BatchDeleteObject{
+		Object: &s3.DeleteObjectInput{
+			Bucket:                    iter.Bucket,
+			Key:                       iter.objects[0].Key,
+			BypassGovernanceRetention: ptr.Bool(iter.BypassGovernanceRetention),
+		},
+	}
+}
diff --git a/resources/s3-bucket.go b/resources/s3-bucket.go
index 96a99d1e..d264214e 100644
--- a/resources/s3-bucket.go
+++ b/resources/s3-bucket.go
@@ -9,7 +9,6 @@ import (
 	"github.com/gotidy/ptr"
 	"github.com/sirupsen/logrus"
 
-	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	s3types "github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/aws/smithy-go"
@@ -48,7 +47,7 @@ func (l *S3BucketLister) List(ctx context.Context, o interface{}) ([]resource.Re
 	opts := o.(*nuke.ListerOpts)
 	svc := s3.NewFromConfig(*opts.Config)
 
-	buckets, err := DescribeS3Buckets(ctx, svc)
+	buckets, err := DescribeS3Buckets(ctx, svc, opts)
 	if err != nil {
 		return nil, err
 	}
@@ -57,13 +56,13 @@ func (l *S3BucketLister) List(ctx context.Context, o interface{}) ([]resource.Re
 	for _, bucket := range buckets {
 		newBucket := &S3Bucket{
 			svc:          svc,
-			name:         aws.ToString(bucket.Name),
-			creationDate: aws.ToTime(bucket.CreationDate),
-			tags:         make([]s3types.Tag, 0),
+			Name:         bucket.Name,
+			CreationDate: bucket.CreationDate,
+			Tags:         make([]s3types.Tag, 0),
 		}
 
 		lockCfg, err := svc.GetObjectLockConfiguration(ctx, &s3.GetObjectLockConfigurationInput{
-			Bucket: &newBucket.name,
+			Bucket: newBucket.Name,
 		})
 		if err != nil {
 			// check if aws error is NoSuchObjectLockConfiguration
@@ -92,7 +91,7 @@ func (l *S3BucketLister) List(ctx context.Context, o interface{}) ([]resource.Re
 			continue
 		}
 
-		newBucket.tags = tags.TagSet
+		newBucket.Tags = tags.TagSet
 		resources = append(resources, newBucket)
 	}
 
@@ -105,32 +104,27 @@ type DescribeS3BucketsAPIClient interface {
 	GetBucketLocation(context.Context, *s3.GetBucketLocationInput, ...func(*s3.Options)) (*s3.GetBucketLocationOutput, error)
 }
 
-func DescribeS3Buckets(ctx context.Context, svc DescribeS3BucketsAPIClient) ([]s3types.Bucket, error) {
-	resp, err := svc.ListBuckets(ctx, nil)
-	if err != nil {
-		return nil, err
+func DescribeS3Buckets(ctx context.Context, svc DescribeS3BucketsAPIClient, opts *nuke.ListerOpts) ([]s3types.Bucket, error) {
+	buckets := make([]s3types.Bucket, 0)
+
+	params := &s3.ListBucketsInput{
+		BucketRegion: ptr.String(opts.Region.Name),
+		MaxBuckets:   ptr.Int32(100),
 	}
 
-	buckets := make([]s3types.Bucket, 0)
-	for _, out := range resp.Buckets {
-		bucketLocationResponse, err := svc.GetBucketLocation(ctx, &s3.GetBucketLocationInput{Bucket: out.Name})
+	for {
+		resp, err := svc.ListBuckets(ctx, params)
 		if err != nil {
-			continue
+			return nil, err
 		}
 
-		location := string(bucketLocationResponse.LocationConstraint)
-		if location == "" {
-			location = "us-east-1"
-		}
+		buckets = append(buckets, resp.Buckets...)
 
-		region := svc.Options().Region
-		if region == "" {
-			region = "us-east-1"
+		if resp.ContinuationToken == nil {
+			break
 		}
 
-		if location == region {
-			buckets = append(buckets, out)
-		}
+		params.ContinuationToken = resp.ContinuationToken
 	}
 
 	return buckets, nil
@@ -139,22 +133,22 @@ func DescribeS3Buckets(ctx context.Context, svc DescribeS3BucketsAPIClient) ([]s
 type S3Bucket struct {
 	svc          *s3.Client
 	settings     *libsettings.Setting
-	name         string
-	creationDate time.Time
-	tags         []s3types.Tag
+	Name         *string
+	CreationDate *time.Time
+	Tags         []s3types.Tag
 	ObjectLock   s3types.ObjectLockEnabled
 }
 
 func (r *S3Bucket) Remove(ctx context.Context) error {
 	_, err := r.svc.DeleteBucketPolicy(ctx, &s3.DeleteBucketPolicyInput{
-		Bucket: &r.name,
+		Bucket: r.Name,
 	})
 	if err != nil {
 		return err
 	}
 
 	_, err = r.svc.PutBucketLogging(ctx, &s3.PutBucketLoggingInput{
-		Bucket:              &r.name,
+		Bucket:              r.Name,
 		BucketLoggingStatus: &s3types.BucketLoggingStatus{},
 	})
 	if err != nil {
@@ -177,7 +171,7 @@ func (r *S3Bucket) Remove(ctx context.Context) error {
 	}
 
 	_, err = r.svc.DeleteBucket(ctx, &s3.DeleteBucketInput{
-		Bucket: &r.name,
+		Bucket: r.Name,
 	})
 
 	return err
@@ -188,12 +182,12 @@ func (r *S3Bucket) RemoveAllLegalHolds(ctx context.Context) error {
 		return nil
 	}
 
-	if r.ObjectLock == "" || r.ObjectLock != s3types.ObjectLockEnabledEnabled {
+	if r.ObjectLock != s3types.ObjectLockEnabledEnabled {
 		return nil
 	}
 
 	params := &s3.ListObjectsV2Input{
-		Bucket: &r.name,
+		Bucket: r.Name,
 	}
 
 	for {
@@ -206,7 +200,7 @@ func (r *S3Bucket) RemoveAllLegalHolds(ctx context.Context) error {
 
 		for _, obj := range res.Contents {
 			_, err := r.svc.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
-				Bucket:    &r.name,
+				Bucket:    r.Name,
 				Key:       obj.Key,
 				LegalHold: &s3types.ObjectLockLegalHold{Status: s3types.ObjectLockLegalHoldStatusOff},
 			})
@@ -225,7 +219,7 @@ func (r *S3Bucket) RemoveAllLegalHolds(ctx context.Context) error {
 
 func (r *S3Bucket) RemoveAllVersions(ctx context.Context) error {
 	params := &s3.ListObjectVersionsInput{
-		Bucket: &r.name,
+		Bucket: r.Name,
 	}
 
 	var setBypass bool
@@ -242,7 +236,7 @@ func (r *S3Bucket) RemoveAllVersions(ctx context.Context) error {
 
 func (r *S3Bucket) RemoveAllObjects(ctx context.Context) error {
 	params := &s3.ListObjectsV2Input{
-		Bucket: &r.name,
+		Bucket: r.Name,
 	}
 
 	var setBypass bool
@@ -262,176 +256,9 @@ func (r *S3Bucket) Settings(settings *libsettings.Setting) {
 }
 
 func (r *S3Bucket) Properties() types.Properties {
-	properties := types.NewProperties().
-		Set("Name", r.name).
-		Set("CreationDate", r.creationDate.Format(time.RFC3339)).
-		Set("ObjectLock", r.ObjectLock)
-
-	for _, tag := range r.tags {
-		properties.SetTag(tag.Key, tag.Value)
-	}
-
-	return properties
+	return types.NewPropertiesFromStruct(r)
 }
 
 func (r *S3Bucket) String() string {
-	return fmt.Sprintf("s3://%s", r.name)
-}
-
-func bypassGovernanceRetention(input *s3.DeleteObjectsInput) {
-	input.BypassGovernanceRetention = ptr.Bool(true)
-}
-
-type s3DeleteVersionListIterator struct {
-	Bucket                    *string
-	Paginator                 *s3.ListObjectVersionsPaginator
-	objects                   []s3types.ObjectVersion
-	lastNotify                time.Time
-	BypassGovernanceRetention *bool
-	err                       error
-}
-
-func newS3DeleteVersionListIterator(
-	svc *s3.Client,
-	input *s3.ListObjectVersionsInput,
-	bypass bool,
-	opts ...func(*s3DeleteVersionListIterator)) awsmod.BatchDeleteIterator {
-	iter := &s3DeleteVersionListIterator{
-		Bucket:                    input.Bucket,
-		Paginator:                 s3.NewListObjectVersionsPaginator(svc, input),
-		BypassGovernanceRetention: ptr.Bool(bypass),
-	}
-
-	for _, opt := range opts {
-		opt(iter)
-	}
-
-	return iter
-}
-
-// Next will use the S3API client to iterate through a list of objects.
-func (iter *s3DeleteVersionListIterator) Next() bool {
-	if len(iter.objects) > 0 {
-		iter.objects = iter.objects[1:]
-		if len(iter.objects) > 0 {
-			return true
-		}
-	}
-
-	if !iter.Paginator.HasMorePages() {
-		return false
-	}
-
-	page, err := iter.Paginator.NextPage(context.TODO())
-	if err != nil {
-		iter.err = err
-		return false
-	}
-
-	iter.objects = page.Versions
-	for _, entry := range page.DeleteMarkers {
-		iter.objects = append(iter.objects, s3types.ObjectVersion{
-			Key:       entry.Key,
-			VersionId: entry.VersionId,
-		})
-	}
-
-	if len(iter.objects) > 500 && (iter.lastNotify.IsZero() || time.Since(iter.lastNotify) > 120*time.Second) {
-		logrus.Infof(
-			"S3Bucket: %s - empty bucket operation in progress, this could take a while, please be patient",
-			*iter.Bucket)
-		iter.lastNotify = time.Now().UTC()
-	}
-
-	return len(iter.objects) > 0
-}
-
-// Err will return the last known error from Next.
-func (iter *s3DeleteVersionListIterator) Err() error {
-	return iter.err
-}
-
-// DeleteObject will return the current object to be deleted.
-func (iter *s3DeleteVersionListIterator) DeleteObject() awsmod.BatchDeleteObject {
-	return awsmod.BatchDeleteObject{
-		Object: &s3.DeleteObjectInput{
-			Bucket:                    iter.Bucket,
-			Key:                       iter.objects[0].Key,
-			VersionId:                 iter.objects[0].VersionId,
-			BypassGovernanceRetention: iter.BypassGovernanceRetention,
-		},
-	}
-}
-
-type s3ObjectDeleteListIterator struct {
-	Bucket                    *string
-	Paginator                 *s3.ListObjectsV2Paginator
-	objects                   []s3types.Object
-	lastNotify                time.Time
-	BypassGovernanceRetention bool
-	err                       error
-}
-
-func newS3ObjectDeleteListIterator(
-	svc *s3.Client,
-	input *s3.ListObjectsV2Input,
-	bypass bool,
-	opts ...func(*s3ObjectDeleteListIterator)) awsmod.BatchDeleteIterator {
-	iter := &s3ObjectDeleteListIterator{
-		Bucket:                    input.Bucket,
-		Paginator:                 s3.NewListObjectsV2Paginator(svc, input),
-		BypassGovernanceRetention: bypass,
-	}
-
-	for _, opt := range opts {
-		opt(iter)
-	}
-	return iter
-}
-
-// Next will use the S3API client to iterate through a list of objects.
-func (iter *s3ObjectDeleteListIterator) Next() bool {
-	if len(iter.objects) > 0 {
-		iter.objects = iter.objects[1:]
-		if len(iter.objects) > 0 {
-			return true
-		}
-	}
-
-	if !iter.Paginator.HasMorePages() {
-		return false
-	}
-
-	page, err := iter.Paginator.NextPage(context.TODO())
-	if err != nil {
-		iter.err = err
-		return false
-	}
-
-	iter.objects = page.Contents
-
-	if len(iter.objects) > 500 && (iter.lastNotify.IsZero() || time.Since(iter.lastNotify) > 120*time.Second) {
-		logrus.Infof(
-			"S3Bucket: %s - empty bucket operation in progress, this could take a while, please be patient",
-			*iter.Bucket)
-		iter.lastNotify = time.Now().UTC()
-	}
-
-	return len(iter.objects) > 0
-}
-
-// Err will return the last known error from Next.
-func (iter *s3ObjectDeleteListIterator) Err() error {
-	return iter.err
-}
-
-// DeleteObject will return the current object to be deleted.
-func (iter *s3ObjectDeleteListIterator) DeleteObject() awsmod.BatchDeleteObject {
-	return awsmod.BatchDeleteObject{
-		Object: &s3.DeleteObjectInput{
-			Bucket:                    iter.Bucket,
-			Key:                       iter.objects[0].Key,
-			BypassGovernanceRetention: ptr.Bool(iter.BypassGovernanceRetention),
-		},
-	}
+	return fmt.Sprintf("s3://%s", *r.Name)
 }
diff --git a/resources/s3-bucket_test.go b/resources/s3-bucket_test.go
index 71cd42a7..22bfc828 100644
--- a/resources/s3-bucket_test.go
+++ b/resources/s3-bucket_test.go
@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gotidy/ptr"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 
@@ -29,14 +30,14 @@ func (readSeekCloser) Close() error { return nil }
 
 type TestS3BucketSuite struct {
 	suite.Suite
-	bucket string
+	bucket *string
 	svc    *s3.Client
 }
 
 func (suite *TestS3BucketSuite) SetupSuite() {
 	var err error
 
-	suite.bucket = fmt.Sprintf("aws-nuke-testing-bucket-%d", time.Now().UnixNano())
+	suite.bucket = ptr.String(fmt.Sprintf("aws-nuke-testing-bucket-%d", time.Now().UnixNano()))
 
 	ctx := context.TODO()
 
@@ -50,7 +51,7 @@ func (suite *TestS3BucketSuite) SetupSuite() {
 
 	// Create the bucket
 	_, err = suite.svc.CreateBucket(ctx, &s3.CreateBucketInput{
-		Bucket: aws.String(suite.bucket),
+		Bucket: suite.bucket,
 		CreateBucketConfiguration: &s3types.CreateBucketConfiguration{
 			LocationConstraint: s3types.BucketLocationConstraint("us-west-2"),
 		},
@@ -61,7 +62,7 @@ func (suite *TestS3BucketSuite) SetupSuite() {
 
 	// enable versioning
 	_, err = suite.svc.PutBucketVersioning(ctx, &s3.PutBucketVersioningInput{
-		Bucket: aws.String(suite.bucket),
+		Bucket: suite.bucket,
 		VersioningConfiguration: &s3types.VersioningConfiguration{
 			Status: s3types.BucketVersioningStatusEnabled,
 		},
@@ -72,7 +73,7 @@ func (suite *TestS3BucketSuite) SetupSuite() {
 
 	// Set the object lock configuration to governance mode
 	_, err = suite.svc.PutObjectLockConfiguration(ctx, &s3.PutObjectLockConfigurationInput{
-		Bucket: aws.String(suite.bucket),
+		Bucket: suite.bucket,
 		ObjectLockConfiguration: &s3types.ObjectLockConfiguration{
 			ObjectLockEnabled: s3types.ObjectLockEnabledEnabled,
 			Rule: &s3types.ObjectLockRule{
@@ -89,7 +90,7 @@ func (suite *TestS3BucketSuite) SetupSuite() {
 
 	// Create an object in the bucket
 	_, err = suite.svc.PutObject(ctx, &s3.PutObjectInput{
-		Bucket:            aws.String(suite.bucket),
+		Bucket:            suite.bucket,
 		Key:               aws.String("test-object"),
 		Body:              readSeekCloser{strings.NewReader("test content")},
 		ChecksumAlgorithm: s3types.ChecksumAlgorithmCrc32,
@@ -101,7 +102,7 @@ func (suite *TestS3BucketSuite) SetupSuite() {
 
 func (suite *TestS3BucketSuite) TearDownSuite() {
 	iterator := newS3DeleteVersionListIterator(suite.svc, &s3.ListObjectVersionsInput{
-		Bucket: &suite.bucket,
+		Bucket: suite.bucket,
 	}, true)
 	if err := awsmod.NewBatchDeleteWithClient(suite.svc).Delete(context.TODO(), iterator, bypassGovernanceRetention); err != nil {
 		if !strings.Contains(err.Error(), "NoSuchBucket") {
@@ -110,7 +111,7 @@ func (suite *TestS3BucketSuite) TearDownSuite() {
 	}
 
 	iterator2 := newS3ObjectDeleteListIterator(suite.svc, &s3.ListObjectsV2Input{
-		Bucket: &suite.bucket,
+		Bucket: suite.bucket,
 	}, true)
 	if err := awsmod.NewBatchDeleteWithClient(suite.svc).Delete(context.TODO(), iterator2, bypassGovernanceRetention); err != nil {
 		if !strings.Contains(err.Error(), "NoSuchBucket") {
@@ -119,7 +120,7 @@ func (suite *TestS3BucketSuite) TearDownSuite() {
 	}
 
 	_, err := suite.svc.DeleteBucket(context.TODO(), &s3.DeleteBucketInput{
-		Bucket: aws.String(suite.bucket),
+		Bucket: suite.bucket,
 	})
 	if err != nil {
 		if !strings.Contains(err.Error(), "NoSuchBucket") {
@@ -135,7 +136,7 @@ type TestS3BucketObjectLockSuite struct {
 func (suite *TestS3BucketObjectLockSuite) TestS3BucketObjectLock() {
 	// Verify the object lock configuration
 	result, err := suite.svc.GetObjectLockConfiguration(context.TODO(), &s3.GetObjectLockConfigurationInput{
-		Bucket: aws.String(suite.bucket),
+		Bucket: suite.bucket,
 	})
 	if err != nil {
 		suite.T().Fatalf("failed to get object lock configuration, %v", err)
@@ -150,7 +151,7 @@ func (suite *TestS3BucketObjectLockSuite) TestS3BucketRemove() {
 	// Create the S3Bucket object
 	bucket := &S3Bucket{
 		svc:      suite.svc,
-		name:     suite.bucket,
+		Name:     suite.bucket,
 		settings: &libsettings.Setting{},
 	}
 
@@ -165,12 +166,12 @@ type TestS3BucketBypassGovernanceSuite struct {
 func (suite *TestS3BucketBypassGovernanceSuite) TestS3BucketRemoveWithBypass() {
 	// Create the S3Bucket object
 	bucket := &S3Bucket{
-		svc:        suite.svc,
-		name:       suite.bucket,
-		ObjectLock: s3types.ObjectLockEnabledEnabled,
+		svc: suite.svc,
 		settings: &libsettings.Setting{
 			"BypassGovernanceRetention": true,
 		},
+		Name:       suite.bucket,
+		ObjectLock: s3types.ObjectLockEnabledEnabled,
 	}
 
 	err := bucket.Remove(context.TODO())
diff --git a/resources/s3-multipart-uploads.go b/resources/s3-multipart-uploads.go
index feda2d96..81d9a7e6 100644
--- a/resources/s3-multipart-uploads.go
+++ b/resources/s3-multipart-uploads.go
@@ -2,7 +2,6 @@ package resources
 
 import (
 	"context"
-
 	"fmt"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -34,7 +33,7 @@ func (l *S3MultipartUploadLister) List(ctx context.Context, o interface{}) ([]re
 
 	resources := make([]resource.Resource, 0)
 
-	buckets, err := DescribeS3Buckets(ctx, svc)
+	buckets, err := DescribeS3Buckets(ctx, svc, opts)
 	if err != nil {
 		return nil, err
 	}
diff --git a/resources/s3-objects.go b/resources/s3-objects.go
index bf89833e..adc436ee 100644
--- a/resources/s3-objects.go
+++ b/resources/s3-objects.go
@@ -2,7 +2,6 @@ package resources
 
 import (
 	"context"
-
 	"fmt"
 	"time"
 
@@ -37,7 +36,7 @@ func (l *S3ObjectLister) List(ctx context.Context, o interface{}) ([]resource.Re
 
 	resources := make([]resource.Resource, 0)
 
-	buckets, err := DescribeS3Buckets(ctx, svc)
+	buckets, err := DescribeS3Buckets(ctx, svc, opts)
 	if err != nil {
 		return nil, err
 	}

From ebb69b0a24ae462cc34bf4ebe380a149507dd441 Mon Sep 17 00:00:00 2001
From: Erik Kristensen <erik@erikkristensen.com>
Date: Mon, 23 Dec 2024 16:01:49 -0700
Subject: [PATCH 2/6] refactor(s3-object): standardization

---
 resources/{s3-objects.go => s3-object.go} | 58 +++++++++-----------
 resources/s3-object_test.go               | 65 +++++++++++++++++++++++
 2 files changed, 91 insertions(+), 32 deletions(-)
 rename resources/{s3-objects.go => s3-object.go} (59%)
 create mode 100644 resources/s3-object_test.go

diff --git a/resources/s3-objects.go b/resources/s3-object.go
similarity index 59%
rename from resources/s3-objects.go
rename to resources/s3-object.go
index adc436ee..e5ca6c3b 100644
--- a/resources/s3-objects.go
+++ b/resources/s3-object.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/gotidy/ptr"
 
-	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 
 	"github.com/ekristen/libnuke/pkg/registry"
@@ -59,11 +58,11 @@ func (l *S3ObjectLister) List(ctx context.Context, o interface{}) ([]resource.Re
 
 				resources = append(resources, &S3Object{
 					svc:          svc,
-					bucket:       aws.ToString(bucket.Name),
-					creationDate: aws.ToTime(bucket.CreationDate),
-					key:          *out.Key,
-					versionID:    out.VersionId,
-					latest:       ptr.ToBool(out.IsLatest),
+					Bucket:       bucket.Name,
+					CreationDate: bucket.CreationDate,
+					Key:          out.Key,
+					VersionID:    out.VersionId,
+					IsLatest:     out.IsLatest,
 				})
 			}
 
@@ -74,11 +73,11 @@ func (l *S3ObjectLister) List(ctx context.Context, o interface{}) ([]resource.Re
 
 				resources = append(resources, &S3Object{
 					svc:          svc,
-					bucket:       aws.ToString(bucket.Name),
-					creationDate: aws.ToTime(bucket.CreationDate),
-					key:          *out.Key,
-					versionID:    out.VersionId,
-					latest:       ptr.ToBool(out.IsLatest),
+					Bucket:       bucket.Name,
+					CreationDate: bucket.CreationDate,
+					Key:          out.Key,
+					VersionID:    out.VersionId,
+					IsLatest:     out.IsLatest,
 				})
 			}
 
@@ -97,21 +96,21 @@ func (l *S3ObjectLister) List(ctx context.Context, o interface{}) ([]resource.Re
 
 type S3Object struct {
 	svc          *s3.Client
-	bucket       string
-	creationDate time.Time
-	key          string
-	versionID    *string
-	latest       bool
+	Bucket       *string
+	CreationDate *time.Time
+	Key          *string
+	VersionID    *string
+	IsLatest     *bool
 }
 
-func (e *S3Object) Remove(ctx context.Context) error {
+func (r *S3Object) Remove(ctx context.Context) error {
 	params := &s3.DeleteObjectInput{
-		Bucket:    &e.bucket,
-		Key:       &e.key,
-		VersionId: e.versionID,
+		Bucket:    r.Bucket,
+		Key:       r.Key,
+		VersionId: r.VersionID,
 	}
 
-	_, err := e.svc.DeleteObject(ctx, params)
+	_, err := r.svc.DeleteObject(ctx, params)
 	if err != nil {
 		return err
 	}
@@ -119,18 +118,13 @@ func (e *S3Object) Remove(ctx context.Context) error {
 	return nil
 }
 
-func (e *S3Object) Properties() types.Properties {
-	return types.NewProperties().
-		Set("Bucket", e.bucket).
-		Set("Key", e.key).
-		Set("VersionID", e.versionID).
-		Set("IsLatest", e.latest).
-		Set("CreationDate", e.creationDate)
+func (r *S3Object) Properties() types.Properties {
+	return types.NewPropertiesFromStruct(r)
 }
 
-func (e *S3Object) String() string {
-	if e.versionID != nil && *e.versionID != "null" && !e.latest {
-		return fmt.Sprintf("s3://%s/%s#%s", e.bucket, e.key, *e.versionID)
+func (r *S3Object) String() string {
+	if r.VersionID != nil && *r.VersionID != "null" && !ptr.ToBool(r.IsLatest) {
+		return fmt.Sprintf("s3://%s/%s#%s", *r.Bucket, *r.Key, *r.VersionID)
 	}
-	return fmt.Sprintf("s3://%s/%s", e.bucket, e.key)
+	return fmt.Sprintf("s3://%s/%s", *r.Bucket, *r.Key)
 }
diff --git a/resources/s3-object_test.go b/resources/s3-object_test.go
new file mode 100644
index 00000000..e196af3f
--- /dev/null
+++ b/resources/s3-object_test.go
@@ -0,0 +1,65 @@
+package resources
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/gotidy/ptr"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestS3ObjectProperties(t *testing.T) {
+	tests := []struct {
+		bucket       string
+		key          string
+		creationDate time.Time
+		versionID    string
+		isLatest     bool
+	}{
+		{
+			bucket:       "test-bucket",
+			key:          "test-key",
+			creationDate: time.Now(),
+			versionID:    "null",
+			isLatest:     true,
+		},
+		{
+			bucket:       "test-bucket",
+			key:          "test-key",
+			creationDate: time.Now(),
+			versionID:    "test-version-id",
+			isLatest:     false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.bucket, func(t *testing.T) {
+			obj := &S3Object{
+				Bucket:       ptr.String(test.bucket),
+				Key:          ptr.String(test.key),
+				VersionID:    ptr.String(test.versionID),
+				CreationDate: ptr.Time(test.creationDate),
+				IsLatest:     ptr.Bool(test.isLatest),
+			}
+
+			got := obj.Properties()
+			assert.Equal(t, test.bucket, got.Get("Bucket"))
+			assert.Equal(t, test.key, got.Get("Key"))
+			assert.Equal(t, test.versionID, got.Get("VersionID"))
+			assert.Equal(t, test.creationDate.Format(time.RFC3339), got.Get("CreationDate"))
+
+			if test.isLatest {
+				assert.Equal(t, "true", got.Get("IsLatest"))
+			} else {
+				assert.Equal(t, "false", got.Get("IsLatest"))
+			}
+
+			uri := fmt.Sprintf("s3://%s/%s", test.bucket, test.key)
+			if test.versionID != "" && test.versionID != "null" && !test.isLatest {
+				uri = fmt.Sprintf("%s#%s", uri, test.versionID)
+			}
+			assert.Equal(t, uri, obj.String())
+		})
+	}
+}

From d6fce3923c3cbe1298261c661ef3b736bc8bdc83 Mon Sep 17 00:00:00 2001
From: Erik Kristensen <erik@erikkristensen.com>
Date: Mon, 23 Dec 2024 16:02:23 -0700
Subject: [PATCH 3/6] refactor(s3-multipart-upload): standardization

---
 ...part-uploads.go => s3-multipart-upload.go} | 34 +++++++--------
 resources/s3-multipart-upload_test.go         | 41 +++++++++++++++++++
 2 files changed, 56 insertions(+), 19 deletions(-)
 rename resources/{s3-multipart-uploads.go => s3-multipart-upload.go} (70%)
 create mode 100644 resources/s3-multipart-upload_test.go

diff --git a/resources/s3-multipart-uploads.go b/resources/s3-multipart-upload.go
similarity index 70%
rename from resources/s3-multipart-uploads.go
rename to resources/s3-multipart-upload.go
index 81d9a7e6..159a6eee 100644
--- a/resources/s3-multipart-uploads.go
+++ b/resources/s3-multipart-upload.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 
 	"github.com/ekristen/libnuke/pkg/registry"
@@ -56,9 +55,9 @@ func (l *S3MultipartUploadLister) List(ctx context.Context, o interface{}) ([]re
 
 				resources = append(resources, &S3MultipartUpload{
 					svc:      svc,
-					bucket:   aws.ToString(bucket.Name),
-					key:      *upload.Key,
-					uploadID: *upload.UploadId,
+					Bucket:   bucket.Name,
+					Key:      upload.Key,
+					UploadID: upload.UploadId,
 				})
 			}
 
@@ -76,19 +75,19 @@ func (l *S3MultipartUploadLister) List(ctx context.Context, o interface{}) ([]re
 
 type S3MultipartUpload struct {
 	svc      *s3.Client
-	bucket   string
-	key      string
-	uploadID string
+	Bucket   *string
+	Key      *string
+	UploadID *string
 }
 
-func (e *S3MultipartUpload) Remove(ctx context.Context) error {
+func (r *S3MultipartUpload) Remove(ctx context.Context) error {
 	params := &s3.AbortMultipartUploadInput{
-		Bucket:   &e.bucket,
-		Key:      &e.key,
-		UploadId: &e.uploadID,
+		Bucket:   r.Bucket,
+		Key:      r.Key,
+		UploadId: r.UploadID,
 	}
 
-	_, err := e.svc.AbortMultipartUpload(ctx, params)
+	_, err := r.svc.AbortMultipartUpload(ctx, params)
 	if err != nil {
 		return err
 	}
@@ -96,13 +95,10 @@ func (e *S3MultipartUpload) Remove(ctx context.Context) error {
 	return nil
 }
 
-func (e *S3MultipartUpload) Properties() types.Properties {
-	return types.NewProperties().
-		Set("Bucket", e.bucket).
-		Set("Key", e.key).
-		Set("UploadID", e.uploadID)
+func (r *S3MultipartUpload) Properties() types.Properties {
+	return types.NewPropertiesFromStruct(r)
 }
 
-func (e *S3MultipartUpload) String() string {
-	return fmt.Sprintf("s3://%s/%s#%s", e.bucket, e.key, e.uploadID)
+func (r *S3MultipartUpload) String() string {
+	return fmt.Sprintf("s3://%s/%s#%s", *r.Bucket, *r.Key, *r.UploadID)
 }
diff --git a/resources/s3-multipart-upload_test.go b/resources/s3-multipart-upload_test.go
new file mode 100644
index 00000000..9cbe404b
--- /dev/null
+++ b/resources/s3-multipart-upload_test.go
@@ -0,0 +1,41 @@
+package resources
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/gotidy/ptr"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestS3MultipartUploadProperties(t *testing.T) {
+	tests := []struct {
+		bucket   string
+		key      string
+		uploadID string
+	}{
+		{
+			bucket:   "test-bucket",
+			key:      "test-key",
+			uploadID: "test-upload-id",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.bucket, func(t *testing.T) {
+			obj := &S3MultipartUpload{
+				Bucket:   ptr.String(test.bucket),
+				Key:      ptr.String(test.key),
+				UploadID: ptr.String(test.uploadID),
+			}
+
+			got := obj.Properties()
+			assert.Equal(t, test.bucket, got.Get("Bucket"))
+			assert.Equal(t, test.key, got.Get("Key"))
+			assert.Equal(t, test.uploadID, got.Get("UploadID"))
+
+			uri := fmt.Sprintf("s3://%s/%s#%s", test.bucket, test.key, test.uploadID)
+			assert.Equal(t, uri, obj.String())
+		})
+	}
+}

From e8efa66c99c326d7e11e69cd4e2548ac127b4611 Mon Sep 17 00:00:00 2001
From: Erik Kristensen <erik@erikkristensen.com>
Date: Mon, 23 Dec 2024 16:12:40 -0700
Subject: [PATCH 4/6] refactor(s3-access-point): standardization

---
 ...s3-access-points.go => s3-access-point.go} | 46 +++++++++---------
 resources/s3-access-point_test.go             | 48 +++++++++++++++++++
 2 files changed, 72 insertions(+), 22 deletions(-)
 rename resources/{s3-access-points.go => s3-access-point.go} (57%)
 create mode 100644 resources/s3-access-point_test.go

diff --git a/resources/s3-access-points.go b/resources/s3-access-point.go
similarity index 57%
rename from resources/s3-access-points.go
rename to resources/s3-access-point.go
index a94ba068..e5d58e3c 100644
--- a/resources/s3-access-points.go
+++ b/resources/s3-access-point.go
@@ -5,7 +5,6 @@ import (
 
 	"github.com/gotidy/ptr"
 
-	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/s3control"
 
 	"github.com/ekristen/libnuke/pkg/registry"
@@ -46,15 +45,20 @@ func (l *S3AccessPointLister) List(_ context.Context, o interface{}) ([]resource
 
 		for _, accessPoint := range resp.AccessPointList {
 			resources = append(resources, &S3AccessPoint{
-				svc:         svc,
-				accountID:   opts.AccountID,
-				accessPoint: accessPoint,
+				svc:           svc,
+				accountID:     opts.AccountID,
+				Name:          accessPoint.Name,
+				ARN:           accessPoint.AccessPointArn,
+				Alias:         accessPoint.Alias,
+				Bucket:        accessPoint.Bucket,
+				NetworkOrigin: accessPoint.NetworkOrigin,
 			})
 		}
 
 		if resp.NextToken == nil {
 			break
 		}
+
 		params.NextToken = resp.NextToken
 	}
 
@@ -62,30 +66,28 @@ func (l *S3AccessPointLister) List(_ context.Context, o interface{}) ([]resource
 }
 
 type S3AccessPoint struct {
-	svc         *s3control.S3Control
-	accountID   *string
-	accessPoint *s3control.AccessPoint
+	svc           *s3control.S3Control
+	accountID     *string
+	Name          *string
+	ARN           *string
+	Alias         *string
+	Bucket        *string
+	NetworkOrigin *string
 }
 
-func (e *S3AccessPoint) Remove(_ context.Context) error {
-	_, err := e.svc.DeleteAccessPoint(&s3control.DeleteAccessPointInput{
-		AccountId: e.accountID,
-		Name:      aws.String(*e.accessPoint.Name),
+func (r *S3AccessPoint) Remove(_ context.Context) error {
+	_, err := r.svc.DeleteAccessPoint(&s3control.DeleteAccessPointInput{
+		AccountId: r.accountID,
+		Name:      r.Name,
 	})
 	return err
 }
 
-func (e *S3AccessPoint) Properties() types.Properties {
-	properties := types.NewProperties()
-	properties.Set("AccessPointArn", e.accessPoint.AccessPointArn).
-		Set("Alias", e.accessPoint.Alias).
-		Set("Bucket", e.accessPoint.Bucket).
-		Set("Name", e.accessPoint.Name).
-		Set("NetworkOrigin", e.accessPoint.NetworkOrigin)
-
-	return properties
+func (r *S3AccessPoint) Properties() types.Properties {
+	return types.NewPropertiesFromStruct(r).
+		Set("AccessPointArn", r.ARN) // TODO(ek): this is an alias, should be deprecated for ARN
 }
 
-func (e *S3AccessPoint) String() string {
-	return ptr.ToString(e.accessPoint.AccessPointArn)
+func (r *S3AccessPoint) String() string {
+	return ptr.ToString(r.ARN) // TODO(ek): this should be the Name not the ARN
 }
diff --git a/resources/s3-access-point_test.go b/resources/s3-access-point_test.go
new file mode 100644
index 00000000..8ca90d6b
--- /dev/null
+++ b/resources/s3-access-point_test.go
@@ -0,0 +1,48 @@
+package resources
+
+import (
+	"fmt"
+	"github.com/gotidy/ptr"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestS3AccessPointProperties(t *testing.T) {
+	tests := []struct {
+		accountID     string
+		name          string
+		alias         string
+		bucket        string
+		networkOrigin string
+	}{
+		{
+			accountID:     "123456789012",
+			name:          "test-access-point",
+			alias:         "some-alias",
+			bucket:        "some-bucket",
+			networkOrigin: "some-network-origin",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			obj := &S3AccessPoint{
+				accountID:     ptr.String(tc.accountID),
+				ARN:           ptr.String(fmt.Sprintf("arn:aws:s3:::%s:%s", tc.accountID, tc.name)),
+				Name:          ptr.String(tc.name),
+				Alias:         ptr.String(tc.alias),
+				Bucket:        ptr.String(tc.bucket),
+				NetworkOrigin: ptr.String(tc.networkOrigin),
+			}
+
+			got := obj.Properties()
+			assert.Equal(t, tc.name, got.Get("Name"))
+			assert.Equal(t, fmt.Sprintf("arn:aws:s3:::%s:%s", tc.accountID, tc.name), got.Get("AccessPointArn"))
+			assert.Equal(t, tc.alias, got.Get("Alias"))
+			assert.Equal(t, tc.bucket, got.Get("Bucket"))
+			assert.Equal(t, tc.networkOrigin, got.Get("NetworkOrigin"))
+
+			assert.Equal(t, fmt.Sprintf("arn:aws:s3:::%s:%s", tc.accountID, tc.name), obj.String())
+		})
+	}
+}

From 9677bb56311e97071bdd53386f3826a10997784e Mon Sep 17 00:00:00 2001
From: Erik Kristensen <erik@erikkristensen.com>
Date: Mon, 23 Dec 2024 16:13:47 -0700
Subject: [PATCH 5/6] docs(s3): auto-generated

---
 docs/resources/s3-access-point.md     | 7 +++++++
 docs/resources/s3-bucket.md           | 4 ++++
 docs/resources/s3-multipart-upload.md | 5 +++++
 docs/resources/s3-object.md           | 7 +++++++
 4 files changed, 23 insertions(+)

diff --git a/docs/resources/s3-access-point.md b/docs/resources/s3-access-point.md
index e7ac6720..710ed99a 100644
--- a/docs/resources/s3-access-point.md
+++ b/docs/resources/s3-access-point.md
@@ -11,8 +11,15 @@ generated: true
 S3AccessPoint
 ```
 
+## Properties
 
 
+- `ARN`: No Description
+- `Alias`: No Description
+- `Bucket`: No Description
+- `Name`: No Description
+- `NetworkOrigin`: No Description
+
 !!! note - Using Properties
     Properties are what [Filters](../config-filtering.md) are written against in your configuration. You use the property
     names to write filters for what you want to **keep** and omit from the nuke process.
diff --git a/docs/resources/s3-bucket.md b/docs/resources/s3-bucket.md
index 8212bffa..71bdf295 100644
--- a/docs/resources/s3-bucket.md
+++ b/docs/resources/s3-bucket.md
@@ -23,7 +23,11 @@ AWS::S3::Bucket
 ## Properties
 
 
+- `CreationDate`: No Description
+- `Name`: No Description
 - `ObjectLock`: No Description
+- `tag:<key>:`: This resource has tags with property `Tags`. These are key/value pairs that are
+	added as their own property with the prefix of `tag:` (e.g. [tag:example: "value"]) 
 
 !!! note - Using Properties
     Properties are what [Filters](../config-filtering.md) are written against in your configuration. You use the property
diff --git a/docs/resources/s3-multipart-upload.md b/docs/resources/s3-multipart-upload.md
index bb17d334..da9ead91 100644
--- a/docs/resources/s3-multipart-upload.md
+++ b/docs/resources/s3-multipart-upload.md
@@ -11,8 +11,13 @@ generated: true
 S3MultipartUpload
 ```
 
+## Properties
 
 
+- `Bucket`: No Description
+- `Key`: No Description
+- `UploadID`: No Description
+
 !!! note - Using Properties
     Properties are what [Filters](../config-filtering.md) are written against in your configuration. You use the property
     names to write filters for what you want to **keep** and omit from the nuke process.
diff --git a/docs/resources/s3-object.md b/docs/resources/s3-object.md
index 5c70b67e..13eb4392 100644
--- a/docs/resources/s3-object.md
+++ b/docs/resources/s3-object.md
@@ -11,8 +11,15 @@ generated: true
 S3Object
 ```
 
+## Properties
 
 
+- `Bucket`: No Description
+- `CreationDate`: No Description
+- `IsLatest`: No Description
+- `Key`: No Description
+- `VersionID`: No Description
+
 !!! note - Using Properties
     Properties are what [Filters](../config-filtering.md) are written against in your configuration. You use the property
     names to write filters for what you want to **keep** and omit from the nuke process.

From 027cbe25548d0999eaf766c8a872735f6485c2a9 Mon Sep 17 00:00:00 2001
From: Erik Kristensen <erik@erikkristensen.com>
Date: Mon, 23 Dec 2024 17:32:50 -0700
Subject: [PATCH 6/6] chore(s3-access-point): fix lint violation

---
 resources/s3-access-point_test.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/resources/s3-access-point_test.go b/resources/s3-access-point_test.go
index 8ca90d6b..b7e119ed 100644
--- a/resources/s3-access-point_test.go
+++ b/resources/s3-access-point_test.go
@@ -2,9 +2,10 @@ package resources
 
 import (
 	"fmt"
+	"testing"
+
 	"github.com/gotidy/ptr"
 	"github.com/stretchr/testify/assert"
-	"testing"
 )
 
 func TestS3AccessPointProperties(t *testing.T) {