mirrors/versitygw
1
0
Fork 0
mirror of https://github.com/versity/versitygw.git synced 2026-07-31 12:33:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Prevents bucket deletion when it contains object versions by returning ErrVersionedBucketNotEmpty error. Enabled object deletion with versionId and delete markers creation with DeleteObject when the versioning status is Suspended

Browse Source
This commit is contained in:
jonaustin09
2024-10-18 15:36:52 -04:00
parent a78c826d0f
commit c803af4688
10 changed files with 112 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/azure/azure.go
+3 -3
View File
@@ -250,8 +250,8 @@ func (az *Azure) HeadBucket(ctx context.Context, input *s3.HeadBucketInput) (*s3
return &s3.HeadBucketOutput{}, nil
}
func (az *Azure) DeleteBucket(ctx context.Context, input *s3.DeleteBucketInput) error {
pager := az.client.NewListBlobsFlatPager(*input.Bucket, nil)
func (az *Azure) DeleteBucket(ctx context.Context, bucket string) error {
pager := az.client.NewListBlobsFlatPager(bucket, nil)
pg, err := pager.NextPage(ctx)
if err != nil {
@@ -261,7 +261,7 @@ func (az *Azure) DeleteBucket(ctx context.Context, input *s3.DeleteBucketInput)
if len(pg.Segment.BlobItems) > 0 {
return s3err.GetAPIError(s3err.ErrBucketNotEmpty)
}
_, err = az.client.DeleteContainer(ctx, *input.Bucket, nil)
_, err = az.client.DeleteContainer(ctx, bucket, nil)
return azureErrToS3Err(err)
}
backend/backend.go
+2 -2
View File
@@ -37,7 +37,7 @@ type Backend interface {
GetBucketAcl(context.Context, *s3.GetBucketAclInput) ([]byte, error)
CreateBucket(_ context.Context, _ *s3.CreateBucketInput, defaultACL []byte) error
PutBucketAcl(_ context.Context, bucket string, data []byte) error
DeleteBucket(context.Context, *s3.DeleteBucketInput) error
DeleteBucket(_ context.Context, bucket string) error
PutBucketVersioning(_ context.Context, bucket string, status types.BucketVersioningStatus) error
GetBucketVersioning(_ context.Context, bucket string) (s3response.GetBucketVersioningOutput, error)
PutBucketPolicy(_ context.Context, bucket string, policy []byte) error
@@ -123,7 +123,7 @@ func (BackendUnsupported) CreateBucket(context.Context, *s3.CreateBucketInput, [
func (BackendUnsupported) PutBucketAcl(_ context.Context, bucket string, data []byte) error {
return s3err.GetAPIError(s3err.ErrNotImplemented)
}
func (BackendUnsupported) DeleteBucket(context.Context, *s3.DeleteBucketInput) error {
func (BackendUnsupported) DeleteBucket(_ context.Context, bucket string) error {
return s3err.GetAPIError(s3err.ErrNotImplemented)
}
func (BackendUnsupported) PutBucketVersioning(_ context.Context, bucket string, status types.BucketVersioningStatus) error {
backend/posix/posix.go
+51 -33
View File
@@ -376,39 +376,59 @@ func (p *Posix) CreateBucket(ctx context.Context, input *s3.CreateBucketInput, a
return nil
}
func (p *Posix) DeleteBucket(_ context.Context, input *s3.DeleteBucketInput) error {
if input.Bucket == nil {
return s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
names, err := os.ReadDir(*input.Bucket)
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchBucket)
}
if err != nil {
return fmt.Errorf("readdir bucket: %w", err)
}
if len(names) == 1 && names[0].Name() == metaTmpDir {
// if .sgwtmp is only item in directory
// then clean this up before trying to remove the bucket
err = os.RemoveAll(filepath.Join(*input.Bucket, metaTmpDir))
func (p *Posix) isBucketEmpty(bucket string) error {
if p.versioningEnabled() {
ents, err := os.ReadDir(filepath.Join(p.versioningDir, bucket))
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("remove temp dir: %w", err)
return fmt.Errorf("readdir bucket: %w", err)
}
if err == nil {
if len(ents) == 1 && ents[0].Name() != metaTmpDir {
return s3err.GetAPIError(s3err.ErrVersionedBucketNotEmpty)
} else if len(ents) > 1 {
return s3err.GetAPIError(s3err.ErrVersionedBucketNotEmpty)
}
}
}
err = os.Remove(*input.Bucket)
if err != nil && err.(*os.PathError).Err == syscall.ENOTEMPTY {
ents, err := os.ReadDir(bucket)
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("readdir bucket: %w", err)
}
if errors.Is(err, fs.ErrNotExist) {
return s3err.GetAPIError(s3err.ErrNoSuchBucket)
}
if len(ents) == 1 && ents[0].Name() != metaTmpDir {
return s3err.GetAPIError(s3err.ErrBucketNotEmpty)
} else if len(ents) > 1 {
return s3err.GetAPIError(s3err.ErrBucketNotEmpty)
}
return nil
}
func (p *Posix) DeleteBucket(_ context.Context, bucket string) error {
if bucket == "" {
return s3err.GetAPIError(s3err.ErrInvalidBucketName)
}
// Check if the bucket is empty
err := p.isBucketEmpty(bucket)
if err != nil {
return err
}
// Remove the bucket
err = os.RemoveAll(bucket)
if err != nil {
return fmt.Errorf("remove bucket: %w", err)
}
err = p.meta.DeleteAttributes(*input.Bucket, "")
if err != nil {
return fmt.Errorf("remove bucket attributes: %w", err)
// Remove the bucket from versioning directory
if p.versioningEnabled() {
err = os.RemoveAll(filepath.Join(p.versioningDir, bucket))
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("remove bucket version: %w", err)
}
}
return nil
@@ -2393,10 +2413,9 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
if err != nil {
return nil, err
}
vEnabled := p.isBucketVersioningEnabled(vStatus)
// Directory objects can't have versions
if !isDir && p.versioningEnabled() && vEnabled {
if !isDir && p.versioningEnabled() && vStatus != "" {
if getString(input.VersionId) == "" {
// if the versionId is not specified, make the current version a delete marker
fi, err := os.Stat(objpath)
@@ -2533,6 +2552,8 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return nil, fmt.Errorf("remove obj version %w", err)
}
p.removeParents(filepath.Join(p.versioningDir, bucket), filepath.Join(genObjVersionKey(object), *input.VersionId))
return &s3.DeleteObjectOutput{
DeleteMarker: &isDelMarker,
VersionId: input.VersionId,
@@ -2552,6 +2573,8 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return nil, fmt.Errorf("delete object: %w", err)
}
p.removeParents(filepath.Join(p.versioningDir, bucket), filepath.Join(genObjVersionKey(object), *input.VersionId))
return &s3.DeleteObjectOutput{
DeleteMarker: &isDelMarker,
VersionId: input.VersionId,
@@ -2597,15 +2620,12 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) (
return nil, fmt.Errorf("delete object attributes: %w", err)
}
err = p.removeParents(bucket, object)
if err != nil {
return nil, err
}
p.removeParents(bucket, object)
return &s3.DeleteObjectOutput{}, nil
}
func (p *Posix) removeParents(bucket, object string) error {
func (p *Posix) removeParents(bucket, object string) {
// this will remove all parent directories that were not
// specifically uploaded with a put object. we detect
// this with a special attribute to indicate these. stop
@@ -2614,7 +2634,6 @@ func (p *Posix) removeParents(bucket, object string) error {
objPath := object
for {
parent := filepath.Dir(objPath)
if parent == string(filepath.Separator) || parent == "." {
// stop removing parents if we hit the bucket directory.
break
@@ -2635,7 +2654,6 @@ func (p *Posix) removeParents(bucket, object string) error {
objPath = parent
}
return nil
}
func (p *Posix) DeleteObjects(ctx context.Context, input *s3.DeleteObjectsInput) (s3response.DeleteResult, error) {
backend/s3proxy/s3.go
+4 -2
View File
@@ -125,8 +125,10 @@ func (s *S3Proxy) CreateBucket(ctx context.Context, input *s3.CreateBucketInput,
return handleError(err)
}
func (s *S3Proxy) DeleteBucket(ctx context.Context, input *s3.DeleteBucketInput) error {
_, err := s.client.DeleteBucket(ctx, input)
func (s *S3Proxy) DeleteBucket(ctx context.Context, bucket string) error {
_, err := s.client.DeleteBucket(ctx, &s3.DeleteBucketInput{
Bucket: &bucket,
})
return handleError(err)
}
s3api/controllers/backend_moq_test.go
+14 -14
View File
@@ -41,7 +41,7 @@ var _ backend.Backend = &BackendMock{}
// CreateMultipartUploadFunc: func(contextMoqParam context.Context, createMultipartUploadInput *s3.CreateMultipartUploadInput) (s3response.InitiateMultipartUploadResult, error) {
// panic("mock out the CreateMultipartUpload method")
// },
// DeleteBucketFunc: func(contextMoqParam context.Context, deleteBucketInput *s3.DeleteBucketInput) error {
// DeleteBucketFunc: func(contextMoqParam context.Context, bucket string) error {
// panic("mock out the DeleteBucket method")
// },
// DeleteBucketOwnershipControlsFunc: func(contextMoqParam context.Context, bucket string) error {
@@ -202,7 +202,7 @@ type BackendMock struct {
CreateMultipartUploadFunc func(contextMoqParam context.Context, createMultipartUploadInput *s3.CreateMultipartUploadInput) (s3response.InitiateMultipartUploadResult, error)
// DeleteBucketFunc mocks the DeleteBucket method.
DeleteBucketFunc func(contextMoqParam context.Context, deleteBucketInput *s3.DeleteBucketInput) error
DeleteBucketFunc func(contextMoqParam context.Context, bucket string) error
// DeleteBucketOwnershipControlsFunc mocks the DeleteBucketOwnershipControls method.
DeleteBucketOwnershipControlsFunc func(contextMoqParam context.Context, bucket string) error
@@ -388,8 +388,8 @@ type BackendMock struct {
DeleteBucket []struct {
// ContextMoqParam is the contextMoqParam argument value.
ContextMoqParam context.Context
// DeleteBucketInput is the deleteBucketInput argument value.
DeleteBucketInput *s3.DeleteBucketInput
// Bucket is the bucket argument value.
Bucket string
}
// DeleteBucketOwnershipControls holds details about calls to the DeleteBucketOwnershipControls method.
DeleteBucketOwnershipControls []struct {
@@ -1012,21 +1012,21 @@ func (mock *BackendMock) CreateMultipartUploadCalls() []struct {
}
// DeleteBucket calls DeleteBucketFunc.
func (mock *BackendMock) DeleteBucket(contextMoqParam context.Context, deleteBucketInput *s3.DeleteBucketInput) error {
func (mock *BackendMock) DeleteBucket(contextMoqParam context.Context, bucket string) error {
if mock.DeleteBucketFunc == nil {
panic("BackendMock.DeleteBucketFunc: method is nil but Backend.DeleteBucket was just called")
}
callInfo := struct {
ContextMoqParam context.Context
DeleteBucketInput *s3.DeleteBucketInput
ContextMoqParam context.Context
Bucket string
}{
ContextMoqParam: contextMoqParam,
DeleteBucketInput: deleteBucketInput,
ContextMoqParam: contextMoqParam,
Bucket: bucket,
}
mock.lockDeleteBucket.Lock()
mock.calls.DeleteBucket = append(mock.calls.DeleteBucket, callInfo)
mock.lockDeleteBucket.Unlock()
return mock.DeleteBucketFunc(contextMoqParam, deleteBucketInput)
return mock.DeleteBucketFunc(contextMoqParam, bucket)
}
// DeleteBucketCalls gets all the calls that were made to DeleteBucket.
@@ -1034,12 +1034,12 @@ func (mock *BackendMock) DeleteBucket(contextMoqParam context.Context, deleteBuc
//
// len(mockedBackend.DeleteBucketCalls())
func (mock *BackendMock) DeleteBucketCalls() []struct {
ContextMoqParam context.Context
DeleteBucketInput *s3.DeleteBucketInput
ContextMoqParam context.Context
Bucket string
} {
var calls []struct {
ContextMoqParam context.Context
DeleteBucketInput *s3.DeleteBucketInput
ContextMoqParam context.Context
Bucket string
}
mock.lockDeleteBucket.RLock()
calls = mock.calls.DeleteBucket
s3api/controllers/base.go
+1 -4
View File
@@ -2468,10 +2468,7 @@ func (c S3ApiController) DeleteBucket(ctx *fiber.Ctx) error {
})
}
err = c.be.DeleteBucket(ctx.Context(),
&s3.DeleteBucketInput{
Bucket: &bucket,
})
err = c.be.DeleteBucket(ctx.Context(), bucket)
return SendResponse(ctx, err,
&MetaOpts{
Logger: c.logger,
s3api/controllers/base_test.go
+1 -1
View File
@@ -1217,7 +1217,7 @@ func TestS3ApiController_DeleteBucket(t *testing.T) {
app := fiber.New()
s3ApiController := S3ApiController{
be: &BackendMock{
DeleteBucketFunc: func(context.Context, *s3.DeleteBucketInput) error {
DeleteBucketFunc: func(_ context.Context, bucket string) error {
return nil
},
DeleteBucketTaggingFunc: func(contextMoqParam context.Context, bucket string) error {
s3err/s3err.go
+6
View File
@@ -57,6 +57,7 @@ const (
ErrAccessDenied
ErrMethodNotAllowed
ErrBucketNotEmpty
ErrVersionedBucketNotEmpty
ErrBucketAlreadyExists
ErrBucketAlreadyOwnedByYou
ErrNoSuchBucket
@@ -160,6 +161,11 @@ var errorCodeResponse = map[ErrorCode]APIError{
Description: "The bucket you tried to delete is not empty.",
HTTPStatusCode: http.StatusConflict,
},
ErrVersionedBucketNotEmpty: {
Code: "BucketNotEmpty",
Description: "The bucket you tried to delete is not empty. You must delete all versions in the bucket.",
HTTPStatusCode: http.StatusConflict,
},
ErrBucketAlreadyExists: {
Code: "BucketAlreadyExists",
Description: "The requested bucket name is not available. The bucket name can not be an existing collection, and the bucket namespace is shared by all users of the system. Please select a different name and try again.",
tests/integration/group-tests.go
+3
View File
@@ -545,6 +545,8 @@ func TestVersioning(s *S3Conf) {
GetBucketVersioning_non_existing_bucket(s)
GetBucketVersioning_empty_response(s)
GetBucketVersioning_success(s)
// DeleteBucket action
Versioning_DeleteBucket_not_empty(s)
// PutObject action
Versioning_PutObject_suspended_null_versionId_obj(s)
Versioning_PutObject_null_versionId_obj(s)
@@ -936,6 +938,7 @@ func GetIntTests() IntTests {
"GetBucketVersioning_non_existing_bucket": GetBucketVersioning_non_existing_bucket,
"GetBucketVersioning_empty_response": GetBucketVersioning_empty_response,
"GetBucketVersioning_success": GetBucketVersioning_success,
"Versioning_DeleteBucket_not_empty": Versioning_DeleteBucket_not_empty,
"Versioning_PutObject_suspended_null_versionId_obj": Versioning_PutObject_suspended_null_versionId_obj,
"Versioning_PutObject_null_versionId_obj": Versioning_PutObject_null_versionId_obj,
"Versioning_PutObject_overwrite_null_versionId_obj": Versioning_PutObject_overwrite_null_versionId_obj,
tests/integration/tests.go
+27
View File
@@ -10769,6 +10769,28 @@ func GetBucketVersioning_success(s *S3Conf) error {
}, withVersioning(types.BucketVersioningStatusEnabled))
}
func Versioning_DeleteBucket_not_empty(s *S3Conf) error {
testName := "Versioning_DeleteBucket_not_empty"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "my-obj"
_, err := createObjVersions(s3client, bucket, obj, 2)
if err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.DeleteBucket(ctx, &s3.DeleteBucketInput{
Bucket: &bucket,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrVersionedBucketNotEmpty)); err != nil {
return err
}
return nil
}, withVersioning(types.BucketVersioningStatusEnabled))
}
func Versioning_PutObject_suspended_null_versionId_obj(s *S3Conf) error {
testName := "Versioning_PutObject_suspended_null_versionId_obj"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -11144,6 +11166,11 @@ func Versioning_CopyObject_special_chars(s *S3Conf) error {
return fmt.Errorf("expected the copied object versionId to be %v, instead got %v", *res.VersionId, *out.VersionId)
}
err = teardown(s, dstBucket)
if err != nil {
return err
}
return nil
}, withVersioning(types.BucketVersioningStatusEnabled))
}