ci: remove s3tests skip - test will be fixed in s3-tests repo

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

test/s3/retention/object_lock_validation_test.go

@@ -77,14 +77,14 @@ func TestObjectLockValidation(t *testing.T) {
 	require.NoError(t, err, "Setting Object Lock retention should succeed")
 	t.Log("   Object Lock retention applied successfully")
 
-	// Verify retention allows simple DELETE (creates delete marker) but blocks version deletion
-	// AWS S3 behavior: Simple DELETE (without version ID) is ALWAYS allowed and creates delete marker
+	// Verify retention blocks simple DELETE (COMPLIANCE mode is strict WORM)
+	// AWS S3 behavior: Simple DELETE (without version ID) is blocked by COMPLIANCE retention
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
 		Bucket: aws.String(bucketName),
 		Key:    aws.String(key),
 	})
-	require.NoError(t, err, "Simple DELETE should succeed and create delete marker (AWS S3 behavior)")
-	t.Log("   Simple DELETE succeeded (creates delete marker - correct AWS behavior)")
+	require.Error(t, err, "Simple DELETE should be blocked by COMPLIANCE retention (AWS S3 behavior)")
+	t.Log("   Simple DELETE correctly blocked by COMPLIANCE retention")
 
 	// Now verify that DELETE with version ID is properly blocked by retention
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{

test/s3/retention/s3_retention_test.go

@@ -329,12 +329,12 @@ func TestRetentionModeCompliance(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, types.ObjectLockRetentionModeCompliance, retentionResp.Retention.Mode)
 
-	// Try simple DELETE - should succeed and create delete marker (AWS S3 behavior)
+	// Try simple DELETE - should fail for COMPLIANCE mode (strict WORM)
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
 		Bucket: aws.String(bucketName),
 		Key:    aws.String(key),
 	})
-	require.NoError(t, err, "Simple DELETE should succeed and create delete marker")
+	require.Error(t, err, "Simple DELETE should be blocked by COMPLIANCE retention")
 
 	// Try DELETE with version ID - should fail for COMPLIANCE mode
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
@@ -388,12 +388,12 @@ func TestLegalHoldWorkflow(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, types.ObjectLockLegalHoldStatusOn, legalHoldResp.LegalHold.Status)
 
-	// Try simple DELETE - should succeed and create delete marker (AWS S3 behavior)
+	// Try simple DELETE - should fail due to legal hold
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
 		Bucket: aws.String(bucketName),
 		Key:    aws.String(key),
 	})
-	require.NoError(t, err, "Simple DELETE should succeed and create delete marker")
+	require.Error(t, err, "Simple DELETE should be blocked by legal hold")
 
 	// Try DELETE with version ID - should fail due to legal hold
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
@@ -591,12 +591,12 @@ func TestRetentionAndLegalHoldCombination(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	// Try simple DELETE - should succeed and create delete marker (AWS S3 behavior)
+	// Try simple DELETE - should fail due to legal hold + COMPLIANCE retention
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
 		Bucket: aws.String(bucketName),
 		Key:    aws.String(key),
 	})
-	require.NoError(t, err, "Simple DELETE should succeed and create delete marker")
+	require.Error(t, err, "Simple DELETE should be blocked by legal hold")
 
 	// Try DELETE with version ID and bypass - should still fail due to legal hold
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
@@ -607,7 +607,7 @@ func TestRetentionAndLegalHoldCombination(t *testing.T) {
 	})
 	require.Error(t, err, "Legal hold should prevent deletion even with governance bypass")
 
