Skip to content

Commit

Permalink
BB-462 Expiring non-versioned objects fails
Browse files Browse the repository at this point in the history
Note: Only buckets with versioning enabled can be configured for replication.

To expire non-versioned objects correctly, we currently rely on the InvalidBucketState error returned when the S3C Backbeat API is used for a non-versioned bucket's object, until CLDSRV-461 is implemented to add support for non-versioned buckets to the Backbeat API.
  • Loading branch information
nicolas2bert committed Oct 25, 2023
1 parent 12db952 commit d8ab9f8
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 3 deletions.
13 changes: 12 additions & 1 deletion extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ class LifecycleDeleteObjectTask extends BackbeatTask {
encodedVersionId: version,
}, log, (err, blob) => {
if (err) {
log.error('error getting metadata blob from S3', Object.assign({
// <!> Only in S3C <!> Backbeat API returns 'InvalidBucketState' error if the bucket is not versioned.
// In this case, instead of logging an error, it should be logged as a debug message,
// to avoid causing unnecessary concern to the customer.
// TODO: After the implementation of CLDSRV-461, we could remove this check.
const logLevel = err.code === 'InvalidBucketState' ? 'debug' : 'error';
log[logLevel]('error getting metadata blob from S3', Object.assign({
method: 'LifecycleDeleteObjectTask._getMetadata',
error: err.message,
}, entry.getLogInfo()));
Expand Down Expand Up @@ -187,6 +192,12 @@ class LifecycleDeleteObjectTask extends BackbeatTask {
}
return this._getMetadata(entry, log, (err, objMD) => {
if (err) {
// <!> Only in S3C <!> Backbeat API returns 'InvalidBucketState' error if the bucket is not versioned,
// so we can skip checking object replication for non-versioned buckets.
// TODO: After the implementation of CLDSRV-461, we could remove this check.
if (err.code === 'InvalidBucketState') {
return done();
}
return done(err);
}
const replicationStatus = objMD.getReplicationStatus();
Expand Down
6 changes: 5 additions & 1 deletion lib/BackbeatMetadataProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,11 @@ class BackbeatMetadataProxy extends BackbeatTask {
if (err) {
// eslint-disable-next-line no-param-reassign
err.origin = 'source';
if (err.ObjNotFound || err.code === 'ObjNotFound') {
// <!> Only in S3C <!> Backbeat API returns 'InvalidBucketState' error if the bucket is not versioned.
// In this case, instead of logging an error, it should be logged as a debug message,
// to avoid causing unnecessary concern to the customer.
// TODO: After the implementation of CLDSRV-461, we could remove this check.
if (err.ObjNotFound || err.code === 'ObjNotFound' || err.code === 'InvalidBucketState') {
return cbOnce(err);
}
log.error('an error occurred when getting metadata from S3', {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "backbeat",
"version": "7.10.13",
"version": "7.10.14",
"description": "Asynchronous queue and job manager",
"main": "index.js",
"scripts": {
Expand Down
31 changes: 31 additions & 0 deletions tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ const LifecycleDeleteObjectTask = require(

const day = 1000 * 60 * 60 * 24;

const invalidBucketStateError = {
code: 'InvalidBucketState',
requestId: 'd4c33f72964c85667de4:89ee7213ce42b2a8d420',
statusCode: 409,
retryable: false,
};

const {
S3ClientMock,
BackbeatClientMock,
Expand All @@ -35,6 +42,10 @@ describe('LifecycleDeleteObjectTask', () => {
task = new LifecycleDeleteObjectTask(objectProcessor);
});

afterEach(() => {
backbeatClient.setError(null);
});

it('should not return error for 404s', done => {
const entry = ActionQueueEntry.create('deleteObject')
.setAttribute('target.owner', 'testowner')
Expand Down Expand Up @@ -143,6 +154,26 @@ describe('LifecycleDeleteObjectTask', () => {
});
});

// TODO: After the implementation of CLDSRV-461, we could remove this test.
it('should expire non-versioned object',
done => {
objMd.setLegalHold(true);
const entry = ActionQueueEntry.create('deleteObject')
.setAttribute('target.owner', 'testowner')
.setAttribute('target.bucket', 'testbucket')
.setAttribute('target.accountId', 'testid')
.setAttribute('target.key', 'testkey')
.setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z');
s3Client.setResponse(null, {});
// <!> Only in S3C <!> Backbeat API returns 'InvalidBucketState' error if the bucket is not versioned
backbeatClient.setError(invalidBucketStateError);
task.processActionEntry(entry, err => {
assert.strictEqual(s3Client.calls.deleteObject, 1);
assert.ifError(err);
done();
});
});

it('should expire current version of locked object with legal hold',
done => {
objMd.setLegalHold(true);
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/lifecycle/mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,30 @@ class BackbeatClientMock {
constructor() {
this.mdObj = null;
this.receivedMd = null;
this.error = null;
}

setError(error) {
this.error = error;
}

setMdObj(mdObj) {
this.mdObj = mdObj;
}

getMetadata(params, log, cb) {
if (this.error) {
return cb(this.error);
}

return cb(null, { Body: this.mdObj.getSerialized() });
}

putMetadata(params, log, cb) {
if (this.error) {
return cb(this.error);
}

this.receivedMd = JSON.parse(params.mdBlob);
return cb();
}
Expand Down

0 comments on commit d8ab9f8

Please sign in to comment.