CLDSRV-936 Lifecycle noncurrent expiration stalls when listing truncates on a bare master

lib/api/apiUtils/object/lifecycle.js

@@ -1,4 +1,4 @@
-const { versioning } = require('arsenal');
+const { versioning, errorInstances } = require('arsenal');
 const versionIdUtils = versioning.VersionID;
 
 const { lifecycleListing } = require('../../../../constants');
@@ -7,12 +7,11 @@ const { CURRENT_TYPE, NON_CURRENT_TYPE, ORPHAN_DM_TYPE } = lifecycleListing;
 function _makeTags(tags) {
     const res = [];
     Object.entries(tags).forEach(([key, value]) =>
-        res.push(
-            {
-                Key: key,
-                Value: value,
-            }
-        ));
+        res.push({
+            Key: key,
+            Value: value,
+        }),
+    );
     return res;
 }
 
@@ -51,8 +50,7 @@ function processCurrents(bucketName, listParams, isBucketVersioned, list) {
         // NOTE: The current versions listed to be lifecycle should include version id
         // if the bucket is versioned.
         if (isBucketVersioned) {
-            const versionId = (v.IsNull || v.VersionId === undefined) ?
-                'null' : versionIdUtils.encode(v.VersionId);
+            const versionId = v.IsNull || v.VersionId === undefined ? 'null' : versionIdUtils.encode(v.VersionId);
             content.VersionId = versionId;
         }
 
@@ -70,6 +68,19 @@ function _encodeVersionId(vid) {
     return versionId;
 }
 
+// A 'null' or absent marker means "no marker". Any other value is decoded,
+// returning InvalidArgument if it is not a valid version id.
+function decodeVersionIdMarker(vid) {
+    if (!vid || vid === 'null') {
+        return undefined;
+    }
+    const decoded = versionIdUtils.decode(vid);
+    if (decoded instanceof Error) {
+        return errorInstances.InvalidArgument.customizeDescription('Invalid version id marker specified');
+    }
+    return decoded;
+}
+
 function processNonCurrents(bucketName, listParams, list) {
     const nextVersionIdMarker = _encodeVersionId(list.NextVersionIdMarker);
     const versionIdMarker = _encodeVersionId(listParams.versionIdMarker);
@@ -90,8 +101,7 @@ function processNonCurrents(bucketName, listParams, list) {
 
     list.Contents.forEach(item => {
         const v = item.value;
-        const versionId = (v.IsNull || v.VersionId === undefined) ?
-            'null' : versionIdUtils.encode(v.VersionId);
+        const versionId = v.IsNull || v.VersionId === undefined ? 'null' : versionIdUtils.encode(v.VersionId);
 
         const content = {
             Key: item.key,
@@ -131,8 +141,7 @@ function processOrphans(bucketName, listParams, list) {
 
     list.Contents.forEach(item => {
         const v = item.value;
-        const versionId = (v.IsNull || v.VersionId === undefined) ?
-            'null' : versionIdUtils.encode(v.VersionId);
+        const versionId = v.IsNull || v.VersionId === undefined ? 'null' : versionIdUtils.encode(v.VersionId);
         data.Contents.push({
             Key: item.key,
             LastModified: v.LastModified,
@@ -150,8 +159,10 @@ function processOrphans(bucketName, listParams, list) {
 }
 
 function getLocationConstraintErrorMessage(locationName) {
-    return 'value of the location you are attempting to set ' +
-        `- ${locationName} - is not listed in the locationConstraint config`;
+    return (
+        'value of the location you are attempting to set ' +
+        `- ${locationName} - is not listed in the locationConstraint config`
+    );
 }
 
 /**
@@ -170,8 +181,11 @@ function validateMaxScannedEntries(params, config, min) {
     if (params['max-scanned-lifecycle-listing-entries']) {
         const maxEntriesParams = Number.parseInt(params['max-scanned-lifecycle-listing-entries'], 10);
 
-        if (Number.isNaN(maxEntriesParams) || maxEntriesParams < min ||
-            maxEntriesParams > maxScannedLifecycleListingEntries) {
+        if (
+            Number.isNaN(maxEntriesParams) ||
+            maxEntriesParams < min ||
+            maxEntriesParams > maxScannedLifecycleListingEntries
+        ) {
             return { isValid: false };
         }
 
@@ -187,4 +201,5 @@ module.exports = {
     processOrphans,
     getLocationConstraintErrorMessage,
     validateMaxScannedEntries,
+    decodeVersionIdMarker,
 };

lib/api/backbeat/listLifecycleNonCurrents.js

@@ -1,24 +1,26 @@
-const { errors, errorInstances, versioning } = require('arsenal');
+const { errors, errorInstances } = require('arsenal');
 const constants = require('../../../constants');
 const services = require('../../services');
 const { standardMetadataValidateBucket } = require('../../metadata/metadataUtils');
 const { pushMetric } = require('../../utapi/utilities');
-const versionIdUtils = versioning.VersionID;
 const monitoring = require('../../utilities/monitoringHandler');
-const { getLocationConstraintErrorMessage, processNonCurrents,
-    validateMaxScannedEntries } = require('../apiUtils/object/lifecycle');
+const {
+    getLocationConstraintErrorMessage,
+    processNonCurrents,
+    validateMaxScannedEntries,
+    decodeVersionIdMarker,
+} = require('../apiUtils/object/lifecycle');
 const { config } = require('../../Config');
 
-function handleResult(listParams, requestMaxKeys, authInfo,
-    bucketName, list, log, callback) {
+function handleResult(listParams, requestMaxKeys, authInfo, bucketName, list, log, callback) {
     // eslint-disable-next-line no-param-reassign
     listParams.maxKeys = requestMaxKeys;
     const res = processNonCurrents(bucketName, listParams, list);
 
     pushMetric('listLifecycleNonCurrents', log, { authInfo, bucket: bucketName });
     monitoring.promMetrics('GET', bucketName, '200', 'listLifecycleNonCurrents');
     return callback(null, res);
 }
 
 /**
  * listLifecycleNonCurrents - Return list of non-current versions in bucket
@@ -36,20 +38,21 @@
     const bucketName = request.bucketName;
 
     log.debug('processing request', { method: 'listLifecycleNonCurrents' });
-    const requestMaxKeys = params['max-keys'] ?
-        Number.parseInt(params['max-keys'], 10) : 1000;
+    const requestMaxKeys = params['max-keys'] ? Number.parseInt(params['max-keys'], 10) : 1000;
     if (Number.isNaN(requestMaxKeys) || requestMaxKeys < 0) {
-        monitoring.promMetrics(
-            'GET', bucketName, 400, 'listLifecycleNonCurrents');
+        monitoring.promMetrics('GET', bucketName, 400, 'listLifecycleNonCurrents');
         return callback(errors.InvalidArgument);
     }
     const actualMaxKeys = Math.min(constants.listingHardLimit, requestMaxKeys);
 
     // 3 is required as a minimum because we must scan at least three entries to determine version eligibility.
     // Two entries representing the master key and the following one representing the non-current version.
     const minEntriesToBeScanned = 3;
-    const { isValid, maxScannedLifecycleListingEntries } =
-        validateMaxScannedEntries(params, config, minEntriesToBeScanned);
+    const { isValid, maxScannedLifecycleListingEntries } = validateMaxScannedEntries(
+        params,
+        config,
+        minEntriesToBeScanned,
+    );
     if (!isValid) {
         monitoring.promMetrics('GET', bucketName, 400, 'listLifecycleNonCurrents');
         return callback(errors.InvalidArgument);
@@ -80,44 +83,43 @@
         maxScannedLifecycleListingEntries,
     };
 
-    listParams.versionIdMarker = params['version-id-marker'] ?
-        versionIdUtils.decode(params['version-id-marker']) : undefined;
+    const versionIdMarker = decodeVersionIdMarker(params['version-id-marker']);
+    if (versionIdMarker instanceof Error) {
+        log.debug('invalid version id marker', { versionIdMarker: params['version-id-marker'] });
+        monitoring.promMetrics('GET', bucketName, 400, 'listLifecycleNonCurrents');
+        return callback(versionIdMarker);
+    }
+    listParams.versionIdMarker = versionIdMarker;
 
     return standardMetadataValidateBucket(metadataValParams, request.actionImplicitDenies, log, (err, bucket) => {
         if (err) {
             log.debug('error processing request', { method: 'metadataValidateBucket', error: err });
-            monitoring.promMetrics(
-                'GET', bucketName, err.code, 'listLifecycleNonCurrents');
+            monitoring.promMetrics('GET', bucketName, err.code, 'listLifecycleNonCurrents');
             return callback(err, null);
         }
 
         const vcfg = bucket.getVersioningConfiguration();
         const isBucketVersioned = vcfg && (vcfg.Status === 'Enabled' || vcfg.Status === 'Suspended');
         if (!isBucketVersioned) {
             log.debug('bucket is not versioned');
-            return callback(errorInstances.InvalidRequest.customizeDescription(
-                'bucket is not versioned'), null);
+            return callback(errorInstances.InvalidRequest.customizeDescription('bucket is not versioned'), null);
         }
 
         if (!requestMaxKeys) {
             const emptyList = {
                 Contents: [],
                 IsTruncated: false,
             };
-            return handleResult(listParams, requestMaxKeys, authInfo,
-                bucketName, emptyList, log, callback);
+            return handleResult(listParams, requestMaxKeys, authInfo, bucketName, emptyList, log, callback);
         }
 
-        return services.getLifecycleListing(bucketName, listParams, log,
-        (err, list) => {
+        return services.getLifecycleListing(bucketName, listParams, log, (err, list) => {
             if (err) {
                 log.debug('error processing request', { method: 'services.getLifecycleListing', error: err });
-                monitoring.promMetrics(
-                    'GET', bucketName, err.code, 'listLifecycleNonCurrents');
+                monitoring.promMetrics('GET', bucketName, err.code, 'listLifecycleNonCurrents');
                 return callback(err, null);
             }
-            return handleResult(listParams, requestMaxKeys, authInfo,
-                bucketName, list, log, callback);
+            return handleResult(listParams, requestMaxKeys, authInfo, bucketName, list, log, callback);
         });
     });
 }

package.json

@@ -1,6 +1,6 @@
 {
     "name": "@zenko/cloudserver",
-    "version": "9.3.11",
+    "version": "9.3.12",
     "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
     "main": "index.js",
     "engines": {