Added required validations in lifecycle rules

docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md

@@ -50,4 +50,5 @@ Attached a table with tests that where investigated and their status (this table
 | test_get_bucket_encryption_s3 | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) | 
 | test_get_bucket_encryption_kms | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) | 
 | test_delete_bucket_encryption_s3 | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) | 
-| test_delete_bucket_encryption_kms | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) | 
+| test_delete_bucket_encryption_kms | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |
+| 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

@@ -9,6 +9,63 @@ const S3Error = require('../s3_errors').S3Error;
 
 const true_regex = /true/i;
 
+/**
+ * validate_lifecycle_rule validates lifecycle rule structure and logical constraints
+ *
+ * validations:
+ * - ID must be ≤ MAX_RULE_ID_LENGTH
+ * - Status must be "Enabled" or "Disabled"
+ * - multiple Filters must be under "And"
+ * - only one Expiration field is allowed
+ * - Expiration.Date must be midnight UTC format
+ * - AbortIncompleteMultipartUpload cannot be combined with Tags or ObjectSize filters
+ *
+ * @param {Object} rule - lifecycle rule to validate
+ * @throws {S3Error} - on validation failure
+ */
+function validate_lifecycle_rule(rule) {
+
+    if (rule.ID?.length === 1 && rule.ID[0].length > s3_const.MAX_RULE_ID_LENGTH) {
+        dbg.error('Rule should not have ID length exceed allowed limit of ', s3_const.MAX_RULE_ID_LENGTH, ' characters', rule);
+        throw new S3Error({ ...S3Error.InvalidArgument, message: `ID length should not exceed allowed limit of ${s3_const.MAX_RULE_ID_LENGTH}` });
+    }
+
+    if (!rule.Status || rule.Status.length !== 1 ||
+        (rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_ENABLED && rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_DISABLED)) {
+        dbg.error(`Rule should have a status value of "${s3_const.LIFECYCLE_STATUS.STAT_ENABLED}" or "${s3_const.LIFECYCLE_STATUS.STAT_DISABLED}".`, rule);
+        throw new S3Error(S3Error.MalformedXML);
+    }
+
+    if (rule.Filter?.[0] && Object.keys(rule.Filter[0]).length > 1 && !rule.Filter[0]?.And) {
+        dbg.error('Rule should combine multiple filters using "And"', rule);
+        throw new S3Error(S3Error.MalformedXML);
+    }
+
+    if (rule.Expiration?.[0] && Object.keys(rule.Expiration[0]).length > 1) {
+        dbg.error('Rule should specify only one expiration field: Days, Date, or ExpiredObjectDeleteMarker', rule);
+        throw new S3Error(S3Error.MalformedXML);
+    }
+
+    if (rule.Expiration?.length === 1 && rule.Expiration[0]?.Date) {
+        const date = new Date(rule.Expiration[0].Date[0]);
+        if (isNaN(date.getTime()) || date.getTime() !== Date.UTC(date.getUTCFullYear(), date.getUTCMonth(), date.getUTCDate())) {
+            dbg.error('Date value must conform to the ISO 8601 format and at midnight UTC (00:00:00). Provided:', rule.Expiration[0].Date[0]);
+            throw new S3Error({ ...S3Error.InvalidArgument, message: "'Date' must be at midnight GMT" });
+        }
+    }
+
+    if (rule.AbortIncompleteMultipartUpload?.length === 1 && rule.Filter?.length === 1) {
+        if (rule.Filter[0]?.Tag) {
+            dbg.error('Rule should not include AbortIncompleteMultipartUpload with Tags', rule);
+            throw new S3Error({ ...S3Error.InvalidArgument, message: 'AbortIncompleteMultipartUpload cannot be specified with Tags' });
+        }
+        if (rule.Filter[0]?.ObjectSizeGreaterThan || rule.Filter[0]?.ObjectSizeLessThan) {
+            dbg.error('Rule should not include AbortIncompleteMultipartUpload with Object Size', rule);
+            throw new S3Error({ ...S3Error.InvalidArgument, message: 'AbortIncompleteMultipartUpload cannot be specified with Object Size' });
+        }
+    }
+}
+
 // parse lifecycle rule filter
 function parse_filter(filter) {
     const current_rule_filter = {};
@@ -89,13 +146,11 @@ async function put_bucket_lifecycle(req) {
             filter: {},
         };
 
+        // validate rule
+        validate_lifecycle_rule(rule);
+
         if (rule.ID?.length === 1) {
-            if (rule.ID[0].length > s3_const.MAX_RULE_ID_LENGTH) {
-                dbg.error('Rule should not have ID length exceed allowed limit of ', s3_const.MAX_RULE_ID_LENGTH, ' characters', rule);
-                throw new S3Error({ ...S3Error.InvalidArgument, message: `ID length should not exceed allowed limit of ${s3_const.MAX_RULE_ID_LENGTH}` });
-            } else {
-                current_rule.id = rule.ID[0];
-            }
+            current_rule.id = rule.ID[0];
         } else {
             // Generate a random ID if missing
             current_rule.id = crypto.randomUUID();
@@ -108,11 +163,6 @@ async function put_bucket_lifecycle(req) {
         }
         id_set.add(current_rule.id);
 
-        if (!rule.Status || rule.Status.length !== 1 ||
-            (rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_ENABLED && rule.Status[0] !== s3_const.LIFECYCLE_STATUS.STAT_DISABLED)) {
-            dbg.error(`Rule should have a status value of "${s3_const.LIFECYCLE_STATUS.STAT_ENABLED}" or "${s3_const.LIFECYCLE_STATUS.STAT_DISABLED}".`, rule);
-            throw new S3Error(S3Error.MalformedXML);
-        }
         current_rule.status = rule.Status[0];
 
         if (rule.Prefix) {

src/test/lifecycle/common.js

@@ -162,8 +162,10 @@ function size_gt_lt_lifecycle_configuration(Bucket, gt, lt) {
                     Date: midnight,
                 },
                 Filter: {
-                    ObjectSizeLessThan: lt,
-                    ObjectSizeGreaterThan: gt
+                    And: {
+                        ObjectSizeLessThan: lt,
+                        ObjectSizeGreaterThan: gt,
+                    },
                 },
                 Status: 'Enabled',
             }, ],
@@ -368,7 +370,7 @@ function duplicate_id_lifecycle_configuration(Bucket, Key) {
         Bucket,
         LifecycleConfiguration: {
             Rules: [{
-                ID1,
+                ID: ID1,
                 Expiration: {
                     Days: 17,
                 },
@@ -378,7 +380,7 @@ function duplicate_id_lifecycle_configuration(Bucket, Key) {
                 Status: 'Enabled',
             },
             {
-                ID2,
+                ID: ID2,
                 Expiration: {
                     Days: 18,
                 },
@@ -622,7 +624,7 @@ exports.test_rule_id_length = async function(Bucket, Key, s3) {
         await s3.putBucketLifecycleConfiguration(putLifecycleParams);
         assert.fail(`Expected error for ID length exceeding maximum allowed characters ${s3_const.MAX_RULE_ID_LENGTH}, but request was successful`);
     } catch (error) {
-        assert(error.code === 'InvalidArgument', `Expected InvalidArgument: id length exceeding ${s3_const.MAX_RULE_ID_LENGTH} characters`);
+        assert(error.Code === 'InvalidArgument', `Expected InvalidArgument: id length exceeding ${s3_const.MAX_RULE_ID_LENGTH} characters`);
     }
 };
 
@@ -633,7 +635,7 @@ exports.test_rule_duplicate_id = async function(Bucket, Key, s3) {
         await s3.putBucketLifecycleConfiguration(putLifecycleParams);
         assert.fail('Expected error for duplicate rule ID, but request was successful');
     } catch (error) {
-        assert(error.code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
+        assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: duplicate ID found in the rules');
     }
 };
 
@@ -647,6 +649,80 @@ exports.test_rule_status_value = async function(Bucket, Key, s3) {
         await s3.putBucketLifecycleConfiguration(putLifecycleParams);
         assert.fail('Expected MalformedXML error due to wrong status value, but received a different response');
     } catch (error) {
-        assert(error.code === 'MalformedXML', `Expected MalformedXML error: due to invalid status value`);
+        assert(error.Code === 'MalformedXML', `Expected MalformedXML error: due to invalid status value`);
+    }
+};
+
+exports.test_invalid_filter_format = async function(Bucket, Key, s3) {
+    const putLifecycleParams = tags_lifecycle_configuration(Bucket, Key);
+
+    // append prefix for invalid filter: "And" condition is missing, but multiple filters are present
+    putLifecycleParams.LifecycleConfiguration.Rules[0].Filter.Prefix = 'test-prefix';
+
+    try {
+        await s3.putBucketLifecycleConfiguration(putLifecycleParams);
+        assert.fail('Expected MalformedXML error due to missing "And" condition for multiple filters');
+    } catch (error) {
+        assert(error.Code === 'MalformedXML', 'Expected MalformedXML error: due to missing "And" condition');
+    }
+};
+
+exports.test_invalid_expiration_date_format = async function(Bucket, Key, s3) {
+    const putLifecycleParams = date_lifecycle_configuration(Bucket, Key);
+
+    // set expiration with a Date that is not at midnight UTC (incorrect time specified)
+    putLifecycleParams.LifecycleConfiguration.Rules[0].Expiration.Date = new Date('2025-01-01T15:30:00Z');
+
+    try {
+        await s3.putBucketLifecycleConfiguration(putLifecycleParams);
+        assert.fail('Expected error due to incorrect date format (not at midnight UTC), but request was successful');
+    } catch (error) {
+        assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument error: date must be at midnight UTC');
+    }
+};
+
+exports.test_expiration_multiple_fields = async function(Bucket, Key, s3) {
+    const putLifecycleParams = days_lifecycle_configuration(Bucket, Key);
+
+    // append ExpiredObjectDeleteMarker for invalid expiration with multiple fields
+    putLifecycleParams.LifecycleConfiguration.Rules[0].Expiration.ExpiredObjectDeleteMarker = false;
+
+    try {
+        await s3.putBucketLifecycleConfiguration(putLifecycleParams);
+        assert.fail('Expected MalformedXML error due to multiple expiration fields');
+    } catch (error) {
+        assert(error.Code === 'MalformedXML', 'Expected MalformedXML error: due to multiple expiration fields');
+    }
+};
+
+exports.test_abortincompletemultipartupload_with_tags = async function(Bucket, Key, s3) {
+    const putLifecycleParams = tags_lifecycle_configuration(Bucket);
+
+    // invalid combination of AbortIncompleteMultipartUpload with tags
+    putLifecycleParams.LifecycleConfiguration.Rules[0].AbortIncompleteMultipartUpload = {
+        DaysAfterInitiation: 5
+    };
+
+    try {
+        await s3.putBucketLifecycleConfiguration(putLifecycleParams);
+        assert.fail('Expected InvalidArgument error due to AbortIncompleteMultipartUpload specified with tags');
+    } catch (error) {
+        assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: AbortIncompleteMultipartUpload cannot be specified with tags');
+    }
+};
+
+exports.test_abortincompletemultipartupload_with_sizes = async function(Bucket, Key, s3) {
+    const putLifecycleParams = filter_size_lifecycle_configuration(Bucket);
+
+    // invalid combination of AbortIncompleteMultipartUpload with object size filters
+    putLifecycleParams.LifecycleConfiguration.Rules[0].AbortIncompleteMultipartUpload = {
+        DaysAfterInitiation: 5
+    };
+
+    try {
+        await s3.putBucketLifecycleConfiguration(putLifecycleParams);
+        assert.fail('Expected InvalidArgument error due to AbortIncompleteMultipartUpload specified with object size');
+    } catch (error) {
+        assert(error.Code === 'InvalidArgument', 'Expected InvalidArgument: AbortIncompleteMultipartUpload cannot be specified with object size');
     }
 };

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

@@ -153,4 +153,5 @@ s3tests/functional/test_headers.py::test_bucket_create_bad_date_none_aws2
 s3tests_boto3/functional/test_s3.py::test_versioned_concurrent_object_create_and_remove
 s3tests_boto3/functional/test_s3.py::test_object_presigned_put_object_with_acl_tenant
 s3tests_boto3/functional/test_s3.py::test_get_undefined_public_block
-s3tests_boto3/functional/test_s3.py::test_get_public_block_deny_bucket_policy
+s3tests_boto3/functional/test_s3.py::test_get_public_block_deny_bucket_policy
+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

@@ -142,4 +142,5 @@ s3tests_boto3/functional/test_s3.py::test_get_public_block_deny_bucket_policy
 s3tests_boto3/functional/test_s3.py::test_get_bucket_encryption_s3
 s3tests_boto3/functional/test_s3.py::test_get_bucket_encryption_kms
 s3tests_boto3/functional/test_s3.py::test_delete_bucket_encryption_s3
-s3tests_boto3/functional/test_s3.py::test_delete_bucket_encryption_kms
+s3tests_boto3/functional/test_s3.py::test_delete_bucket_encryption_kms
+s3tests_boto3/functional/test_s3.py::test_lifecycle_expiration_tags1

src/test/system_tests/test_lifecycle.js

@@ -54,9 +54,6 @@ async function main() {
     await commonTests.test_rule_id(Bucket, Key, s3);
     await commonTests.test_filter_size(Bucket, s3);
     await commonTests.test_and_prefix_size(Bucket, Key, s3);
-    await commonTests.test_rule_id_length(Bucket, Key, s3);
-    await commonTests.test_rule_duplicate_id(Bucket, Key, s3);
-    await commonTests.test_rule_status_value(Bucket, Key, s3);
 
     const getObjectParams = {
         Bucket,

src/test/unit_tests/test_lifecycle.js

@@ -104,6 +104,30 @@ mocha.describe('lifecycle', () => {
         mocha.it('test multipath', async () => {
             await commonTests.test_multipart(Bucket, Key, s3);
         });
+        mocha.it('test rule ID length', async () => {
+            await commonTests.test_rule_id_length(Bucket, Key, s3);
+        });
+        mocha.it('test rule duplicate ID', async () => {
+            await commonTests.test_rule_duplicate_id(Bucket, Key, s3);
+        });
+        mocha.it('test rule status value', async () => {
+            await commonTests.test_rule_status_value(Bucket, Key, s3);
+        });
+        mocha.it('test invalid filter format', async () => {
+            await commonTests.test_invalid_filter_format(Bucket, Key, s3);
+        });
+        mocha.it('test invalid expiration date format', async () => {
+            await commonTests.test_invalid_expiration_date_format(Bucket, Key, s3);
+        });
+        mocha.it('test expiration with multiple fields', async () => {
+            await commonTests.test_expiration_multiple_fields(Bucket, Key, s3);
+        });
+        mocha.it('test AbortIncompleteMultipartUpload with tags', async () => {
+            await commonTests.test_abortincompletemultipartupload_with_tags(Bucket, Key, s3);
+        });
+        mocha.it('test AbortIncompleteMultipartUpload with object sizes', async () => {
+            await commonTests.test_abortincompletemultipartupload_with_sizes(Bucket, Key, s3);
+        });
     });
 
     mocha.describe('bucket-lifecycle-bg-worker', function() {