With retention, skip actions expiring all versions (#19657)

ILM actions due to ExpiredObjectDeleteAllVersions and
DelMarkerExpiration are ignored when object locking is enabled on a
bucket.
Note: This applies to object versions which may not have retention
configured on them. This applies to all object versions in this bucket,
including those created before the retention config was applied.

cmd/data-scanner.go

@@ -1199,17 +1199,15 @@ func evalActionFromLifecycle(ctx context.Context, lc lifecycle.Lifecycle, lr loc
 		console.Debugf(applyActionsLogPrefix+" lifecycle: Secondary scan: %v\n", event.Action)
 	}
 
-	if event.Action == lifecycle.NoneAction {
-		return event
-	}
-
-	if obj.IsLatest && event.Action == lifecycle.DeleteAllVersionsAction {
-		if lr.LockEnabled && enforceRetentionForDeletion(ctx, obj) {
+	switch event.Action {
+	case lifecycle.DeleteAllVersionsAction, lifecycle.DelMarkerDeleteAllVersionsAction:
+		// Skip if bucket has object locking enabled; To prevent the
+		// possibility of violating an object retention on one of the
+		// noncurrent versions of this object.
+		if lr.LockEnabled {
 			return lifecycle.Event{Action: lifecycle.NoneAction}
 		}
-	}
 
-	switch event.Action {
 	case lifecycle.DeleteVersionAction, lifecycle.DeleteRestoredVersionAction:
 		// Defensive code, should never happen
 		if obj.VersionID == "" {

cmd/data-scanner_test.go

@@ -20,12 +20,15 @@ package cmd
 import (
 	"context"
 	"encoding/xml"
+	"fmt"
+	"strings"
 	"sync"
 	"testing"
 	"time"
 
 	"github.com/google/uuid"
 	"github.com/minio/minio/internal/bucket/lifecycle"
+	"github.com/minio/minio/internal/bucket/object/lock"
 	"github.com/minio/minio/internal/bucket/versioning"
 )
 
@@ -141,3 +144,96 @@ func TestApplyNewerNoncurrentVersionsLimit(t *testing.T) {
 		}
 	}
 }
+
+func TestEvalActionFromLifecycle(t *testing.T) {
+	// Tests cover only ExpiredObjectDeleteAllVersions and DelMarkerExpiration actions
+	obj := ObjectInfo{
+		Name:        "foo",
+		ModTime:     time.Now().Add(-31 * 24 * time.Hour),
+		Size:        100 << 20,
+		VersionID:   uuid.New().String(),
+		IsLatest:    true,
+		NumVersions: 4,
+	}
+	delMarker := ObjectInfo{
+		Name:         "foo-deleted",
+		ModTime:      time.Now().Add(-61 * 24 * time.Hour),
+		Size:         0,
+		VersionID:    uuid.New().String(),
+		IsLatest:     true,
+		DeleteMarker: true,
+		NumVersions:  4,
+	}
+	deleteAllILM := `<LifecycleConfiguration>
+			    <Rule>
+		               <Expiration>
+		                  <Days>30</Days>
+	                          <ExpiredObjectAllVersions>true</ExpiredObjectAllVersions>
+	                       </Expiration>
+	                       <Filter></Filter>
+	                       <Status>Enabled</Status>
+			       <ID>DeleteAllVersions</ID>
+	                    </Rule>
+	                 </LifecycleConfiguration>`
+	delMarkerILM := `<LifecycleConfiguration>
+                            <Rule>
+                              <ID>DelMarkerExpiration</ID>
+                              <Filter></Filter>
+                              <Status>Enabled</Status>
+                              <DelMarkerExpiration>
+                                <Days>60</Days>
+                              </DelMarkerExpiration>
+                             </Rule>
+                       </LifecycleConfiguration>`
+	deleteAllLc, err := lifecycle.ParseLifecycleConfig(strings.NewReader(deleteAllILM))
+	if err != nil {
+		t.Fatalf("Failed to parse deleteAllILM test ILM policy %v", err)
+	}
+	delMarkerLc, err := lifecycle.ParseLifecycleConfig(strings.NewReader(delMarkerILM))
+	if err != nil {
+		t.Fatalf("Failed to parse delMarkerILM test ILM policy %v", err)
+	}
+	tests := []struct {
+		ilm       lifecycle.Lifecycle
+		retention lock.Retention
+		obj       ObjectInfo
+		want      lifecycle.Action
+	}{
+		{
+			// with object locking
+			ilm:       *deleteAllLc,
+			retention: lock.Retention{LockEnabled: true},
+			obj:       obj,
+			want:      lifecycle.NoneAction,
+		},
+		{
+			// without object locking
+			ilm:       *deleteAllLc,
+			retention: lock.Retention{},
+			obj:       obj,
+			want:      lifecycle.DeleteAllVersionsAction,
+		},
+		{
+			// with object locking
+			ilm:       *delMarkerLc,
+			retention: lock.Retention{LockEnabled: true},
+			obj:       delMarker,
+			want:      lifecycle.NoneAction,
+		},
+		{
+			// without object locking
+			ilm:       *delMarkerLc,
+			retention: lock.Retention{},
+			obj:       delMarker,
+			want:      lifecycle.DelMarkerDeleteAllVersionsAction,
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("TestEvalAction-%d", i), func(t *testing.T) {
+			if got := evalActionFromLifecycle(context.TODO(), test.ilm, test.retention, nil, test.obj); got.Action != test.want {
+				t.Fatalf("Expected %v but got %v", test.want, got)
+			}
+		})
+	}
+}