-	// Remove legal hold (must specify version ID since latest version is now delete marker)
+	// Remove legal hold (must specify version ID)
 	_, err = client.PutObjectLegalHold(context.TODO(), &s3.PutObjectLegalHoldInput{
 		Bucket:    aws.String(bucketName),
 		Key:       aws.String(key),

test/s3/retention/s3_worm_integration_test.go

@@ -42,12 +42,12 @@ func TestWORMRetentionIntegration(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	// Try simple DELETE - should succeed and create delete marker (AWS S3 behavior)
+	// Try simple DELETE - should fail due to GOVERNANCE retention
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
 		Bucket: aws.String(bucketName),
 		Key:    aws.String(key),
 	})
-	require.NoError(t, err, "Simple DELETE should succeed and create delete marker")
+	require.Error(t, err, "Simple DELETE should be blocked by GOVERNANCE retention")
 
 	// Try DELETE with version ID - should fail due to GOVERNANCE retention
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
@@ -325,12 +325,12 @@ func TestRetentionWithMultipartUpload(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	// Try simple DELETE - should succeed and create delete marker (AWS S3 behavior)
+	// Try simple DELETE - should fail due to GOVERNANCE retention
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{
 		Bucket: aws.String(bucketName),
 		Key:    aws.String(key),
 	})
-	require.NoError(t, err, "Simple DELETE should succeed and create delete marker")
+	require.Error(t, err, "Simple DELETE should be blocked by GOVERNANCE retention")
 
 	// Try DELETE with version ID - should fail due to GOVERNANCE retention
 	_, err = client.DeleteObject(context.TODO(), &s3.DeleteObjectInput{

weed/s3api/s3api_object_handlers_delete.go

@@ -19,6 +19,21 @@ const (
 	deleteMultipleObjectsLimit = 1000
 )
 
+// objectLockVersionToCheckForDelete resolves which version should be validated for Object Lock protections.
+// For versioned delete without explicit versionId, this targets the latest version (empty versionId for enabled,
+// "null" for suspended) because DeleteObject affects that version's visibility.
+// This always returns a version ID to check; object lock protections are fail-closed.
+func objectLockVersionToCheckForDelete(versioningState, requestedVersionID string) string {
+	if requestedVersionID != "" {
+		return requestedVersionID
+	}
+
+	if versioningState == s3_constants.VersioningSuspended {
+		return "null"
+	}
+	return ""
+}
+
 func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Request) {
 
 	bucket, object := s3_constants.GetBucketAndObject(r)
@@ -52,18 +67,18 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone)
 	}
 
+	lockCheckVersionID := objectLockVersionToCheckForDelete(versioningState, versionId)
+	governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
+	if err := s3a.enforceObjectLockProtections(r, bucket, object, lockCheckVersionID, governanceBypassAllowed); err != nil {
+		glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object, lockCheckVersionID, err)
+		s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
+		return
+	}
+
 	if versioningConfigured {
 		// Handle versioned delete based on specific versioning state
 		if versionId != "" {
 			// Delete specific version (same for both enabled and suspended)
-			// Check object lock permissions before deleting specific version
-			governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
-			if err := s3a.enforceObjectLockProtections(r, bucket, object, versionId, governanceBypassAllowed); err != nil {
-				glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
-				s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
-				return
-			}
-
 			// Delete specific version
 			err := s3a.deleteSpecificObjectVersion(bucket, object, versionId)
 			if err != nil {
@@ -77,9 +92,7 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		} else {
 			// Delete without version ID - behavior depends on versioning state
 			if versioningEnabled {
-				// Enabled versioning: Create delete marker (logical delete)
-				// AWS S3 behavior: Delete marker creation is NOT blocked by object retention
-				// because it's a logical delete that doesn't actually remove the retained version
+				// Enabled versioning: create delete marker (logical delete)
 				deleteMarkerVersionId, err := s3a.createDeleteMarker(bucket, object)
 				if err != nil {
 					glog.Errorf("Failed to create delete marker: %v", err)
@@ -94,14 +107,6 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 				// Suspended versioning: Actually delete the "null" version object
 				glog.V(2).Infof("DeleteObjectHandler: deleting null version for suspended versioning %s/%s", bucket, object)
 
-				// Check object lock permissions before deleting "null" version
-				governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
-				if err := s3a.enforceObjectLockProtections(r, bucket, object, "null", governanceBypassAllowed); err != nil {
-					glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
-					s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
-					return
-				}
-
 				// Delete the "null" version (the regular file)
 				err := s3a.deleteSpecificObjectVersion(bucket, object, "null")
 				if err != nil {
@@ -116,14 +121,6 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		}
 	} else {
 		// Handle regular delete (non-versioned)
-		// Check object lock permissions before deleting object
-		governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
-		if err := s3a.enforceObjectLockProtections(r, bucket, object, "", governanceBypassAllowed); err != nil {
-			glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, err)
-			s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
-			return
-		}
-
 		// Normalize trailing-slash object keys (e.g. "path/") to the
 		// underlying directory entry path so DeleteEntry gets a valid name.
 		target := util.NewFullPath(s3a.bucketDir(bucket), object)
@@ -249,20 +246,19 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 				continue
 			}
 
-			// Check object lock permissions before deletion (only for versioned buckets)
-			if versioningConfigured {
-				// Validate governance bypass for this specific object
-				governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key)
-				if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, object.VersionId, governanceBypassAllowed); err != nil {
-					glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, object.VersionId, err)
-					deleteErrors = append(deleteErrors, DeleteError{
-						Code:      s3err.GetAPIError(s3err.ErrAccessDenied).Code,
-						Message:   s3err.GetAPIError(s3err.ErrAccessDenied).Description,
-						Key:       object.Key,
-						VersionId: object.VersionId,
-					})
-					continue
-				}
+			// Check object lock permissions before deletion (applies to all buckets: versioned or non-versioned)
+			lockCheckVersionID := objectLockVersionToCheckForDelete(versioningState, object.VersionId)
+			// Validate governance bypass for this specific object
+			governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key)
+			if err := s3a.enforceObjectLockProtections(r, bucket, object.Key, lockCheckVersionID, governanceBypassAllowed); err != nil {
+				glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, lockCheckVersionID, err)
+				deleteErrors = append(deleteErrors, DeleteError{
+					Code:      s3err.GetAPIError(s3err.ErrAccessDenied).Code,
+					Message:   s3err.GetAPIError(s3err.ErrAccessDenied).Description,
+					Key:       object.Key,
+					VersionId: object.VersionId,
+				})
+				continue
 			}
 
 			var deleteVersionId string

