diff --git a/extensions/lifecycle/tasks/LifecycleTask.js b/extensions/lifecycle/tasks/LifecycleTask.js index 8a8311840..6bf7556f8 100644 --- a/extensions/lifecycle/tasks/LifecycleTask.js +++ b/extensions/lifecycle/tasks/LifecycleTask.js @@ -31,6 +31,8 @@ const errorTransitionInProgress = errors.InternalError. customizeDescription('transition is currently in progress'); const errorTransitionColdObject = errors.InternalError. customizeDescription('transitioning a cold object is forbidden'); +const errorTransitionNonLocalizedObject = errors.InternalError. + customizeDescription('transitioning a non-localized object is forbidden'); const errorObjectTemporarilyRestored = errors.InternalError. customizeDescription('object temporarily restored'); const errorReplicationInProgress = errors.InternalError. @@ -1270,12 +1272,21 @@ class LifecycleTask extends BackbeatTask { return next(errorReplicationInProgress); } const dataStoreName = objectMD.getDataStoreName(); - const isObjectCold = dataStoreName && locationsConfig[dataStoreName] - && locationsConfig[dataStoreName].isCold; + const locationConfig = (dataStoreName + && locationsConfig[dataStoreName]) || {}; // We do not transition cold objects - if (isObjectCold) { + if (locationConfig.isCold) { return next(errorTransitionColdObject); } + // Clean room: the object data still lives on the source + // (isCRR) location. Localization is the only valid transition + // out of such a location, and it is triggered by its own path + // (the queue populator), not by lifecycle rules. The version + // is simply re-evaluated on a later scan, once localization + // has completed. + if (locationConfig.isCRR) { + return next(errorTransitionNonLocalizedObject); + } // If transition is in progress, do not re-publish entry // to data-mover or cold-archive topic. if (objectMD.getTransitionInProgress()) { diff --git a/extensions/replication/ReplicationQueuePopulator.js b/extensions/replication/ReplicationQueuePopulator.js index e8d9b3234..9d9d89907 100644 --- a/extensions/replication/ReplicationQueuePopulator.js +++ b/extensions/replication/ReplicationQueuePopulator.js @@ -87,8 +87,10 @@ class ReplicationQueuePopulator extends QueuePopulatorExtension { // location and first needs to be localized. This is unrelated to // replicationInfo, which tracks replication of a *local* object to // remote sites, hence the check before any replication condition. - if (locationConfig.isCRR && this.localizationConfig) { - this._publishLocalizationAction(entry, queueEntry, value); + if (locationConfig.isCRR) { + if (this.localizationConfig) { + this._publishLocalizationAction(entry, queueEntry, value); + } return; } // Allow a non-versioned object if being replicated from an NFS bucket. diff --git a/tests/unit/lifecycle/LifecycleTask.spec.js b/tests/unit/lifecycle/LifecycleTask.spec.js index 7150bbe14..ae46563b2 100644 --- a/tests/unit/lifecycle/LifecycleTask.spec.js +++ b/tests/unit/lifecycle/LifecycleTask.spec.js @@ -11,6 +11,7 @@ const LifecycleTask = require( const LifecycleTaskV2 = require( '../../../extensions/lifecycle/tasks/LifecycleTaskV2'); const ActionQueueEntry = require('../../../lib/models/ActionQueueEntry'); +const ReplicationAPI = require('../../../extensions/replication/ReplicationAPI'); const { LifecycleMetrics } = require('../../../extensions/lifecycle/LifecycleMetrics'); const fakeLogger = require('../../utils/fakeLogger'); const { withActiveSpan } = require('../../utils/withActiveSpan'); @@ -2473,6 +2474,89 @@ describe('lifecycle task helper methods', () => { }); }); + describe('_applyTransitionRule', () => { + const CRR_LOCATION = 'location-crr-source'; + const testParams = { + bucket: 'test-bucket', + owner: 'test-owner', + objectKey: 'test-key', + versionId: 'test-version-id', + eTag: '"test-etag"', + lastModified: '2023-01-01T00:00:00.000Z', + site: 'test-site', + accountId: 'test-account-id', + transitionTime: Date.now(), + bucketData: { + target: { + bucket: 'test-bucket', + owner: 'test-owner', + accountId: 'test-account-id', + }, + }, + }; + + let lifecycleTask; + let objectMD; + let sendDataMoverAction; + let putObjectMD; + + function setupObjectMD(dataStoreName) { + objectMD = { + getReplicationStatus: () => 'COMPLETED', + getDataStoreName: () => dataStoreName, + getDataStoreVersionId: () => 'version-123', + getTransitionInProgress: () => false, + getArchive: () => undefined, + getContentLength: () => 1024, + getUserMetadata: () => null, + setTransitionInProgress: sinon.spy(), + setOriginOp: sinon.spy(), + getSerialized: () => '{}', + }; + sinon.stub(lifecycleTask, '_getObjectMD') + .callsFake((params, log, cb) => cb(null, objectMD)); + } + + beforeEach(() => { + lifecycleTask = new LifecycleTask(lp); + lifecycleTask.pausedLocations = new Set(); + lifecycleTask.circuitBreakers = { tripped: () => false }; + lifecycleTask.producer = {}; + lifecycleTask.transitionTasksTopic = 'test-transition-topic'; + sendDataMoverAction = sinon.stub( + ReplicationAPI, 'sendDataMoverAction') + .callsFake((producer, entry, log, cb) => cb()); + putObjectMD = sinon.stub(lifecycleTask, '_putObjectMD') + .callsFake((params, log, cb) => cb()); + }); + + it('should not transition an object still on an isCRR location', + done => { + setupObjectMD(CRR_LOCATION); + + lifecycleTask._applyTransitionRule(testParams, fakeLogger, err => { + assert.strictEqual(err.description, + 'transitioning a non-localized object is forbidden'); + sinon.assert.notCalled(sendDataMoverAction); + sinon.assert.notCalled(objectMD.setTransitionInProgress); + sinon.assert.notCalled(putObjectMD); + done(); + }); + }); + + it('should transition an object on a regular location', done => { + setupObjectMD('us-east-1'); + + lifecycleTask._applyTransitionRule(testParams, fakeLogger, err => { + assert.ifError(err); + sinon.assert.calledOnce(sendDataMoverAction); + sinon.assert.calledOnce(objectMD.setTransitionInProgress); + sinon.assert.calledOnce(putObjectMD); + done(); + }); + }); + }); + describe('_sendObjectAction', () => { it('should emit trigger metrics with the entry location', done => { const lifecycleTask = new LifecycleTask(lp); diff --git a/tests/unit/replication/ReplicationQueuePopulator.spec.js b/tests/unit/replication/ReplicationQueuePopulator.spec.js index bb9a95d16..baa31e1dc 100644 --- a/tests/unit/replication/ReplicationQueuePopulator.spec.js +++ b/tests/unit/replication/ReplicationQueuePopulator.spec.js @@ -597,12 +597,33 @@ describe('replication queue populator: clean room localization', () => { sinon.assert.notCalled(params.metricsHandler.localizationObjects); }); - it('should fall back to replication when localization is disabled', () => { + it('should not replicate a non-localized object when localization is ' + + 'disabled', () => { delete params.config.localization; rqp = new RecordingQueuePopulatorMock(params); rqp._filterKeyOp(makeEntry(makeValue())); - assert.strictEqual(rqp.published.length, 1); - assert.strictEqual(rqp.published[0].topic, TOPIC); + assert.strictEqual(rqp.published.length, 0); + }); + + // the data still lives on the source location: there is nothing local to + // replicate, whatever replicationInfo says. + [true, false].forEach(localizationEnabled => { + it('should never replicate a pending non-localized object ' + + `(localization ${localizationEnabled ? 'enabled' : 'disabled'})`, + () => { + if (!localizationEnabled) { + delete params.config.localization; + } + rqp = new RecordingQueuePopulatorMock(params); + const value = makeValue({ + replicationInfo: { ...repInfo, status: 'PENDING' }, + }); + rqp._filterKeyOp(makeEntry(value)); + + assert.strictEqual( + rqp.published.filter(p => p.topic === TOPIC).length, 0); + sinon.assert.notCalled(params.metricsHandler.objects); + }); }); });