[#31] Add force bucket delete flag #436
5 changed files with 99 additions and 9 deletions
|
@ -237,9 +237,18 @@ func (h *handler) DeleteBucketHandler(w http.ResponseWriter, r *http.Request) {
|
|||
sessionToken = boxData.Gate.SessionTokenForDelete()
|
||||
}
|
||||
|
||||
skipObjCheck := false
|
||||
if value, ok := r.Header[api.AmzForceBucketDelete]; ok {
|
||||
s := value[0]
|
||||
if s == "true" {
|
||||
alexvanin marked this conversation as resolved
Outdated
|
||||
skipObjCheck = true
|
||||
}
|
||||
}
|
||||
|
||||
if err = h.obj.DeleteBucket(r.Context(), &layer.DeleteBucketParams{
|
||||
BktInfo: bktInfo,
|
||||
SessionToken: sessionToken,
|
||||
SkipCheck: skipObjCheck,
|
||||
}); err != nil {
|
||||
h.logAndSendError(w, "couldn't delete bucket", reqInfo, err)
|
||||
return
|
||||
|
|
|
@ -85,6 +85,24 @@ func TestDeleteBucketOnNotFoundError(t *testing.T) {
|
|||
deleteBucket(t, hc, bktName, http.StatusNoContent)
|
||||
}
|
||||
|
||||
func TestForceDeleteBucket(t *testing.T) {
|
||||
hc := prepareHandlerContext(t)
|
||||
|
||||
bktName, objName := "bucket-for-removal", "object-to-delete"
|
||||
bktInfo := createTestBucket(hc, bktName)
|
||||
|
||||
putObject(hc, bktName, objName)
|
||||
|
||||
nodeVersion, err := hc.tree.GetUnversioned(hc.context, bktInfo, objName)
|
||||
require.NoError(t, err)
|
||||
var addr oid.Address
|
||||
addr.SetContainer(bktInfo.CID)
|
||||
addr.SetObject(nodeVersion.OID)
|
||||
|
||||
deleteBucketForce(t, hc, bktName, http.StatusConflict, "false")
|
||||
deleteBucketForce(t, hc, bktName, http.StatusNoContent, "true")
|
||||
}
|
||||
|
||||
func TestDeleteMultipleObjectCheckUniqueness(t *testing.T) {
|
||||
hc := prepareHandlerContext(t)
|
||||
|
||||
|
@ -517,6 +535,13 @@ func deleteObjectsBase(hc *handlerContext, bktName string, objVersions [][2]stri
|
|||
return w
|
||||
}
|
||||
|
||||
func deleteBucketForce(t *testing.T, tc *handlerContext, bktName string, code int, value string) {
|
||||
dkirillov
commented
Minor (non-blocking): it's better to write
Minor (non-blocking): it's better to write
```
func deleteBucketForce(t *testing.T, tc *handlerContext, bktName string, value bool, code int) {
```
|
||||
w, r := prepareTestRequest(tc, bktName, "", nil)
|
||||
r.Header.Set(api.AmzForceBucketDelete, value)
|
||||
tc.Handler().DeleteBucketHandler(w, r)
|
||||
assertStatus(t, w, code)
|
||||
}
|
||||
|
||||
func deleteBucket(t *testing.T, tc *handlerContext, bktName string, code int) {
|
||||
w, r := prepareTestRequest(tc, bktName, "", nil)
|
||||
tc.Handler().DeleteBucketHandler(w, r)
|
||||
|
|
|
@ -62,6 +62,7 @@ const (
|
|||
AmzMaxParts = "X-Amz-Max-Parts"
|
||||
AmzPartNumberMarker = "X-Amz-Part-Number-Marker"
|
||||
AmzStorageClass = "X-Amz-Storage-Class"
|
||||
AmzForceBucketDelete = "X-Amz-Force-Delete-Bucket"
|
||||
|
||||
AmzServerSideEncryptionCustomerAlgorithm = "x-amz-server-side-encryption-customer-algorithm"
|
||||
AmzServerSideEncryptionCustomerKey = "x-amz-server-side-encryption-customer-key"
|
||||
|
|
|
@ -170,6 +170,7 @@ type (
|
|||
DeleteBucketParams struct {
|
||||
BktInfo *data.BucketInfo
|
||||
SessionToken *session.Container
|
||||
SkipCheck bool
|
||||
}
|
||||
|
||||
// ListObjectVersionsParams stores list objects versions parameters.
|
||||
|
@ -804,16 +805,18 @@ func (n *Layer) ResolveBucket(ctx context.Context, name string) (cid.ID, error)
|
|||
}
|
||||
|
||||
func (n *Layer) DeleteBucket(ctx context.Context, p *DeleteBucketParams) error {
|
||||
res, _, err := n.getAllObjectsVersions(ctx, commonVersionsListingParams{
|
||||
BktInfo: p.BktInfo,
|
||||
MaxKeys: 1,
|
||||
})
|
||||
if !p.SkipCheck {
|
||||
res, _, err := n.getAllObjectsVersions(ctx, commonVersionsListingParams{
|
||||
BktInfo: p.BktInfo,
|
||||
MaxKeys: 1,
|
||||
})
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if len(res) != 0 {
|
||||
return errors.GetAPIError(errors.ErrBucketNotEmpty)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
alexvanin marked this conversation as resolved
Outdated
alexvanin
commented
I appreciate small changes, it looks very nice and clean. However, I am not sure it is worth keeping I appreciate small changes, it looks very nice and clean.
However, I am not sure it is worth keeping `n.getAllObjectVersions` when we can skip it entirely if `SkipCheck == true`.
dkirillov
commented
Also it would be nice to have tests for this new behavior Also it would be nice to have tests for this new behavior
|
||||
if len(res) != 0 {
|
||||
return errors.GetAPIError(errors.ErrBucketNotEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
n.cache.DeleteBucket(p.BktInfo)
|
||||
|
|
52
docs/extensions.md
Normal file
52
docs/extensions.md
Normal file
|
@ -0,0 +1,52 @@
|
|||
# S3 API Extension
|
||||
|
||||
## Bucket operations management
|
||||
|
||||
### Action to delete bucket (DeleteBucket)
|
||||
|
||||
Deletes bucket with all objects in it.
|
||||
|
||||
#### Request Parameters
|
||||
|
||||
- **Bucket**
|
||||
|
||||
Specifies the bucket being deleted.
|
||||
|
||||
|
||||
#### Errors
|
||||
|
||||
- **NoSuchEntity**
|
||||
|
||||
The request was rejected because it referenced a resource entity that does not exist.
|
||||
|
||||
HTTP Status Code: 404
|
||||
|
||||
- **ServiceFailure**
|
||||
|
||||
The request processing has failed because of an unknown error, exception or failure.
|
||||
|
||||
HTTP Status Code: 500
|
||||
|
||||
|
||||
#### Example
|
||||
|
||||
Sample Request
|
||||
|
||||
```text
|
||||
DELETE / HTTP/1.1
|
||||
X-Amz-Force-Delete-Bucket: true
|
||||
alexvanin marked this conversation as resolved
Outdated
alexvanin
commented
There should be There should be `X-Amz-Force-Delete-Bucket: true` header as well.
|
||||
Host: data.s3.<Region>.frostfs-s3-gw.com
|
||||
Date: Wed, 01 Mar 2024 12:00:00 GMT
|
||||
Authorization: authorization string
|
||||
```
|
||||
|
||||
Sample Response
|
||||
|
||||
```text
|
||||
HTTP/1.1 204 No Content
|
||||
x-amz-id-2: JuKZqmXuiwFeDQxhD7M8KtsKobSzWA1QEjLbTMTagkKdBX2z7Il/jGhDeJ3j6s80
|
||||
x-amz-request-id: 32FE2CEB32F5EE25
|
||||
Date: Wed, 01 Mar 2006 12:00:00 GMT
|
||||
Connection: close
|
||||
Server: AmazonS3
|
||||
```
|
Loading…
Reference in a new issue
This is compatibility-breaking change. We should not fail in case of missing this header.