Merge pull request #1568 from versity/sis/object-lock-on-existing-buckets

fix: correct a few object lock behaviors

auth/object_lock.go

@@ -41,7 +41,7 @@ func ParseBucketLockConfigurationInput(input []byte) ([]byte, error) {
 		return nil, s3err.GetAPIError(s3err.ErrMalformedXML)
 	}
 
-	if lockConfig.ObjectLockEnabled != "" && lockConfig.ObjectLockEnabled != types.ObjectLockEnabledEnabled {
+	if lockConfig.ObjectLockEnabled != types.ObjectLockEnabledEnabled {
 		return nil, s3err.GetAPIError(s3err.ErrMalformedXML)
 	}
 
@@ -272,31 +272,35 @@ func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects [
 			}
 
 			if retention.Mode != "" && retention.RetainUntilDate != nil {
-				if retention.RetainUntilDate.After(time.Now()) {
-					switch retention.Mode {
-					case types.ObjectLockRetentionModeGovernance:
-						if !bypass {
-							return s3err.GetAPIError(s3err.ErrObjectLocked)
-						} else {
-							policy, err := be.GetBucketPolicy(ctx, bucket)
-							if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchBucketPolicy)) {
-								return s3err.GetAPIError(s3err.ErrObjectLocked)
-							}
-							if err != nil {
-								return err
-							}
-							if isBucketPublic {
-								err = VerifyPublicBucketPolicy(policy, bucket, key, BypassGovernanceRetentionAction)
-							} else {
-								err = VerifyBucketPolicy(policy, userAccess, bucket, key, BypassGovernanceRetentionAction)
-							}
-							if err != nil {
-								return s3err.GetAPIError(s3err.ErrObjectLocked)
-							}
-						}
-					case types.ObjectLockRetentionModeCompliance:
+				if retention.RetainUntilDate.Before(time.Now()) {
+					// if the object retention is expired, the object
+					// is allowed for write operations(delete, modify)
+					return nil
+				}
+
+				switch retention.Mode {
+				case types.ObjectLockRetentionModeGovernance:
+					if !bypass {
 						return s3err.GetAPIError(s3err.ErrObjectLocked)
+					} else {
+						policy, err := be.GetBucketPolicy(ctx, bucket)
+						if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchBucketPolicy)) {
+							return s3err.GetAPIError(s3err.ErrObjectLocked)
+						}
+						if err != nil {
+							return err
+						}
+						if isBucketPublic {
+							err = VerifyPublicBucketPolicy(policy, bucket, key, BypassGovernanceRetentionAction)
+						} else {
+							err = VerifyBucketPolicy(policy, userAccess, bucket, key, BypassGovernanceRetentionAction)
+						}
+						if err != nil {
+							return s3err.GetAPIError(s3err.ErrObjectLocked)
+						}
 					}
+				case types.ObjectLockRetentionModeCompliance:
+					return s3err.GetAPIError(s3err.ErrObjectLocked)
 				}
 			}
 		}

backend/azure/azure.go

@@ -1644,24 +1644,6 @@ func (az *Azure) DeleteBucketCors(ctx context.Context, bucket string) error {
 }
 
 func (az *Azure) PutObjectLockConfiguration(ctx context.Context, bucket string, config []byte) error {
-	cfg, err := az.getContainerMetaData(ctx, bucket, string(keyBucketLock))
-	if err != nil {
-		return err
-	}
-
-	if len(cfg) == 0 {
-		return s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotAllowed)
-	}
-
-	var bucketLockCfg auth.BucketLockConfig
-	if err := json.Unmarshal(cfg, &bucketLockCfg); err != nil {
-		return fmt.Errorf("unmarshal object lock config: %w", err)
-	}
-
-	if !bucketLockCfg.Enabled {
-		return s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotAllowed)
-	}
-
 	return az.setContainerMetaData(ctx, bucket, string(keyBucketLock), config)
 }
 

backend/posix/posix.go

