Skip to content

Commit

Permalink
BB-401 Incompatibility of ExpiredObjectDeleteMarker logic with AWS
Browse files Browse the repository at this point in the history 
Backbeat performs automatic ExpiredObjectDeleteMarker cleanup for compatibility with Amazon S3,
 - either when the delete markers meet the age criteria
 - or when the ExpiredObjectDeleteMarker tag is set to true.

Furthermore, the process of "verifying whether the deletion marker was generated by a lifecycle service account" is ineffective due to the implementation of the assumeRole logic. Consequently, this verification will consistently yield false results, leading us to remove it.
  • Loading branch information
@nicolas2bert
nicolas2bert committed May 22, 2023
1 parent 4ada919 commit e442088
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 61 deletions.
25 changes: 8 additions & 17 deletions extensions/lifecycle/tasks/LifecycleTask.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ const transitionOneDayEarlier = process.env.TRANSITION_ONE_DAY_EARLIER === 'true
// moves lifecycle expiration deadlines 1 day earlier, mostly for testing
const expireOneDayEarlier = process.env.EXPIRE_ONE_DAY_EARLIER === 'true';

function isLifecycleUser(canonicalID) {
const canonicalIDArray = canonicalID.split('/');
const serviceName = canonicalIDArray[canonicalIDArray.length - 1];
return serviceName === 'lifecycle';
}

