Handle overlapping and conflicting ILM rules (#15812)

internal/bucket/lifecycle/lifecycle.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"sort"
 	"strings"
 	"time"
 
@@ -309,13 +310,63 @@ func (o ObjectOpts) ExpiredObjectDeleteMarker() bool {
 	return o.DeleteMarker && o.NumVersions == 1
 }
 
-// ComputeAction returns the action to perform by evaluating all lifecycle rules
+type lifecycleEvent struct {
-// against the object name and its modification time.
+	EventAction Action
-func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
+	RuleID      string
-	action := NoneAction
+	Due         time.Time
-	if obj.ModTime.IsZero() {
+}
-		return action
+
+type lifecycleEvents []lifecycleEvent
+
+func (es lifecycleEvents) Len() int {
+	return len(es)
+}
+
+func (es lifecycleEvents) Swap(i, j int) {
+	es[i], es[j] = es[j], es[i]
+}
+
+func (es lifecycleEvents) Less(i, j int) bool {
+	if es[i].Due.Equal(es[j].Due) {
+		// Prefer Expiration over Transition for both current and noncurrent
+		// versions
+		switch es[i].EventAction {
+		case DeleteAction, DeleteVersionAction:
+			return true
 		}
+		switch es[j].EventAction {
+		case DeleteAction, DeleteVersionAction:
+			return false
+		}
+		return true
+	}
+
+	// Prefer earlier occurring event
+	return es[i].Due.Before(es[j].Due)
+}
+
+// eval returns the lifecycle event applicable at now. If now is the zero value of time.Time, it returns the upcoming lifecycle event.
+func (lc Lifecycle) eval(obj ObjectOpts, now time.Time) lifecycleEvent {
+	var events []lifecycleEvent
+	if obj.ModTime.IsZero() {
+		return lifecycleEvent{}
+	}
+
+	// Handle expiry of restored object; NB Restored Objects have expiry set on
+	// them as part of RestoreObject API. They aren't governed by lifecycle
+	// rules.
+	if !obj.RestoreExpires.IsZero() && now.After(obj.RestoreExpires) {
+		action := DeleteRestoredAction
+		if !obj.IsLatest {
+			action = DeleteRestoredVersionAction
+		}
+
+		events = append(events, lifecycleEvent{
+			EventAction: action,
+			Due:         now,
+		})
+	}
+
 	for _, rule := range lc.FilterActionableRules(obj) {
 		if obj.ExpiredObjectDeleteMarker() {
 			if rule.Expiration.DeleteMarker.val {
@@ -323,85 +374,111 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 				// Only latest marker is removed. If set to true, the delete marker will be expired;
 				// if set to false the policy takes no action. This cannot be specified with Days or
 				// Date in a Lifecycle Expiration Policy.
-				return DeleteVersionAction
+				events = append(events, lifecycleEvent{
+					EventAction: DeleteVersionAction,
+					RuleID:      rule.ID,
+					Due:         now,
+				})
+				// No other conflicting actions apply to an expired object delete marker
+				break
 			}
 
 			if !rule.Expiration.IsDaysNull() {
 				// Specifying the Days tag will automatically perform ExpiredObjectDeleteMarker cleanup
 				// once delete markers are old enough to satisfy the age criteria.
 				// https://docs.aws.amazon.com/AmazonS3/latest/userguide/lifecycle-configuration-examples.html
-				if time.Now().UTC().After(ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days))) {
+				if expectedExpiry := ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days)); now.After(expectedExpiry) {
-					return DeleteVersionAction
+					events = append(events, lifecycleEvent{
+						EventAction: DeleteVersionAction,
+						RuleID:      rule.ID,
+						Due:         expectedExpiry,
+					})
+					// No other conflicting actions apply to an expired object delete marker
+					break
 				}
 			}
 		}
 
-		if !rule.NoncurrentVersionExpiration.IsDaysNull() {
+		// Skip rules with newer noncurrent versions specified. These rules are
-			// Skip rules with newer noncurrent versions specified.
+		// not handled at an individual version level. ComputeAction applies
-			// These rules are not handled at an individual version
+		// only to a specific version.
-			// level. ComputeAction applies only to a specific
-			// version.
 		if !obj.IsLatest && rule.NoncurrentVersionExpiration.NewerNoncurrentVersions > 0 {
 			continue
 		}
-			if obj.VersionID != "" && !obj.IsLatest && !obj.SuccessorModTime.IsZero() {
+
+		if !obj.IsLatest && !rule.NoncurrentVersionExpiration.IsDaysNull() {
 			// Non current versions should be deleted if their age exceeds non current days configuration
 			// https://docs.aws.amazon.com/AmazonS3/latest/dev/intro-lifecycle-rules.html#intro-lifecycle-rules-actions
-				if time.Now().UTC().After(ExpectedExpiryTime(obj.SuccessorModTime, int(rule.NoncurrentVersionExpiration.NoncurrentDays))) {
+			if expectedExpiry := ExpectedExpiryTime(obj.SuccessorModTime, int(rule.NoncurrentVersionExpiration.NoncurrentDays)); now.After(expectedExpiry) {
-					return DeleteVersionAction
+				events = append(events, lifecycleEvent{
-				}
+					EventAction: DeleteVersionAction,
+					RuleID:      rule.ID,
+					Due:         expectedExpiry,
+				})
 			}
 		}
 
-		if !rule.NoncurrentVersionTransition.IsNull() {
+		if !obj.IsLatest && !rule.NoncurrentVersionTransition.IsNull() {
-			if obj.VersionID != "" && !obj.IsLatest && !obj.SuccessorModTime.IsZero() && !obj.DeleteMarker && obj.TransitionStatus != TransitionComplete {
+			if !obj.DeleteMarker && obj.TransitionStatus != TransitionComplete {
 				// Non current versions should be transitioned if their age exceeds non current days configuration
 				// https://docs.aws.amazon.com/AmazonS3/latest/dev/intro-lifecycle-rules.html#intro-lifecycle-rules-actions
-				if due, ok := rule.NoncurrentVersionTransition.NextDue(obj); ok && time.Now().UTC().After(due) {
+				if due, ok := rule.NoncurrentVersionTransition.NextDue(obj); ok && now.After(due) {
-					return TransitionVersionAction
+					events = append(events, lifecycleEvent{
+						EventAction: TransitionVersionAction,
+						RuleID:      rule.ID,
+						Due:         due,
+					})
 				}
 			}
 		}
 
 		// Remove the object or simply add a delete marker (once) in a versioned bucket
-		if obj.VersionID == "" || obj.IsLatest && !obj.DeleteMarker {
+		if obj.IsLatest && !obj.DeleteMarker {
 			switch {
 			case !rule.Expiration.IsDateNull():
 				if time.Now().UTC().After(rule.Expiration.Date.Time) {
-					return DeleteAction
+					events = append(events, lifecycleEvent{
+						EventAction: DeleteAction,
+						RuleID:      rule.ID,
+						Due:         rule.Expiration.Date.Time,
+					})
 				}
 			case !rule.Expiration.IsDaysNull():
-				if time.Now().UTC().After(ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days))) {
+				if expectedExpiry := ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days)); now.After(expectedExpiry) {
-					return DeleteAction
+					events = append(events, lifecycleEvent{
+						EventAction: DeleteAction,
+						RuleID:      rule.ID,
+						Due:         expectedExpiry,
+					})
 				}
 			}
 
 			if obj.TransitionStatus != TransitionComplete {
-				if due, ok := rule.Transition.NextDue(obj); ok {
+				if due, ok := rule.Transition.NextDue(obj); ok && now.After(due) {
-					if time.Now().UTC().After(due) {
+					events = append(events, lifecycleEvent{
-						action = TransitionAction
+						EventAction: TransitionAction,
+						RuleID:      rule.ID,
+						Due:         due,
+					})
+				}
+			}
 		}
 	}
 
-				if !obj.RestoreExpires.IsZero() && time.Now().UTC().After(obj.RestoreExpires) {
+	if len(events) > 0 {
-					if obj.VersionID != "" {
+		sort.Sort(lifecycleEvents(events))
-						action = DeleteRestoredVersionAction
+		return events[0]
-					} else {
-						action = DeleteRestoredAction
-					}
-				}
-			}
-			if !obj.RestoreExpires.IsZero() && time.Now().UTC().After(obj.RestoreExpires) {
-				if obj.VersionID != "" {
-					action = DeleteRestoredVersionAction
-				} else {
-					action = DeleteRestoredAction
-				}
 	}
 
+	return lifecycleEvent{
+		EventAction: NoneAction,
 	}
-	}
+}
-	return action
+
+// ComputeAction returns the action to perform by evaluating all lifecycle rules
+// against the object name and its modification time.
+func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
+	return lc.eval(obj, time.Now().UTC()).EventAction
 }
 
 // ExpectedExpiryTime calculates the expiry, transition or restore date/time based on a object modtime.
@@ -418,90 +495,18 @@ func ExpectedExpiryTime(modTime time.Time, days int) time.Time {
 	return t.Truncate(24 * time.Hour)
 }
 
-// PredictExpiryTime returns the expiry date/time of a given object
-// after evaluating the current lifecycle document.
-func (lc Lifecycle) PredictExpiryTime(obj ObjectOpts) (string, time.Time) {
-	if obj.DeleteMarker {
-		// We don't need to send any x-amz-expiration for delete marker.
-		return "", time.Time{}
-	}
-
-	var finalExpiryDate time.Time
-	var finalExpiryRuleID string
-
-	// Iterate over all actionable rules and find the earliest
-	// expiration date and its associated rule ID.
-	for _, rule := range lc.FilterActionableRules(obj) {
-		// We don't know the index of this version and hence can't
-		// reliably compute expected expiry time.
-		if !obj.IsLatest && rule.NoncurrentVersionExpiration.NewerNoncurrentVersions > 0 {
-			continue
-		}
-		if !rule.NoncurrentVersionExpiration.IsDaysNull() && !obj.IsLatest && obj.VersionID != "" {
-			return rule.ID, ExpectedExpiryTime(obj.SuccessorModTime, int(rule.NoncurrentVersionExpiration.NoncurrentDays))
-		}
-
-		if !rule.Expiration.IsDateNull() {
-			if finalExpiryDate.IsZero() || finalExpiryDate.After(rule.Expiration.Date.Time) {
-				finalExpiryRuleID = rule.ID
-				finalExpiryDate = rule.Expiration.Date.Time
-			}
-		}
-		if !rule.Expiration.IsDaysNull() {
-			expectedExpiry := ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days))
-			if finalExpiryDate.IsZero() || finalExpiryDate.After(expectedExpiry) {
-				finalExpiryRuleID = rule.ID
-				finalExpiryDate = expectedExpiry
-			}
-		}
-	}
-	return finalExpiryRuleID, finalExpiryDate
-}
-
-// PredictTransitionTime returns the transition date/time of a given object
-// after evaluating the current lifecycle document.
-func (lc Lifecycle) PredictTransitionTime(obj ObjectOpts) (string, time.Time) {
-	if obj.DeleteMarker {
-		// We don't need to send any x-minio-transition for delete marker.
-		return "", time.Time{}
-	}
-
-	if obj.TransitionStatus == TransitionComplete {
-		return "", time.Time{}
-	}
-
-	// Iterate over all actionable rules and find the earliest
-	// transition date and its associated rule ID.
-	var finalTransitionDate time.Time
-	var finalTransitionRuleID string
-	for _, rule := range lc.FilterActionableRules(obj) {
-		if due, ok := rule.Transition.NextDue(obj); ok {
-			if finalTransitionDate.IsZero() || finalTransitionDate.After(due) {
-				finalTransitionRuleID = rule.ID
-				finalTransitionDate = due
-			}
-		}
-		if due, ok := rule.NoncurrentVersionTransition.NextDue(obj); ok {
-			if finalTransitionDate.IsZero() || finalTransitionDate.After(due) {
-				finalTransitionRuleID = rule.ID
-				finalTransitionDate = due
-			}
-		}
-	}
-	return finalTransitionRuleID, finalTransitionDate
-}
-
 // SetPredictionHeaders sets time to expiry and transition headers on w for a
 // given obj.
 func (lc Lifecycle) SetPredictionHeaders(w http.ResponseWriter, obj ObjectOpts) {
-	if ruleID, expiry := lc.PredictExpiryTime(obj); !expiry.IsZero() {
+	event := lc.eval(obj, time.Time{})
+	switch event.EventAction {
+	case DeleteAction, DeleteVersionAction:
 		w.Header()[xhttp.AmzExpiration] = []string{
-			fmt.Sprintf(`expiry-date="%s", rule-id="%s"`, expiry.Format(http.TimeFormat), ruleID),
+			fmt.Sprintf(`expiry-date="%s", rule-id="%s"`, event.Due.Format(http.TimeFormat), event.RuleID),
 		}
-	}
+	case TransitionAction, TransitionVersionAction:
-	if ruleID, transition := lc.PredictTransitionTime(obj); !transition.IsZero() {
 		w.Header()[xhttp.MinIOTransition] = []string{
-			fmt.Sprintf(`transition-date="%s", rule-id="%s"`, transition.Format(http.TimeFormat), ruleID),
+			fmt.Sprintf(`transition-date="%s", rule-id="%s"`, event.Due.Format(http.TimeFormat), event.RuleID),
 		}
 	}
 }
@@ -519,8 +524,8 @@ func (lc Lifecycle) TransitionTier(obj ObjectOpts) string {
 	return ""
 }
 
-// NoncurrentVersionsExpirationLimit returns the maximum limit on number of
+// NoncurrentVersionsExpirationLimit returns the number of noncurrent versions
-// noncurrent versions across rules.
+// to be retained from the first applicable rule per S3 behavior.
 func (lc Lifecycle) NoncurrentVersionsExpirationLimit(obj ObjectOpts) (string, int, int) {
 	var lim int
 	var days int
@@ -529,18 +534,7 @@ func (lc Lifecycle) NoncurrentVersionsExpirationLimit(obj ObjectOpts) (string, i
 		if rule.NoncurrentVersionExpiration.NewerNoncurrentVersions == 0 {
 			continue
 		}
-		// Pick the highest number of NewerNoncurrentVersions value
+		return rule.ID, int(rule.NoncurrentVersionExpiration.NoncurrentDays), rule.NoncurrentVersionExpiration.NewerNoncurrentVersions
-		// among overlapping rules.
-		if lim == 0 || lim < rule.NoncurrentVersionExpiration.NewerNoncurrentVersions {
-			lim = rule.NoncurrentVersionExpiration.NewerNoncurrentVersions
-		}
-		// Pick the earliest applicable NoncurrentDays among overlapping
-		// rules. Note: ruleID is that of the rule which determines the
-		// time of expiry.
-		if ndays := int(rule.NoncurrentVersionExpiration.NoncurrentDays); days == 0 || days > ndays {
-			days = ndays
-			ruleID = rule.ID
-		}
 	}
 	return ruleID, days, lim
 }

internal/bucket/lifecycle/lifecycle_test.go

@@ -439,6 +439,96 @@ func TestComputeActions(t *testing.T) {
 			isNoncurrent:           true,
 			expectedAction:         DeleteVersionAction,
 		},
+		{
+			inputConfig: `<LifecycleConfiguration>
+                             <Rule>
+                               <ID>Rule 1</ID>
+                               <Filter>
+                               </Filter>
+                               <Status>Enabled</Status>
+                               <Expiration>
+                                 <Days>365</Days>
+                               </Expiration>
+                             </Rule>
+                             <Rule>
+                               <ID>Rule 2</ID>
+                               <Filter>
+                                 <Prefix>logs/</Prefix>
+                               </Filter>
+                               <Status>Enabled</Status>
+                               <Transition>
+                                 <StorageClass>STANDARD_IA</StorageClass>
+                                 <Days>30</Days>
+                               </Transition>
+                              </Rule>
+                          </LifecycleConfiguration>`,
+			objectName:     "logs/obj-1",
+			objectModTime:  time.Now().UTC().Add(-31 * 24 * time.Hour),
+			expectedAction: TransitionAction,
+		},
+		{
+			inputConfig: `<LifecycleConfiguration>
+                             <Rule>
+                               <ID>Rule 1</ID>
+                               <Filter>
+                                 <Prefix>logs/</Prefix>
+                               </Filter>
+                               <Status>Enabled</Status>
+                               <Expiration>
+                                 <Days>365</Days>
+                               </Expiration>
+                             </Rule>
+                             <Rule>
+                               <ID>Rule 2</ID>
+                               <Filter>
+                                 <Prefix>logs/</Prefix>
+                               </Filter>
+                               <Status>Enabled</Status>
+                               <Transition>
+                                 <StorageClass>STANDARD_IA</StorageClass>
+                                 <Days>365</Days>
+                               </Transition>
+                             </Rule>
+                          </LifecycleConfiguration>`,
+			objectName:     "logs/obj-1",
+			objectModTime:  time.Now().UTC().Add(-366 * 24 * time.Hour),
+			expectedAction: DeleteAction,
+		},
+		{
+			inputConfig: `<LifecycleConfiguration>
+                            <Rule>
+                              <ID>Rule 1</ID>
+                              <Filter>
+                                <Tag>
+                                   <Key>tag1</Key>
+                                   <Value>value1</Value>
+                                </Tag>
+                              </Filter>
+                              <Status>Enabled</Status>
+                              <Transition>
+                                <StorageClass>GLACIER</StorageClass>
+                                <Days>365</Days>
+                              </Transition>
+                            </Rule>
+                            <Rule>
+                              <ID>Rule 2</ID>
+                              <Filter>
+                                <Tag>
+                                   <Key>tag2</Key>
+                                   <Value>value2</Value>
+                                </Tag>
+                              </Filter>
+                              <Status>Enabled</Status>
+                              <Expiration>
+                                <Days>14</Days>
+                              </Expiration>
+                             </Rule>
+                         </LifecycleConfiguration>`,
+			objectName:     "obj-1",
+			objectTags:     "tag1=value1&tag2=value2",
+			objectModTime:  time.Now().UTC().Add(-15 * 24 * time.Hour),
+			expectedAction: DeleteAction,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -461,7 +551,6 @@ func TestComputeActions(t *testing.T) {
 				t.Fatalf("Expected action: `%v`, got: `%v`", tc.expectedAction, resultAction)
 			}
 		})
-
 	}
 }
 
@@ -668,8 +757,8 @@ func TestNoncurrentVersionsLimit(t *testing.T) {
 	lc := Lifecycle{
 		Rules: rules,
 	}
-	if ruleID, days, lim := lc.NoncurrentVersionsExpirationLimit(ObjectOpts{Name: "obj"}); ruleID != "1" || days != 1 || lim != 10 {
+	if ruleID, days, lim := lc.NoncurrentVersionsExpirationLimit(ObjectOpts{Name: "obj"}); ruleID != "1" || days != 1 || lim != 1 {
-		t.Fatalf("Expected (ruleID, days, lim) to be (\"1\", 1, 10) but got (%s, %d, %d)", ruleID, days, lim)
+		t.Fatalf("Expected (ruleID, days, lim) to be (\"1\", 1, 1) but got (%s, %d, %d)", ruleID, days, lim)
 	}
 }