@@ -624,7 +624,7 @@ func (p *Posix) getBucketVersioningStatus(ctx context.Context, bucket string) (t
 	if errors.Is(err, s3err.GetAPIError(s3err.ErrVersioningNotConfigured)) {
 		return "", nil
 	}
-	if err != nil && !errors.Is(err, s3err.GetAPIError(s3err.ErrVersioningNotConfigured)) {
+	if err != nil {
 		return "", err
 	}
 
@@ -4805,21 +4805,21 @@ func (p *Posix) PutObjectLockConfiguration(ctx context.Context, bucket string, c
 		return fmt.Errorf("stat bucket: %w", err)
 	}
 
-	cfg, err := p.meta.RetrieveAttribute(nil, bucket, "", bucketLockKey)
-	if errors.Is(err, meta.ErrNoSuchKey) {
-		return s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotAllowed)
-	}
-	if err != nil {
-		return fmt.Errorf("get object lock config: %w", err)
-	}
+	if p.versioningEnabled() {
+		// if versioning is enabled on gateway level and bucket versioning
+		// status is not `Enabled`, object lock can't be enabled.
+		// if object lock has been enabled on bucket creation
+		// it means the versioning has been enabled alongside with object lock
+		// and it can't be suspended ever again
+		status, err := p.getBucketVersioningStatus(ctx, bucket)
+		if err != nil {
+			return err
+		}
 
-	var bucketLockCfg auth.BucketLockConfig
-	if err := json.Unmarshal(cfg, &bucketLockCfg); err != nil {
-		return fmt.Errorf("unmarshal object lock config: %w", err)
-	}
-
-	if !bucketLockCfg.Enabled {
-		return s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotAllowed)
+		if status != types.BucketVersioningStatusEnabled {
+			// if versioning is enabled on gateway level
+			return s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotAllowed)
+		}
 	}
 
 	err = p.meta.StoreAttribute(nil, bucket, "", bucketLockKey, config)

s3err/s3err.go

