From 6d5bc045bc9295db0ae5e384a5c951296eed2589 Mon Sep 17 00:00:00 2001
From: Krishnan Parthasarathi <krisis@users.noreply.github.com>
Date: Wed, 22 May 2024 18:12:48 -0700
Subject: [PATCH] Disallow ExpiredObjectAllVersions with object lock (#19792)

Relaxes restrictions on Expiration and NoncurrentVersionExpiration
placed by https://github.com/minio/minio/pull/19785.
ref: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-lock-managing.html#object-lock-managing-lifecycle

> Object lifecycle management configurations continue functioning
normally on protected objects, including placing delete markers.
However, a locked version of an object cannot be deleted by a S3
Lifecycle expiration policy. Object Lock is maintained regardless of
the object's storage class and throughout S3 Lifecycle
transitions between storage classes.
---
 internal/bucket/lifecycle/lifecycle.go      |  7 ++--
 internal/bucket/lifecycle/lifecycle_test.go | 36 ++++++++++++++-------
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/internal/bucket/lifecycle/lifecycle.go b/internal/bucket/lifecycle/lifecycle.go
index d98bd9e5f..68da9251c 100644
--- a/internal/bucket/lifecycle/lifecycle.go
+++ b/internal/bucket/lifecycle/lifecycle.go
@@ -36,7 +36,7 @@ var (
 	errLifecycleNoRule       = Errorf("Lifecycle configuration should have at least one rule")
 	errLifecycleDuplicateID  = Errorf("Rule ID must be unique. Found same ID for more than one rule")
 	errXMLNotWellFormed      = Errorf("The XML you provided was not well-formed or did not validate against our published schema")
-	errLifecycleBucketLocked = Errorf("--expire-day, --expire-delete-marker, --expire-all-object-versions and --noncurrent-expire-days can't be used for locked bucket")
+	errLifecycleBucketLocked = Errorf("ExpiredObjectAllVersions element and DelMarkerExpiration action cannot be used on an object locked bucket")
 )
 
 const (
@@ -253,10 +253,7 @@ func (lc Lifecycle) Validate(lr lock.Retention) error {
 		if err := r.Validate(); err != nil {
 			return err
 		}
-		if (r.Expiration.DeleteMarker.val || // DeleteVersionAction
-			!r.DelMarkerExpiration.Empty() || // DelMarkerDeleteAllVersionsAction
-			!r.NoncurrentVersionExpiration.IsDaysNull() || // DeleteVersionAction
-			!r.Expiration.IsDaysNull()) && lr.LockEnabled {
+		if lr.LockEnabled && (r.Expiration.DeleteAll.val || !r.DelMarkerExpiration.Empty()) {
 			return errLifecycleBucketLocked
 		}
 	}
diff --git a/internal/bucket/lifecycle/lifecycle_test.go b/internal/bucket/lifecycle/lifecycle_test.go
index 28ffc5f90..dfe6bbeb9 100644
--- a/internal/bucket/lifecycle/lifecycle_test.go
+++ b/internal/bucket/lifecycle/lifecycle_test.go
@@ -63,25 +63,39 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) {
 			expectedParsingErr:    nil,
 			expectedValidationErr: nil,
 		},
-		{ // invalid lifecycle config
+		{ // Using ExpiredObjectAllVersions element with an object locked bucket
 			inputConfig: `<LifecycleConfiguration>
-								  <Rule>
-								  <ID>testRule1</ID>
+                                        <Rule>
+                                          <ID>ExpiredObjectAllVersions with object locking</ID>
 		                          <Filter>
 		                             <Prefix>prefix</Prefix>
 		                          </Filter>
 		                          <Status>Enabled</Status>
-		                          <Expiration><Days>3</Days></Expiration>
-		                          </Rule>
-		                              <Rule>
-								  <ID>testRule2</ID>
+		                          <Expiration>
+			                    <Days>3</Days>
+				            <ExpiredObjectAllVersions>true</ExpiredObjectAllVersions>
+			                  </Expiration>
+		                        </Rule>
+		                      </LifecycleConfiguration>`,
+			expectedParsingErr:    nil,
+			expectedValidationErr: errLifecycleBucketLocked,
+			lr: lock.Retention{
+				LockEnabled: true,
+			},
+		},
+		{ // Using DelMarkerExpiration action with an object locked bucket
+			inputConfig: `<LifecycleConfiguration>
+                                        <Rule>
+                                          <ID>DeleteMarkerExpiration with object locking</ID>
 		                          <Filter>
-		                             <Prefix>another-prefix</Prefix>
+		                             <Prefix>prefix</Prefix>
 		                          </Filter>
 		                          <Status>Enabled</Status>
-		                          <Expiration><Days>3</Days></Expiration>
-		                          </Rule>
-		                          </LifecycleConfiguration>`,
+		                          <DelMarkerExpiration>
+			                    <Days>3</Days>
+			                  </DelMarkerExpiration>
+		                        </Rule>
+		                      </LifecycleConfiguration>`,
 			expectedParsingErr:    nil,
 			expectedValidationErr: errLifecycleBucketLocked,
 			lr: lock.Retention{