Skip to content

Commit 81f5bac

Browse files
delthasdelthas
committed
Override an unversioned object originOp when moving it to a version
This prevents triggering a possible s3:ObjectCreated:Put bucket notification when moving this object to a version, when deleting it (when creating a deletion marker). Issue: CLDSRV-816 Signed-off-by: Thomas Flament <thomas.flament@scality.com>
1 parent 60b7aa4 commit 81f5bac

File tree

5 files changed

+109
-3
lines changed

5 files changed

+109
-3
lines changed

‎lib/api/apiUtils/object/versioning.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ function _storeNullVersionMD(bucketName, objKey, nullVersionId, objMD, log, cb)
9898
isNull2: true,
9999
});
100100
}
101+
nullVersionMD.originOp = 's3:StoreNullVersion';
101102
metadata.putObjectMD(bucketName, objKey, nullVersionMD, { versionId }, log, err => {
102103
if (err) {
103104
log.debug('error from metadata storing null version as new version',

‎tests/unit/api/multipartUpload.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2396,6 +2396,7 @@ describe('complete mpu with versioning', () => {
23962396
assert.strictEqual(objVal.completeInProgress, true);
23972397
} else {
23982398
assert.strictEqual(params.replayId, testUploadId);
2399+
assert.strictEqual(objVal.originOp, 's3:ObjectCreated:CompleteMultipartUpload');
23992400
metadataBackend.putObject = origPutObject;
24002401
}
24012402
origPutObject(
@@ -2413,6 +2414,7 @@ describe('complete mpu with versioning', () => {
24132414
metadataBackend.putObject =
24142415
(putBucketName, objName, objVal, params, log, cb) => {
24152416
assert.strictEqual(params.oldReplayId, testUploadId);
2417+
assert.strictEqual(objVal.originOp, 's3:ObjectCreated:Put');
24162418
metadataBackend.putObject = origPutObject;
24172419
origPutObject(
24182420
putBucketName, objName, objVal, params, log, cb);

‎tests/unit/api/objectCopy.js‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,15 @@ describe('objectCopy with versioning', () => {
113113
objectCopy(authInfo, testObjectCopyRequest, sourceBucketName, objectKey,
114114
undefined, log, err => {
115115
assert.ifError(err, `Unexpected err: ${err}`);
116+
sinon.assert.calledWith(
117+
metadata.putObjectMD.lastCall,
118+
destBucketName,
119+
objectKey,
120+
sinon.match({ _data: { originOp: 's3:ObjectCreated:Copy' } }),
121+
sinon.match.any,
122+
sinon.match.any,
123+
sinon.match.any
124+
);
116125
setImmediate(() => {
117126
versioningTestUtils
118127
.assertDataStoreValues(ds, expectedValues);
@@ -276,7 +285,9 @@ describe('non-versioned objectCopy', () => {
276285
undefined, log, next),
277286
async () => {
278287
sinon.assert.calledWith(metadata.putObjectMD.lastCall,
279-
destBucketName, objectKey, any, sinon.match({
288+
destBucketName, objectKey, sinon.match({
289+
_data: { originOp: 's3:ObjectCreated:Copy' },
290+
}), sinon.match({
280291
needOplogUpdate: undefined,
281292
originOp: undefined,
282293
}), any, any);
@@ -291,7 +302,9 @@ describe('non-versioned objectCopy', () => {
291302
undefined, log, next),
292303
async () => {
293304
sinon.assert.calledWith(metadata.putObjectMD.lastCall,
294-
destBucketName, objectKey, any, sinon.match({
305+
destBucketName, objectKey, sinon.match({
306+
_data: { originOp: 's3:ObjectCreated:Copy' },
307+
}), sinon.match({
295308
needOplogUpdate: undefined,
296309
originOp: undefined,
297310
}), any, any);

‎tests/unit/api/objectDelete.js‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const services = require('../../../lib/services');
77
const { bucketPut } = require('../../../lib/api/bucketPut');
88
const bucketPutACL = require('../../../lib/api/bucketPutACL');
99
const constants = require('../../../constants');
10-
const { cleanup, DummyRequestLogger, makeAuthInfo } = require('../helpers');
10+
const { cleanup, DummyRequestLogger, makeAuthInfo, versioningTestUtils} = require('../helpers');
1111
const objectPut = require('../../../lib/api/objectPut');
1212
const { objectDelete, objectDeleteInternal } = require('../../../lib/api/objectDelete');
1313
const objectGet = require('../../../lib/api/objectGet');
@@ -16,6 +16,7 @@ const mpuUtils = require('../utils/mpuUtils');
1616
const metadataswitch = require('../metadataswitch');
1717
const { fakeMetadataArchive } = require('../../functional/aws-node-sdk/test/utils/init');
1818
const bucketPutNotification = require('../../../lib/api/bucketPutNotification');
19+
const bucketPutVersioning = require('../../../lib/api/bucketPutVersioning');
1920

2021
const any = sinon.match.any;
2122
const originalDeleteObject = services.deleteObject;
@@ -32,6 +33,8 @@ const lateDate = new Date();
3233
earlyDate.setMinutes(earlyDate.getMinutes() - 30);
3334
lateDate.setMinutes(lateDate.getMinutes() + 30);
3435

36+
const enableVersioningRequest = versioningTestUtils.createBucketPutVersioningReq(bucketName, 'Enabled');
37+
3538
function testAuth(bucketOwner, authUser, bucketPutReq, objPutReq, objDelReq,
3639
log, cb) {
3740
bucketPut(bucketOwner, bucketPutReq, log, () => {
@@ -371,3 +374,65 @@ describe('objectDelete API', () => {
371374
});
372375
});
373376
});
377+
378+
describe('objectDelete API with versioning', () => {
379+
let testPutObjectRequest;
380+
381+
beforeEach(() => {
382+
cleanup();
383+
testPutObjectRequest = new DummyRequest({
384+
bucketName,
385+
namespace,
386+
objectKey,
387+
headers: {},
388+
url: `/${bucketName}/${objectKey}`,
389+
}, postBody);
390+
391+
sinon.stub(services, 'deleteObject').callsFake(originalDeleteObject);
392+
sinon.spy(metadataswitch, 'putObjectMD');
393+
sinon.spy(metadataswitch, 'deleteObjectMD');
394+
});
395+
396+
afterEach(() => {
397+
sinon.restore();
398+
});
399+
400+
const testBucketPutRequest = new DummyRequest({
401+
bucketName,
402+
namespace,
403+
headers: {},
404+
url: `/${bucketName}`,
405+
});
406+
const testDeleteRequest = new DummyRequest({
407+
bucketName,
408+
namespace,
409+
objectKey,
410+
headers: {},
411+
url: `/${bucketName}/${objectKey}`,
412+
});
413+
414+
it('should upgrade master-only document to a version document when storing a delete marker version', done => {
415+
async.series([
416+
next => bucketPut(authInfo, testBucketPutRequest, log, next),
417+
next => objectPut(authInfo, testPutObjectRequest, undefined, log, next),
418+
next => bucketPutVersioning(authInfo, enableVersioningRequest, log, next),
419+
next => objectDelete(authInfo, testDeleteRequest, log, next),
420+
async () => {
421+
const calls = metadataswitch.putObjectMD.getCalls();
422+
sinon.assert.calledWith(calls[calls.length - 2],
423+
bucketName, objectKey, sinon.match({
424+
versionId: sinon.match.truthy,
425+
isNull: true,
426+
originOp: 's3:StoreNullVersion',
427+
}), any, any, any);
428+
},
429+
async () => {
430+
// New version document (delete marker) was created with the right originOp.
431+
sinon.assert.calledWith(metadataswitch.putObjectMD.lastCall,
432+
bucketName, objectKey, sinon.match({
433+
_data: { originOp: 's3:ObjectRemoved:DeleteMarkerCreated' },
434+
}), any, any, any);
435+
},
436+
], done);
437+
});
438+
});

‎tests/unit/api/objectPut.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,31 @@ describe('objectPut API with versioning', () => {
980980
});
981981
});
982982

983+
it('should set originOp when moving master-only document to a version document', done => {
984+
async.series([
985+
next => bucketPut(authInfo, testPutBucketRequest, log, next),
986+
next => objectPut(authInfo, testPutObjectRequest, undefined, log, next),
987+
next => bucketPutVersioning(authInfo, enableVersioningRequest, log, next),
988+
next => objectPut(authInfo, testPutObjectRequest, undefined, log, next),
989+
async () => {
990+
// Old master-only document was moved to a proper version, with originOp overridden to prevent
991+
// unexpected bucket notifications.
992+
const calls = metadata.putObjectMD.getCalls();
993+
sinon.assert.calledWith(calls[calls.length - 2],
994+
bucketName, objectName, sinon.match({
995+
originOp: 's3:StoreNullVersion',
996+
}), any, any, any);
997+
},
998+
async () => {
999+
// New version document was created with the right originOp.
1000+
sinon.assert.calledWith(metadata.putObjectMD.lastCall,
1001+
bucketName, objectName, sinon.match({
1002+
_data: { originOp: 's3:ObjectCreated:Put' },
1003+
}), any, any, any);
1004+
},
1005+
], done);
1006+
});
1007+
9831008
it('should not pass needOplogUpdate when writing new object', done => {
9841009
async.series([
9851010
next => bucketPut(authInfo, testPutBucketRequest, log, next),

0 commit comments

Comments
 (0)