From d4af132fc45a58df45e9b58f8e6cd9583c9c6cae Mon Sep 17 00:00:00 2001
From: Anis Elleuch <vadmeste@users.noreply.github.com>
Date: Sun, 5 Jul 2020 04:56:02 +0100
Subject: [PATCH] lifecycle: Expiry should not delete versions (#9972)

Currently, lifecycle expiry is deleting all object versions which is not
correct, unless noncurrent versions field is specified.

Also, only delete the delete marker if it is the only version of the
given object.
---
 cmd/data-crawler.go               | 20 +++++++++++++++-----
 cmd/xl-storage.go                 |  2 +-
 pkg/bucket/lifecycle/lifecycle.go | 14 +++++++++-----
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/cmd/data-crawler.go b/cmd/data-crawler.go
index 08a84c4de..723b0fb2e 100644
--- a/cmd/data-crawler.go
+++ b/cmd/data-crawler.go
@@ -509,8 +509,9 @@ func (i *crawlItem) transformMetaDir() {
 
 // actionMeta contains information used to apply actions.
 type actionMeta struct {
-	oi      ObjectInfo
-	trustOI bool // Set true if oi can be trusted and has been read with quorum.
+	oi          ObjectInfo
+	trustOI     bool // Set true if oi can be trusted and has been read with quorum.
+	numVersions int  // The number of versions of this object
 }
 
 // applyActions will apply lifecycle checks on to a scanned item.
@@ -535,12 +536,13 @@ func (i *crawlItem) applyActions(ctx context.Context, o ObjectLayer, meta action
 			VersionID:    meta.oi.VersionID,
 			DeleteMarker: meta.oi.DeleteMarker,
 			IsLatest:     meta.oi.IsLatest,
+			NumVersions:  meta.numVersions,
 		})
 	if i.debug {
 		logger.Info(color.Green("applyActions:")+" lifecycle: %q, Initial scan: %v", i.objectPath(), action)
 	}
 	switch action {
-	case lifecycle.DeleteAction:
+	case lifecycle.DeleteAction, lifecycle.DeleteVersionAction:
 	default:
 		// No action.
 		return size
@@ -586,14 +588,22 @@ func (i *crawlItem) applyActions(ctx context.Context, o ObjectLayer, meta action
 		}
 		versionID = obj.VersionID
 		switch action {
-		case lifecycle.DeleteAction:
+		case lifecycle.DeleteAction, lifecycle.DeleteVersionAction:
 		default:
 			// No action.
 			return size
 		}
 	}
 
-	obj, err := o.DeleteObject(ctx, i.bucket, i.objectPath(), ObjectOptions{VersionID: versionID})
+	opts := ObjectOptions{}
+	switch action {
+	case lifecycle.DeleteVersionAction:
+		opts.VersionID = versionID
+	case lifecycle.DeleteAction:
+		opts.Versioned = globalBucketVersioningSys.Enabled(i.bucket)
+	}
+
+	obj, err := o.DeleteObject(ctx, i.bucket, i.objectPath(), opts)
 	if err != nil {
 		// Assume it is still there.
 		logger.LogIf(ctx, err)
diff --git a/cmd/xl-storage.go b/cmd/xl-storage.go
index 20033fa37..87efcfd68 100644
--- a/cmd/xl-storage.go
+++ b/cmd/xl-storage.go
@@ -408,7 +408,7 @@ func (s *xlStorage) CrawlAndGetDataUsage(ctx context.Context, cache dataUsageCac
 
 		var totalSize int64
 		for _, version := range fivs.Versions {
-			size := item.applyActions(ctx, objAPI, actionMeta{oi: version.ToObjectInfo(item.bucket, item.objectPath())})
+			size := item.applyActions(ctx, objAPI, actionMeta{numVersions: len(fivs.Versions), oi: version.ToObjectInfo(item.bucket, item.objectPath())})
 			if !version.Deleted {
 				totalSize += size
 			}
diff --git a/pkg/bucket/lifecycle/lifecycle.go b/pkg/bucket/lifecycle/lifecycle.go
index 5e62c2752..613689d45 100644
--- a/pkg/bucket/lifecycle/lifecycle.go
+++ b/pkg/bucket/lifecycle/lifecycle.go
@@ -40,6 +40,8 @@ const (
 	NoneAction Action = iota
 	// DeleteAction means the object needs to be removed after evaluting lifecycle rules
 	DeleteAction
+	// DeleteVersionAction deletes a particular version
+	DeleteVersionAction
 )
 
 // Lifecycle - Configuration for bucket lifecycle.
@@ -176,6 +178,7 @@ type ObjectOpts struct {
 	VersionID    string
 	IsLatest     bool
 	DeleteMarker bool
+	NumVersions  int
 }
 
 // ComputeAction returns the action to perform by evaluating all lifecycle rules
@@ -187,27 +190,28 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 	}
 
 	for _, rule := range lc.FilterActionableRules(obj) {
-		if obj.DeleteMarker && obj.IsLatest && bool(rule.Expiration.DeleteMarker) {
+		if obj.DeleteMarker && obj.NumVersions == 1 && bool(rule.Expiration.DeleteMarker) {
 			// Indicates whether MinIO will remove a delete marker with no noncurrent versions.
 			// 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 DeleteAction
+			return DeleteVersionAction
 		}
 
 		if !rule.NoncurrentVersionExpiration.IsDaysNull() {
 			if obj.VersionID != "" && !obj.IsLatest {
 				// Non current versions should be deleted.
 				if time.Now().After(expectedExpiryTime(obj.ModTime, rule.NoncurrentVersionExpiration.NoncurrentDays)) {
-					return DeleteAction
+					return DeleteVersionAction
 				}
 				return NoneAction
 			}
 			return NoneAction
 		}
 
-		// All other expiration only applies to latest versions.
-		if obj.IsLatest {
+		// All other expiration only applies to latest versions
+		// (except if this is a delete marker)
+		if obj.IsLatest && !obj.DeleteMarker {
 			switch {
 			case !rule.Expiration.IsDateNull():
 				if time.Now().UTC().After(rule.Expiration.Date.Time) {