[#226] Fix status code in GET/HEAD delete marker #242

Merged
alexvanin merged 1 commit from dkirillov/frostfs-s3-gw:bugfix/226-get_head_delete_marker into master 2023-10-31 13:32:26 +00:00
4 changed files with 47 additions and 1 deletions

View file

@ -17,6 +17,7 @@ This document outlines major changes between releases.
- Use correct keys in `list-multipart-uploads` response (#185)
- Fix parsing `key-marker` for object list versions (#243)
- Fix marshaling errors in `DeleteObjects` method (#222)
- Fix status code in GET/HEAD delete marker (#226)
### Added
- Add `trace_id` value into log record when tracing is enabled (#142)

View file

@ -363,6 +363,27 @@ func TestDeleteMarkers(t *testing.T) {
require.Len(t, listOIDsFromMockedFrostFS(t, tc, bktName), 0, "shouldn't be any object in frostfs")
}
func TestGetHeadDeleteMarker(t *testing.T) {
hc := prepareHandlerContext(t)
bktName, objName := "bucket-for-removal", "object-to-delete"
createTestBucket(hc, bktName)
putBucketVersioning(t, hc, bktName, true)
putObject(hc, bktName, objName)
deleteMarkerVersionID, _ := deleteObject(t, hc, bktName, objName, emptyVersion)
w := headObjectBase(hc, bktName, objName, deleteMarkerVersionID)
require.Equal(t, w.Code, http.StatusMethodNotAllowed)
require.Equal(t, w.Result().Header.Get(api.AmzDeleteMarker), "true")
w, r := prepareTestRequest(hc, bktName, objName, nil)
hc.Handler().GetObjectHandler(w, r)
assertStatus(hc.t, w, http.StatusNotFound)
require.Equal(t, w.Result().Header.Get(api.AmzDeleteMarker), "true")
}
func TestDeleteObjectFromListCache(t *testing.T) {
tc := prepareHandlerContext(t)

View file

@ -3,6 +3,7 @@ package handler
import (
"context"
"errors"
"fmt"
"net/http"
"strconv"
"strings"
@ -28,6 +29,7 @@ func (h *handler) reqLogger(ctx context.Context) *zap.Logger {
}
func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo *middleware.ReqInfo, err error, additional ...zap.Field) {
err = handleDeleteMarker(w, err)
code := middleware.WriteErrorResponse(w, reqInfo, transformToS3Error(err))
fields := []zap.Field{
zap.Int("status", code),
@ -44,6 +46,16 @@ func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo
h.log.Error(logs.RequestFailed, fields...) // consider using h.reqLogger (it requires accept context.Context or http.Request)
}
func handleDeleteMarker(w http.ResponseWriter, err error) error {
var target layer.DeleteMarkerError
if !errors.As(err, &target) {
return err
}
w.Header().Set(api.AmzDeleteMarker, "true")
return fmt.Errorf("%w: %s", s3errors.GetAPIError(target.ErrorCode), err)
}
func (h *handler) logAndSendErrorNoHeader(w http.ResponseWriter, logText string, reqInfo *middleware.ReqInfo, err error, additional ...zap.Field) {
middleware.WriteErrorResponseNoHeader(w, reqInfo, transformToS3Error(err))
fields := []zap.Field{

View file

@ -80,8 +80,16 @@ type (
Marker string
ContinuationToken string
}
DeleteMarkerError struct {
ErrorCode apiErrors.ErrorCode
}
)
func (e DeleteMarkerError) Error() string {
return "object is delete marker"
}
const (
continuationToken = "<continuation-token>"
)
@ -389,7 +397,7 @@ func (n *layer) headLastVersionIfNotDeleted(ctx context.Context, bkt *data.Bucke
}
if node.IsDeleteMarker() {
return nil, fmt.Errorf("%w: found version is delete marker", apiErrors.GetAPIError(apiErrors.ErrNoSuchKey))
return nil, DeleteMarkerError{ErrorCode: apiErrors.ErrNoSuchKey}
}
meta, err := n.objectHead(ctx, bkt, node.OID)
@ -445,6 +453,10 @@ func (n *layer) headVersion(ctx context.Context, bkt *data.BucketInfo, p *HeadOb
return extObjInfo, nil
}
if foundVersion.IsDeleteMarker() {
return nil, DeleteMarkerError{ErrorCode: apiErrors.ErrMethodNotAllowed}
}
meta, err := n.objectHead(ctx, bkt, foundVersion.OID)
if err != nil {
if client.IsErrObjectNotFound(err) {