From e6b4ea7618e04478f28160a6a47aa59622a1fe56 Mon Sep 17 00:00:00 2001 From: Poorna Krishnamoorthy Date: Wed, 10 Feb 2021 17:25:04 -0800 Subject: [PATCH] More fixes for delete marker replication (#11504) continuation of PR#11491 for multiple server pools and bi-directional replication. Moving proxying for GET/HEAD to handler level rather than server pool layer as this was also causing incorrect proxying of HEAD. Also fixing metadata update on CopyObject - minio-go was not passing source version ID in X-Amz-Copy-Source header --- cmd/bucket-replication.go | 10 ++++- cmd/erasure-server-pool.go | 14 ------ cmd/gateway/s3/gateway-s3.go | 2 +- cmd/object-handlers.go | 83 +++++++++++++++++++++++++----------- go.mod | 2 +- go.sum | 2 + 6 files changed, 72 insertions(+), 41 deletions(-) diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index dd56e8abb..ca5c8d712 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -184,6 +184,8 @@ func checkReplicateDelete(ctx context.Context, bucket string, dobj ObjectToDelet if gerr != nil { validReplStatus := false switch oi.ReplicationStatus { + // Point to note: even if two way replication with Deletemarker or Delete sync is enabled, + // deletion of a REPLICA object/deletemarker will not trigger a replication to the other cluster. case replication.Pending, replication.Completed, replication.Failed: validReplStatus = true } @@ -648,9 +650,14 @@ func replicateObject(ctx context.Context, objInfo ObjectInfo, objectAPI ObjectLa replicationStatus := replication.Completed if rtype != replicateAll { // replicate metadata for object tagging/copy with metadata replacement + srcOpts := miniogo.CopySrcOptions{ + Bucket: dest.Bucket, + Object: object, + VersionID: objInfo.VersionID} dstOpts := miniogo.PutObjectOptions{Internal: miniogo.AdvancedPutOptions{SourceVersionID: objInfo.VersionID}} c := &miniogo.Core{Client: tgt.Client} - if _, err = c.CopyObject(ctx, dest.Bucket, object, dest.Bucket, object, getCopyObjMetadata(objInfo, dest), dstOpts); err != nil { + + if _, err = c.CopyObject(ctx, dest.Bucket, object, dest.Bucket, object, getCopyObjMetadata(objInfo, dest), srcOpts, dstOpts); err != nil { replicationStatus = replication.Failed logger.LogIf(ctx, fmt.Errorf("Unable to replicate metadata for object %s/%s(%s): %s", bucket, objInfo.Name, objInfo.VersionID, err)) } @@ -876,6 +883,7 @@ func proxyGetToReplicationTarget(ctx context.Context, bucket, object string, rs if err != nil { return nil, false } + reader.ObjInfo = oi.Clone() return reader, true } diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go index b91514d4c..5deb6450a 100644 --- a/cmd/erasure-server-pool.go +++ b/cmd/erasure-server-pool.go @@ -521,13 +521,6 @@ func (z *erasureServerPools) GetObjectNInfo(ctx context.Context, bucket, object } return gr, nil } - if isProxyable(ctx, bucket) { - // proxy to replication target if active-active replication is in place. - reader, proxy := proxyGetToReplicationTarget(ctx, bucket, object, rs, h, opts) - if reader != nil && proxy { - return reader, nil - } - } if opts.VersionID != "" { return gr, VersionNotFound{Bucket: bucket, Object: object, VersionID: opts.VersionID} } @@ -573,13 +566,6 @@ func (z *erasureServerPools) GetObjectInfo(ctx context.Context, bucket, object s return objInfo, nil } object = decodeDirObject(object) - // proxy HEAD to replication target if active-active replication configured on bucket - if isProxyable(ctx, bucket) { - oi, proxy, err := proxyHeadToReplicationTarget(ctx, bucket, object, opts) - if proxy { - return oi, err - } - } if opts.VersionID != "" { return objInfo, VersionNotFound{Bucket: bucket, Object: object, VersionID: opts.VersionID} } diff --git a/cmd/gateway/s3/gateway-s3.go b/cmd/gateway/s3/gateway-s3.go index 8505b2e8d..0c3c4a302 100644 --- a/cmd/gateway/s3/gateway-s3.go +++ b/cmd/gateway/s3/gateway-s3.go @@ -515,7 +515,7 @@ func (l *s3Objects) CopyObject(ctx context.Context, srcBucket string, srcObject srcInfo.UserDefined[k] = v[0] } - if _, err = l.Client.CopyObject(ctx, srcBucket, srcObject, dstBucket, dstObject, srcInfo.UserDefined, miniogo.PutObjectOptions{}); err != nil { + if _, err = l.Client.CopyObject(ctx, srcBucket, srcObject, dstBucket, dstObject, srcInfo.UserDefined, miniogo.CopySrcOptions{}, miniogo.PutObjectOptions{}); err != nil { return objInfo, minio.ErrorRespToObjectError(err, srcBucket, srcObject) } return l.GetObjectInfo(ctx, dstBucket, dstObject, dstOpts) diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index a1a71ea6d..482788751 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -411,19 +411,40 @@ func (api objectAPIHandlers) GetObjectHandler(w http.ResponseWriter, r *http.Req gr, err := getObjectNInfo(ctx, bucket, object, rs, r.Header, readLock, opts) if err != nil { - if isErrPreconditionFailed(err) { - return + var ( + reader *GetObjectReader + proxy bool + ) + if isProxyable(ctx, bucket) { + // proxy to replication target if active-active replication is in place. + reader, proxy = proxyGetToReplicationTarget(ctx, bucket, object, rs, r.Header, opts) + if reader != nil && proxy { + gr = reader + } } - if globalBucketVersioningSys.Enabled(bucket) && gr != nil { - // Versioning enabled quite possibly object is deleted might be delete-marker - // if present set the headers, no idea why AWS S3 sets these headers. - if gr.ObjInfo.VersionID != "" && gr.ObjInfo.DeleteMarker { - w.Header()[xhttp.AmzVersionID] = []string{gr.ObjInfo.VersionID} - w.Header()[xhttp.AmzDeleteMarker] = []string{strconv.FormatBool(gr.ObjInfo.DeleteMarker)} + if reader == nil || !proxy { + if isErrPreconditionFailed(err) { + return } + if globalBucketVersioningSys.Enabled(bucket) && gr != nil { + if !gr.ObjInfo.VersionPurgeStatus.Empty() { + // Shows the replication status of a permanent delete of a version + w.Header()[xhttp.MinIODeleteReplicationStatus] = []string{string(gr.ObjInfo.VersionPurgeStatus)} + } + if !gr.ObjInfo.ReplicationStatus.Empty() && gr.ObjInfo.DeleteMarker { + w.Header()[xhttp.MinIODeleteMarkerReplicationStatus] = []string{string(gr.ObjInfo.ReplicationStatus)} + } + + // Versioning enabled quite possibly object is deleted might be delete-marker + // if present set the headers, no idea why AWS S3 sets these headers. + if gr.ObjInfo.VersionID != "" && gr.ObjInfo.DeleteMarker { + w.Header()[xhttp.AmzVersionID] = []string{gr.ObjInfo.VersionID} + w.Header()[xhttp.AmzDeleteMarker] = []string{strconv.FormatBool(gr.ObjInfo.DeleteMarker)} + } + } + writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) + return } - writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r)) - return } defer gr.Close() @@ -577,23 +598,37 @@ func (api objectAPIHandlers) HeadObjectHandler(w http.ResponseWriter, r *http.Re objInfo, err := getObjectInfo(ctx, bucket, object, opts) if err != nil { - if globalBucketVersioningSys.Enabled(bucket) { - if !objInfo.VersionPurgeStatus.Empty() { - // Shows the replication status of a permanent delete of a version - w.Header()[xhttp.MinIODeleteReplicationStatus] = []string{string(objInfo.VersionPurgeStatus)} + var ( + proxy bool + perr error + oi ObjectInfo + ) + // proxy HEAD to replication target if active-active replication configured on bucket + if isProxyable(ctx, bucket) { + oi, proxy, perr = proxyHeadToReplicationTarget(ctx, bucket, object, opts) + if proxy && perr == nil { + objInfo = oi } - if !objInfo.ReplicationStatus.Empty() && objInfo.DeleteMarker { - w.Header()[xhttp.MinIODeleteMarkerReplicationStatus] = []string{string(objInfo.ReplicationStatus)} - } - // Versioning enabled quite possibly object is deleted might be delete-marker - // if present set the headers, no idea why AWS S3 sets these headers. - if objInfo.VersionID != "" && objInfo.DeleteMarker { - w.Header()[xhttp.AmzVersionID] = []string{objInfo.VersionID} - w.Header()[xhttp.AmzDeleteMarker] = []string{strconv.FormatBool(objInfo.DeleteMarker)} + } + if !proxy || perr != nil { + if globalBucketVersioningSys.Enabled(bucket) { + if !objInfo.VersionPurgeStatus.Empty() { + // Shows the replication status of a permanent delete of a version + w.Header()[xhttp.MinIODeleteReplicationStatus] = []string{string(objInfo.VersionPurgeStatus)} + } + if !objInfo.ReplicationStatus.Empty() && objInfo.DeleteMarker { + w.Header()[xhttp.MinIODeleteMarkerReplicationStatus] = []string{string(objInfo.ReplicationStatus)} + } + // Versioning enabled quite possibly object is deleted might be delete-marker + // if present set the headers, no idea why AWS S3 sets these headers. + if objInfo.VersionID != "" && objInfo.DeleteMarker { + w.Header()[xhttp.AmzVersionID] = []string{objInfo.VersionID} + w.Header()[xhttp.AmzDeleteMarker] = []string{strconv.FormatBool(objInfo.DeleteMarker)} + } } + writeErrorResponseHeadersOnly(w, toAPIError(ctx, err)) + return } - writeErrorResponseHeadersOnly(w, toAPIError(ctx, err)) - return } // Automatically remove the object/version is an expiry lifecycle rule can be applied diff --git a/go.mod b/go.mod index e67e7a44e..2ee6f460e 100644 --- a/go.mod +++ b/go.mod @@ -49,7 +49,7 @@ require ( github.com/minio/cli v1.22.0 github.com/minio/highwayhash v1.0.1 github.com/minio/md5-simd v1.1.1 // indirect - github.com/minio/minio-go/v7 v7.0.8 + github.com/minio/minio-go/v7 v7.0.9-0.20210210235136-83423dddb072 github.com/minio/selfupdate v0.3.1 github.com/minio/sha256-simd v0.1.1 github.com/minio/simdjson-go v0.2.1 diff --git a/go.sum b/go.sum index b66792378..9267cc1c4 100644 --- a/go.sum +++ b/go.sum @@ -419,6 +419,8 @@ github.com/minio/md5-simd v1.1.1 h1:9ojcLbuZ4gXbB2sX53MKn8JUZ0sB/2wfwsEcRw+I08U= github.com/minio/md5-simd v1.1.1/go.mod h1:XpBqgZULrMYD3R+M28PcmP0CkI7PEMzB3U77ZrKZ0Gw= github.com/minio/minio-go/v7 v7.0.8 h1:snnHtYkHz3TKrQJY1jTQGOZqnue79pbYTukuwqz/QvM= github.com/minio/minio-go/v7 v7.0.8/go.mod h1:pEZBUa+L2m9oECoIA6IcSK8bv/qggtQVLovjeKK5jYc= +github.com/minio/minio-go/v7 v7.0.9-0.20210210235136-83423dddb072 h1:zlheLAzZ66jYLUsa81R8gwPtSgKRI5FMJyAKuaJpkHE= +github.com/minio/minio-go/v7 v7.0.9-0.20210210235136-83423dddb072/go.mod h1:pEZBUa+L2m9oECoIA6IcSK8bv/qggtQVLovjeKK5jYc= github.com/minio/selfupdate v0.3.1 h1:BWEFSNnrZVMUWXbXIgLDNDjbejkmpAmZvy/nCz1HlEs= github.com/minio/selfupdate v0.3.1/go.mod h1:b8ThJzzH7u2MkF6PcIra7KaXO9Khf6alWPvMSyTDCFM= github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU=