Skip to content

Commit 4954a67

Browse files
achouhan09romayalon
achouhan09
authored and
romayalon
committed
1. Added required validations in lifecycle rules for filter, expiration and AboutIncompleteMultipartUpload
2. Added tests for the above validations Signed-off-by: Aayush Chouhan <achouhan@redhat.com> (cherry picked from commit 0c49cb6)
1 parent dad2f7e commit 4954a67

File tree

4 files changed

+175
-21
lines changed

4 files changed

+175
-21
lines changed

‎src/endpoint/s3/ops/s3_put_bucket_lifecycle.js

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,63 @@ const S3Error = require('../s3_errors').S3Error;
99

1010
const true_regex = /true/i;
1111

12+
/**
13+
* validate_lifecycle_rule validates lifecycle rule structure and logical constraints
14+
*
15+
* validations:
16+
* - ID must be ≤ MAX_RULE_ID_LENGTH
17+
* - Status must be "Enabled" or "Disabled"
18+
* - multiple Filters must be under "And"
19+
* - only one Expiration field is allowed
20+
* - Expiration.Date must be midnight UTC format
21+
* - AbortIncompleteMultipartUpload cannot be combined with Tags or ObjectSize filters
22+
*
23+
* @param {Object} rule - lifecycle rule to validate
24+
* @throws {S3Error} - on validation failure
25+
*/
26+
function validate_lifecycle_rule(rule) {
27+
28+
if (rule.ID?.length === 1 && rule.ID[0].length > s3_const.MAX_RULE_ID_LENGTH) {
29+
dbg.error('Rule should not have ID length exceed allowed limit of ', s3_const.MAX_RULE_ID_LENGTH, ' characters', rule);
30+
throw new S3Error({ ...S3Error.InvalidArgument, message: `ID length should not exceed allowed limit of ${s3_const.MAX_RULE_ID_LENGTH}` });
31+
}
32+
33+
if (!rule.Status || rule.Status.length !== 1 ||
34+
(rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_ENABLED && rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_DISABLED)) {
35+
dbg.error(`Rule should have a status value of "${s3_const.LIFECYCLE_STATUS.STAT_ENABLED}" or "${s3_const.LIFECYCLE_STATUS.STAT_DISABLED}".`, rule);
36+
throw new S3Error(S3Error.MalformedXML);
37+
}
38+
39+
if (rule.Filter?.[0] && Object.keys(rule.Filter[0]).length > 1 && !rule.Filter[0]?.And) {
40+
dbg.error('Rule should combine multiple filters using "And"', rule);
41+
throw new S3Error(S3Error.MalformedXML);
42+
}
43+
44+
if (rule.Expiration?.[0] && Object.keys(rule.Expiration[0]).length > 1) {
45+
dbg.error('Rule should specify only one expiration field: Days, Date, or ExpiredObjectDeleteMarker', rule);
46+
throw new S3Error(S3Error.MalformedXML);
47+
}
48+
49+
if (rule.Expiration?.length === 1 && rule.Expiration[0]?.Date) {
50+
const date = new Date(rule.Expiration[0].Date[0]);
51+
if (isNaN(date.getTime()) || date.getTime() !== Date.UTC(date.getUTCFullYear(), date.getUTCMonth(), date.getUTCDate())) {
52+
dbg.error('Date value must conform to the ISO 8601 format and at midnight UTC (00:00:00). Provided:', rule.Expiration[0].Date[0]);
53+
throw new S3Error({ ...S3Error.InvalidArgument, message: "'Date' must be at midnight GMT" });
54+
}
55+
}
56+
57+
if (rule.AbortIncompleteMultipartUpload?.length === 1 && rule.Filter?.length === 1) {
58+
if (rule.Filter[0]?.Tag) {
59+
dbg.error('Rule should not include AbortIncompleteMultipartUpload with Tags', rule);
60+
throw new S3Error({ ...S3Error.InvalidArgument, message: 'AbortIncompleteMultipartUpload cannot be specified with Tags' });
61+
}
62+
if (rule.Filter[0]?.ObjectSizeGreaterThan || rule.Filter[0]?.ObjectSizeLessThan) {
63+
dbg.error('Rule should not include AbortIncompleteMultipartUpload with Object Size', rule);
64+
throw new S3Error({ ...S3Error.InvalidArgument, message: 'AbortIncompleteMultipartUpload cannot be specified with Object Size' });
65+
}
66+
}
67+
}
68+
1269
// parse lifecycle rule filter
1370
function parse_filter(filter) {
1471
const current_rule_filter = {};
@@ -89,13 +146,11 @@ async function put_bucket_lifecycle(req) {
89146
filter: {},
90147
};
91148

149+
// validate rule
150+
validate_lifecycle_rule(rule);
151+
92152
if (rule.ID?.length === 1) {
93-
if (rule.ID[0].length > s3_const.MAX_RULE_ID_LENGTH) {
94-
dbg.error('Rule should not have ID length exceed allowed limit of ', s3_const.MAX_RULE_ID_LENGTH, ' characters', rule);
95-
throw new S3Error({ ...S3Error.InvalidArgument, message: `ID length should not exceed allowed limit of ${s3_const.MAX_RULE_ID_LENGTH}` });
96-
} else {
97-
current_rule.id = rule.ID[0];
98-
}
153+
current_rule.id = rule.ID[0];
99154
} else {
100155
// Generate a random ID if missing
101156
current_rule.id = crypto.randomUUID();
@@ -108,11 +163,6 @@ async function put_bucket_lifecycle(req) {
108163
}
109164
id_set.add(current_rule.id);
110165

111-
if (!rule.Status || rule.Status.length !== 1 ||
112-
(rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_ENABLED && rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_DISABLED)) {
113-
dbg.error(`Rule should have a status value of "${s3_const.LIFECYCLE_STATUS.STAT_ENABLED}" or "${s3_const.LIFECYCLE_STATUS.STAT_DISABLED}".`, rule);
114-
throw new S3Error(S3Error.MalformedXML);
115-
}
116166
current_rule.status = rule.Status[0];
117167

118168
if (rule.Prefix) {

‎src/test/lifecycle/common.js

Lines changed: 83 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,10 @@ function size_gt_lt_lifecycle_configuration(Bucket, gt, lt) {
161161
Date: midnight,
162162
},
163163
Filter: {
164-
ObjectSizeLessThan: lt,
165-
ObjectSizeGreaterThan: gt
164+
And: {
165+
ObjectSizeLessThan: lt,
166+
ObjectSizeGreaterThan: gt,
167+
},
166168
},
167169
Status: 'Enabled',
168170
}, ],
@@ -366,7 +368,7 @@ function duplicate_id_lifecycle_configuration(Bucket, Key) {
366368
Bucket,
367369
LifecycleConfiguration: {
368370
Rules: [{
369-
ID1,
371+
ID: ID1,
370372
Expiration: {
371373
Days: 17,
372374
},
@@ -376,7 +378,7 @@ function duplicate_id_lifecycle_configuration(Bucket, Key) {
376378
Status: 'Enabled',
377379
},
378380
{
379-
ID2,
381+
ID: ID2,
380382
Expiration: {
381383
Days: 18,
382384
},
@@ -555,7 +557,7 @@ exports.test_rule_id_length = async function(Bucket, Key, s3) {
555557
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
556558
assert.fail(`Expected error for ID length exceeding maximum allowed characters ${s3_const.MAX_RULE_ID_LENGTH}, but request was successful`);
557559
} catch (error) {
558-
assert(error.code === 'InvalidArgument', `Expected InvalidArgument: id length exceeding ${s3_const.MAX_RULE_ID_LENGTH} characters`);
560+
assert(error.Code === 'InvalidArgument', `Expected InvalidArgument: id length exceeding ${s3_const.MAX_RULE_ID_LENGTH} characters`);
559561
}
560562
};
561563

@@ -566,7 +568,7 @@ exports.test_rule_duplicate_id = async function(Bucket, Key, s3) {
566568
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
567569
assert.fail('Expected error for duplicate rule ID, but request was successful');
568570
} catch (error) {
569-
assert(error.code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
571+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
570572
}
571573
};
572574

@@ -580,6 +582,80 @@ exports.test_rule_status_value = async function(Bucket, Key, s3) {
580582
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
581583
assert.fail('Expected MalformedXML error due to wrong status value, but received a different response');
582584
} catch (error) {
583-
assert(error.code === 'MalformedXML', `Expected MalformedXML error: due to invalid status value`);
585+
assert(error.Code === 'MalformedXML', `Expected MalformedXML error: due to invalid status value`);
586+
}
587+
};
588+
589+
exports.test_invalid_filter_format = async function(Bucket, Key, s3) {
590+
const putLifecycleParams = tags_lifecycle_configuration(Bucket, Key);
591+
592+
// append prefix for invalid filter: "And" condition is missing, but multiple filters are present
593+
putLifecycleParams.LifecycleConfiguration.Rules[0].Filter.Prefix = 'test-prefix';
594+
595+
try {
596+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
597+
assert.fail('Expected MalformedXML error due to missing "And" condition for multiple filters');
598+
} catch (error) {
599+
assert(error.Code === 'MalformedXML', 'Expected MalformedXML error: due to missing "And" condition');
600+
}
601+
};
602+
603+
exports.test_invalid_expiration_date_format = async function(Bucket, Key, s3) {
604+
const putLifecycleParams = date_lifecycle_configuration(Bucket, Key);
605+
606+
// set expiration with a Date that is not at midnight UTC (incorrect time specified)
607+
putLifecycleParams.LifecycleConfiguration.Rules[0].Expiration.Date = new Date('2025-01-01T15:30:00Z');
608+
609+
try {
610+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
611+
assert.fail('Expected error due to incorrect date format (not at midnight UTC), but request was successful');
612+
} catch (error) {
613+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument error: date must be at midnight UTC');
614+
}
615+
};
616+
617+
exports.test_expiration_multiple_fields = async function(Bucket, Key, s3) {
618+
const putLifecycleParams = days_lifecycle_configuration(Bucket, Key);
619+
620+
// append ExpiredObjectDeleteMarker for invalid expiration with multiple fields
621+
putLifecycleParams.LifecycleConfiguration.Rules[0].Expiration.ExpiredObjectDeleteMarker = false;
622+
623+
try {
624+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
625+
assert.fail('Expected MalformedXML error due to multiple expiration fields');
626+
} catch (error) {
627+
assert(error.Code === 'MalformedXML', 'Expected MalformedXML error: due to multiple expiration fields');
628+
}
629+
};
630+
631+
exports.test_abortincompletemultipartupload_with_tags = async function(Bucket, Key, s3) {
632+
const putLifecycleParams = tags_lifecycle_configuration(Bucket);
633+
634+
// invalid combination of AbortIncompleteMultipartUpload with tags
635+
putLifecycleParams.LifecycleConfiguration.Rules[0].AbortIncompleteMultipartUpload = {
636+
DaysAfterInitiation: 5
637+
};
638+
639+
try {
640+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
641+
assert.fail('Expected InvalidArgument error due to AbortIncompleteMultipartUpload specified with tags');
642+
} catch (error) {
643+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: AbortIncompleteMultipartUpload cannot be specified with tags');
644+
}
645+
};
646+
647+
exports.test_abortincompletemultipartupload_with_sizes = async function(Bucket, Key, s3) {
648+
const putLifecycleParams = filter_size_lifecycle_configuration(Bucket);
649+
650+
// invalid combination of AbortIncompleteMultipartUpload with object size filters
651+
putLifecycleParams.LifecycleConfiguration.Rules[0].AbortIncompleteMultipartUpload = {
652+
DaysAfterInitiation: 5
653+
};
654+
655+
try {
656+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
657+
assert.fail('Expected InvalidArgument error due to AbortIncompleteMultipartUpload specified with object size');
658+
} catch (error) {
659+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: AbortIncompleteMultipartUpload cannot be specified with object size');
584660
}
585661
};

‎src/test/system_tests/test_lifecycle.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,6 @@ async function main() {
5454
await commonTests.test_rule_id(Bucket, Key, s3);
5555
await commonTests.test_filter_size(Bucket, s3);
5656
await commonTests.test_and_prefix_size(Bucket, Key, s3);
57-
await commonTests.test_rule_id_length(Bucket, Key, s3);
58-
await commonTests.test_rule_duplicate_id(Bucket, Key, s3);
59-
await commonTests.test_rule_status_value(Bucket, Key, s3);
6057

6158
const getObjectParams = {
6259
Bucket,

‎src/test/unit_tests/test_lifecycle.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,37 @@ mocha.describe('lifecycle', () => {
8484
mocha.it('test and prefix size', async () => {
8585
await commonTests.test_and_prefix_size(Bucket, Key, s3);
8686
});
87+
// on 5.18 we need to backport https://github.com/noobaa/noobaa-core/pull/8885 for supporting the next 2 tests
88+
// mocha.it('test version', async () => {
89+
// await commonTests.test_version(Bucket, Key, s3);
90+
// });
91+
// mocha.it('test multipath', async () => {
92+
// await commonTests.test_multipart(Bucket, Key, s3);
93+
// });
94+
mocha.it('test rule ID length', async () => {
95+
await commonTests.test_rule_id_length(Bucket, Key, s3);
96+
});
97+
mocha.it('test rule duplicate ID', async () => {
98+
await commonTests.test_rule_duplicate_id(Bucket, Key, s3);
99+
});
100+
mocha.it('test rule status value', async () => {
101+
await commonTests.test_rule_status_value(Bucket, Key, s3);
102+
});
103+
mocha.it('test invalid filter format', async () => {
104+
await commonTests.test_invalid_filter_format(Bucket, Key, s3);
105+
});
106+
mocha.it('test invalid expiration date format', async () => {
107+
await commonTests.test_invalid_expiration_date_format(Bucket, Key, s3);
108+
});
109+
mocha.it('test expiration with multiple fields', async () => {
110+
await commonTests.test_expiration_multiple_fields(Bucket, Key, s3);
111+
});
112+
mocha.it('test AbortIncompleteMultipartUpload with tags', async () => {
113+
await commonTests.test_abortincompletemultipartupload_with_tags(Bucket, Key, s3);
114+
});
115+
mocha.it('test AbortIncompleteMultipartUpload with object sizes', async () => {
116+
await commonTests.test_abortincompletemultipartupload_with_sizes(Bucket, Key, s3);
117+
});
87118
});
88119

89120
mocha.describe('bucket-lifecycle-bg-worker', function() {

0 commit comments

Comments
 (0)