@@ -549,7 +549,7 @@ var errorCodeResponse = map[ErrorCode]APIError{
 	},
 	ErrObjectLockConfigurationNotAllowed: {
 		Code:           "InvalidBucketState",
-		Description:    "Object Lock configuration cannot be enabled on existing buckets.",
+		Description:    "Versioning must be 'Enabled' on the bucket to apply a Object Lock configuration",
 		HTTPStatusCode: http.StatusConflict,
 	},
 	ErrObjectLocked: {

tests/integration/group-tests.go

@@ -597,7 +597,9 @@ func TestCORSMiddleware(s *S3Conf) {
 func TestPutObjectLockConfiguration(s *S3Conf) {
 	PutObjectLockConfiguration_non_existing_bucket(s)
 	PutObjectLockConfiguration_empty_config(s)
-	PutObjectLockConfiguration_not_enabled_on_bucket_creation(s)
+	if !s.versioningEnabled {
+		PutObjectLockConfiguration_not_enabled_on_bucket_creation(s)
+	}
 	PutObjectLockConfiguration_invalid_status(s)
 	PutObjectLockConfiguration_invalid_mode(s)
 	PutObjectLockConfiguration_both_years_and_days(s)
@@ -615,7 +617,6 @@ func TestPutObjectRetention(s *S3Conf) {
 	PutObjectRetention_non_existing_bucket(s)
 	PutObjectRetention_non_existing_object(s)
 	PutObjectRetention_unset_bucket_object_lock_config(s)
-	PutObjectRetention_disabled_bucket_object_lock_config(s)
 	PutObjectRetention_expired_retain_until_date(s)
 	PutObjectRetention_invalid_mode(s)
 	PutObjectRetention_overwrite_compliance_mode(s)
@@ -640,7 +641,6 @@ func TestPutObjectLegalHold(s *S3Conf) {
 	PutObjectLegalHold_invalid_body(s)
 	PutObjectLegalHold_invalid_status(s)
 	PutObjectLegalHold_unset_bucket_object_lock_config(s)
-	PutObjectLegalHold_disabled_bucket_object_lock_config(s)
 	PutObjectLegalHold_success(s)
 }
 
@@ -773,7 +773,9 @@ func TestFullFlow(s *S3Conf) {
 	TestGetObjectRetention(s)
 	TestPutObjectLegalHold(s)
 	TestGetObjectLegalHold(s)
-	TestWORMProtection(s)
+	if !s.versioningEnabled {
+		TestWORMProtection(s)
+	}
 	TestAccessControl(s)
 	TestRouter(s)
 	// FIXME: The tests should pass for azure as well
@@ -926,7 +928,14 @@ func TestAccessControl(s *S3Conf) {
 func TestPublicBuckets(s *S3Conf) {
 	PublicBucket_default_private_bucket(s)
 	PublicBucket_public_bucket_policy(s)
-	PublicBucket_public_object_policy(s)
+	if !s.versioningEnabled {
+		// This test targets gateway actions when bucket grants
+		// public access to object operations: no specific
+		// bucket versioning operations. As object version cleanup
+		// is hard to perform, run the test only on the versioning-disabled
+		// gateway instance
+		PublicBucket_public_object_policy(s)
+	}
 	PublicBucket_public_acl(s)
 	PublicBucket_signed_streaming_payload(s)
 	PublicBucket_incorrect_sha256_hash(s)
@@ -993,6 +1002,7 @@ func TestVersioning(s *S3Conf) {
 	Versioning_UploadPartCopy_non_existing_versionId(s)
 	Versioning_UploadPartCopy_from_an_object_version(s)
 	// Object lock configuration
+	Versioning_object_lock_not_enabled_on_bucket_creation(s)
 	Versioning_Enable_object_lock(s)
 	Versioning_status_switch_to_suspended_with_object_lock(s)
 	// Object-Lock Retention
@@ -1438,7 +1448,6 @@ func GetIntTests() IntTests {
 		"PutObjectRetention_non_existing_bucket":                                  PutObjectRetention_non_existing_bucket,
 		"PutObjectRetention_non_existing_object":                                  PutObjectRetention_non_existing_object,
 		"PutObjectRetention_unset_bucket_object_lock_config":                      PutObjectRetention_unset_bucket_object_lock_config,
-		"PutObjectRetention_disabled_bucket_object_lock_config":                   PutObjectRetention_disabled_bucket_object_lock_config,
 		"PutObjectRetention_expired_retain_until_date":                            PutObjectRetention_expired_retain_until_date,
 		"PutObjectRetention_invalid_mode":                                         PutObjectRetention_invalid_mode,
 		"PutObjectRetention_overwrite_compliance_mode":                            PutObjectRetention_overwrite_compliance_mode,
@@ -1457,7 +1466,6 @@ func GetIntTests() IntTests {
 		"PutObjectLegalHold_invalid_body":                                         PutObjectLegalHold_invalid_body,
 		"PutObjectLegalHold_invalid_status":                                       PutObjectLegalHold_invalid_status,
 		"PutObjectLegalHold_unset_bucket_object_lock_config":                      PutObjectLegalHold_unset_bucket_object_lock_config,
-		"PutObjectLegalHold_disabled_bucket_object_lock_config":                   PutObjectLegalHold_disabled_bucket_object_lock_config,
 		"PutObjectLegalHold_success":                                              PutObjectLegalHold_success,
 		"GetObjectLegalHold_non_existing_bucket":                                  GetObjectLegalHold_non_existing_bucket,
 		"GetObjectLegalHold_non_existing_object":                                  GetObjectLegalHold_non_existing_object,
@@ -1590,6 +1598,7 @@ func GetIntTests() IntTests {
 		"Versioning_Multipart_Upload_overwrite_an_object":                         Versioning_Multipart_Upload_overwrite_an_object,
 		"Versioning_UploadPartCopy_non_existing_versionId":                        Versioning_UploadPartCopy_non_existing_versionId,
 		"Versioning_UploadPartCopy_from_an_object_version":                        Versioning_UploadPartCopy_from_an_object_version,
+		"Versioning_object_lock_not_enabled_on_bucket_creation":                   Versioning_object_lock_not_enabled_on_bucket_creation,
 		"Versioning_Enable_object_lock":                                           Versioning_Enable_object_lock,
 		"Versioning_status_switch_to_suspended_with_object_lock":                  Versioning_status_switch_to_suspended_with_object_lock,
 		"Versioning_PutObjectRetention_invalid_versionId":                         Versioning_PutObjectRetention_invalid_versionId,

tests/integration/tests.go

@@ -3046,14 +3046,10 @@ func PutObject_with_object_lock(s *S3Conf) error {
 			return fmt.Errorf("expected object lock mode to be %v, instead got %v", types.ObjectLockModeCompliance, out.ObjectLockMode)
 		}
 		if out.ObjectLockLegalHoldStatus != types.ObjectLockLegalHoldStatusOn {
-			return fmt.Errorf("expected object lock legal hold status to be %v, instead got %v", types.ObjectLockLegalHoldStatusOn, out.ObjectLockLegalHoldStatus)
+			return fmt.Errorf("expected object lock mode to be %v, instead got %v", types.ObjectLockLegalHoldStatusOn, out.ObjectLockLegalHoldStatus)
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: obj, removeLegalHold: true, isCompliance: true}})
 	}, withLock())
 }
 
@@ -8121,12 +8117,7 @@ func CopyObject_with_legal_hold(s *S3Conf) error {
 				types.ObjectLockLegalHoldStatusOn, res.LegalHold.Status)
 		}
 
-		err = changeBucketObjectLockStatus(s3client, bucket, false)
-		if err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: dstObj, removeOnlyLeglHold: true}})
 	}, withLock())
 }
 
