Skip to content

Commit 8cf67dc

Browse files
tomassrnkaclaude
andcommitted
fix(storage): paginate S3 prefix deletes and guard empty prefix
DeleteObjectsWithPrefix listed only the first ListObjectsV2 page (max 1000 keys), silently leaving any further objects undeleted; walk all pages, deleting each batch (<=1000 keys). Also reject an empty prefix, which would otherwise delete the whole bucket. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f1fedb4 commit 8cf67dc

2 files changed

Lines changed: 64 additions & 13 deletions

File tree

packages/shared/pkg/storage/storage_aws.go

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,26 +75,59 @@ func newAWSStorage(ctx context.Context, bucketName string) (*awsStorage, error)
7575
}
7676

7777
func (s *awsStorage) DeleteObjectsWithPrefix(ctx context.Context, prefix string) error {
78-
ctx, cancel := context.WithTimeout(ctx, awsOperationTimeout)
79-
defer cancel()
80-
81-
list, err := s.client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{Bucket: &s.bucketName, Prefix: &prefix})
82-
if err != nil {
83-
return err
78+
if prefix == "" {
79+
return errors.New("refusing to delete objects with an empty prefix")
8480
}
8581

86-
objects := make([]types.ObjectIdentifier, 0, len(list.Contents))
87-
for _, obj := range list.Contents {
88-
objects = append(objects, types.ObjectIdentifier{Key: obj.Key})
82+
// A large prefix spans many pages, so scope the timeout per round-trip
83+
// below instead of wrapping the whole walk.
84+
paginator := s3.NewListObjectsV2Paginator(s.client, &s3.ListObjectsV2Input{Bucket: &s.bucketName, Prefix: &prefix})
85+
86+
deleted := false
87+
for paginator.HasMorePages() {
88+
objects, err := func() ([]types.ObjectIdentifier, error) {
89+
pageCtx, cancel := context.WithTimeout(ctx, awsOperationTimeout)
90+
defer cancel()
91+
92+
list, err := paginator.NextPage(pageCtx)
93+
if err != nil {
94+
return nil, err
95+
}
96+
97+
objects := make([]types.ObjectIdentifier, 0, len(list.Contents))
98+
for _, obj := range list.Contents {
99+
objects = append(objects, types.ObjectIdentifier{Key: obj.Key})
100+
}
101+
102+
return objects, nil
103+
}()
104+
if err != nil {
105+
return err
106+
}
107+
108+
// AWS S3 delete operation requires at least one object to delete.
109+
if len(objects) == 0 {
110+
continue
111+
}
112+
113+
if err := s.deleteObjects(ctx, objects); err != nil {
114+
return err
115+
}
116+
117+
deleted = true
89118
}
90119

91-
// AWS S3 delete operation requires at least one object to delete.
92-
if len(objects) == 0 {
120+
if !deleted {
93121
logger.L().Warn(ctx, "No objects found to delete with the given prefix", zap.String("prefix", prefix), zap.String("bucket", s.bucketName))
94-
95-
return nil
96122
}
97123

124+
return nil
125+
}
126+
127+
func (s *awsStorage) deleteObjects(ctx context.Context, objects []types.ObjectIdentifier) error {
128+
ctx, cancel := context.WithTimeout(ctx, awsOperationTimeout)
129+
defer cancel()
130+
98131
output, err := s.client.DeleteObjects(
99132
ctx, &s3.DeleteObjectsInput{
100133
Bucket: aws.String(s.bucketName),
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package storage
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestAWSDeleteObjectsWithPrefixRejectsEmptyPrefix(t *testing.T) {
11+
t.Parallel()
12+
13+
s := &awsStorage{bucketName: "test-bucket"}
14+
15+
err := s.DeleteObjectsWithPrefix(context.Background(), "")
16+
require.Error(t, err)
17+
require.Contains(t, err.Error(), "empty prefix")
18+
}

0 commit comments

Comments
 (0)