From a4f9c62e2aac4319ac726ab540ec40e8cb5c6b0a Mon Sep 17 00:00:00 2001 From: Francois Ferrand Date: Mon, 24 Aug 2026 10:48:03 +0200 Subject: [PATCH 1/3] Do not garbage-collect the from-location when it is an isCRR location Data sitting on an isCRR location is production data owned by a remote site: we may read it, but deleting it is never ours to do. The copy engine reuses the lifecycle transition pipeline, which garbage-collects the from-location once the new one is merged into the metadata - against an isCRR source that would wipe the remote production copy. Unlike a transition, a localization merge must therefore leave the from-location alone. Keying this on the location type rather than making it a clean-room special case also gives us replay safety for duplicate copy actions: the second merge supersedes the first and collects its copy, which is only safe because the first never touched the remote source. Issue: BB-813 --- conf/locationConfig.json | 7 ++ .../tasks/LifecycleUpdateTransitionTask.js | 18 ++++- lib/util/locations.js | 52 ++++++++++++++ tests/unit/lib/util/locations.spec.js | 67 +++++++++++++++++++ .../lifecycle/CircuitBreakerGroup.spec.js | 10 +++ .../LifecycleUpdateTransitionTask.spec.js | 43 ++++++++++++ 6 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 lib/util/locations.js create mode 100644 tests/unit/lib/util/locations.spec.js diff --git a/conf/locationConfig.json b/conf/locationConfig.json index 8ba3dc334c..dceb31ddac 100644 --- a/conf/locationConfig.json +++ b/conf/locationConfig.json @@ -42,5 +42,12 @@ "legacyAwsBehavior": false, "isCold": true, "details": {} + }, + "location-crr-source": { + "type": "scality", + "objectId": "location-crr-source", + "legacyAwsBehavior": false, + "isCRR": true, + "details": {} } } diff --git a/extensions/lifecycle/tasks/LifecycleUpdateTransitionTask.js b/extensions/lifecycle/tasks/LifecycleUpdateTransitionTask.js index 4f57c33c7f..52801e5820 100644 --- a/extensions/lifecycle/tasks/LifecycleUpdateTransitionTask.js +++ b/extensions/lifecycle/tasks/LifecycleUpdateTransitionTask.js @@ -6,6 +6,7 @@ const BackbeatTask = require('../../../lib/tasks/BackbeatTask'); const ActionQueueEntry = require('../../../lib/models/ActionQueueEntry'); const ObjectMD = require('arsenal').models.ObjectMD; const { LifecycleMetrics } = require('../LifecycleMetrics'); +const { filterOutCRRLocations, getCRRLocationNames } = require('../../../lib/util/locations'); /** @typedef { import('../objectProcessor/LifecycleObjectProcessor.js') } LifecycleObjectProcessor */ class LifecycleUpdateTransitionTask extends BackbeatTask { @@ -112,6 +113,21 @@ class LifecycleUpdateTransitionTask extends BackbeatTask { _garbageCollectLocation(entry, locations, log, done) { const { bucket, key, version, eTag, accountId, owner } = this.getTargetAttribute(entry); + // Data stored on a CRR location belongs to the remote site: the copy we + // just made is an extra local copy, the source must be left untouched. + const locationsToGC = filterOutCRRLocations(locations); + if (locationsToGC.length !== locations.length) { + log.info('skipping garbage collection of data on CRR location', { + method: 'LifecycleUpdateTransitionTask._garbageCollectLocation', + bucket, + objectKey: key, + versionId: version, + dataStoreNames: getCRRLocationNames(locations), + }); + } + if (locationsToGC.length === 0) { + return process.nextTick(done); + } const gcEntry = ActionQueueEntry.create('deleteData') .addContext({ origin: 'lifecycle', @@ -126,7 +142,7 @@ class LifecycleUpdateTransitionTask extends BackbeatTask { .setAttribute('serviceName', 'lifecycle-transition') .setAttribute('target.accountId', accountId) .setAttribute('target.owner', owner) - .setAttribute('target.locations', locations); + .setAttribute('target.locations', locationsToGC); this.gcProducer.publishActionEntry(gcEntry); return process.nextTick(done); } diff --git a/lib/util/locations.js b/lib/util/locations.js new file mode 100644 index 0000000000..fb09e6eb14 --- /dev/null +++ b/lib/util/locations.js @@ -0,0 +1,52 @@ +const locationsConfig = require('../../conf/locationConfig.json') || {}; + +/** + * Tell whether a location holds data owned by a remote site. + * + * Data stored on such a location is remote production data: we may read it + * (e.g. to copy it locally), but we must never delete it. + * + * @param {String} dataStoreName - location name + * @return {Boolean} true if the location is a CRR (remote) location + */ +function isCRRLocation(dataStoreName) { + return Boolean(dataStoreName && locationsConfig[dataStoreName] && + locationsConfig[dataStoreName].isCRR); +} + +/** + * Remove from a list of location parts those living on a CRR location, + * i.e. those which must never be garbage-collected. + * + * @param {Object[]} locations - array of location parts + * @return {Object[]} the location parts which are safe to delete + */ +function filterOutCRRLocations(locations) { + if (!Array.isArray(locations)) { + return []; + } + return locations.filter(location => !isCRRLocation(location && location.dataStoreName)); +} + +/** + * List the distinct CRR location names found in a list of location parts, + * for logging purposes. + * + * @param {Object[]} locations - array of location parts + * @return {String[]} distinct CRR location names + */ +function getCRRLocationNames(locations) { + if (!Array.isArray(locations)) { + return []; + } + const names = locations + .map(location => location && location.dataStoreName) + .filter(dataStoreName => isCRRLocation(dataStoreName)); + return [...new Set(names)]; +} + +module.exports = { + isCRRLocation, + filterOutCRRLocations, + getCRRLocationNames, +}; diff --git a/tests/unit/lib/util/locations.spec.js b/tests/unit/lib/util/locations.spec.js new file mode 100644 index 0000000000..c86e0fe355 --- /dev/null +++ b/tests/unit/lib/util/locations.spec.js @@ -0,0 +1,67 @@ +const assert = require('assert'); + +const { + isCRRLocation, + filterOutCRRLocations, + getCRRLocationNames, +} = require('../../../../lib/util/locations'); + +const crrPart = { + key: 'crrKey', + size: 10, + start: 0, + dataStoreName: 'location-crr-source', + dataStoreType: 'scality', +}; +const localPart = { + key: 'localKey', + size: 10, + start: 0, + dataStoreName: 'us-east-1', + dataStoreType: 'file', +}; + +describe('locations util', () => { + describe('isCRRLocation', () => { + it('should return true for a location flagged isCRR', () => { + assert.strictEqual(isCRRLocation('location-crr-source'), true); + }); + + it('should return false for a regular location', () => { + assert.strictEqual(isCRRLocation('us-east-1'), false); + }); + + it('should return false for an unknown or missing location', () => { + assert.strictEqual(isCRRLocation('does-not-exist'), false); + assert.strictEqual(isCRRLocation(undefined), false); + }); + }); + + describe('filterOutCRRLocations', () => { + it('should drop the parts living on a CRR location', () => { + assert.deepStrictEqual( + filterOutCRRLocations([localPart, crrPart]), [localPart]); + }); + + it('should keep all parts when none is on a CRR location', () => { + assert.deepStrictEqual( + filterOutCRRLocations([localPart]), [localPart]); + }); + + it('should return an empty array when locations is not an array', () => { + assert.deepStrictEqual(filterOutCRRLocations(undefined), []); + }); + }); + + describe('getCRRLocationNames', () => { + it('should list the distinct CRR location names', () => { + assert.deepStrictEqual( + getCRRLocationNames([localPart, crrPart, crrPart]), + ['location-crr-source']); + }); + + it('should return an empty array when there is no CRR location', () => { + assert.deepStrictEqual(getCRRLocationNames([localPart]), []); + }); + }); +}); diff --git a/tests/unit/lifecycle/CircuitBreakerGroup.spec.js b/tests/unit/lifecycle/CircuitBreakerGroup.spec.js index 56e825d455..c8c8737b6f 100644 --- a/tests/unit/lifecycle/CircuitBreakerGroup.spec.js +++ b/tests/unit/lifecycle/CircuitBreakerGroup.spec.js @@ -436,6 +436,11 @@ describe('extractBucketProcessorCircuitBreakerConfigs', () => { '${location}', 'location-dmf-v1', ), + formatProbeConfig( + topicSpecificLocationTemplateProbe, + '${location}', + 'location-crr-source', + ), ], }, global: [], @@ -493,6 +498,11 @@ describe('extractBucketProcessorCircuitBreakerConfigs', () => { '${location}', 'location-dmf-v1', ), + formatProbeConfig( + topicSpecificLocationTemplateProbe, + '${location}', + 'location-crr-source', + ), ], }, global: [], diff --git a/tests/unit/lifecycle/LifecycleUpdateTransitionTask.spec.js b/tests/unit/lifecycle/LifecycleUpdateTransitionTask.spec.js index 61748e96e3..d05a1abd6b 100644 --- a/tests/unit/lifecycle/LifecycleUpdateTransitionTask.spec.js +++ b/tests/unit/lifecycle/LifecycleUpdateTransitionTask.spec.js @@ -148,6 +148,49 @@ describe('LifecycleUpdateTransitionTask', () => { }); }); + it('should update metadata but not GC the from-location when it is a CRR ' + + 'location', done => { + const crrLocation = [Object.assign({}, oldLocation[0], + { dataStoreName: 'location-crr-source' })]; + mdObj.setLocation(crrLocation); + task.processActionEntry(actionEntry, err => { + assert.ifError(err); + const receivedMd = backbeatMetadataProxyClient.getReceivedMd(); + assert.deepStrictEqual(receivedMd.location, newLocation); + assert.strictEqual(gcProducer.getReceivedEntry(), null); + done(); + }); + }); + + it('should only GC the parts which are not on a CRR location', done => { + const crrPart = Object.assign({}, oldLocation[0], + { key: 'crrKey', dataStoreName: 'location-crr-source' }); + mdObj.setLocation([crrPart, ...oldLocation]); + task.processActionEntry(actionEntry, err => { + assert.ifError(err); + const receivedGcEntry = gcProducer.getReceivedEntry(); + assert.deepStrictEqual( + receivedGcEntry.getAttribute('target.locations'), oldLocation); + done(); + }); + }); + + it('should still GC the new location on rollback even if the ' + + 'from-location is a CRR location', done => { + mdObj.setLocation([Object.assign({}, oldLocation[0], + { dataStoreName: 'location-crr-source' })]); + actionEntry.setAttribute('target.eTag', + '"6713e7cf89b6b16d5abf11d1fabac587"'); + task.processActionEntry(actionEntry, err => { + assert.ifError(err); + assert.strictEqual(backbeatMetadataProxyClient.getReceivedMd(), null); + const receivedGcEntry = gcProducer.getReceivedEntry(); + assert.deepStrictEqual( + receivedGcEntry.getAttribute('target.locations'), newLocation); + done(); + }); + }); + it('should reset transition-in-progress flag when transition fails', done => { actionEntry.setError(errors.InternalError); task.processActionEntry(actionEntry, err => { From c5a5e88d72e4e87488476edaa18a87129bbb1fb3 Mon Sep 17 00:00:00 2001 From: Francois Ferrand Date: Mon, 24 Aug 2026 10:48:10 +0200 Subject: [PATCH 2/3] Skip isCRR location parts in the GC service Same no-GC rule as in the copy engine, applied where data actually gets deleted: whatever published the deleteData action, parts living on an isCRR location are remote production data and get skipped rather than deleted. That covers every publisher, notably restored-object expiration, where expiring a clean-room object that was never localized would otherwise delete the production copy. Reaching the GC service with such a location means something upstream is wrong, so it warns, but deleting is never the right answer: the entry is still completed and the offset committed, and no completion metric is emitted for a delete that did not happen. Issue: BB-818 --- extensions/gc/tasks/GarbageCollectorTask.js | 25 ++++- tests/unit/gc/GarbageCollectorTask.spec.js | 109 ++++++++++++++++++++ 2 files changed, 131 insertions(+), 3 deletions(-) diff --git a/extensions/gc/tasks/GarbageCollectorTask.js b/extensions/gc/tasks/GarbageCollectorTask.js index dae27c5807..812691165e 100644 --- a/extensions/gc/tasks/GarbageCollectorTask.js +++ b/extensions/gc/tasks/GarbageCollectorTask.js @@ -5,6 +5,7 @@ const { ObjectMD } = require('arsenal').models; const BackbeatTask = require('../../../lib/tasks/BackbeatTask'); const { BatchDeleteCommand } = require('@scality/cloudserverclient'); const { GarbageCollectorMetrics } = require('../GarbageCollectorMetrics'); +const { filterOutCRRLocations, getCRRLocationNames } = require('../../../lib/util/locations'); /** @typedef { import('../GarbageCollector.js') } GarbageCollector */ class GarbageCollectorTask extends BackbeatTask { @@ -142,8 +143,26 @@ class GarbageCollectorTask extends BackbeatTask { _executeDeleteDataOnce(entry, log, done) { const { locations } = entry.getAttribute('target'); const ruleType = entry.getContextAttribute('ruleType'); + // Last line of defense: whoever published this entry, data on a CRR + // location belongs to the remote site and must never be deleted. + const locationsToDelete = filterOutCRRLocations(locations); + if (locationsToDelete.length !== (locations || []).length) { + log.warn('refusing to delete data on CRR location', { + method: 'GarbageCollectorTask._executeDeleteDataOnce', + bucket: entry.getAttribute('source.bucket'), + objectKey: entry.getAttribute('source.objectKey'), + dataStoreNames: getCRRLocationNames(locations), + ruleType, + ...entry.getLogInfo(), + }); + } + if (locationsToDelete.length === 0) { + entry.setEnd(null); + log.info('action execution ended, nothing to delete', entry.getLogInfo()); + return process.nextTick(done); + } const params = { - Locations: locations.map(location => ({ + Locations: locationsToDelete.map(location => ({ key: location.key, dataStoreName: location.dataStoreName, size: location.size, @@ -159,7 +178,7 @@ class GarbageCollectorTask extends BackbeatTask { }), }; - this._batchDeleteData(params, entry, log, err => { + return this._batchDeleteData(params, entry, log, err => { // ruleType can be either `transition` or `restore` (for restore-expiration) GarbageCollectorMetrics.onS3Request(log, 'batchdelete', ruleType, err); entry.setEnd(err); @@ -184,7 +203,7 @@ class GarbageCollectorTask extends BackbeatTask { } GarbageCollectorMetrics.onGcCompleted(log, ruleType, - locations[0]?.dataStoreName, Date.now() - entry.getAttribute('timestamp')); + locationsToDelete[0]?.dataStoreName, Date.now() - entry.getAttribute('timestamp')); return done(); }); } diff --git a/tests/unit/gc/GarbageCollectorTask.spec.js b/tests/unit/gc/GarbageCollectorTask.spec.js index 3cd4d7fdbc..36a8ae4133 100644 --- a/tests/unit/gc/GarbageCollectorTask.spec.js +++ b/tests/unit/gc/GarbageCollectorTask.spec.js @@ -387,4 +387,113 @@ describe('GarbageCollectorTask', () => { }); }); + describe('with CRR locations', () => { + let log; + + function createDeleteDataEntry(locations) { + return ActionQueueEntry.create('deleteData') + .addContext({ + origin: 'lifecycle', + ruleType: 'transition', + bucketName: bucket, + objectKey: key, + versionId: version, + }) + .setAttribute('serviceName', 'lifecycle-transition') + .setAttribute('source', { + bucket, + objectKey: key, + storageClass: 'sourceStorageClass', + }) + .setAttribute('target', { + bucket, + key: version, + version: key, + accountId, + owner, + locations, + }); + } + + const crrLocation = { + key: 'crrKey', + dataStoreName: 'location-crr-source', + size: 10, + dataStoreVersionId: 'crrVersionId', + }; + const regularLocation = { + key: 'locationKey', + dataStoreName: 'us-east-1', + size: 20, + dataStoreVersionId: 'dataStoreVersionId', + }; + + beforeEach(() => { + log = { + info: sinon.spy(), + warn: sinon.spy(), + debug: sinon.spy(), + error: sinon.spy(), + getSerializedUids: () => 'uids', + }; + log.end = () => log; + gcTask.logger = { newRequestLogger: () => log }; + backbeatClient.batchDeleteResponse = { error: null, res: null }; + }); + + it('should not delete anything and warn when all locations are on a ' + + 'CRR location', done => { + const entry = createDeleteDataEntry([crrLocation]); + const batchDeleteDataSpy = sinon.spy(gcTask, '_batchDeleteData'); + const onGcCompletedSpy = sinon.spy(GarbageCollectorMetrics, 'onGcCompleted'); + + gcTask.processActionEntry(entry, err => { + assert.ifError(err); + assert.strictEqual(batchDeleteDataSpy.callCount, 0); + assert.strictEqual(backbeatClient.times.batchDeleteResponse, 0); + assert.strictEqual(onGcCompletedSpy.callCount, 0); + assert.strictEqual(log.warn.callCount, 1); + assert.deepStrictEqual( + log.warn.firstCall.args[1].dataStoreNames, + ['location-crr-source']); + assert.strictEqual(entry.getStatus(), 'success'); + batchDeleteDataSpy.restore(); + onGcCompletedSpy.restore(); + done(); + }); + }); + + it('should only delete the parts which are not on a CRR location', done => { + const entry = createDeleteDataEntry([crrLocation, regularLocation]); + const batchDeleteDataSpy = sinon.spy(gcTask, '_batchDeleteData'); + + gcTask.processActionEntry(entry, err => { + assert.ifError(err); + assert.strictEqual(batchDeleteDataSpy.callCount, 1); + assert.deepStrictEqual( + batchDeleteDataSpy.firstCall.args[0].Locations, + [regularLocation]); + assert.strictEqual(log.warn.callCount, 1); + batchDeleteDataSpy.restore(); + done(); + }); + }); + + it('should delete all locations and not warn when none is on a CRR ' + + 'location', done => { + const entry = createDeleteDataEntry([regularLocation]); + const batchDeleteDataSpy = sinon.spy(gcTask, '_batchDeleteData'); + + gcTask.processActionEntry(entry, err => { + assert.ifError(err); + assert.strictEqual(batchDeleteDataSpy.callCount, 1); + assert.deepStrictEqual( + batchDeleteDataSpy.firstCall.args[0].Locations, + [regularLocation]); + assert.strictEqual(log.warn.callCount, 0); + batchDeleteDataSpy.restore(); + done(); + }); + }); + }); }); From 50c63e971ad83f9f479d99179f3cfc1a7efa27d1 Mon Sep 17 00:00:00 2001 From: Francois Ferrand Date: Mon, 24 Aug 2026 12:19:52 +0200 Subject: [PATCH 3/3] Reclaim local data when a localized version is deleted In a clean room the bucket location constraint is the isCRR source location, so once an object has been localized its metadata looks exactly like a transitioned object: the mongo processor was bailing out with "transitioned to another location", never deleting the metadata and leaving the local copy of the data behind forever. Detect that case and, after the metadata delete, publish a deleteData action for the local parts only. Versions which were never localized still point at the isCRR location and are left alone, as before. Issue: BB-815 --- .../mongoProcessor/MongoQueueProcessor.js | 96 ++++++++++++- .../mongoProcessor/mongoProcessorTask.js | 3 +- .../MongoQueueProcessor.spec.js | 126 ++++++++++++++++++ 3 files changed, 218 insertions(+), 7 deletions(-) diff --git a/extensions/mongoProcessor/MongoQueueProcessor.js b/extensions/mongoProcessor/MongoQueueProcessor.js index cc2843f074..d4ffdcb7bf 100644 --- a/extensions/mongoProcessor/MongoQueueProcessor.js +++ b/extensions/mongoProcessor/MongoQueueProcessor.js @@ -13,6 +13,7 @@ const { extractVersionId } = require('../../lib/util/versioning'); const Config = require('../../lib/Config'); const BackbeatConsumer = require('../../lib/BackbeatConsumer'); +const ActionQueueEntry = require('../../lib/models/ActionQueueEntry'); const QueueEntry = require('../../lib/models/QueueEntry'); const DeleteOpQueueEntry = require('../../lib/models/DeleteOpQueueEntry'); const ObjectQueueEntry = require('../../lib/models/ObjectQueueEntry'); @@ -22,6 +23,8 @@ const { metricsExtension, metricsTypeCompleted, metricsTypePendingOnly } = const getContentType = require('./utils/contentTypeHelper'); const BucketMemState = require('./utils/BucketMemState'); const MongoProcessorMetrics = require('./MongoProcessorMetrics'); +const GarbageCollectorProducer = require('../gc/GarbageCollectorProducer'); +const { isCRRLocation, filterOutCRRLocations } = require('../../lib/util/locations'); // batch metrics by location and send to kafka metrics topic every 5 seconds const METRIC_REPORT_INTERVAL_MS = process.env.CI === 'true' ? 1000 : 5000; @@ -64,13 +67,17 @@ class MongoQueueProcessor { * @param {number} [mongoProcessorConfig.concurrency] - consumer concurrency * @param {Object} mongoClientConfig - config for connecting to mongo * @param {Object} mConfig - metrics config + * @param {Object} [gcConfig] - garbage collector config, required to reclaim + * the data of localized objects */ - constructor(kafkaConfig, mongoProcessorConfig, mongoClientConfig, mConfig) { + constructor(kafkaConfig, mongoProcessorConfig, mongoClientConfig, mConfig, gcConfig) { this.kafkaConfig = kafkaConfig; this.mongoProcessorConfig = mongoProcessorConfig; this.mongoClientConfig = mongoClientConfig; this._mConfig = mConfig; + this._gcConfig = gcConfig; + this._gcProducer = null; this._consumer = null; this._bootstrapList = null; this.logger = new Logger('Backbeat:Ingestion:MongoProcessor'); @@ -119,6 +126,16 @@ class MongoQueueProcessor { } return next(err); }), + next => { + if (!this._gcConfig) { + this.logger.info('no garbage collector configured', { + method: 'MongoQueueProcessor.start', + }); + return next(); + } + this._gcProducer = new GarbageCollectorProducer(); + return this._gcProducer.setupProducer(next); + }, ], error => { if (error) { this.logger.fatal('error starting mongo queue processor'); @@ -377,16 +394,24 @@ class MongoQueueProcessor { async.waterfall([ cb => this._getZenkoObjectMetadata(log, sourceEntry, versionId, cb), (zenkoObjMd, cb) => { + // In a clean room the bucket location constraint is the (isCRR) source + // location: once an object has been localized its data does not live there + // anymore, so it looks exactly like a transitioned object. It must still be + // deleted, and the local copy of the data reclaimed. + const localizedLocations = isCRRLocation(location) ? + filterOutCRRLocations(zenkoObjMd.location) : []; + // Skip if the object is in a different location, i.e. when the delete was caused // by restored-object expiration or transition. It works because the dataStoreName // is updated before actually sending the object to GC to effectively delete the // data. const encode = versionId => (versionId ? VersionID.encode(versionId) : 'null'); - if (zenkoObjMd.dataStoreName !== location || + if (localizedLocations.length === 0 && ( + zenkoObjMd.dataStoreName !== location || zenkoObjMd.location?.length !== 1 || zenkoObjMd.location[0].dataStoreName !== location || zenkoObjMd.location[0].key !== key || - (zenkoObjMd.location[0].dataStoreVersionId || 'null') !== encode(entryVersionId) + (zenkoObjMd.location[0].dataStoreVersionId || 'null') !== encode(entryVersionId)) ) { log.end().info('ignore delete entry, transitioned to another location', { entry: sourceEntry.getLogInfo(), @@ -395,9 +420,9 @@ class MongoQueueProcessor { return done(); } - return cb(null, zenkoObjMd); + return cb(null, zenkoObjMd, localizedLocations); }, - (zenkoObjMd, cb) => { + (zenkoObjMd, localizedLocations, cb) => { const options = {}; // Calling deleteObject with empty options to use deleteObjectNoVer which is used @@ -418,7 +443,19 @@ class MongoQueueProcessor { options.doesNotNeedOpogUpdate = true; } - return this._mongoClient.deleteObject(bucket, key, options, log, cb); + return this._mongoClient.deleteObject(bucket, key, options, log, err => { + if (err) { + return cb(err); + } + // The data copied locally is not referenced by anything anymore: reclaim + // it. Doing it after the metadata delete means a failure here leaks the + // data, instead of losing it. + if (localizedLocations.length > 0) { + this._garbageCollectData(log, sourceEntry, zenkoObjMd, versionId, + localizedLocations); + } + return cb(); + }); }, ], err => { if (err?.is.NoSuchKey) { @@ -448,6 +485,53 @@ class MongoQueueProcessor { }); } + /** + * Publish a garbage collection entry to reclaim the data of an object which has just + * been deleted from mongo. + * @param {Logger.newRequestLogger} log - request logger object + * @param {DeleteOpQueueEntry} sourceEntry - delete object entry + * @param {Object} zenkoObjMd - metadata of the object which was deleted + * @param {string} versionId - decoded version id of the object which was deleted + * @param {Object[]} locations - location parts to delete + * @return {undefined} + */ + _garbageCollectData(log, sourceEntry, zenkoObjMd, versionId, locations) { + const bucket = sourceEntry.getBucket(); + const key = sourceEntry.getObjectKey(); + + if (!this._gcProducer) { + log.error('no garbage collector configured, cannot reclaim the data', { + bucket, + objectKey: key, + versionId, + }); + return; + } + + // The version is already gone, so no lastModified is passed along: the GC must not + // send any conditional header, only the locations matter here. + const gcEntry = ActionQueueEntry.create('deleteData') + .addContext({ + origin: 'localization', + ruleType: 'transition', + reqId: log.getSerializedUids(), + bucketName: bucket, + objectKey: key, + versionId: versionId ? VersionID.encode(versionId) : undefined, + eTag: zenkoObjMd['content-md5'], + }) + .setAttribute('source', { + bucket, + objectKey: key, + storageClass: zenkoObjMd.dataStoreName, + }) + .setAttribute('serviceName', 'md-ingestion') + .setAttribute('target.owner', zenkoObjMd['owner-id']) + .setAttribute('target.locations', locations); + + this._gcProducer.publishActionEntry(gcEntry); + } + /** * Process an object entry * @param {Logger.newRequestLogger} log - request logger object diff --git a/extensions/mongoProcessor/mongoProcessorTask.js b/extensions/mongoProcessor/mongoProcessorTask.js index e0ccf02581..5b0e3412b9 100644 --- a/extensions/mongoProcessor/mongoProcessorTask.js +++ b/extensions/mongoProcessor/mongoProcessorTask.js @@ -21,6 +21,7 @@ const mongoProcessorConfig = config.extensions.mongoProcessor; // TODO: consider whether we would want a separate mongo config // for the consumer side const mongoClientConfig = config.queuePopulator.mongo; +const gcConfig = config.extensions.gc; const log = new werelogs.Logger('Backbeat:MongoProcessor:task'); const mongoProcessorLogConfig = mongoProcessorConfig.log ?? config.log; @@ -28,7 +29,7 @@ werelogs.configure({ level: mongoProcessorLogConfig.logLevel, dump: mongoProcessorLogConfig.dumpLevel }); const mqp = new MongoQueueProcessor(kafkaConfig, mongoProcessorConfig, - mongoClientConfig, mConfig); + mongoClientConfig, mConfig, gcConfig); /** * Handle ProbeServer liveness check diff --git a/tests/unit/mongoProcessor/MongoQueueProcessor.spec.js b/tests/unit/mongoProcessor/MongoQueueProcessor.spec.js index 4ed3b85a10..45331f6435 100644 --- a/tests/unit/mongoProcessor/MongoQueueProcessor.spec.js +++ b/tests/unit/mongoProcessor/MongoQueueProcessor.spec.js @@ -1,9 +1,20 @@ const assert = require('assert'); +const sinon = require('sinon'); + +const { VersionID, VersioningConstants } = require('arsenal').versioning; const MongoQueueProcessor = require('../../../extensions/mongoProcessor/MongoQueueProcessor'); const ObjectQueueEntry = require('../../../lib/models/ObjectQueueEntry'); +const DeleteOpQueueEntry = + require('../../../lib/models/DeleteOpQueueEntry'); + +const VID_SEP = VersioningConstants.VersionId.Separator; + +// see conf/locationConfig.json +const CRR_LOCATION = 'location-crr-source'; +const LOCAL_LOCATION = 'us-east-1'; function _makeProcessor(bootstrapList) { const proc = Object.create(MongoQueueProcessor.prototype); @@ -234,3 +245,118 @@ describe('MongoQueueProcessor._updateReplicationInfo', () => { assert.strictEqual(bySite['cloud-b'].status, 'PENDING'); }); }); + +describe('MongoQueueProcessor._processDeleteOpQueueEntry', () => { + const bucket = 'cleanroom-bucket'; + const objectKey = 'docs/report.pdf'; + const versionId = '98765432109876999999RG001 1'; + const encodedVersionId = VersionID.encode(versionId); + + function _makeLog() { + const log = { + debug: () => {}, + info: () => {}, + error: () => {}, + warn: () => {}, + getSerializedUids: () => 'req-uid', + }; + log.end = () => log; + return log; + } + + function _makeDeleteProcessor(zenkoObjMd, gcProducer) { + const proc = Object.create(MongoQueueProcessor.prototype); + proc.logger = { debug: () => {} }; + proc._gcProducer = gcProducer; + proc._mongoClient = { + deleteObject: sinon.stub().callsFake((b, k, opts, log, cb) => cb()), + }; + proc._getZenkoObjectMetadata = + sinon.stub().callsFake((log, entry, vid, cb) => cb(null, zenkoObjMd)); + proc._produceMetricCompletionEntry = () => {}; + proc._normalizePendingMetric = () => {}; + return proc; + } + + function _makeEntry() { + return new DeleteOpQueueEntry(bucket, `${objectKey}${VID_SEP}${versionId}`, {}); + } + + it('deletes a localized version and publishes its local data for GC', done => { + const zenkoObjMd = { + 'dataStoreName': LOCAL_LOCATION, + 'owner-id': 'owner-canonical-id', + 'content-md5': 'etag-value', + 'location': [{ dataStoreName: LOCAL_LOCATION, key: 'local-data-key' }], + }; + const gcProducer = { publishActionEntry: sinon.stub() }; + const proc = _makeDeleteProcessor(zenkoObjMd, gcProducer); + + proc._processDeleteOpQueueEntry(_makeLog(), _makeEntry(), CRR_LOCATION, {}, err => { + assert.ifError(err); + assert.strictEqual(proc._mongoClient.deleteObject.callCount, 1); + assert.strictEqual(gcProducer.publishActionEntry.callCount, 1); + + const gcEntry = gcProducer.publishActionEntry.firstCall.args[0]; + assert.strictEqual(gcEntry.getActionType(), 'deleteData'); + assert.deepStrictEqual(gcEntry.getAttribute('target.locations'), + zenkoObjMd.location); + assert.strictEqual(gcEntry.getAttribute('target.owner'), 'owner-canonical-id'); + assert.strictEqual(gcEntry.getAttribute('serviceName'), 'md-ingestion'); + // the version is gone: no conditional header must be sent by the GC + assert.strictEqual(gcEntry.getAttribute('source').lastModified, undefined); + assert.strictEqual(gcEntry.getContextAttribute('versionId'), encodedVersionId); + done(); + }); + }); + + it('does not publish anything for a version which was never localized', done => { + const zenkoObjMd = { + dataStoreName: CRR_LOCATION, + location: [{ + dataStoreName: CRR_LOCATION, + key: objectKey, + dataStoreVersionId: encodedVersionId, + }], + }; + const gcProducer = { publishActionEntry: sinon.stub() }; + const proc = _makeDeleteProcessor(zenkoObjMd, gcProducer); + + proc._processDeleteOpQueueEntry(_makeLog(), _makeEntry(), CRR_LOCATION, {}, err => { + assert.ifError(err); + assert.strictEqual(proc._mongoClient.deleteObject.callCount, 1); + assert.strictEqual(gcProducer.publishActionEntry.callCount, 0); + done(); + }); + }); + + it('still ignores an object transitioned outside of a clean room', done => { + const zenkoObjMd = { + dataStoreName: 'location-dmf-v1', + location: [{ dataStoreName: 'location-dmf-v1', key: 'cold-key' }], + }; + const gcProducer = { publishActionEntry: sinon.stub() }; + const proc = _makeDeleteProcessor(zenkoObjMd, gcProducer); + + proc._processDeleteOpQueueEntry(_makeLog(), _makeEntry(), LOCAL_LOCATION, {}, err => { + assert.ifError(err); + assert.strictEqual(proc._mongoClient.deleteObject.callCount, 0); + assert.strictEqual(gcProducer.publishActionEntry.callCount, 0); + done(); + }); + }); + + it('deletes the metadata even when no garbage collector is configured', done => { + const zenkoObjMd = { + dataStoreName: LOCAL_LOCATION, + location: [{ dataStoreName: LOCAL_LOCATION, key: 'local-data-key' }], + }; + const proc = _makeDeleteProcessor(zenkoObjMd, null); + + proc._processDeleteOpQueueEntry(_makeLog(), _makeEntry(), CRR_LOCATION, {}, err => { + assert.ifError(err); + assert.strictEqual(proc._mongoClient.deleteObject.callCount, 1); + done(); + }); + }); +});