@@ -8176,12 +8167,7 @@ func CopyObject_with_retention_lock(s *S3Conf) error {
 				retDate.Format(time.RFC1123), res.Retention.RetainUntilDate.Format(time.RFC1123))
 		}
 
-		err = changeBucketObjectLockStatus(s3client, bucket, false)
-		if err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: dstObj}})
 	}, withLock())
 }
 
@@ -9177,11 +9163,7 @@ func CreateMultipartUpload_with_object_lock(s *S3Conf) error {
 				types.ObjectLockModeGovernance, resp.ObjectLockMode)
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: obj, removeLegalHold: true}})
 	}, withLock())
 }
 
@@ -15962,10 +15944,11 @@ func PutObjectLockConfiguration_not_enabled_on_bucket_creation(s *S3Conf) error
 			},
 		})
 		cancel()
-		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotAllowed)); err != nil {
-			return err
-		}
-		return nil
+		// this test cases address the successful object lock status upload
+		// on versioning-disabled gateway mode, where versioning is not supported
+		// and object lock may be enabled without bucket versioning status check
+		// Note: this is not S3 compatible feature.
+		return err
 	})
 }
 
@@ -16219,10 +16202,6 @@ func PutObjectRetention_non_existing_bucket(s *S3Conf) error {
 func PutObjectRetention_non_existing_object(s *S3Conf) error {
 	testName := "PutObjectRetention_non_existing_object"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
-
 		date := time.Now().Add(time.Hour * 3)
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		_, err := s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
@@ -16271,52 +16250,9 @@ func PutObjectRetention_unset_bucket_object_lock_config(s *S3Conf) error {
 	})
 }
 
-func PutObjectRetention_disabled_bucket_object_lock_config(s *S3Conf) error {
-	testName := "PutObjectRetention_disabled_bucket_object_lock_config"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutObjectLockConfiguration(ctx, &s3.PutObjectLockConfigurationInput{
-			Bucket:                  &bucket,
-			ObjectLockConfiguration: &types.ObjectLockConfiguration{},
-		})
-		cancel()
-		if err != nil {
-			return err
-		}
-
-		date := time.Now().Add(time.Hour * 3)
-		key := "my-obj"
-
-		_, err = putObjects(s3client, []string{key}, bucket)
-		if err != nil {
-			return err
-		}
-
-		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
-		_, err = s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
-			Bucket: &bucket,
-			Key:    &key,
-			Retention: &types.ObjectLockRetention{
-				Mode:            types.ObjectLockRetentionModeCompliance,
-				RetainUntilDate: &date,
-			},
-		})
-		cancel()
-		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)); err != nil {
-			return err
-		}
-
-		return nil
-	}, withLock())
-}
-
 func PutObjectRetention_expired_retain_until_date(s *S3Conf) error {
 	testName := "PutObjectRetention_expired_retain_until_date"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
-
 		date := time.Now().Add(-time.Hour * 3)
 
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
@@ -16397,11 +16333,7 @@ func PutObjectRetention_overwrite_compliance_mode(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: obj, isCompliance: true}})
 	}, withLock())
 }
 
