Revert "use deleteObject to expire objects in S3c"

extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js

@@ -103,25 +103,62 @@
         return done();
     }
 
-    _getS3Action(actionType, accountId) {
-        let reqMethod;
-        if (actionType === 'deleteObject') {
-            const backbeatClient = this.getBackbeatClient(accountId);
-            if (!backbeatClient) {
-                return null;
-            }
-            // Zenko supports the "deleteObjectFromExpiration" API, which
-            // sets the proper originOp in the metadata to trigger a
-            // nortification when an object gets expired.
-            if (typeof backbeatClient.deleteObjectFromExpiration === 'function') {
-                return backbeatClient.deleteObjectFromExpiration.bind(backbeatClient);
-            }
-            reqMethod = 'deleteObject';
-        } else {
-            reqMethod = 'abortMultipartUpload';
-        }
+    _abortMPU(reqParams, accountId, transitionTime, startTime, location, log, done) {
         const client = this.getS3Client(accountId);
-        return client[reqMethod].bind(client);
+        if (!client) {
+            log.error('failed to get s3 client', {
+                accountId,
+                actionType: 'deleteMPU',
+                method: 'LifecycleDeleteObjectTask._abortMPU',
+            });
+            done(errors.InternalError
+                .customizeDescription('Unable to obtain s3 client'));
+            return;
+        }
+        LifecycleMetrics.onLifecycleStarted(log,
+            'expiration:mpu',
+            location, startTime - transitionTime);
+        const req = client.abortMultipartUpload(reqParams);
+        attachReqUids(req, log);
+        req.send(done);
+    }
+
+    _deleteObject(reqParams, accountId, transitionTime, startTime, location, log, done) {
+        const logDetails = {
+            accountId,
+            actionType: 'deleteObject',
+            method: 'LifecycleDeleteObjectTask._deleteObject',
+        };
+        const client = this.getBackbeatClient(accountId);
+        if (!client) {
+            log.error('failed to get s3 backbeat client', logDetails);
+            done(errors.InternalError
+                .customizeDescription('Unable to obtain s3 client'));
+            return;
+        }
+        LifecycleMetrics.onLifecycleStarted(log,
+            'expiration',
+            location, startTime - transitionTime);
+        const req = client.deleteObjectFromExpiration(reqParams);
+        attachReqUids(req, log);
+        req.send(err => {
+            if (err?.statusCode === errors.MethodNotAllowed.code) {
+                log.warn('deleteObjectFromExpiration API not supported, falling back to deleteObject',
+                    logDetails);
+                // fallback to s3 deleteObject when using a cloudserver that
+                // doesn't support deleteObjectFromExpiration
+                const s3Client = this.getS3Client(accountId);
+                if (!s3Client) {
+                    log.error('failed to get s3 client', logDetails);
+                    return done(errors.InternalError
+                        .customizeDescription('Unable to obtain client'));
+                }
+                const s3Req = s3Client.deleteObject(reqParams);
+                attachReqUids(s3Req, log);
+                return s3Req.send(done);
+            }
+            return done(err);
+        });
     }
 
     _executeDelete(entry, startTime, log, done) {
@@ -135,37 +172,24 @@
         if (version !== undefined) {
             reqParams.VersionId = version;
         }
-        let reqMethod;
 
         const actionType = entry.getActionType();
         const transitionTime = entry.getAttribute('transitionTime');
         const location = this.objectMD?.dataStoreName || entry.getAttribute('details.dataStoreName');
-        let req = null;
 
-        const s3Action = this._getS3Action(actionType, accountId);
-        if (!s3Action) {
-            log.error('failed to get s3 action', {
-                accountId,
-                actionType,
-                method: 'LifecycleDeleteObjectTask._executeDelete',
-            });
-            return done(errors.InternalError
-                .customizeDescription('Unable to obtain s3 action'));
-        }
-        LifecycleMetrics.onLifecycleStarted(log,
-            actionType === 'deleteMPU' ? 'expiration:mpu' : 'expiration',
-            location, startTime - transitionTime);
+        let reqMethod = 'deleteObject';
+        let actionMethod = this._deleteObject.bind(this);
         if (actionType === 'deleteMPU') {
+            reqMethod = 'abortMultipartUpload';
             reqParams.UploadId = entry.getAttribute('details.UploadId');
+            actionMethod = this._abortMPU.bind(this);
         }
-        req = s3Action(reqParams);
-        attachReqUids(req, log);
-        return req.send(err => {
+
+        actionMethod(reqParams, accountId, transitionTime, startTime, location, log, err => {
             LifecycleMetrics.onS3Request(log, reqMethod, 'expiration', err);
             LifecycleMetrics.onLifecycleCompleted(log,
                 actionType === 'deleteMPU' ? 'expiration:mpu' : 'expiration',
                 location, Date.now() - entry.getAttribute('transitionTime'));
-
             if (err) {
                 log.error(
                     `an error occurred on ${reqMethod} to S3`, Object.assign({

tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js

@@ -260,7 +260,7 @@ describe('LifecycleDeleteObjectTask', () => {
         });
     });
 
-    it('should expire object using the deleteObjectFromExpiration method if supported', done => {
+    it('should expire object using the deleteObjectFromExpiration method', done => {
         const entry = ActionQueueEntry.create('deleteObject')
             .setAttribute('target.owner', 'testowner')
             .setAttribute('target.bucket', 'testbucket')
@@ -278,9 +278,7 @@ describe('LifecycleDeleteObjectTask', () => {
         });
     });
 
-    it('should expire object using the deleteObject method when not in Zenko', done => {
-        backbeatClient = new BackbeatClientMock({ isS3c: true });
-        sinon.stub(task, 'getBackbeatClient').returns(backbeatClient);
+    it('should fallback to deleteObject method if deleteObjectFromExpiration is not supported', done => {
         const entry = ActionQueueEntry.create('deleteObject')
             .setAttribute('target.owner', 'testowner')
             .setAttribute('target.bucket', 'testbucket')
@@ -289,11 +287,32 @@ describe('LifecycleDeleteObjectTask', () => {
             .setAttribute('target.version', 'testversion')
             .setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z');
         s3Client.setResponse(null, {});
-        backbeatClient.setResponse(null, {});
+        const methodNotAllowedErr = new Error('MethodNotAllowed');
+        methodNotAllowedErr.statusCode = 405;
+        backbeatClient.setResponse(methodNotAllowedErr, {});
         task.processActionEntry(entry, err => {
             assert.ifError(err);
+            assert.strictEqual(backbeatClient.times.deleteObjectFromExpiration, 1);
             assert.strictEqual(s3Client.calls.deleteObject, 1);
-            assert.strictEqual(backbeatClient.times.deleteObjectFromExpiration, 0);
+            done();
+        });
+    });
+
+    it('should fail to fallback if it fails to get the s3 client', done => {
+        sinon.stub(task, 'getS3Client').returns(null);
+        const entry = ActionQueueEntry.create('deleteObject')
+            .setAttribute('target.owner', 'testowner')
+            .setAttribute('target.bucket', 'testbucket')
+            .setAttribute('target.accountId', 'testid')
+            .setAttribute('target.key', 'testkey')
+            .setAttribute('target.version', 'testversion')
+            .setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z');
+        s3Client.setResponse(null, {});
+        const methodNotAllowedErr = new Error('MethodNotAllowed');
+        methodNotAllowedErr.statusCode = 405;
+        backbeatClient.setResponse(methodNotAllowedErr, {});
+        task.processActionEntry(entry, err => {
+            assert(err);
             done();
         });
     });
@@ -334,4 +353,22 @@ describe('LifecycleDeleteObjectTask', () => {
             done();
         });
     });
+
+    it('should return an error when it can\'t get the S3 client', done => {
+        sinon.stub(task, 'getS3Client').returns(null);
+        const entry = ActionQueueEntry.create('deleteMPU')
+            .setAttribute('target.owner', 'testowner')
+            .setAttribute('target.bucket', 'testbucket')
+            .setAttribute('target.accountId', 'testid')
+            .setAttribute('target.key', 'testkey')
+            .setAttribute('target.version', 'testversion')
+            .setAttribute('details.UploadId', 'someUploadId')
+            .setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z');
+        s3Client.setResponse(null, {});
+        backbeatClient.setResponse(null, {});
+        task.processActionEntry(entry, err => {
+            assert(err);
+            done();
+        });
+    });
 });

tests/unit/mocks.js

@@ -61,21 +61,18 @@ class MockRequestAPI extends EventEmitter {
 }
 
 class BackbeatClientMock {
-    constructor(params) {
+    constructor() {
         this.response = null;
         this.batchDeleteResponse = {};
         this.times = {
             batchDeleteResponse: 0,
             deleteObjectFromExpiration: 0,
         };
-        if (params?.isS3c) {
-            this.deleteObjectFromExpiration = undefined;
-        }
     }
 
-    deleteObjectFromExpiration() {
+    deleteObjectFromExpiration(params) {
         this.times.deleteObjectFromExpiration += 1;
-        return new MockRequestAPI(this.response.error, this.response.data);
+        return new MockRequestAPI(params, this.response);
     }
 
     setResponse(error, data) {