diff --git a/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js b/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js index 9f41b86c5..cd6d82e5f 100644 --- a/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js +++ b/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js @@ -103,25 +103,62 @@ class LifecycleDeleteObjectTask extends BackbeatTask { 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 @@ class LifecycleDeleteObjectTask extends BackbeatTask { 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({ diff --git a/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js b/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js index 45b6132fa..c96435bf1 100644 --- a/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js +++ b/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(); + }); + }); }); diff --git a/tests/unit/mocks.js b/tests/unit/mocks.js index bbd2c6912..4bef5b282 100644 --- a/tests/unit/mocks.js +++ b/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) {