/**
* compare 2 version by their stale dates returning:
* - LT (-1) if v1 is less than v2
Expand Down Expand Up @@ -1074,24 +1068,21 @@ class LifecycleTask extends BackbeatTask {
const eodm = rules.Expiration
&& rules.Expiration.ExpiredObjectDeleteMarker;

// Backbeat performs automatic ExpiredObjectDeleteMarker cleanup
// for compatibility with Amazon S3,
// - either when the delete markers meet the age criteria
// - or when the ExpiredObjectDeleteMarker tag is set to true.
const applicableExpRule = rules.Expiration && (
(rules.Expiration.Days !== undefined
&& daysSinceInitiated >= rules.Expiration.Days)
|| (rules.Expiration.Date !== undefined
&& rules.Expiration.Date < Date.now())
|| eodm !== false
);
const validLifecycleUserCase = (
isLifecycleUser(deleteMarker.Owner.ID)
&& eodm !== false
|| eodm === true
);

// if there are no other versions with the same Key as this DM,
// if a valid Expiration rule exists or if the DM was created
// by a lifecycle service account and eodm rule is not
// explicitly set to false, apply and permanently delete this DM
if (matchingNoncurrentKeys.length === 0 && (applicableExpRule
|| validLifecycleUserCase)) {
// if there are no other versions with the same Key as this DM and
// if a valid Expiration rule exists, apply and permanently delete this DM
if (matchingNoncurrentKeys.length === 0 && applicableExpRule) {
const entry = ActionQueueEntry.create('deleteObject')
.addContext({
origin: 'lifecycle',
Expand Down
124 changes: 80 additions & 44 deletions tests/unit/lifecycle/LifecycleTask.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,31 +304,35 @@ describe('lifecycle task helper methods', () => {
describe('_checkAndApplyEODMRule', () => {
let lct2;

const oldLastModified = new Date(Date.now() - (2 * DAY)).toISOString();
const lastModified = new Date(Date.now()).toISOString();

const bucketData = {
target: {
owner: 'test-user',
bucket: 'test-bucket',
},
};
// lifecycle service account created delete marker
const deleteMarkerLC = {
// user created delete marker
const deleteMarker = {
Owner: {
DisplayName: 'Lifecycle Service Account',
ID: '86346e5bda4c2158985574c9942089c36ca650dc509/lifecycle',
DisplayName: 'Not Lifecycle Service Account',
ID: '86346e5bda4c2158985574c9942089c36ca650dc509',
},
Key: 'test-key',
VersionId:
'834373731313631393339313839393939393952473030312020353820',
LastModified: lastModified,
};
// user created delete marker
const deleteMarkerNotLC = {
const deleteMarkerOld = {
Owner: {
DisplayName: 'Not Lifecycle Service Account',
ID: '86346e5bda4c2158985574c9942089c36ca650dc509',
},
Key: 'test-key',
VersionId:
'834373731313631393339313839393939393952473030312020353820',
LastModified: oldLastModified,
};
const listOfVersions = [
{
Expand Down Expand Up @@ -362,100 +366,132 @@ describe('lifecycle task helper methods', () => {
lct2.reset();
});

it('should send an entry to Kafka when ExpiredObjectDeleteMarker is ' +
'enabled and delete marker was created by the lifecycle service ' +
'account', () => {
it('should NOT send any entry to Kafka when delete marker is not eligible based on its age', () => {
const rules = {
Expiration: { ExpiredObjectDeleteMarker: true },
Expiration: { Days: 5 },
};

lct2._checkAndApplyEODMRule(bucketData, deleteMarkerLC,
lct2._checkAndApplyEODMRule(bucketData, deleteMarker,
listOfVersions, rules, fakeLogger, err => {
assert.ifError(err);

const latestEntry = lct2.getLatestEntry();
const expectedTarget = Object.assign({}, bucketData.target, {
key: deleteMarkerLC.Key,
version: deleteMarkerLC.VersionId,
});
assert.strictEqual(latestEntry.getActionType(), 'deleteObject');
assert.deepStrictEqual(
latestEntry.getAttribute('target'), expectedTarget);
assert.equal(latestEntry, undefined);
});
});

it('should send an entry to Kafka when the delete marker was created ' +
'by the lifecycle service account and ExpiredObjectDeleteMarker rule ' +
'was NOT explicitly set to false', () => {
const rules = {};
it('should send any entry to Kafka when delete marker meets the age criteria and ' +
'ExpiredObjectDeleteMarker is not set', () => {
const rules = {
Expiration: { Days: 1 },
};

lct2._checkAndApplyEODMRule(bucketData, deleteMarkerLC,
lct2._checkAndApplyEODMRule(bucketData, deleteMarkerOld,
listOfVersions, rules, fakeLogger, err => {
assert.ifError(err);

const latestEntry = lct2.getLatestEntry();
const expectedTarget = Object.assign({}, bucketData.target, {
key: deleteMarkerLC.Key,
version: deleteMarkerLC.VersionId,
key: deleteMarkerOld.Key,
version: deleteMarkerOld.VersionId,
});
assert(latestEntry, 'entry has not been sent');
assert.strictEqual(latestEntry.getActionType(), 'deleteObject');
assert.deepStrictEqual(
latestEntry.getAttribute('target'), expectedTarget);
});
});

it('should NOT send any entry to Kafka when Expiration rule is not ' +
'enabled and the delete marker was not created by the lifecycle ' +
'service account', () => {
const rules = {};
it('should send any entry to Kafka when delete marker meets the age criteria and ' +
'ExpiredObjectDeleteMarker is set to false', () => {
const rules = {
Expiration: { Days: 1, ExpiredObjectDeleteMarker: false },
};

lct2._checkAndApplyEODMRule(bucketData, deleteMarkerNotLC,
lct2._checkAndApplyEODMRule(bucketData, deleteMarkerOld,
listOfVersions, rules, fakeLogger, err => {
assert.ifError(err);

const latestEntry = lct2.getLatestEntry();
assert.equal(latestEntry, undefined);
const expectedTarget = Object.assign({}, bucketData.target, {
key: deleteMarkerOld.Key,
version: deleteMarkerOld.VersionId,
});
assert(latestEntry, 'entry has not been sent');
assert.strictEqual(latestEntry.getActionType(), 'deleteObject');
assert.deepStrictEqual(
latestEntry.getAttribute('target'), expectedTarget);
});
});

it('should NOT send an entry to Kafka when the delete marker was ' +
'created by the lifecycle service account but ' +
'ExpiredObjectDeleteMarker rule is explicitly set to false', () => {
it('should send an entry to Kafka when ExpiredObjectDeleteMarker is enabled', () => {
const rules = {
Expiration: { ExpiredObjectDeleteMarker: false },
Expiration: { ExpiredObjectDeleteMarker: true },
};

lct2._checkAndApplyEODMRule(bucketData, deleteMarkerLC,
lct2._checkAndApplyEODMRule(bucketData, deleteMarker,
listOfVersions, rules, fakeLogger, err => {
assert.ifError(err);

const latestEntry = lct2.getLatestEntry();
assert.equal(latestEntry, undefined);
const expectedTarget = Object.assign({}, bucketData.target, {
key: deleteMarker.Key,
version: deleteMarker.VersionId,
});
assert(latestEntry, 'entry has not been sent');
assert.strictEqual(latestEntry.getActionType(), 'deleteObject');
assert.deepStrictEqual(
latestEntry.getAttribute('target'), expectedTarget);
});
});

it('should send an entry to Kafka when ExpiredObjectDeleteMarker is ' +
'enabled and delete marker was not created by the lifecycle service ' +
'account', () => {
'enabled and delete marker is not eligible based on its age', () => {
const rules = {
Expiration: { ExpiredObjectDeleteMarker: true },
Expiration: { Days: 5, ExpiredObjectDeleteMarker: true },
};

lct2._checkAndApplyEODMRule(bucketData, deleteMarkerNotLC,
lct2._checkAndApplyEODMRule(bucketData, deleteMarker,
listOfVersions, rules, fakeLogger, err => {
assert.ifError(err);

const latestEntry = lct2.getLatestEntry();

const expectedTarget = Object.assign({}, bucketData.target, {
key: deleteMarkerNotLC.Key,
version: deleteMarkerNotLC.VersionId,
key: deleteMarker.Key,
version: deleteMarker.VersionId,
});
assert(latestEntry, 'entry has not been sent');
assert.strictEqual(latestEntry.getActionType(), 'deleteObject');
assert.deepStrictEqual(
latestEntry.getAttribute('target'), expectedTarget);
});
});

it('should NOT send an entry to Kafka when no Expiration rule is set', () => {
const rules = {};

lct2._checkAndApplyEODMRule(bucketData, deleteMarker,
listOfVersions, rules, fakeLogger, err => {
assert.ifError(err);

const latestEntry = lct2.getLatestEntry();
assert.equal(latestEntry, undefined);
});
});

it('should NOT send an entry to Kafka if ExpiredObjectDeleteMarker rule is explicitly set to false', () => {
const rules = {
Expiration: { ExpiredObjectDeleteMarker: false },
};

lct2._checkAndApplyEODMRule(bucketData, deleteMarker,
listOfVersions, rules, fakeLogger, err => {
assert.ifError(err);

const latestEntry = lct2.getLatestEntry();
assert.equal(latestEntry, undefined);
});
});
});

describe('_rulesHaveTag', () => {
Expand Down

0 comments on commit e442088

Please sign in to comment.