Merge remote-tracking branch 'origin/development/7.70' into user/jonathan/S3C-7352-proto

Pass the `getDeleteMarker` flag to the Metadata backend when the
Cloudserver operation requires to distinguish if the target is a
delete marker or does not exist, to set response header
"x-amz-delete-marker" or return a specific error code.

lib/api/apiUtils/object/createAndStoreObject.js

@@ -257,6 +257,7 @@ function createAndStoreObject(bucketName, bucketMD, objectKey, objMD, authInfo,
             metadataStoreParams.versionId = options.versionId;
             metadataStoreParams.versioning = options.versioning;
             metadataStoreParams.isNull = options.isNull;
+            metadataStoreParams.deleteNullKey = options.deleteNullKey;
             if (options.extraMD) {
                 Object.assign(metadataStoreParams, options.extraMD);
             }

lib/api/apiUtils/object/versioning.js

@@ -62,17 +62,34 @@ function checkQueryVersionId(query) {
     return undefined;
 }
 
-function _storeNullVersionMD(bucketName, objKey, objMD, options, log, cb) {
+function _storeNullVersionMD(bucketName, objKey, nullVersionId, objMD, log, cb) {
-    metadata.putObjectMD(bucketName, objKey, objMD, options, log, err => {
+    // In compatibility mode, create null versioned keys instead of null keys
+    let versionId;
+    let nullVersionMD;
+    if (config.nullVersionCompatMode) {
+        versionId = nullVersionId;
+        nullVersionMD = Object.assign({}, objMD, {
+            versionId: nullVersionId,
+            isNull: true,
+        });
+    } else {
+        versionId = 'null';
+        nullVersionMD = Object.assign({}, objMD, {
+            versionId: nullVersionId,
+            isNull: true,
+            isNull2: true,
+        });
+    }
+    metadata.putObjectMD(bucketName, objKey, nullVersionMD, { versionId }, log, err => {
         if (err) {
             log.debug('error from metadata storing null version as new version',
             { error: err });
         }
-        cb(err, options);
+        cb(err);
     });
 }
 
-/** get location of null version data for deletion
+/** check existence and get location of null version data for deletion
 * @param {string} bucketName - name of bucket
 * @param {string} objKey - name of object key
 * @param {object} options - metadata options for getting object MD
@@ -83,50 +100,54 @@ function _storeNullVersionMD(bucketName, objKey, objMD, options, log, cb) {
 * @param {function} cb - callback
 * @return {undefined} - and call callback with (err, dataToDelete)
 */
-function _getNullVersionsToDelete(bucketName, objKey, options, mst, log, cb) {
+function _prepareNullVersionDeletion(bucketName, objKey, options, mst, log, cb) {
+    const nullOptions = {};
+    if (!options.deleteData) {
+        return process.nextTick(cb, null, nullOptions);
+    }
     if (options.versionId === mst.versionId) {
-        // no need to get delete location, we already have the master's metadata
+        // no need to get another key as the master is the target
-        const dataToDelete = mst.objLocation;
+        nullOptions.dataToDelete = mst.objLocation;
-        return process.nextTick(cb, null, dataToDelete);
+        return process.nextTick(cb, null, nullOptions);
+    }
+    if (options.versionId === 'null') {
+        // deletion of the null key will be done by the main metadata
+        // PUT via this option
+        nullOptions.deleteNullKey = true;
     }
     return metadata.getObjectMD(bucketName, objKey, options, log,
         (err, versionMD) => {
+            // the null key may not exist, hence it's a normal
+            // situation to have a NoSuchKey error, in which case
+            // there is nothing to delete
+            if (err && err.is.NoSuchKey) {
+                return cb(null, {});
+            }
             if (err) {
-                log.debug('err from metadata getting specified version', {
+                log.warn('could not get null version metadata', {
                     error: err,
-                    method: '_getNullVersionsToDelete',
+                    method: '_prepareNullVersionDeletion',
                 });
                 return cb(err);
             }
-            if (!versionMD.location) {
+            if (versionMD.location) {
-                return cb();
+                const dataToDelete = Array.isArray(versionMD.location) ?
+                      versionMD.location : [versionMD.location];
+                nullOptions.dataToDelete = dataToDelete;
             }
-            const dataToDelete = Array.isArray(versionMD.location) ?
+            return cb(null, nullOptions);
-                versionMD.location : [versionMD.location];
-            return cb(null, dataToDelete);
         });
 }
 
-function _deleteNullVersionMD(bucketName, objKey, options, mst, log, cb) {
+function _deleteNullVersionMD(bucketName, objKey, options, log, cb) {
-    return _getNullVersionsToDelete(bucketName, objKey, options, mst, log,
+    return metadata.deleteObjectMD(bucketName, objKey, options, log, err => {
-        (err, nullDataToDelete) => {
+        if (err) {
-            if (err) {
+            log.warn('metadata error deleting null versioned key',
-                log.warn('could not find null version metadata', {
+                     { bucketName, objKey, error: err, method: '_deleteNullVersionMD' });
-                    error: err,
+            return cb(err);
-                    method: '_deleteNullVersionMD',
+        }
-                });
+        return cb();
-                return cb(err);
+    });
-            }
-            return metadata.deleteObjectMD(bucketName, objKey, options, log,
-                err => {
-                    if (err) {
-                        log.warn('metadata error deleting null version',
-                        { error: err, method: '_deleteNullVersionMD' });
-                        return cb(err);
-                    }
-                    return cb(null, nullDataToDelete);
-                });
-        });
 }
 
 /**
@@ -136,6 +157,10 @@ function _deleteNullVersionMD(bucketName, objKey, options, mst, log, cb) {
  * @param {object} mst - state of master version, as returned by
  * getMasterState()
  * @param {string} vstat - bucket versioning status: 'Enabled' or 'Suspended'
+ * @param {boolean} nullVersionCompatMode - if true, behaves in null
+ * version compatibility mode and return appropriate values: this mode
+ * does not attempt to create null keys but create null versioned keys
+ * instead
  *
  * @return {object} result object with the following attributes:
  * - {object} options: versioning-related options to pass to the
@@ -145,7 +170,7 @@ function _deleteNullVersionMD(bucketName, objKey, options, mst, log, cb) {
  * - {object} [delOptions]: options for metadata to delete the null
      version key, if needed
  */
-function processVersioningState(mst, vstat) {
+function processVersioningState(mst, vstat, nullVersionCompatMode) {
     const versioningSuspended = (vstat === 'Suspended');
     const masterIsNull = mst.exists && (mst.isNull || !mst.versionId);
 
@@ -157,38 +182,71 @@ function processVersioningState(mst, vstat) {
             if (mst.objLocation) {
                 options.dataToDelete = mst.objLocation;
             }
-            if (mst.isNull) {
+            // backward-compat: a null version key may exist even with
+            // a null master (due to S3C-7526), if so, delete it (its
+            // data will be deleted as part of the master cleanup, so
+            // no "deleteData" param is needed)
+            //
+            // "isNull2" attribute is set in master metadata when
+            // null keys are used, which is used as an optimization to
+            // avoid having to check the versioned key since there can
+            // be no more versioned key to clean up
+            if (mst.isNull && !mst.isNull2) {
                 const delOptions = { versionId: mst.versionId };
-                if (mst.uploadId) {
-                    delOptions.replayId = mst.uploadId;
-                }
                 return { options, delOptions };
             }
             return { options };
         }
         if (mst.nullVersionId) {
-            const delOptions = { versionId: mst.nullVersionId };
+            // backward-compat: delete the null versioned key and data
+            const delOptions = { versionId: mst.nullVersionId, deleteData: true };
             if (mst.nullUploadId) {
                 delOptions.replayId = mst.nullUploadId;
             }
             return { options, delOptions };
         }
-        return { options };
+        // clean up the eventual null key's location data prior to put
+
+        // NOTE: due to metadata v1 internal format, we cannot guess
+        // from the master key whether there is an associated null
+        // key, because the master key may be removed whenever the
+        // latest version becomes a delete marker. Hence we need to
+        // pessimistically try to get the null key metadata and delete
+        // it if it exists.
+        const delOptions = { versionId: 'null', deleteData: true };
+        return { options, delOptions };
     }
+
     // versioning is enabled: create a new version
     const options = { versioning: true };
     if (masterIsNull) {
-        // store master version in a new key
+        // if master is a null version or a non-versioned key,
+        // copy it to a new null key
         const nullVersionId = mst.isNull ? mst.versionId : nonVersionedObjId;
-        options.extraMD = {
+        if (nullVersionCompatMode) {
-            nullVersionId,
+            options.extraMD = {
-        };
+                nullVersionId,
-        if (mst.uploadId) {
+            };
-            options.extraMD.nullUploadId = mst.uploadId;
+            if (mst.uploadId) {
+                options.extraMD.nullUploadId = mst.uploadId;
+            }
+            return { options, nullVersionId };
+        }
+        if (mst.isNull && !mst.isNull2) {
+            // if master null version was put with an older
+            // Cloudserver (or in compat mode), there is a
+            // possibility that it also has a null versioned key
+            // associated, so we need to delete it as we write the
+            // null key
+            const delOptions = {
+                versionId: nullVersionId,
+            };
+            return { options, nullVersionId, delOptions };
         }
         return { options, nullVersionId };
     }
-    // versioning is enabled, copy reference to null version ID if it exists
+    // backward-compat: keep a reference to the existing null
+    // versioned key
     if (mst.nullVersionId) {
         options.extraMD = {
             nullVersionId: mst.nullVersionId,
@@ -222,6 +280,7 @@ function getMasterState(objMD) {
         versionId: objMD.versionId,
         uploadId: objMD.uploadId,
         isNull: objMD.isNull,
+        isNull2: objMD.isNull2,
         nullVersionId: objMD.nullVersionId,
         nullUploadId: objMD.nullUploadId,
     };
@@ -245,9 +304,6 @@ function getMasterState(objMD) {
  *  ('' overwrites the master version)
  * options.versioning - (true/undefined) metadata instruction to create new ver
  * options.isNull - (true/undefined) whether new version is null or not
- * options.nullVersionId - if storing a null version in version history, the
- *  version id of the null version
- * options.deleteNullVersionData - whether to delete the data of the null ver
  */
 function versioningPreprocessing(bucketName, bucketMD, objectKey, objMD,
     log, callback) {
@@ -260,39 +316,38 @@ function versioningPreprocessing(bucketName, bucketMD, objectKey, objMD,
     }
     // bucket is versioning configured
     const { options, nullVersionId, delOptions } =
-          processVersioningState(mst, vCfg.Status);
+          processVersioningState(mst, vCfg.Status, config.nullVersionCompatMode);
     return async.series([
-        function storeVersion(next) {
+        function storeNullVersionMD(next) {
             if (!nullVersionId) {
                 return process.nextTick(next);
             }
-            const versionMD = Object.assign({}, objMD, { versionId: nullVersionId, isNull: true });
+            return _storeNullVersionMD(bucketName, objectKey, nullVersionId, objMD, log, next);
-            const params = { versionId: nullVersionId };
-            return _storeNullVersionMD(bucketName, objectKey, versionMD, params, log, next);
         },
-        function deleteNullVersion(next) {
+        function prepareNullVersionDeletion(next) {
             if (!delOptions) {
                 return process.nextTick(next);
             }
-            return _deleteNullVersionMD(bucketName, objectKey, delOptions, mst,
+            return _prepareNullVersionDeletion(
-                log, (err, nullDataToDelete) => {
+                bucketName, objectKey, delOptions, mst, log,
+                (err, nullOptions) => {
                     if (err) {
-                        log.warn('unexpected error deleting null version md', {
+                        return next(err);
-                            error: err,
-                            method: 'versioningPreprocessing',
-                        });
-                        // it's possible there was a concurrent request to
-                        // delete the null version, so proceed with putting a
-                        // new version
-                        if (err.is.NoSuchKey) {
-                            return next(null, options);
-                        }
-                        return next(errors.InternalError);
                     }
-                    Object.assign(options, { dataToDelete: nullDataToDelete });
+                    Object.assign(options, nullOptions);
                     return next();
                 });
         },
+        function deleteNullVersionMD(next) {
+            if (delOptions &&
+                delOptions.versionId &&
+                delOptions.versionId !== 'null') {
+                // backward-compat: delete old null versioned key
+                return _deleteNullVersionMD(
+                    bucketName, objectKey, { versionId: delOptions.versionId }, log, next);
+            }
+            return process.nextTick(next);
+        },
     ], err => callback(err, options));
 }
 
@@ -302,12 +357,19 @@ function versioningPreprocessing(bucketName, bucketMD, objectKey, objMD,
  * @param {object} bucketMD - bucket metadata
  * @param {object} objectMD - obj metadata
  * @param {string} [reqVersionId] - specific version ID sent as part of request
+ * @param {boolean} nullVersionCompatMode - if true, behaves in null
+ * version compatibility mode and return appropriate values:
+ * - in normal mode, returns an 'isNull' boolean sent to Metadata (true or false)
+ * - in compatibility mode, does not return an 'isNull' property
  * @return {object} options object with params:
- * options.deleteData - (true/undefined) whether to delete data (if undefined
+ * {boolean} [options.deleteData=true|undefined] - whether to delete data (if undefined
  *  means creating a delete marker instead)
- * options.versionId - specific versionId to delete
+ * {string} [options.versionId] - specific versionId to delete
+ * {boolean} [options.isNull=true|false|undefined] - if set, tells the
+ * Metadata backend if we're deleting a null version or not a null
+ * version. Not set if `nullVersionCompatMode` is true.
  */
-function preprocessingVersioningDelete(bucketName, bucketMD, objectMD, reqVersionId) {
+function preprocessingVersioningDelete(bucketName, bucketMD, objectMD, reqVersionId, nullVersionCompatMode) {
     const options = {};
     if (!bucketMD.getVersioningConfiguration() || reqVersionId) {
         // delete data if bucket is non-versioned or the request
@@ -316,14 +378,31 @@ function preprocessingVersioningDelete(bucketName, bucketMD, objectMD, reqVersio
     }
     if (bucketMD.getVersioningConfiguration() && reqVersionId) {
         if (reqVersionId === 'null') {
-            // deleting the 'null' version if it exists: use its
+            // deleting the 'null' version if it exists:
-            // internal versionId if it exists
+            //
+            // - use its internal versionId if it is a "real" null
+            //   version (not non-versioned)
+            //
+            // - send the "isNull" param to Metadata if:
+            //
+            //   - in non-compat mode (mandatory for v1 format)
+            //
+            //   - OR if the version is already a null key put by a
+            //     non-compat mode Cloudserver, to let Metadata know that
+            //     the null key is to be deleted. This is the case if the
+            //     "isNull2" param is set.
             if (objectMD.versionId !== undefined) {
                 options.versionId = objectMD.versionId;
+                if (objectMD.isNull2) {
+                    options.isNull = true;
+                }
             }
         } else {
             // deleting a specific version
             options.versionId = reqVersionId;
+            if (!nullVersionCompatMode) {
+                options.isNull = false;
+            }
         }
     }
     return options;

lib/api/completeMultipartUpload.js

@@ -337,6 +337,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
                     metaStoreParams.versionId = options.versionId;
                     metaStoreParams.versioning = options.versioning;
                     metaStoreParams.isNull = options.isNull;
+                    metaStoreParams.deleteNullKey = options.deleteNullKey;
                     if (options.extraMD) {
                         Object.assign(metaStoreParams, options.extraMD);
                     }

lib/api/multiObjectDelete.js

@@ -285,7 +285,8 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request,
                 return callback(null, objMD, versionId);
             },
             (objMD, versionId, callback) => {
-                const options = preprocessingVersioningDelete(bucketName, bucket, objMD, versionId);
+                const options = preprocessingVersioningDelete(
+                    bucketName, bucket, objMD, versionId, config.nullVersionCompatMode);
                 const deleteInfo = {};
                 if (options && options.deleteData) {
                     deleteInfo.deleted = true;

lib/api/objectCopy.js

@@ -214,6 +214,7 @@ function objectCopy(authInfo, request, sourceBucket,
         bucketName: sourceBucket,
         objectKey: sourceObject,
         versionId: sourceVersionId,
+        getDeleteMarker: true,
         requestType: 'objectGet',
         request,
     };

lib/api/objectDelete.js

@@ -138,11 +138,10 @@ function objectDelete(authInfo, request, log, cb) {
         },
         function deleteOperation(bucketMD, objectMD, next) {
             const delOptions = preprocessingVersioningDelete(
-                bucketName, bucketMD, objectMD, reqVersionId);
+                bucketName, bucketMD, objectMD, reqVersionId, config.nullVersionCompatMode);
             const deleteInfo = {
                 removeDeleteMarker: false,
                 newDeleteMarker: false,
-                isNull: delOptions.isNull,
             };
             if (delOptions && delOptions.deleteData) {
                 if (objectMD.isDeleteMarker) {

lib/api/objectDeleteTagging.js

@@ -11,6 +11,7 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 const metadata = require('../metadata/wrapper');
 const getReplicationInfo = require('./apiUtils/object/getReplicationInfo');
 const { data } = require('../data/wrapper');
+const { config } = require('../Config');
 const REPLICATION_ACTION = 'DELETE_TAGGING';
 
 /**
@@ -41,8 +42,9 @@ function objectDeleteTagging(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectDeleteTagging',
         versionId: reqVersionId,
+        getDeleteMarker: true,
+        requestType: 'objectDeleteTagging',
         request,
     };
 
@@ -64,6 +66,8 @@ function objectDeleteTagging(authInfo, request, log, callback) {
               if (objectMD.isDeleteMarker) {
                   log.trace('version is a delete marker',
                   { method: 'objectDeleteTagging' });
+                  // FIXME wee should return a `x-amz-delete-marker: true` header,
+                  // see S3C-7592
                   return next(errors.MethodNotAllowed, bucket);
               }
               return next(null, bucket, objectMD);
@@ -71,8 +75,13 @@ function objectDeleteTagging(authInfo, request, log, callback) {
         (bucket, objectMD, next) => {
             // eslint-disable-next-line no-param-reassign
             objectMD.tags = {};
-            const params = objectMD.versionId ? { versionId:
+            const params = {};
-              objectMD.versionId } : {};
+            if (objectMD.versionId) {
+                params.versionId = objectMD.versionId;
+                if (!config.nullVersionCompatMode) {
+                    params.isNull = objectMD.isNull || false;
+                }
+            }
             const replicationInfo = getReplicationInfo(objectKey, bucket, true,
                 0, REPLICATION_ACTION, objectMD);
             if (replicationInfo) {

lib/api/objectGet.js

@@ -45,6 +45,7 @@ function objectGet(authInfo, request, returnTagCount, log, callback) {
         bucketName,
         objectKey,
         versionId,
+        getDeleteMarker: true,
         requestType: 'objectGet',
         request,
     };

lib/api/objectGetACL.js

@@ -54,6 +54,8 @@ function objectGetACL(authInfo, request, log, callback) {
     }
     const versionId = decodedVidResult;
 
+    // FIXME pass 'getDeleteMarker: true' option to set
+    // 'x-amz-delete-marker' header (see S3C-7592)
     const metadataValParams = {
         authInfo,
         bucketName,
@@ -90,10 +92,14 @@ function objectGetACL(authInfo, request, log, callback) {
                         if (versionId) {
                             log.trace('requested version is delete marker',
                                 { method: 'objectGetACL' });
+                            // FIXME wee should return a `x-amz-delete-marker: true` header,
+                            // see S3C-7592
                             return next(errors.MethodNotAllowed);
                         }
                         log.trace('most recent version is delete marker',
                             { method: 'objectGetACL' });
+                        // FIXME wee should return a `x-amz-delete-marker: true` header,
+                        // see S3C-7592
                         return next(errors.NoSuchKey);
                     }
                     return next(null, bucket, objectMD);

lib/api/objectGetLegalHold.js

@@ -33,12 +33,14 @@ function objectGetLegalHold(authInfo, request, log, callback) {
     }
     const versionId = decodedVidResult;
 
+    // FIXME pass 'getDeleteMarker: true' option to set
+    // 'x-amz-delete-marker' header (see S3C-7592)
     const metadataValParams = {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectGetLegalHold',
         versionId,
+        requestType: 'objectGetLegalHold',
         request,
     };
 
@@ -61,10 +63,14 @@ function objectGetLegalHold(authInfo, request, log, callback) {
                     if (versionId) {
                         log.trace('requested version is delete marker',
                             { method: 'objectGetLegalHold' });
+                        // FIXME wee should return a `x-amz-delete-marker: true` header,
+                        // see S3C-7592
                         return next(errors.MethodNotAllowed);
                     }
                     log.trace('most recent version is delete marker',
                         { method: 'objectGetLegalHold' });
+                    // FIXME wee should return a `x-amz-delete-marker: true` header,
+                    // see S3C-7592
                     return next(errors.NoSuchKey);
                 }
                 if (!bucket.isObjectLockEnabled()) {

lib/api/objectGetRetention.js

@@ -33,12 +33,14 @@ function objectGetRetention(authInfo, request, log, callback) {
     }
     const reqVersionId = decodedVidResult;
 
+    // FIXME pass 'getDeleteMarker: true' option to set
+    // 'x-amz-delete-marker' header (see S3C-7592)
     const metadataValParams = {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectGetRetention',
         versionId: reqVersionId,
+        requestType: 'objectGetRetention',
         request,
     };
 
@@ -61,10 +63,14 @@ function objectGetRetention(authInfo, request, log, callback) {
                     if (reqVersionId) {
                         log.trace('requested version is delete marker',
                             { method: 'objectGetRetention' });
+                        // FIXME wee should return a `x-amz-delete-marker: true` header,
+                        // see S3C-7592
                         return next(errors.MethodNotAllowed);
                     }
                     log.trace('most recent version is delete marker',
                         { method: 'objectGetRetention' });
+                    // FIXME wee should return a `x-amz-delete-marker: true` header,
+                    // see S3C-7592
                     return next(errors.NoSuchKey);
                 }
                 if (!bucket.isObjectLockEnabled()) {

lib/api/objectGetTagging.js

@@ -34,12 +34,14 @@ function objectGetTagging(authInfo, request, log, callback) {
     }
     const reqVersionId = decodedVidResult;
 
+    // FIXME pass 'getDeleteMarker: true' option to set
+    // 'x-amz-delete-marker' header (see S3C-7592)
     const metadataValParams = {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectGetTagging',
         versionId: reqVersionId,
+        requestType: 'objectGetTagging',
         request,
     };
 
@@ -63,9 +65,13 @@ function objectGetTagging(authInfo, request, log, callback) {
                       log.trace('requested version is delete marker',
                       { method: 'objectGetTagging' });
                       return next(errors.MethodNotAllowed);
+                      // FIXME wee should return a `x-amz-delete-marker: true` header,
+                      // see S3C-7592
                   }
                   log.trace('most recent version is delete marker',
                   { method: 'objectGetTagging' });
+                  // FIXME wee should return a `x-amz-delete-marker: true` header,
+                  // see S3C-7592
                   return next(errors.NoSuchKey);
               }
               return next(null, bucket, objectMD);

lib/api/objectHead.js

@@ -45,6 +45,7 @@ function objectHead(authInfo, request, log, callback) {
         bucketName,
         objectKey,
         versionId,
+        getDeleteMarker: true,
         requestType: 'objectHead',
         request,
     };

lib/api/objectPutACL.js

@@ -11,6 +11,7 @@ const { decodeVersionId, getVersionIdResHeader }
     = require('./apiUtils/object/versioning');
 const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
 const monitoring = require('../utilities/metrics');
+const { config } = require('../Config');
 
 /*
    Format of xml request:
@@ -85,8 +86,9 @@ function objectPutACL(authInfo, request, log, cb) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectPutACL',
         versionId: reqVersionId,
+        getDeleteMarker: true,
+        requestType: 'objectPutACL',
     };
 
     const possibleGrants = ['FULL_CONTROL', 'WRITE_ACP', 'READ', 'READ_ACP'];
@@ -123,13 +125,14 @@ function objectPutACL(authInfo, request, log, cb) {
                     if (objectMD.isDeleteMarker) {
                         log.trace('delete marker detected',
                         { method: 'objectPutACL' });
+                        // FIXME wee should return a `x-amz-delete-marker: true` header,
+                        // see S3C-7592
                         return next(errors.MethodNotAllowed, bucket);
                     }
                     return next(null, bucket, objectMD);
                 });
         },
         function parseAclFromXml(bucket, objectMD, next) {
-            metadataValParams.versionId = objectMD.versionId;
             // If not setting acl through headers, parse body
             let jsonGrants;
             let aclOwnerID;
@@ -278,8 +281,13 @@ function objectPutACL(authInfo, request, log, cb) {
         },
         function addAclsToObjMD(bucket, objectMD, ACLParams, next) {
             // Add acl's to object metadata
-            const params = metadataValParams.versionId ?
+            const params = {};
-                { versionId: metadataValParams.versionId } : {};
+            if (objectMD.versionId) {
+                params.versionId = objectMD.versionId;
+                if (!config.nullVersionCompatMode) {
+                    params.isNull = objectMD.isNull || false;
+                }
+            }
             acl.addObjectACL(bucket, objectKey, objectMD,
                 ACLParams, params, log, err => next(err, bucket, objectMD));
         },

lib/api/objectPutCopyPart.js

@@ -44,6 +44,7 @@ function objectPutCopyPart(authInfo, request, sourceBucket,
         bucketName: sourceBucket,
         objectKey: sourceObject,
         versionId: reqVersionId,
+        getDeleteMarker: true,
         requestType: 'objectGet',
         request,
     };

lib/api/objectPutLegalHold.js

@@ -8,6 +8,7 @@ const getReplicationInfo = require('./apiUtils/object/getReplicationInfo');
 const metadata = require('../metadata/wrapper');
 const { metadataValidateBucketAndObj } = require('../metadata/metadataUtils');
 const { pushMetric } = require('../utapi/utilities');
+const { config } = require('../Config');
 
 const { parseLegalHoldXml } = s3middleware.objectLegalHold;
 
@@ -40,8 +41,9 @@ function objectPutLegalHold(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectPutLegalHold',
         versionId,
+        getDeleteMarker: true,
+        requestType: 'objectPutLegalHold',
         request,
     };
 
@@ -63,6 +65,8 @@ function objectPutLegalHold(authInfo, request, log, callback) {
             if (objectMD.isDeleteMarker) {
                 log.trace('version is a delete marker',
                     { method: 'objectPutLegalHold' });
+                // FIXME wee should return a `x-amz-delete-marker: true` header,
+                // see S3C-7592
                 return next(errors.MethodNotAllowed, bucket);
             }
             if (!bucket.isObjectLockEnabled()) {
@@ -82,8 +86,13 @@ function objectPutLegalHold(authInfo, request, log, callback) {
         (bucket, legalHold, objectMD, next) => {
             // eslint-disable-next-line no-param-reassign
             objectMD.legalHold = legalHold;
-            const params = objectMD.versionId ?
+            const params = {};
-                { versionId: objectMD.versionId } : {};
+            if (objectMD.versionId) {
+                params.versionId = objectMD.versionId;
+                if (!config.nullVersionCompatMode) {
+                    params.isNull = objectMD.isNull || false;
+                }
+            }
             const replicationInfo = getReplicationInfo(objectKey, bucket, true,
                 0, REPLICATION_ACTION, objectMD);
             if (replicationInfo) {

lib/api/objectPutRetention.js

@@ -10,6 +10,7 @@ const { pushMetric } = require('../utapi/utilities');
 const getReplicationInfo = require('./apiUtils/object/getReplicationInfo');
 const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 const metadata = require('../metadata/wrapper');
+const { config } = require('../Config');
 
 const { parseRetentionXml } = s3middleware.retention;
 const REPLICATION_ACTION = 'PUT_RETENTION';
@@ -41,8 +42,9 @@ function objectPutRetention(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectPutRetention',
         versionId: reqVersionId,
+        getDeleteMarker: true,
+        requestType: 'objectPutRetention',
         request,
     };
 
@@ -64,6 +66,8 @@ function objectPutRetention(authInfo, request, log, callback) {
             if (objectMD.isDeleteMarker) {
                 log.trace('version is a delete marker',
                     { method: 'objectPutRetention' });
+                // FIXME wee should return a `x-amz-delete-marker: true` header,
+                // see S3C-7592
                 return next(errors.MethodNotAllowed, bucket);
             }
             if (!bucket.isObjectLockEnabled()) {
@@ -112,8 +116,13 @@ function objectPutRetention(authInfo, request, log, callback) {
             /* eslint-disable no-param-reassign */
             objectMD.retentionMode = retentionInfo.mode;
             objectMD.retentionDate = retentionInfo.date;
-            const params = objectMD.versionId ?
+            const params = {};
-                { versionId: objectMD.versionId } : {};
+            if (objectMD.versionId) {
+                params.versionId = objectMD.versionId;
+                if (!config.nullVersionCompatMode) {
+                    params.isNull = objectMD.isNull || false;
+                }
+            }
             const replicationInfo = getReplicationInfo(objectKey, bucket, true,
                 0, REPLICATION_ACTION, objectMD);
             if (replicationInfo) {

lib/api/objectPutTagging.js

@@ -12,6 +12,7 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders');
 const metadata = require('../metadata/wrapper');
 const { data } = require('../data/wrapper');
 const { parseTagXml } = s3middleware.tagging;
+const { config } = require('../Config');
 const REPLICATION_ACTION = 'PUT_TAGGING';
 
 /**
@@ -42,8 +43,9 @@ function objectPutTagging(authInfo, request, log, callback) {
         authInfo,
         bucketName,
         objectKey,
-        requestType: 'objectPutTagging',
         versionId: reqVersionId,
+        getDeleteMarker: true,
+        requestType: 'objectPutTagging',
         request,
     };
 
@@ -65,6 +67,8 @@ function objectPutTagging(authInfo, request, log, callback) {
               if (objectMD.isDeleteMarker) {
                   log.trace('version is a delete marker',
                   { method: 'objectPutTagging' });
+                  // FIXME wee should return a `x-amz-delete-marker: true` header,
+                  // see S3C-7592
                   return next(errors.MethodNotAllowed, bucket);
               }
               return next(null, bucket, objectMD);
@@ -77,8 +81,13 @@ function objectPutTagging(authInfo, request, log, callback) {
         (bucket, tags, objectMD, next) => {
             // eslint-disable-next-line no-param-reassign
             objectMD.tags = tags;
-            const params = objectMD.versionId ? { versionId:
+            const params = {};
-              objectMD.versionId } : {};
+            if (objectMD.versionId) {
+                params.versionId = objectMD.versionId;
+                if (!config.nullVersionCompatMode) {
+                    params.isNull = objectMD.isNull || false;
+                }
+            }
             const replicationInfo = getReplicationInfo(objectKey, bucket, true,
                 0, REPLICATION_ACTION, objectMD);
             if (replicationInfo) {

lib/metadata/metadataUtils.js

@@ -66,7 +66,7 @@ function getNullVersionFromMaster(bucketName, objectKey, log, cb) {
  */
 function metadataGetObject(bucketName, objectKey, versionId, log, cb) {
     // versionId may be 'null', which asks metadata to fetch the null key specifically
-    const options = { versionId };
+    const options = { versionId, getDeleteMarker: true };
     return metadata.getObjectMD(bucketName, objectKey, options, log,
         (err, objMD) => {
             if (err) {
@@ -140,11 +140,15 @@ function validateBucket(bucket, params, log) {
  * @return {undefined} - and call callback with params err, bucket md
  */
 function metadataValidateBucketAndObj(params, log, callback) {
-    const { authInfo, bucketName, objectKey, versionId, requestType, request } = params;
+    const { authInfo, bucketName, objectKey, versionId, getDeleteMarker,
+            requestType, request } = params;
     async.waterfall([
         next => {
             // versionId may be 'null', which asks metadata to fetch the null key specifically
             const getOptions = { versionId };
+            if (getDeleteMarker) {
+                getOptions.getDeleteMarker = true;
+            }
             return metadata.getBucketAndObjectMD(bucketName, objectKey, getOptions, log, next);
         },
         (getResult, next) => {

lib/services.js

@@ -7,6 +7,7 @@ const ObjectMD = require('arsenal').models.ObjectMD;
 const BucketInfo = require('arsenal').models.BucketInfo;
 const acl = require('./metadata/acl');
 const constants = require('../constants');
+const { config } = require('./Config');
 const { data } = require('./data/wrapper');
 const metadata = require('./metadata/wrapper');
 const logger = require('./utilities/logger');
@@ -97,7 +98,7 @@ const services = {
             lastModifiedDate, versioning, versionId, uploadId,
             tagging, taggingCopy, replicationInfo, defaultRetention,
             dataStoreName, retentionMode, retentionDate, legalHold,
-            originOp, oldReplayId } = params;
+            originOp, oldReplayId, deleteNullKey } = params;
         log.trace('storing object in metadata');
         assert.strictEqual(typeof bucketName, 'string');
         const md = new ObjectMD();
@@ -165,8 +166,15 @@ const services = {
             options.oldReplayId = oldReplayId;
         }
 
+        if (deleteNullKey) {
+            options.deleteNullKey = deleteNullKey;
+        }
+
         // information to store about the version and the null version id
         // in the object metadata
+
+        // NOTE nullVersionId and nullUploadId are only maintained in
+        // v0 metadata compatibility mode
         const { isNull, nullVersionId, nullUploadId, isDeleteMarker } = params;
         md.setIsNull(isNull)
             .setNullVersionId(nullVersionId)
@@ -175,6 +183,9 @@ const services = {
         if (versionId && versionId !== 'null') {
             md.setVersionId(versionId);
         }
+        if (isNull && !config.nullVersionCompatMode) {
+            md.setIsNull2(true);
+        }
         if (taggingCopy) {
             // If copying tags to an object (taggingCopy) we do not
             // need to validate them again

package.json

@@ -20,7 +20,7 @@
   "homepage": "https://github.com/scality/S3#readme",
   "dependencies": {
     "@hapi/joi": "^17.1.0",
-    "arsenal": "git+https://github.com/scality/arsenal#7.10.46",
+    "arsenal": "git+https://github.com/scality/arsenal#4c39ef64",
     "async": "~2.5.0",
     "aws-sdk": "2.905.0",
     "azure-storage": "^2.1.0",

tests/functional/aws-node-sdk/lib/utility/versioning-util.js

@@ -82,8 +82,17 @@ function enableVersioningThenPutObject(bucket, object, callback) {
     });
 }
 
-/** createDualNullVersion - create a null version that is stored in metadata
+/** createDualNullVersion
- *  both in the master version and a separate version
+ *
+ * - PREVIOUSLY: created a null version that was stored in metadata
+ *   both in the master version and a separate version
+ *
+ * - CURRENTLY: only one null version key is present in metadata
+ *   after the second put, as the first null key is cleaned up
+ *
+ *  Even though there is only one key at the end, it is still useful
+ *  to keep this function for regression testing
+ *
  *  @param {AWS.S3} s3 - aws sdk s3 instance
  *  @param {string} bucketName - name of bucket in versioning suspended state
  *  @param {string} keyName - name of key

tests/functional/aws-node-sdk/test/object/getObjectLegalHold.js

@@ -107,6 +107,19 @@ describe('GET object legal hold', () => {
             });
         });
 
+        it('should return NoSuchKey if latest version is delete marker', done => {
+            s3.deleteObject({ Bucket: bucket, Key: key }, err => {
+                assert.ifError(err);
+                s3.getObjectLegalHold({
+                    Bucket: bucket,
+                    Key: key,
+                }, err => {
+                    checkError(err, 'NoSuchKey', 404);
+                    done();
+                });
+            });
+        });
+
         it('should return InvalidRequest error getting legal hold of object ' +
             'inside object lock disabled bucket', done => {
             s3.getObjectLegalHold({

tests/functional/aws-node-sdk/test/versioning/objectCopy.js

@@ -688,7 +688,7 @@ describe('Object Version Copy', () => {
             });
 
         it('should return NoSuchKey if attempt to copy version with ' +
-        ' delete marker', done => {
+        'delete marker', done => {
             s3.deleteObject({
                 Bucket: sourceBucketName,
                 Key: sourceObjName,

tests/functional/aws-node-sdk/test/versioning/objectDelete.js

@@ -138,7 +138,7 @@ describe('aws-node-sdk test delete object', () => {
         // delete bucket after testing
         after(done => {
             removeAllVersions({ Bucket: bucket }, err => {
-                if (err.code === 'NoSuchBucket') {
+                if (err && err.code === 'NoSuchBucket') {
                     return done();
                 } else if (err) {
                     return done(err);
@@ -348,7 +348,7 @@ describe('aws-node-sdk test delete object', () => {
             });
         });
 
-        it('should delete the versionned object', done => {
+        it('should delete the versioned object', done => {
             const version = versionIds.shift();
             s3.deleteObject({
                 Bucket: bucket,
@@ -370,12 +370,15 @@ describe('aws-node-sdk test delete object', () => {
                 Bucket: bucket,
                 Key: key,
                 VersionId: version,
-            }, (err, res) => {
+            }, function test(err, res) {
                 if (err) {
                     return done(err);
                 }
                 assert.strictEqual(res.VersionId, version);
                 assert.equal(res.DeleteMarker, true);
+                // deleting a delete marker should set the x-amz-delete-marker header
+                const headers = this.httpResponse.headers;
+                assert.strictEqual(headers['x-amz-delete-marker'], 'true');
                 return done();
             });
         });

tests/functional/aws-node-sdk/test/versioning/objectDeleteTagging.js

@@ -131,7 +131,7 @@ describe('Delete object tagging with versioning', () => {
             });
         });
 
-        it('should return 405 MethodNotAllowed deletting tag set without ' +
+        it('should return 405 MethodNotAllowed deleting tag set without ' +
          'version id if version specified is a delete marker', done => {
             async.waterfall([
                 next => s3.putBucketVersioning({ Bucket: bucketName,

tests/functional/aws-node-sdk/test/versioning/objectGet.js

@@ -137,9 +137,11 @@ describe('get behavior on versioning-enabled bucket', () => {
                 s3.getObject({
                     Bucket: bucket,
                     Key: key,
-                    VersionId: this.test.deleteVersionId },
+                    VersionId: this.test.deleteVersionId,
-                err => {
+                }, function test1(err) {
                     _assertError(err, 405, 'MethodNotAllowed');
+                    const headers = this.httpResponse.headers;
+                    assert.strictEqual(headers['x-amz-delete-marker'], 'true');
                     done();
                 });
             });
@@ -148,9 +150,11 @@ describe('get behavior on versioning-enabled bucket', () => {
             'latest version is a delete marker', done => {
                 s3.getObject({
                     Bucket: bucket,
-                    Key: key },
+                    Key: key,
-                err => {
+                }, function test2(err) {
                     _assertError(err, 404, 'NoSuchKey');
+                    const headers = this.httpResponse.headers;
+                    assert.strictEqual(headers['x-amz-delete-marker'], 'true');
                     done();
                 });
             });
@@ -175,9 +179,11 @@ describe('get behavior on versioning-enabled bucket', () => {
                 s3.getObject({
                     Bucket: bucket,
                     Key: key,
-                    VersionId: this.test.deleteVersionId },
+                    VersionId: this.test.deleteVersionId,
-                err => {
+                }, function test3(err) {
                     _assertError(err, 405, 'MethodNotAllowed');
+                    const headers = this.httpResponse.headers;
+                    assert.strictEqual(headers['x-amz-delete-marker'], 'true');
                     done();
                 });
             });
@@ -200,9 +206,11 @@ describe('get behavior on versioning-enabled bucket', () => {
             done => {
                 s3.getObject({
                     Bucket: bucket,
-                    Key: key },
+                    Key: key,
-                err => {
+                }, function test4(err) {
                     _assertError(err, 404, 'NoSuchKey');
+                    const headers = this.httpResponse.headers;
+                    assert.strictEqual(headers['x-amz-delete-marker'], 'true');
                     done();
                 });
             });

tests/functional/aws-node-sdk/test/versioning/objectPut.js

@@ -218,7 +218,7 @@ describe('put and get object with versioning', function testSuite() {
                 });
             });
 
-            it('should create new versions but still keep nullVersionId',
+            it('should create new versions but still keep the null version',
             done => {
                 const versionIds = [];
                 const params = { Bucket: bucket, Key: key };

tests/unit/api/apiUtils/versioning.js

@@ -24,6 +24,25 @@ describe('versioning helpers', () => {
                         isNull: true,
                         versionId: '',
                     },
+                    delOptions: {
+                        deleteData: true,
+                        versionId: 'null',
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                    },
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    delOptions: {
+                        deleteData: true,
+                        versionId: 'null',
+                    },
                 },
             },
             {
@@ -41,6 +60,25 @@ describe('versioning helpers', () => {
                         isNull: true,
                         versionId: '',
                     },
+                    delOptions: {
+                        deleteData: true,
+                        versionId: 'null',
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                    },
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    delOptions: {
+                        deleteData: true,
+                        versionId: 'null',
+                    },
                 },
             },
             {
@@ -59,10 +97,29 @@ describe('versioning helpers', () => {
                         isNull: true,
                         versionId: '',
                     },
+                    delOptions: {
+                        deleteData: true,
+                        versionId: 'null',
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                    },
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    delOptions: {
+                        deleteData: true,
+                        versionId: 'null',
+                    },
                 },
             },
             {
-                description: 'prior null object version exists',
+                description: 'prior legacy null object version exists',
                 objMD: {
                     versionId: 'vnull',
                     isNull: true,
@@ -70,13 +127,15 @@ describe('versioning helpers', () => {
                 versioningEnabledExpectedRes: {
                     options: {
                         versioning: true,
-                        extraMD: {
-                            nullVersionId: 'vnull',
-                        },
                     },
                     // instruct to first copy the null version onto a
-                    // newly created version key preserving the version ID
+                    // newly created null key with version ID in its metadata
                     nullVersionId: 'vnull',
+                    // delete possibly existing null versioned key
+                    // that is identical to the null master
+                    delOptions: {
+                        versionId: 'vnull',
+                    },
                 },
                 versioningSuspendedExpectedRes: {
                     options: {
@@ -87,15 +146,96 @@ describe('versioning helpers', () => {
                         versionId: 'vnull',
                     },
                 },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                        extraMD: {
+                            nullVersionId: 'vnull',
+                        },
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created version key preserving the version ID
+                    nullVersionId: 'vnull',
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    delOptions: {
+                        versionId: 'vnull',
+                    },
+                },
             },
             {
-                description: 'prior MPU object null version exists',
+                description: 'prior non-legacy null object version exists',
+                objMD: {
+                    versionId: 'vnull',
+                    isNull: true,
+                    isNull2: true, // flag marking that it's a non-legacy null version
+                },
+                versioningEnabledExpectedRes: {
+                    options: {
+                        versioning: true,
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created null key with version ID in its metadata
+                    nullVersionId: 'vnull',
+                },
+                versioningSuspendedExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                        extraMD: {
+                            nullVersionId: 'vnull',
+                        },
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created version key preserving the version ID
+                    nullVersionId: 'vnull',
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                },
+            },
+            {
+                description: 'prior MPU object legacy null version exists',
                 objMD: {
                     versionId: 'vnull',
                     isNull: true,
                     uploadId: 'fooUploadId',
                 },
                 versioningEnabledExpectedRes: {
+                    options: {
+                        versioning: true,
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created null key with version ID in its metadata
+                    nullVersionId: 'vnull',
+                    // delete possibly existing null versioned key
+                    // that is identical to the null master
+                    delOptions: {
+                        versionId: 'vnull',
+                    },
+                },
+                versioningSuspendedExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    delOptions: {
+                        versionId: 'vnull',
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
                     options: {
                         versioning: true,
                         extraMD: {
@@ -107,30 +247,66 @@ describe('versioning helpers', () => {
                     // newly created version key preserving the version ID
                     nullVersionId: 'vnull',
                 },
-                versioningSuspendedExpectedRes: {
+                versioningSuspendedCompatExpectedRes: {
                     options: {
                         isNull: true,
                         versionId: '',
                     },
                     delOptions: {
                         versionId: 'vnull',
-                        replayId: 'fooUploadId',
                     },
                 },
             },
             {
-                description:
+                description: 'prior MPU object non-legacy null version exists',
-                'prior object exists, put before versioning was first enabled',
+                objMD: {
+                    versionId: 'vnull',
+                    isNull: true,
+                    isNull2: true, // flag marking that it's a non-legacy null version
+                    uploadId: 'fooUploadId',
+                },
+                versioningEnabledExpectedRes: {
+                    options: {
+                        versioning: true,
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created null key with version ID in its metadata
+                    nullVersionId: 'vnull',
+                },
+                versioningSuspendedExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                        extraMD: {
+                            nullVersionId: 'vnull',
+                            nullUploadId: 'fooUploadId',
+                        },
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created version key preserving the version ID
+                    nullVersionId: 'vnull',
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                },
+            },
+            {
+                description: 'prior object exists, put before versioning was first enabled',
                 objMD: {},
                 versioningEnabledExpectedRes: {
                     options: {
                         versioning: true,
-                        extraMD: {
-                            nullVersionId: INF_VID,
-                        },
                     },
                     // instruct to first copy the null version onto a
-                    // newly created version key as the oldest version
+                    // newly created null key as the oldest version
                     nullVersionId: INF_VID,
                 },
                 versioningSuspendedExpectedRes: {
@@ -139,14 +315,44 @@ describe('versioning helpers', () => {
                         versionId: '',
                     },
                 },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                        extraMD: {
+                            nullVersionId: INF_VID,
+                        },
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created version key as the oldest version
+                    nullVersionId: INF_VID,
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                },
             },
             {
-                description: 'prior MPU object exists, put before versioning ' +
+                description: 'prior MPU object exists, put before versioning was first enabled',
-                    'was first enabled',
                 objMD: {
                     uploadId: 'fooUploadId',
                 },
                 versioningEnabledExpectedRes: {
+                    options: {
+                        versioning: true,
+                    },
+                    // instruct to first copy the null version onto a
+                    // newly created null key as the oldest version
+                    nullVersionId: INF_VID,
+                },
+                versioningSuspendedExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
                     options: {
                         versioning: true,
                         extraMD: {
@@ -158,7 +364,7 @@ describe('versioning helpers', () => {
                     // newly created version key as the oldest version
                     nullVersionId: INF_VID,
                 },
-                versioningSuspendedExpectedRes: {
+                versioningSuspendedCompatExpectedRes: {
                     options: {
                         isNull: true,
                         versionId: '',
@@ -166,8 +372,7 @@ describe('versioning helpers', () => {
                 },
             },
             {
-                description:
+                description: 'prior non-null object version exists with ref to null version',
-                'prior non-null object version exists with ref to null version',
                 objMD: {
                     versionId: 'v1',
                     nullVersionId: 'vnull',
@@ -185,14 +390,34 @@ describe('versioning helpers', () => {
                         isNull: true,
                         versionId: '',
                     },
+                    // backward-compat: delete old null version key
                     delOptions: {
                         versionId: 'vnull',
+                        deleteData: true,
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                        extraMD: {
+                            nullVersionId: 'vnull',
+                        },
+                    },
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    // backward-compat: delete old null version key
+                    delOptions: {
+                        versionId: 'vnull',
+                        deleteData: true,
                     },
                 },
             },
             {
-                description: 'prior MPU object non-null version exists with ' +
+                description: 'prior MPU object non-null version exists with ref to null version',
-                    'ref to null version',
                 objMD: {
                     versionId: 'v1',
                     uploadId: 'fooUploadId',
@@ -211,14 +436,34 @@ describe('versioning helpers', () => {
                         isNull: true,
                         versionId: '',
                     },
+                    // backward-compat: delete old null version key
                     delOptions: {
                         versionId: 'vnull',
+                        deleteData: true,
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                        extraMD: {
+                            nullVersionId: 'vnull',
+                        },
+                    },
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    // backward-compat: delete old null version key
+                    delOptions: {
+                        versionId: 'vnull',
+                        deleteData: true,
                     },
                 },
             },
             {
-                description: 'prior object non-null version exists with ' +
+                description: 'prior object non-null version exists with ref to MPU null version',
-                    'ref to MPU null version',
                 objMD: {
                     versionId: 'v1',
                     nullVersionId: 'vnull',
@@ -238,21 +483,48 @@ describe('versioning helpers', () => {
                         isNull: true,
                         versionId: '',
                     },
+                    // backward-compat: delete old null version key
                     delOptions: {
                         versionId: 'vnull',
                         replayId: 'nullFooUploadId',
+                        deleteData: true,
+                    },
+                },
+                versioningEnabledCompatExpectedRes: {
+                    options: {
+                        versioning: true,
+                        extraMD: {
+                            nullVersionId: 'vnull',
+                            nullUploadId: 'nullFooUploadId',
+                        },
+                    },
+                },
+                versioningSuspendedCompatExpectedRes: {
+                    options: {
+                        isNull: true,
+                        versionId: '',
+                    },
+                    // backward-compat: delete old null version key
+                    delOptions: {
+                        versionId: 'vnull',
+                        replayId: 'nullFooUploadId',
+                        deleteData: true,
                     },
                 },
             },
         ].forEach(testCase =>
-            ['Enabled', 'Suspended'].forEach(versioningStatus => it(
+            [false, true].forEach(nullVersionCompatMode =>
-            `${testCase.description}, versioning Status=${versioningStatus}`,
+                ['Enabled', 'Suspended'].forEach(versioningStatus => it(
-            () => {
+                `${testCase.description}${nullVersionCompatMode ? ' (null compat)' : ''}` +
-                const mst = getMasterState(testCase.objMD);
+                `, versioning Status=${versioningStatus}`,
-                const res = processVersioningState(mst, versioningStatus);
+                () => {
-                const expectedRes = testCase[`versioning${versioningStatus}ExpectedRes`];
+                    const mst = getMasterState(testCase.objMD);
-                assert.deepStrictEqual(res, expectedRes);
+                    const res = processVersioningState(mst, versioningStatus, nullVersionCompatMode);
-            })));
+                    const resultName = `versioning${versioningStatus}` +
+                          `${nullVersionCompatMode ? 'Compat' : ''}ExpectedRes`;
+                    const expectedRes = testCase[resultName];
+                    assert.deepStrictEqual(res, expectedRes);
+                }))));
     });
 
     describe('preprocessingVersioningDelete', () => {
@@ -263,6 +535,7 @@ describe('versioning helpers', () => {
                     versionId: 'v1',
                 },
                 expectedRes: {},
+                expectedResCompat: {},
             },
             {
                 description: 'delete non-null object version',
@@ -273,10 +546,15 @@ describe('versioning helpers', () => {
                 expectedRes: {
                     deleteData: true,
                     versionId: 'v1',
+                    isNull: false,
+                },
+                expectedResCompat: {
+                    deleteData: true,
+                    versionId: 'v1',
                 },
             },
             {
-                description: 'delete null object version',
+                description: 'delete legacy null object version',
                 objMD: {
                     versionId: 'vnull',
                     isNull: true,
@@ -286,6 +564,29 @@ describe('versioning helpers', () => {
                     deleteData: true,
                     versionId: 'vnull',
                 },
+                expectedResCompat: {
+                    deleteData: true,
+                    versionId: 'vnull',
+                },
+            },
+            {
+                description: 'delete null object version in null key',
+                objMD: {
+                    versionId: 'vnull',
+                    isNull: true,
+                    isNull2: true,
+                },
+                reqVersionId: 'null',
+                expectedRes: {
+                    deleteData: true,
+                    versionId: 'vnull',
+                    isNull: true,
+                },
+                expectedResCompat: {
+                    deleteData: true,
+                    versionId: 'vnull',
+                    isNull: true,
+                },
             },
             {
                 description: 'delete object put before versioning was first enabled',
@@ -293,15 +594,26 @@ describe('versioning helpers', () => {
                 reqVersionId: 'null',
                 expectedRes: {
                     deleteData: true,
+                    // no 'isNull' parameter, as there is no 'versionId', the code will
+                    // not use the version-specific DELETE route but a regular DELETE
+                },
+                expectedResCompat: {
+                    deleteData: true,
                 },
             },
-        ].forEach(testCase => it(testCase.description, () => {
+        ].forEach(testCase =>
-            const mockBucketMD = {
+            [false, true].forEach(nullVersionCompatMode =>
-                getVersioningConfiguration: () => ({ Status: 'Enabled' }),
+                it(`${testCase.description}${nullVersionCompatMode ? ' (null compat)' : ''}`,
-            };
+                () => {
-            const options = preprocessingVersioningDelete(
+                    const mockBucketMD = {
-                'foobucket', mockBucketMD, testCase.objMD, testCase.reqVersionId);
+                        getVersioningConfiguration: () => ({ Status: 'Enabled' }),
-            assert.deepStrictEqual(options, testCase.expectedRes);
+                    };
-        }));
+                    const options = preprocessingVersioningDelete(
+                        'foobucket', mockBucketMD, testCase.objMD, testCase.reqVersionId,
+                        nullVersionCompatMode);
+                    const expectedResAttr = nullVersionCompatMode ?
+                          'expectedResCompat' : 'expectedRes';
+                    assert.deepStrictEqual(options, testCase[expectedResAttr]);
+                })));
     });
 });

tests/unit/api/multipartUpload.js

@@ -1937,13 +1937,13 @@ describe('complete mpu with versioning', () => {
                                         err => next(err, testUploadId));
             },
             (testUploadId, next) => {
-                const origDeleteObject = metadataBackend.deleteObject;
+                const origPutObject = metadataBackend.putObject;
-                metadataBackend.deleteObject =
+                metadataBackend.putObject =
-                    (bucketName, objName, params, log, cb) => {
+                    (putBucketName, objName, objVal, params, log, cb) => {
-                        assert.strictEqual(params.replayId, testUploadId);
+                        assert.strictEqual(params.oldReplayId, testUploadId);
-                        metadataBackend.deleteObject = origDeleteObject;
+                        metadataBackend.putObject = origPutObject;
-                        metadataBackend.deleteObject(
+                        origPutObject(
-                            bucketName, objName, params, log, cb);
+                            putBucketName, objName, objVal, params, log, cb);
                     };
                 // overwrite null version with a non-MPU object
                 objectPut(authInfo, testPutObjectRequests[1],
@@ -2012,13 +2012,13 @@ describe('complete mpu with versioning', () => {
                 versioningTestUtils.assertDataStoreValues(
                     ds, [undefined, objData[1], objData[2]]);
 
-                const origDeleteObject = metadataBackend.deleteObject;
+                const origPutObject = metadataBackend.putObject;
-                metadataBackend.deleteObject =
+                metadataBackend.putObject =
-                    (bucketName, objName, params, log, cb) => {
+                    (putBucketName, objName, objVal, params, log, cb) => {
-                        assert.strictEqual(params.replayId, testUploadId);
+                        assert.strictEqual(params.oldReplayId, testUploadId);
-                        metadataBackend.deleteObject = origDeleteObject;
+                        metadataBackend.putObject = origPutObject;
-                        metadataBackend.deleteObject(
+                        origPutObject(
-                            bucketName, objName, params, log, cb);
+                            putBucketName, objName, objVal, params, log, cb);
                     };
                 // overwrite null version with a non-MPU object
                 objectPut(authInfo, testPutObjectRequests[1],

tests/utilities/objectLock-util.js

@@ -19,7 +19,8 @@ function changeObjectLock(objects, newConfig, cb) {
             objMD.retentionMode = newConfig.mode;
             objMD.retentionDate = newConfig.date;
             objMD.legalHold = false;
-            metadata.putObjectMD(bucket, key, objMD, { versionId: objMD.versionId }, log, err => {
+            const params = { versionId: objMD.versionId, isNull: false };
+            metadata.putObjectMD(bucket, key, objMD, params, log, err => {
                 assert.ifError(err);
                 next();
             });

yarn.lock

@@ -388,7 +388,6 @@ arraybuffer.slice@~0.0.7:
 
 "arsenal@git+https://github.com/scality/Arsenal#7.10.46":
   version "7.10.46"
-  uid bd76402586f1b5117ada9857a9e437727562d55a
   resolved "git+https://github.com/scality/Arsenal#bd76402586f1b5117ada9857a9e437727562d55a"
   dependencies:
     "@types/async" "^3.2.12"
@@ -427,9 +426,9 @@ arraybuffer.slice@~0.0.7:
   optionalDependencies:
     ioctl "^2.0.2"
 
-"arsenal@git+https://github.com/scality/arsenal#7.10.46":
+"arsenal@git+https://github.com/scality/arsenal#4c39ef64":
-  version "7.10.46"
+  version "7.70.4"
-  resolved "git+https://github.com/scality/arsenal#bd76402586f1b5117ada9857a9e437727562d55a"
+  resolved "git+https://github.com/scality/arsenal#4c39ef6469d4d7b630ac03bb8ba41104c46ae36e"
   dependencies:
     "@types/async" "^3.2.12"
     "@types/utf8" "^3.0.1"