weed/s3api/s3api_object_handlers_delete_test.go

@@ -0,0 +1,61 @@
+package s3api
+
+import (
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestObjectLockVersionToCheckForDelete(t *testing.T) {
+	tests := []struct {
+		name              string
+		versioningState   string
+		requestedVersionID string
+		expectedVersionID string
+	}{
+		{
+			name:              "enabled versioning without version id checks latest version",
+			versioningState:   s3_constants.VersioningEnabled,
+			requestedVersionID: "",
+			expectedVersionID: "",
+		},
+		{
+			name:              "suspended versioning without version id checks null version",
+			versioningState:   s3_constants.VersioningSuspended,
+			requestedVersionID: "",
+			expectedVersionID: "null",
+		},
+		{
+			name:              "specific version id is always checked",
+			versioningState:   s3_constants.VersioningEnabled,
+			requestedVersionID: "3LgYQ7f7VxQ3",
+			expectedVersionID: "3LgYQ7f7VxQ3",
+		},
+		{
+			name:              "non-versioned buckets still check current object",
+			versioningState:   "",
+			requestedVersionID: "",
+			expectedVersionID: "",
+		},
+		{
+			name:              "unknown versioning state defaults to empty version",
+			versioningState:   "UnexpectedState",
+			requestedVersionID: "",
+			expectedVersionID: "",
+		},
+		{
+			name:              "suspended versioning with specific version id checks that version",
+			versioningState:   s3_constants.VersioningSuspended,
+			requestedVersionID: "abc123",
+			expectedVersionID: "abc123",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			versionID := objectLockVersionToCheckForDelete(tc.versioningState, tc.requestedVersionID)
+			assert.Equal(t, tc.expectedVersionID, versionID)
+		})
+	}
+}