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.

Sign up for GitHub

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

BB-401 Incompatibility of ExpiredObjectDeleteMarker logic with AWS #2405

Merged
merged 1 commit into from
May 25, 2023
Merged

BB-401 Incompatibility of ExpiredObjectDeleteMarker logic with AWS #2405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 8 additions & 17 deletions extensions/lifecycle/tasks/LifecycleTask.js
View file
Open in desktop
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
Open in desktop
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