Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LifecycleDeleteObjectTask: BackbeatRoutes in S3C do not support non versioned buckets #2541

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: BB-612
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: BB-612
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 @@
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: BB-612
if (err.ObjNotFound || err.code === 'ObjNotFound' || err.code === 'InvalidBucketState') {

Check warning on line 211 in lib/BackbeatMetadataProxy.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

lib/BackbeatMetadataProxy.js#L211

Added line #L211 was not covered by tests
return cbOnce(err);
}
log.error('an error occurred when getting metadata from S3', {
Expand Down
32 changes: 32 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(() => {
backbeatMdProxyClient.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,27 @@ 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
backbeatMdProxyClient.setError(invalidBucketStateError);
backbeatClient.setResponse(null, {});
task.processActionEntry(entry, err => {
assert.strictEqual(backbeatClient.times.deleteObjectFromExpiration, 1);
assert.ifError(err);
done();
});
});

it('should expire current version of locked object with legal hold',
done => {
objMd.setLegalHold(true);
Expand Down
4 changes: 4 additions & 0 deletions tests/unit/mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ class BackbeatMetadataProxyMock {
return cb(null, this.indexesObj);
}

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

putBucketIndexes(bucket, indexes, log, cb) {
if (this.error) {
return cb(this.error);
Expand Down
Loading