@@ -16445,11 +16377,7 @@ func PutObjectRetention_overwrite_compliance_with_compliance(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: obj, isCompliance: true}})
 	}, withLock())
 }
 
@@ -16493,11 +16421,7 @@ func PutObjectRetention_overwrite_governance_with_governance(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: obj}})
 	}, withLock())
 }
 
@@ -16539,11 +16463,7 @@ func PutObjectRetention_overwrite_governance_without_bypass_specified(s *S3Conf)
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: obj}})
 	}, withLock())
 }
 
@@ -16599,21 +16519,13 @@ func PutObjectRetention_overwrite_governance_with_permission(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: obj, isCompliance: true}})
 	}, withLock())
 }
 
 func PutObjectRetention_success(s *S3Conf) error {
 	testName := "PutObjectRetention_success"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
-
 		date := time.Now().Add(time.Hour * 3)
 		key := "my-obj"
 
@@ -16636,11 +16548,7 @@ func PutObjectRetention_success(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, isCompliance: true}})
 	}, withLock())
 }
 
@@ -16727,9 +16635,6 @@ func GetObjectRetention_unset_config(s *S3Conf) error {
 func GetObjectRetention_success(s *S3Conf) error {
 	testName := "GetObjectRetention_success"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
 		key := "my-obj"
 		_, err := putObjects(s3client, []string{key}, bucket)
 		if err != nil {
@@ -16777,11 +16682,7 @@ func GetObjectRetention_success(s *S3Conf) error {
 		// 	return fmt.Errorf("expected retain until date to be %v, instead got %v", retention.RetainUntilDate.Format(iso8601Format), ret.RetainUntilDate.Format(iso8601Format))
 		// }
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, isCompliance: true}})
 	}, withLock())
 }
 
@@ -16805,10 +16706,6 @@ func PutObjectLegalHold_non_existing_bucket(s *S3Conf) error {
 func PutObjectLegalHold_non_existing_object(s *S3Conf) error {
 	testName := "PutObjectLegalHold_non_existing_object"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
-
 		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 		_, err := s3client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
 			Bucket: &bucket,
@@ -16890,50 +16787,9 @@ func PutObjectLegalHold_unset_bucket_object_lock_config(s *S3Conf) error {
 	})
 }
 
-func PutObjectLegalHold_disabled_bucket_object_lock_config(s *S3Conf) error {
-	testName := "PutObjectLegalHold_disabled_bucket_object_lock_config"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		_, err := s3client.PutObjectLockConfiguration(ctx, &s3.PutObjectLockConfigurationInput{
-			Bucket:                  &bucket,
-			ObjectLockConfiguration: &types.ObjectLockConfiguration{},
-		})
-		cancel()
-		if err != nil {
-			return err
-		}
-
-		key := "my-obj"
-
-		_, err = putObjects(s3client, []string{key}, bucket)
-		if err != nil {
-			return err
-		}
-
-		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
-		_, err = s3client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
-			Bucket: &bucket,
-			Key:    &key,
-			LegalHold: &types.ObjectLockLegalHold{
-				Status: types.ObjectLockLegalHoldStatusOn,
-			},
-		})
-		cancel()
-		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidBucketObjectLockConfiguration)); err != nil {
-			return err
-		}
-
-		return nil
-	}, withLock())
-}
-
 func PutObjectLegalHold_success(s *S3Conf) error {
 	testName := "PutObjectLegalHold_success"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
-
 		key := "my-obj"
 
 		_, err := putObjects(s3client, []string{key}, bucket)
@@ -16954,11 +16810,7 @@ func PutObjectLegalHold_success(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, removeOnlyLeglHold: true}})
 	}, withLock())
 }
 
@@ -17045,9 +16897,6 @@ func GetObjectLegalHold_unset_config(s *S3Conf) error {
 func GetObjectLegalHold_success(s *S3Conf) error {
 	testName := "GetObjectLegalHold_success"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
 		key := "my-obj"
 		_, err := putObjects(s3client, []string{key}, bucket)
 		if err != nil {
@@ -17082,11 +16931,7 @@ func GetObjectLegalHold_success(s *S3Conf) error {
 				resp.LegalHold.Status)
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: key, removeOnlyLeglHold: true}})
 	}, withLock())
 }
 
