Skip to content

Commit bbebeac

Browse files
achouhan09achouhan09
authored
Merge pull request #8914 from achouhan09/lifecycle-val
Added required validations in bucket lifecycle configuration rules
2 parents 8f1c3b4 + de96bdd commit bbebeac

File tree

7 files changed

+174
-24
lines changed

7 files changed

+174
-24
lines changed

‎docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,5 @@ Attached a table with tests that where investigated and their status (this table
5050
| test_get_bucket_encryption_s3 | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |
5151
| test_get_bucket_encryption_kms | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |
5252
| test_delete_bucket_encryption_s3 | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |
53-
| test_delete_bucket_encryption_kms | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |
53+
| test_delete_bucket_encryption_kms | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |
54+
| test_lifecycle_expiration_tags1 | Faulty Test | [638](https://github.com/ceph/s3-tests/issues/638) | There can be more such tests having the same issue (`Filter` is not aligned with aws structure in bucket lifecycle configuration) |

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

+61-11
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

+83-7
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,10 @@ function size_gt_lt_lifecycle_configuration(Bucket, gt, lt) {
162162
Date: midnight,
163163
},
164164
Filter: {
165-
ObjectSizeLessThan: lt,
166-
ObjectSizeGreaterThan: gt
165+
And: {
166+
ObjectSizeLessThan: lt,
167+
ObjectSizeGreaterThan: gt,
168+
},
167169
},
168170
Status: 'Enabled',
169171
}, ],
@@ -368,7 +370,7 @@ function duplicate_id_lifecycle_configuration(Bucket, Key) {
368370
Bucket,
369371
LifecycleConfiguration: {
370372
Rules: [{
371-
ID1,
373+
ID: ID1,
372374
Expiration: {
373375
Days: 17,
374376
},
@@ -378,7 +380,7 @@ function duplicate_id_lifecycle_configuration(Bucket, Key) {
378380
Status: 'Enabled',
379381
},
380382
{
381-
ID2,
383+
ID: ID2,
382384
Expiration: {
383385
Days: 18,
384386
},
@@ -622,7 +624,7 @@ exports.test_rule_id_length = async function(Bucket, Key, s3) {
622624
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
623625
assert.fail(`Expected error for ID length exceeding maximum allowed characters ${s3_const.MAX_RULE_ID_LENGTH}, but request was successful`);
624626
} catch (error) {
625-
assert(error.code === 'InvalidArgument', `Expected InvalidArgument: id length exceeding ${s3_const.MAX_RULE_ID_LENGTH} characters`);
627+
assert(error.Code === 'InvalidArgument', `Expected InvalidArgument: id length exceeding ${s3_const.MAX_RULE_ID_LENGTH} characters`);
626628
}
627629
};
628630

@@ -633,7 +635,7 @@ exports.test_rule_duplicate_id = async function(Bucket, Key, s3) {
633635
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
634636
assert.fail('Expected error for duplicate rule ID, but request was successful');
635637
} catch (error) {
636-
assert(error.code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
638+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
637639
}
638640
};
639641

@@ -647,6 +649,80 @@ exports.test_rule_status_value = async function(Bucket, Key, s3) {
647649
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
648650
assert.fail('Expected MalformedXML error due to wrong status value, but received a different response');
649651
} catch (error) {
650-
assert(error.code === 'MalformedXML', `Expected MalformedXML error: due to invalid status value`);
652+
assert(error.Code === 'MalformedXML', `Expected MalformedXML error: due to invalid status value`);
653+
}
654+
};
655+
656+
exports.test_invalid_filter_format = async function(Bucket, Key, s3) {
657+
const putLifecycleParams = tags_lifecycle_configuration(Bucket, Key);
658+
659+
// append prefix for invalid filter: "And" condition is missing, but multiple filters are present
660+
putLifecycleParams.LifecycleConfiguration.Rules[0].Filter.Prefix = 'test-prefix';
661+
662+
try {
663+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
664+
assert.fail('Expected MalformedXML error due to missing "And" condition for multiple filters');
665+
} catch (error) {
666+
assert(error.Code === 'MalformedXML', 'Expected MalformedXML error: due to missing "And" condition');
667+
}
668+
};
669+
670+
exports.test_invalid_expiration_date_format = async function(Bucket, Key, s3) {
671+
const putLifecycleParams = date_lifecycle_configuration(Bucket, Key);
672+
673+
// set expiration with a Date that is not at midnight UTC (incorrect time specified)
674+
putLifecycleParams.LifecycleConfiguration.Rules[0].Expiration.Date = new Date('2025-01-01T15:30:00Z');
675+
676+
try {
677+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
678+
assert.fail('Expected error due to incorrect date format (not at midnight UTC), but request was successful');
679+
} catch (error) {
680+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument error: date must be at midnight UTC');
681+
}
682+
};
683+
684+
exports.test_expiration_multiple_fields = async function(Bucket, Key, s3) {
685+
const putLifecycleParams = days_lifecycle_configuration(Bucket, Key);
686+
687+
// append ExpiredObjectDeleteMarker for invalid expiration with multiple fields
688+
putLifecycleParams.LifecycleConfiguration.Rules[0].Expiration.ExpiredObjectDeleteMarker = false;
689+
690+
try {
691+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
692+
assert.fail('Expected MalformedXML error due to multiple expiration fields');
693+
} catch (error) {
694+
assert(error.Code === 'MalformedXML', 'Expected MalformedXML error: due to multiple expiration fields');
695+
}
696+
};
697+
698+
exports.test_abortincompletemultipartupload_with_tags = async function(Bucket, Key, s3) {
699+
const putLifecycleParams = tags_lifecycle_configuration(Bucket);
700+
701+
// invalid combination of AbortIncompleteMultipartUpload with tags
702+
putLifecycleParams.LifecycleConfiguration.Rules[0].AbortIncompleteMultipartUpload = {
703+
DaysAfterInitiation: 5
704+
};
705+
706+
try {
707+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
708+
assert.fail('Expected InvalidArgument error due to AbortIncompleteMultipartUpload specified with tags');
709+
} catch (error) {
710+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: AbortIncompleteMultipartUpload cannot be specified with tags');
711+
}
712+
};
713+
714+
exports.test_abortincompletemultipartupload_with_sizes = async function(Bucket, Key, s3) {
715+
const putLifecycleParams = filter_size_lifecycle_configuration(Bucket);
716+
717+
// invalid combination of AbortIncompleteMultipartUpload with object size filters
718+
putLifecycleParams.LifecycleConfiguration.Rules[0].AbortIncompleteMultipartUpload = {
719+
DaysAfterInitiation: 5
720+
};
721+
722+
try {
723+
await s3.putBucketLifecycleConfiguration(putLifecycleParams);
724+
assert.fail('Expected InvalidArgument error due to AbortIncompleteMultipartUpload specified with object size');
725+
} catch (error) {
726+
assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: AbortIncompleteMultipartUpload cannot be specified with object size');
651727
}
652728
};

‎src/test/system_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,5 @@ s3tests/functional/test_headers.py::test_bucket_create_bad_date_none_aws2
153153
s3tests_boto3/functional/test_s3.py::test_versioned_concurrent_object_create_and_remove
154154
s3tests_boto3/functional/test_s3.py::test_object_presigned_put_object_with_acl_tenant
155155
s3tests_boto3/functional/test_s3.py::test_get_undefined_public_block
156-
s3tests_boto3/functional/test_s3.py::test_get_public_block_deny_bucket_policy
156+
s3tests_boto3/functional/test_s3.py::test_get_public_block_deny_bucket_policy
157+
s3tests_boto3/functional/test_s3.py::test_lifecycle_expiration_tags1

‎src/test/system_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -142,4 +142,5 @@ s3tests_boto3/functional/test_s3.py::test_get_public_block_deny_bucket_policy
142142
s3tests_boto3/functional/test_s3.py::test_get_bucket_encryption_s3
143143
s3tests_boto3/functional/test_s3.py::test_get_bucket_encryption_kms
144144
s3tests_boto3/functional/test_s3.py::test_delete_bucket_encryption_s3
145-
s3tests_boto3/functional/test_s3.py::test_delete_bucket_encryption_kms
145+
s3tests_boto3/functional/test_s3.py::test_delete_bucket_encryption_kms
146+
s3tests_boto3/functional/test_s3.py::test_lifecycle_expiration_tags1

‎src/test/system_tests/test_lifecycle.js

-3
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

+24
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,30 @@ mocha.describe('lifecycle', () => {
104104
mocha.it('test multipath', async () => {
105105
await commonTests.test_multipart(Bucket, Key, s3);
106106
});
107+
mocha.it('test rule ID length', async () => {
108+
await commonTests.test_rule_id_length(Bucket, Key, s3);
109+
});
110+
mocha.it('test rule duplicate ID', async () => {
111+
await commonTests.test_rule_duplicate_id(Bucket, Key, s3);
112+
});
113+
mocha.it('test rule status value', async () => {
114+
await commonTests.test_rule_status_value(Bucket, Key, s3);
115+
});
116+
mocha.it('test invalid filter format', async () => {
117+
await commonTests.test_invalid_filter_format(Bucket, Key, s3);
118+
});
119+
mocha.it('test invalid expiration date format', async () => {
120+
await commonTests.test_invalid_expiration_date_format(Bucket, Key, s3);
121+
});
122+
mocha.it('test expiration with multiple fields', async () => {
123+
await commonTests.test_expiration_multiple_fields(Bucket, Key, s3);
124+
});
125+
mocha.it('test AbortIncompleteMultipartUpload with tags', async () => {
126+
await commonTests.test_abortincompletemultipartupload_with_tags(Bucket, Key, s3);
127+
});
128+
mocha.it('test AbortIncompleteMultipartUpload with object sizes', async () => {
129+
await commonTests.test_abortincompletemultipartupload_with_sizes(Bucket, Key, s3);
130+
});
107131
});
108132

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

0 commit comments

Comments
 (0)