Skip to content

Commit

Permalink
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.

Issue : BB-555
  • Loading branch information
nicolas2bert authored and benzekrimaha committed Sep 19, 2024
1 parent d886d22 commit 4d15eb9
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 2 deletions.
13 changes: 12 additions & 1 deletion extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ class LifecycleDeleteObjectTask extends BackbeatTask {
}, log, (err, blob) => {
LifecycleMetrics.onS3Request(log, 'getMetadata', 'expiration', err);
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 @@ -215,6 +220,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 @@ -204,7 +204,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
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,
BackbeatMetadataProxyMock,
Expand Down Expand Up @@ -42,6 +49,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 @@ -153,6 +164,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
5 changes: 5 additions & 0 deletions tests/unit/mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class BackbeatClientMock {
batchDeleteResponse: 0,
deleteObjectFromExpiration: 0,
};
this.error = null;
}

deleteObjectFromExpiration() {
Expand All @@ -79,6 +80,10 @@ class BackbeatClientMock {
this.response = { error, data };
}

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

batchDelete(params, cb) {
this.times.batchDeleteResponse += 1;

Expand Down

0 comments on commit 4d15eb9

Please sign in to comment.