@@ -17801,11 +17646,7 @@ func WORMProtection_bucket_object_lock_configuration_compliance_mode(s *S3Conf)
 		if err := checkWORMProtection(s3client, bucket, object); err != nil {
 			return err
 		}
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: object, isCompliance: true}})
 	}, withLock())
 }
 
@@ -17840,11 +17681,7 @@ func WORMProtection_bucket_object_lock_configuration_governance_mode(s *S3Conf)
 		if err := checkWORMProtection(s3client, bucket, object); err != nil {
 			return err
 		}
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: object}})
 	}, withLock())
 }
 
@@ -17896,15 +17733,7 @@ func WORMProtection_bucket_object_lock_governance_bypass_delete(s *S3Conf) error
 			BypassGovernanceRetention: &bypass,
 		})
 		cancel()
-		if err != nil {
-			return err
-		}
-
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return err
 	}, withLock())
 }
 
@@ -17968,15 +17797,7 @@ func WORMProtection_bucket_object_lock_governance_bypass_delete_multiple(s *S3Co
 			},
 		})
 		cancel()
-		if err != nil {
-			return err
-		}
-
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return err
 	}, withLock())
 }
 
@@ -18008,11 +17829,8 @@ func WORMProtection_object_lock_retention_compliance_locked(s *S3Conf) error {
 		if err := checkWORMProtection(s3client, bucket, object); err != nil {
 			return err
 		}
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
 
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: object, isCompliance: true}})
 	}, withLock())
 }
 
@@ -18044,11 +17862,7 @@ func WORMProtection_object_lock_retention_governance_locked(s *S3Conf) error {
 		if err := checkWORMProtection(s3client, bucket, object); err != nil {
 			return err
 		}
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: object}})
 	}, withLock())
 }
 
@@ -18095,15 +17909,7 @@ func WORMProtection_object_lock_retention_governance_bypass_overwrite(s *S3Conf)
 			Key:    &object,
 		})
 		cancel()
-		if err != nil {
-			return err
-		}
-
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return err
 	}, withLock())
 }
 
@@ -18152,15 +17958,7 @@ func WORMProtection_object_lock_retention_governance_bypass_delete(s *S3Conf) er
 			BypassGovernanceRetention: &bypass,
 		})
 		cancel()
-		if err != nil {
-			return err
-		}
-
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return err
 	}, withLock())
 }
 
@@ -18224,25 +18022,13 @@ func WORMProtection_object_lock_retention_governance_bypass_delete_mul(s *S3Conf
 			},
 		})
 		cancel()
-		if err != nil {
-			return err
-		}
-
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return err
 	}, withLock())
 }
 
 func WORMProtection_object_lock_legal_hold_locked(s *S3Conf) error {
 	testName := "WORMProtection_object_lock_legal_hold_locked"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
-			return err
-		}
-
 		object := "my-obj"
 
 		_, err := putObjects(s3client, []string{object}, bucket)
@@ -18268,11 +18054,7 @@ func WORMProtection_object_lock_legal_hold_locked(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: object, removeOnlyLeglHold: true}})
 	}, withLock())
 }
 
@@ -18324,15 +18106,7 @@ func WORMProtection_root_bypass_governance_retention_delete_object(s *S3Conf) er
 			BypassGovernanceRetention: &bypass,
 		})
 		cancel()
-		if err != nil {
-			return err
-		}
-
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return err
 	}, withLock())
 }
 
@@ -20524,12 +20298,6 @@ func PublicBucket_public_object_policy(s *S3Conf) error {
 			}
 		}
 
-		// disable object lock on the bucket
-		err = changeBucketObjectLockStatus(rootClient, bucket, false)
-		if err != nil {
-			return err
-		}
-
 		return nil
 	}, withAnonymousClient(), withLock())
 }
@@ -23886,6 +23654,27 @@ func Versioning_Enable_object_lock(s *S3Conf) error {
 	}, withLock())
 }
 
