Ensure backbeat retrieve the account quotas

lib/api/apiUtils/quotas/quotaUtils.js

@@ -20,27 +20,38 @@ const QuotaService = require('../../../quotas/quotas');
  * @return {number} processed content length
  */
 function processBytesToWrite(apiMethod, bucket, versionId, contentLength, objMD, destObjMD = null) {
+    // Get the size of "hot" data. Object being restored are _considered_ hot, as we "reserve" the
+    // space for them as soon as restore is requested: therefore we must 'free' it immediately on
+    // delete as well.
+    const isCold = objMD => objMD.archive && !objMD.archive.restoreRequestedAt;
+    const getHotContentLength = objMD => {
+        if (isCold(objMD)) {
+            return 0;
+        }
+        return Number.parseInt(objMD['content-length'], 10);
+    };
+
     let bytes = contentLength;
     if (apiMethod === 'objectRestore') {
         // object is being restored
         bytes = Number.parseInt(objMD['content-length'], 10);
     } else if (!bytes && objMD?.['content-length']) {
         if (apiMethod === 'objectCopy' || apiMethod === 'objectPutCopyPart') {
-            if (!destObjMD || bucket.isVersioningEnabled()) {
-                // object is being copied
-                bytes = Number.parseInt(objMD['content-length'], 10);
-            } else if (!bucket.isVersioningEnabled()) {
-                // object is being copied and replaces the target
-                bytes = Number.parseInt(objMD['content-length'], 10) -
-                    Number.parseInt(destObjMD['content-length'], 10);
+            // object is being copied, increases the storage...
+            bytes = Number.parseInt(objMD['content-length'], 10);
+
+            if (destObjMD && !bucket.isVersioningEnabled()) {
+                // but it also replaces the target, which decreases storage
+                bytes -= getHotContentLength(destObjMD);
             }
         } else if (!bucket.isVersioningEnabled() || bucket.isVersioningEnabled() && versionId) {
-            // object is being deleted
-            bytes = -Number.parseInt(objMD['content-length'], 10);
+            // object is being deleted (non versioned) or hard-deleted (versioned, as indicated by
+            // the `versionId` field)
+            bytes = -getHotContentLength(objMD);
         }
     } else if (bytes && objMD?.['content-length'] && !bucket.isVersioningEnabled()) {
         // object is being replaced: store the diff, if the bucket is not versioned
-        bytes = bytes - Number.parseInt(objMD['content-length'], 10);
+        bytes = bytes - getHotContentLength(objMD);
     }
     return bytes || 0;
 }

lib/api/multiObjectDelete.js

@@ -31,7 +31,7 @@ const { overheadField } = require('../../constants');
 const versionIdUtils = versioning.VersionID;
 const { data } = require('../data/wrapper');
 const logger = require('../utilities/logger');
-const { validateQuotas } = require('./apiUtils/quotas/quotaUtils');
+const { processBytesToWrite, validateQuotas } = require('./apiUtils/quotas/quotaUtils');
 
 /*
    Format of xml request:
@@ -333,9 +333,11 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request,
 
                     return callback(null, objMD, versionId);
                 },
-                (objMD, versionId, callback) => validateQuotas(
-                    request, bucket, request.accountQuotas, ['objectDelete'], 'objectDelete',
-                        -objMD?.['content-length'] || 0, false, log, err => callback(err, objMD, versionId)),
+                (objMD, versionId, callback) => {
+                    const bytes = processBytesToWrite('objectDelete', bucket, versionId, 0, objMD);
+                    return validateQuotas(request, bucket, request.accountQuotas, ['objectDelete'],
+                        'objectDelete', bytes, false, log, err => callback(err, objMD, versionId));
+                },
                 (objMD, versionId, callback) => {
                     const options = preprocessingVersioningDelete(
                         bucketName, bucket, objMD, versionId, config.nullVersionCompatMode);

lib/routes/routeBackbeat.js

@@ -38,6 +38,8 @@ const { listLifecycleCurrents } = require('../api/backbeat/listLifecycleCurrents
 const { listLifecycleNonCurrents } = require('../api/backbeat/listLifecycleNonCurrents');
 const { listLifecycleOrphanDeleteMarkers } = require('../api/backbeat/listLifecycleOrphanDeleteMarkers');
 const { objectDeleteInternal } = require('../api/objectDelete');
+const { validateQuotas } = require('../api/apiUtils/quotas/quotaUtils');
+
 const { CURRENT_TYPE, NON_CURRENT_TYPE, ORPHAN_DM_TYPE } = constants.lifecycleListing;
 
 const lifecycleTypeCalls = {
@@ -1176,7 +1178,27 @@ function batchDelete(request, response, userInfo, log, callback) {
                 return callback(err);
             }
             log.debug('batch delete successful', { locations });
-            return _respond(response, null, log, callback);
+
+            // Update inflight metrics for the data which has just been freed
+            const bucket = request.bucketName;
+            const contentLength = locations.reduce((length, loc) => length + loc.size, 0);
+            return async.waterfall([
+                next => metadata.getBucket(bucket, log, next),
+                (bucketMD, next) => validateQuotas(request, bucketMD, request.accountQuotas,
+                    ['objectDelete'], 'objectDelete', -contentLength, false, log, next),
+            ], err => {
+                if (err) {
+                    // Ignore error, as the data has been deleted already: only inflight count
+                    // has not been updated, and will be eventually consistent anyway
+                    log.warn('batch delete failed to update inflights', {
+                        method: 'batchDelete',
+                        locations,
+                        error: err,
+                    });
+                }
+
+                return _respond(response, null, log, callback);
+            });
         });
     });
 }
@@ -1372,6 +1394,19 @@ function routeBackbeat(clientIP, request, response, log) {
     _normalizeBackbeatRequest(request);
     const requestContexts = prepareRequestContexts('objectReplicate', request);
 
+    if (request.resourceType === 'expiration' || request.resourceType === 'batchdelete') {
+        // Reassign a specific apiMethod, as it is needed for quota evaluation (at least), where
+        // "routeBackbeat" cannot be used as it is used for all backbeat API operations...
+        // eslint-disable-next-line no-param-reassign
+        request.apiMethod = 'objectDelete';
+
+        // Request account quotas, as it will not be added for the 'objectReplicate' action which
+        // is used by default for all backbeat operations
+        requestContexts.forEach(context => {
+            context._needQuota = true; // eslint-disable-line no-param-reassign
+        });
+    }
+
     // Ensure backbeat operations like expiration can properly use quotas
     // eslint-disable-next-line no-param-reassign
     request.finalizerHooks = [];
@@ -1388,7 +1423,9 @@ function routeBackbeat(clientIP, request, response, log) {
         const path = request.url.replace('/_/backbeat/api', '/_/');
         const { host, port } = config.backbeat;
         const target = `http://${host}:${port}${path}`;
-        return auth.server.doAuth(request, log, (err, userInfo) => {
+
+        // TODO CLDSRV-591: shall we use the authorization results here?
+        return auth.server.doAuth(request, log, (err, userInfo, authorizationResults, streamingV4Params, infos) => {
             if (err) {
                 log.debug('authentication error', {
                     error: err,
@@ -1398,6 +1435,8 @@ function routeBackbeat(clientIP, request, response, log) {
                 });
                 return responseJSONBody(err, null, response, log);
             }
+            // eslint-disable-next-line no-param-reassign
+            request.accountQuotas = infos?.accountQuota;
             // FIXME for now, any authenticated user can access API
             // routes. We should introduce admin accounts or accounts
             // with admin privileges, and restrict access to those
@@ -1450,7 +1489,8 @@ function routeBackbeat(clientIP, request, response, log) {
     }
 
     return async.waterfall([next => auth.server.doAuth(
-        request, log, (err, userInfo) => {
+        // TODO CLDSRV-591: shall we use the authorization results here?
+        request, log, (err, userInfo, authorizationResults, streamingV4Params, infos) => {
             if (err) {
                 log.debug('authentication error', {
                     error: err,
@@ -1459,6 +1499,8 @@ function routeBackbeat(clientIP, request, response, log) {
                     objectKey: request.objectKey,
                 });
             }
+            // eslint-disable-next-line no-param-reassign
+            request.accountQuotas = infos?.accountQuota;
             return next(err, userInfo);
         }, 's3', requestContexts),
         (userInfo, next) => {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@zenko/cloudserver",
-  "version": "8.8.38",
+  "version": "8.8.39",
   "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
   "main": "index.js",
   "engines": {
@@ -21,7 +21,7 @@
   "dependencies": {
     "@azure/storage-blob": "^12.12.0",
     "@hapi/joi": "^17.1.0",
-    "arsenal": "git+https://github.com/scality/arsenal#8.1.142",
+    "arsenal": "https://github.com/scality/arsenal#8.1.143",
     "async": "~2.5.0",
     "aws-sdk": "2.905.0",
     "bucketclient": "scality/bucketclient#8.1.9",

tests/unit/api/apiUtils/quotas/quotaUtils.js

@@ -114,6 +114,32 @@ describe('validateQuotas (buckets)', () => {
         });
     });
 
+    it('should not return QuotaExceeded if quotas are exceeded but operation is creating a delete marker', done => {
+        const result1 = {
+            bytesTotal: 150,
+        };
+        const result2 = {
+            bytesTotal: 120,
+        };
+        QuotaService._getLatestMetricsCallback.yields(null, result1);
+        QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2);
+
+        validateQuotas(request, mockBucket, {}, ['objectDelete'], 'objectDelete', 0, false, mockLog, err => {
+            assert.ifError(err);
+            assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true);
+            assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith(
+                'bucket',
+                'bucketName_1640995200000',
+                null,
+                {
+                    action: 'objectDelete',
+                    inflight: 0,
+                }
+            ), true);
+            done();
+        });
+    });
+
     it('should not return QuotaExceeded if the quotas are exceeded but operation is a delete', done => {
         const result1 = {
             bytesTotal: 150,
@@ -140,6 +166,32 @@ describe('validateQuotas (buckets)', () => {
         });
     });
 
+    it('should not return QuotaExceeded if the quotas are exceeded but operation is a delete with version', done => {
+        const result1 = {
+            bytesTotal: 150,
+        };
+        const result2 = {
+            bytesTotal: 120,
+        };
+        QuotaService._getLatestMetricsCallback.yields(null, result1);
+        QuotaService._getLatestMetricsCallback.onCall(1).yields(null, result2);
+
+        validateQuotas(request, mockBucket, {}, ['objectDelete'], 'objectDeleteVersion', -50, false, mockLog, err => {
+            assert.ifError(err);
+            assert.strictEqual(QuotaService._getLatestMetricsCallback.calledOnce, true);
+            assert.strictEqual(QuotaService._getLatestMetricsCallback.calledWith(
+                'bucket',
+                'bucketName_1640995200000',
+                null,
+                {
+                    action: 'objectDelete',
+                    inflight: -50,
+                }
+            ), true);
+            done();
+        });
+    });
+
     it('should decrease the inflights by deleting data, and go below 0 to unblock operations', done => {
         const result1 = {
             bytesTotal: 150,
@@ -543,32 +595,76 @@ describe('processBytesToWrite', () => {
         objMD = null;
     });
 
-    it('should return a negative number if the operation is a delete and bucket is not versioned', () => {
-        bucket.isVersioningEnabled.returns(false);
-        objMD = { 'content-length': 100 };
-
-        const bytes = processBytesToWrite('objectDelete', bucket, versionId, contentLength, objMD);
-
-        assert.strictEqual(bytes, -100);
-    });
-
-    it('should return 0 if the operation is a delete and bucket is versioned', () => {
-        bucket.isVersioningEnabled.returns(true);
-        objMD = { 'content-length': 100 };
-
-        const bytes = processBytesToWrite('objectDelete', bucket, versionId, contentLength, objMD);
-
-        assert.strictEqual(bytes, 0);
-    });
-
-    it('should return a negative number for a versioned bucket with a versionid deletion', () => {
-        bucket.isVersioningEnabled.returns(true);
-        objMD = { 'content-length': 100 };
-        versionId = 'versionId';
-
-        const bytes = processBytesToWrite('objectDelete', bucket, versionId, contentLength, objMD);
+    const hotObject = {
+        'content-length': 100,
+        dataStoreName: 'eu-west-1',
+    };
+    const coldObject = {
+        ...hotObject,
+        dataStoreName: 'glacier',
+        archive: {
+            archiveInfo: '{archiveID,archiveVersion}'
+        },
+    };
+    const restoringObject = {
+        ...coldObject,
+        archive: {
+            ...coldObject.archive,
+            restoreRequestedAt: new Date(Date.now() - 2 * 3600 * 1000).toISOString(),
+            restoreRequestedDays: 1,
+        },
+    };
+    const restoredObject = {
+        ...restoringObject,
+        dataStoreName: 'eu-west-1',
+        'x-amz-storage-class': 'glacier',
+        archive: {
+            ...restoringObject.archive,
+            restoreCompletedAt: new Date(Date.now() - 3600 * 1000),
+            restoreWillExpireAt: new Date(Date.now() + 23 * 3600 * 1000),
+        },
+    };
+    const expiredObject = {
+        ...restoredObject,
+        archive: {
+            ...coldObject.archive,
+            restoreRequestedAt: new Date(Date.now() - 25 * 3600 * 1000 - 1000).toString(),
+            restoreRequestedDays: 1,
+            restoreCompletedAt: new Date(Date.now() - 24 * 3600 * 1000 - 1000),
+            restoreWillExpireAt: new Date(Date.now() - 1000),
+        },
+    };
 
-        assert.strictEqual(bytes, -100);
+    [
+        // non versionned case
+        ['the content-length when deleting hot object', hotObject, false, undefined, -100],
+        ['0 when deleting cold object', coldObject, false, undefined, 0],
+        ['the content-length when deleting restoring object', restoringObject, false, undefined, -100],
+        ['the content-length when deleting restored object', restoredObject, false, undefined, -100],
+        ['the content-length when deleting expired object', expiredObject, false, undefined, -100],
+
+        // versionned case
+        ['the content-length when deleting hot object version', hotObject, true, 'versionId', -100],
+        ['0 when deleting cold versioned object version', coldObject, true, 'versionId', 0],
+        ['the content-length when deleting restoring object version', restoringObject, true, 'versionId', -100],
+        ['the content-length when deleting restored object version', restoredObject, true, 'versionId', -100],
+        ['the content-length when deleting expired object version', expiredObject, true, 'versionId', -100],
+
+        // delete marker case
+        ['0 when adding delete marker over hot object', hotObject, true, undefined, 0],
+        ['0 when adding delete marker over cold object', coldObject, true, undefined, 0],
+        ['0 when adding delete marker over restoring object', restoringObject, true, undefined, 0],
+        ['0 when adding delete marker over restored object', restoredObject, true, undefined, 0],
+        ['0 when adding delete marker over expired object', expiredObject, true, undefined, 0],
+    ].forEach(([scenario, object, versionned, reqVersionId, expected]) => {
+        it(`should return ${scenario}`, () => {
+            bucket.isVersioningEnabled.returns(versionned);
+            objMD = object;
+
+            const bytes = processBytesToWrite('objectDelete', bucket, reqVersionId, 0, objMD);
+
+            assert.strictEqual(bytes, expected);
+        });
     });
 
     it('should return 0 for a delete operation if the object metadata is missing', () => {
@@ -647,6 +743,16 @@ describe('processBytesToWrite', () => {
 
         assert.strictEqual(bytes, 100);
     });
+
+    it('should not detect object replacement during copy object operation if the object is cold', () => {
+        bucket.isVersioningEnabled.returns(true);
+        objMD = { 'content-length': 100 };
+        const destObjMD = coldObject;
+
+        const bytes = processBytesToWrite('objectCopy', bucket, versionId, 0, objMD, destObjMD);
+
+        assert.strictEqual(bytes, 100);
+    });
 });
 
 describe('isMetricStale', () => {

tests/unit/api/multiObjectDelete.js

@@ -42,7 +42,11 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => {
         headers: {},
         parsedContentLength: contentLength,
     }, postBody);
-    const bucket = { getVersioningConfiguration: () => null, getQuota: () => 0 };
+    const bucket = {
+        isVersioningEnabled: () => false,
+        getVersioningConfiguration: () => null,
+        getQuota: () => 0,
+    };
 
     beforeEach(done => {
         cleanup();

yarn.lock

@@ -1040,9 +1040,9 @@ arraybuffer.slice@~0.0.7:
   optionalDependencies:
     ioctl "^2.0.2"
 
-"arsenal@git+https://github.com/scality/arsenal#8.1.142":
-  version "8.1.142"
-  resolved "git+https://github.com/scality/arsenal#7402f096c9839815db5f03cbc2f4117536f55c28"
+"arsenal@https://github.com/scality/arsenal#8.1.143":
+  version "8.1.143"
+  resolved "git+https://github.com/scality/arsenal#5a6069f7fdf75ca571e51ae75b4d1b28512515cd"
   dependencies:
     "@azure/identity" "^3.1.1"
     "@azure/storage-blob" "^12.12.0"