+func Versioning_object_lock_not_enabled_on_bucket_creation(s *S3Conf) error {
+	testName := "Versioning_not_enabled_on_bucket_creation"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := s3client.PutObjectLockConfiguration(ctx, &s3.PutObjectLockConfigurationInput{
+			Bucket: &bucket,
+			ObjectLockConfiguration: &types.ObjectLockConfiguration{
+				ObjectLockEnabled: types.ObjectLockEnabledEnabled,
+				Rule: &types.ObjectLockRule{
+					DefaultRetention: &types.DefaultRetention{
+						Mode: types.ObjectLockRetentionModeCompliance,
+						Days: getPtr(int32(10)),
+					},
+				},
+			},
+		})
+		cancel()
+		return checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotAllowed))
+	})
+}
+
 func Versioning_status_switch_to_suspended_with_object_lock(s *S3Conf) error {
 	testName := "Versioning_status_switch_to_suspended_with_object_lock"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -23993,11 +23782,7 @@ func Versioning_Put_GetObjectRetention_success(s *S3Conf) error {
 				types.ObjectLockRetentionModeGovernance, res.Retention.Mode)
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{{key: getString(objVersion.Key), versionId: getString(objVersion.VersionId)}})
 	}, withLock(), withVersioning(types.BucketVersioningStatusEnabled))
 }
 
@@ -24092,11 +23877,13 @@ func Versioning_Put_GetObjectLegalHold_success(s *S3Conf) error {
 				types.ObjectLockLegalHoldStatusOn, res.LegalHold.Status)
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{
+			{
+				key:                getString(objVersion.Key),
+				versionId:          getString(objVersion.VersionId),
+				removeOnlyLeglHold: true,
+			},
+		})
 	}, withLock(), withVersioning(types.BucketVersioningStatusEnabled))
 }
 
@@ -24135,11 +23922,13 @@ func Versioning_WORM_obj_version_locked_with_legal_hold(s *S3Conf) error {
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{
+			{
+				key:                obj,
+				versionId:          getString(version.VersionId),
+				removeOnlyLeglHold: true,
+			},
+		})
 	}, withLock(), withVersioning(types.BucketVersioningStatusEnabled))
 }
 
@@ -24180,11 +23969,12 @@ func Versioning_WORM_obj_version_locked_with_governance_retention(s *S3Conf) err
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{
+			{
+				key:       obj,
+				versionId: getString(version.VersionId),
+			},
+		})
 	}, withLock(), withVersioning(types.BucketVersioningStatusEnabled))
 }
 
@@ -24225,11 +24015,13 @@ func Versioning_WORM_obj_version_locked_with_compliance_retention(s *S3Conf) err
 			return err
 		}
 
-		if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
-			return err
-		}
-
-		return nil
+		return cleanupLockedObjects(s3client, bucket, []objToDelete{
+			{
+				key:          obj,
+				versionId:    getString(version.VersionId),
+				isCompliance: true,
+			},
+		})
 	}, withLock(), withVersioning(types.BucketVersioningStatusEnabled))
 }
 

tests/integration/utils.go

@@ -48,6 +48,8 @@ import (
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
 	"github.com/aws/smithy-go"
 	"github.com/versity/versitygw/s3err"
+	"golang.org/x/sync/errgroup"
+	"golang.org/x/sync/semaphore"
 )
 
 var (
@@ -1100,25 +1102,6 @@ func getMalformedPolicyError(msg string) s3err.APIError {
 	}
 }
 
-// if true enables, otherwise disables
-func changeBucketObjectLockStatus(client *s3.Client, bucket string, status bool) error {
-	cfg := types.ObjectLockConfiguration{}
-	if status {
-		cfg.ObjectLockEnabled = types.ObjectLockEnabledEnabled
-	}
-	ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-	_, err := client.PutObjectLockConfiguration(ctx, &s3.PutObjectLockConfigurationInput{
-		Bucket:                  &bucket,
-		ObjectLockConfiguration: &cfg,
-	})
-	cancel()
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func putBucketVersioningStatus(client *s3.Client, bucket string, status types.BucketVersioningStatus) error {
 	ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
 	_, err := client.PutBucketVersioning(ctx, &s3.PutBucketVersioningInput{
@@ -1835,3 +1818,126 @@ func sprintVersions(objects []types.ObjectVersion) string {
 
 	return strings.Join(names, ",")
 }
+
+// objToDelete represents the metadata of an object that needs to be deleted.
+// It holds details like the key, version, and legal/compliance lock flags.
+type objToDelete struct {
+	key                string // Object key (name) in the bucket
+	versionId          string // Specific object version ID
+	removeLegalHold    bool   // Whether to remove legal hold before deletion
+	removeOnlyLeglHold bool   // Whether to only remove legal hold, without deletion
+	isCompliance       bool   // Whether the object is under Compliance mode retention
+}
+
+// Worker and retry configuration for deleting locked objects
+const (
+	maxDelObjWorkers int64         = 20              // Maximum number of concurrent delete workers
+	maxRetryAttempts int           = 3               // Maximum retries for object deletion
+	lockWaitTime     time.Duration = time.Second * 3 // Wait time for lock expiration before retrying delete
+)
+
+// cleanupLockedObjects removes objects from a bucket that may be protected by
+// Object Lock (legal hold or retention).
+// It handles both Governance and Compliance retention modes and retries deletions
+// when necessary.
+func cleanupLockedObjects(client *s3.Client, bucket string, objs []objToDelete) error {
+	eg, ctx := errgroup.WithContext(context.Background())
+
+	// Semaphore to limit the number of concurrent workers
+	sem := semaphore.NewWeighted(maxDelObjWorkers)
+
+	for _, obj := range objs {
+		obj := obj // capture loop variable
+
+		// Acquire worker slot before processing an object
+		if err := sem.Acquire(ctx, 1); err != nil {
+			return fmt.Errorf("failed to acquire worker space: %w", err)
+		}
+
+		eg.Go(func() error {
+			// Remove legal hold if required
+			if obj.removeLegalHold || obj.removeOnlyLeglHold {
+				ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+				_, err := client.PutObjectLegalHold(ctx, &s3.PutObjectLegalHoldInput{
+					Bucket:    &bucket,
+					Key:       &obj.key,
+					VersionId: getPtr(obj.versionId),
+					LegalHold: &types.ObjectLockLegalHold{
+						Status: types.ObjectLockLegalHoldStatusOff, // Disable legal hold
+					},
+				})
+				cancel()
+				// If object was already deleted, ignore the error
+				if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchKey)) {
+					return nil
+				}
+				if err != nil {
+					return err
+				}
+
+				// If only the legal hold needs to be removed, stop here
+				if obj.removeOnlyLeglHold {
+					return nil
+				}
+			}
+
+			// Apply temporary retention policy to allow deletion
+			// RetainUntilDate is set a few seconds in the future to handle network delays
+			retDate := time.Now().Add(lockWaitTime)
+			mode := types.ObjectLockRetentionModeGovernance
+			if obj.isCompliance {
+				mode = types.ObjectLockRetentionModeCompliance
+			}
+
+			ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+			_, err := client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
+				Bucket:    &bucket,
+				Key:       &obj.key,
+				VersionId: getPtr(obj.versionId),
+				Retention: &types.ObjectLockRetention{
+					Mode:            mode,
+					RetainUntilDate: &retDate,
+				},
+			})
+			cancel()
+
+			// If object was already deleted, ignore the error
+			if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchKey)) {
+				return nil
+			}
+			if err != nil {
+				return err
+			}
+
+			// Wait until retention lock expires before attempting delete
+			time.Sleep(lockWaitTime)
+
+			// Attempt deletion with retries
+			attempts := 0
+			for attempts != maxRetryAttempts {
+				ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+				_, err = client.DeleteObject(ctx, &s3.DeleteObjectInput{
+					Bucket:    &bucket,
+					Key:       &obj.key,
+					VersionId: getPtr(obj.versionId),
+				})
+				cancel()
+				if err != nil {
+					// Retry after a short delay if delete fails
+					time.Sleep(time.Second)
+					attempts++
+					continue
+				}
+
+				// Success, no more retries needed
+				return nil
+			}
+
+			// Return last error if all retries failed
+			return err
+		})
+	}
+
+	// Wait for all goroutines to finish, return any error encountered
+	return eg.Wait()
+}