Newer noncurrent versions (#13815)

- Rename MaxNoncurrentVersions tag to NewerNoncurrentVersions Note: We apply overlapping NewerNoncurrentVersions rules such that we honor the highest among applicable limits. e.g if 2 overlapping rules are configured with 2 and 3 noncurrent versions to be retained, we will retain 3. - Expire newer noncurrent versions after noncurrent days - MinIO extension: allow noncurrent days to be zero, allowing expiry of noncurrent version as soon as more than configured NewerNoncurrentVersions are present. - Allow NewerNoncurrentVersions rules on object-locked buckets - No x-amz-expiration when NewerNoncurrentVersions configured - ComputeAction should skip rules with NewerNoncurrentVersions > 0 - Add unit tests for lifecycle.ComputeAction - Support lifecycle rules with MaxNoncurrentVersions - Extend ExpectedExpiryTime to work with zero days - Fix all-time comparisons to be relative to UTC
2021-12-14 09:41:44 -08:00 · 2021-12-14 09:41:44 -08:00 · 44a9339c0a
parent 113c7ff49a
commit 44a9339c0a
7 changed files with 248 additions and 83 deletions
--- a/cmd/bucket-lifecycle-handlers.go
+++ b/cmd/bucket-lifecycle-handlers.go
@ -24,7 +24,6 @@ import (

 	"github.com/gorilla/mux"
 	"github.com/minio/minio/internal/bucket/lifecycle"
-	"github.com/minio/minio/internal/bucket/object/lock"
 	xhttp "github.com/minio/minio/internal/http"
 	"github.com/minio/minio/internal/logger"
 	"github.com/minio/pkg/bucket/policy"
@ -80,17 +79,6 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r
 		return
 	}

-	// Disallow MaxNoncurrentVersions if bucket has object locking enabled
-	var rCfg lock.Retention
-	if rCfg, err = globalBucketObjectLockSys.Get(bucket); err != nil {
-		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
-		return
-	}
-	if rCfg.LockEnabled && bucketLifecycle.HasMaxNoncurrentVersions() {
-		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrInvalidLifecycleWithObjectLock), r.URL)
-		return
-	}
-
 	// Validate the transition storage ARNs
 	if err = validateTransitionTier(bucketLifecycle); err != nil {
 		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
--- a/cmd/bucket-lifecycle.go
+++ b/cmd/bucket-lifecycle.go
@ -83,19 +83,19 @@ type expiryTask struct {
 type expiryState struct {
 	once                sync.Once
 	byDaysCh            chan expiryTask
-	byMaxNoncurrentCh chan maxNoncurrentTask
+	byNewerNoncurrentCh chan newerNoncurrentTask
 }

 // PendingTasks returns the number of pending ILM expiry tasks.
 func (es *expiryState) PendingTasks() int {
-	return len(es.byDaysCh) + len(es.byMaxNoncurrentCh)
+	return len(es.byDaysCh) + len(es.byNewerNoncurrentCh)
 }

 // close closes work channels exactly once.
 func (es *expiryState) close() {
 	es.once.Do(func() {
 		close(es.byDaysCh)
-		close(es.byMaxNoncurrentCh)
+		close(es.byNewerNoncurrentCh)
 	})
 }

@ -109,13 +109,13 @@ func (es *expiryState) enqueueByDays(oi ObjectInfo, restoredObject bool, rmVersi
 	}
 }

-// enqueueByMaxNoncurrent enqueues object versions expired by
-// MaxNoncurrentVersions limit for expiry.
-func (es *expiryState) enqueueByMaxNoncurrent(bucket string, versions []ObjectToDelete) {
+// enqueueByNewerNoncurrent enqueues object versions expired by
+// NewerNoncurrentVersions limit for expiry.
+func (es *expiryState) enqueueByNewerNoncurrent(bucket string, versions []ObjectToDelete) {
 	select {
 	case <-GlobalContext.Done():
 		es.close()
-	case es.byMaxNoncurrentCh <- maxNoncurrentTask{bucket: bucket, versions: versions}:
+	case es.byNewerNoncurrentCh <- newerNoncurrentTask{bucket: bucket, versions: versions}:
 	default:
 	}
 }
@ -125,7 +125,7 @@ var globalExpiryState *expiryState
 func newExpiryState() *expiryState {
 	return &expiryState{
 		byDaysCh:            make(chan expiryTask, 10000),
-		byMaxNoncurrentCh: make(chan maxNoncurrentTask, 10000),
+		byNewerNoncurrentCh: make(chan newerNoncurrentTask, 10000),
 	}
 }

@ -141,15 +141,15 @@ func initBackgroundExpiry(ctx context.Context, objectAPI ObjectLayer) {
 		}
 	}()
 	go func() {
-		for t := range globalExpiryState.byMaxNoncurrentCh {
+		for t := range globalExpiryState.byNewerNoncurrentCh {
 			deleteObjectVersions(ctx, objectAPI, t.bucket, t.versions)
 		}
 	}()
 }

-// maxNoncurrentTask encapsulates arguments required by worker to expire objects
-// by MaxNoncurrentVersions
-type maxNoncurrentTask struct {
+// newerNoncurrentTask encapsulates arguments required by worker to expire objects
+// by NewerNoncurrentVersions
+type newerNoncurrentTask struct {
 	bucket   string
 	versions []ObjectToDelete
 }
--- a/cmd/data-scanner.go
+++ b/cmd/data-scanner.go
@ -972,14 +972,14 @@ func (i *scannerItem) applyTierObjSweep(ctx context.Context, o ObjectLayer, oi O

 }

-// applyMaxNoncurrentVersionLimit removes noncurrent versions older than the most recent MaxNoncurrentVersions configured.
+// applyNewerNoncurrentVersionLimit removes noncurrent versions older than the most recent NewerNoncurrentVersions configured.
 // Note: This function doesn't update sizeSummary since it always removes versions that it doesn't return.
-func (i *scannerItem) applyMaxNoncurrentVersionLimit(ctx context.Context, o ObjectLayer, fivs []FileInfo) ([]FileInfo, error) {
+func (i *scannerItem) applyNewerNoncurrentVersionLimit(ctx context.Context, _ ObjectLayer, fivs []FileInfo) ([]FileInfo, error) {
 	if i.lifeCycle == nil {
 		return fivs, nil
 	}

-	lim := i.lifeCycle.NoncurrentVersionsExpirationLimit(lifecycle.ObjectOpts{Name: i.objectPath()})
+	_, days, lim := i.lifeCycle.NoncurrentVersionsExpirationLimit(lifecycle.ObjectOpts{Name: i.objectPath()})
 	if lim == 0 || len(fivs) <= lim+1 { // fewer than lim _noncurrent_ versions
 		return fivs, nil
 	}
@ -992,6 +992,7 @@ func (i *scannerItem) applyMaxNoncurrentVersionLimit(ctx context.Context, o Obje
 	toDel := make([]ObjectToDelete, 0, len(overflowVersions))
 	for _, fi := range overflowVersions {
 		obj := fi.ToObjectInfo(i.bucket, i.objectPath())
+		// skip versions with object locking enabled
 		if rcfg.LockEnabled && enforceRetentionForDeletion(ctx, obj) {
 			if i.debug {
 				if obj.VersionID != "" {
@ -1000,22 +1001,34 @@ func (i *scannerItem) applyMaxNoncurrentVersionLimit(ctx context.Context, o Obje
 					console.Debugf(applyVersionActionsLogPrefix+" lifecycle: %s is locked, not deleting\n", obj.Name)
 				}
 			}
+			// add this version back to remaining versions for
+			// subsequent lifecycle policy applications
+			fivs = append(fivs, fi)
 			continue
 		}
+
+		// NoncurrentDays not passed yet.
+		if time.Now().UTC().Before(lifecycle.ExpectedExpiryTime(obj.SuccessorModTime, days)) {
+			// add this version back to remaining versions for
+			// subsequent lifecycle policy applications
+			fivs = append(fivs, fi)
+			continue
+		}
+
 		toDel = append(toDel, ObjectToDelete{
 			ObjectName: fi.Name,
 			VersionID:  fi.VersionID,
 		})
 	}

-	globalExpiryState.enqueueByMaxNoncurrent(i.bucket, toDel)
+	globalExpiryState.enqueueByNewerNoncurrent(i.bucket, toDel)
 	return fivs, nil
 }

 // applyVersionActions will apply lifecycle checks on all versions of a scanned item. Returns versions that remain
 // after applying lifecycle checks configured.
 func (i *scannerItem) applyVersionActions(ctx context.Context, o ObjectLayer, fivs []FileInfo) ([]FileInfo, error) {
-	return i.applyMaxNoncurrentVersionLimit(ctx, o, fivs)
+	return i.applyNewerNoncurrentVersionLimit(ctx, o, fivs)
 }

 // applyActions will apply lifecycle checks on to a scanned item.
--- a/internal/bucket/lifecycle/lifecycle.go
+++ b/internal/bucket/lifecycle/lifecycle.go
@ -33,7 +33,6 @@ var (
 	errLifecycleNoRule       = Errorf("Lifecycle configuration should have at least one rule")
 	errLifecycleDuplicateID  = Errorf("Lifecycle configuration has rule with the same ID. Rule ID must be unique.")
 	errXMLNotWellFormed      = Errorf("The XML you provided was not well-formed or did not validate against our published schema")
-	errLifecycleInvalidNoncurrentExpiration = Errorf("Exactly one of NoncurrentDays (positive integer) or MaxNoncurrentVersions should be specified in a NoncurrentExpiration rule.")
 )

 const (
@ -141,7 +140,7 @@ func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool {
 		if rule.NoncurrentVersionExpiration.NoncurrentDays > 0 {
 			return true
 		}
-		if rule.NoncurrentVersionExpiration.MaxNoncurrentVersions > 0 {
+		if rule.NoncurrentVersionExpiration.NewerNoncurrentVersions > 0 {
 			return true
 		}
 		if !rule.NoncurrentVersionTransition.IsNull() {
@ -150,13 +149,13 @@ func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool {
 		if rule.Expiration.IsNull() && rule.Transition.IsNull() {
 			continue
 		}
-		if !rule.Expiration.IsDateNull() && rule.Expiration.Date.Before(time.Now()) {
+		if !rule.Expiration.IsDateNull() && rule.Expiration.Date.Before(time.Now().UTC()) {
 			return true
 		}
 		if !rule.Expiration.IsDaysNull() {
 			return true
 		}
-		if !rule.Transition.IsDateNull() && rule.Transition.Date.Before(time.Now()) {
+		if !rule.Transition.IsDateNull() && rule.Transition.Date.Before(time.Now().UTC()) {
 			return true
 		}
 		if !rule.Transition.IsNull() { // this allows for Transition.Days to be zero.
@ -238,7 +237,7 @@ func (lc Lifecycle) FilterActionableRules(obj ObjectOpts) []Rule {
 			rules = append(rules, rule)
 			continue
 		}
-		if rule.NoncurrentVersionExpiration.MaxNoncurrentVersions > 0 {
+		if rule.NoncurrentVersionExpiration.NewerNoncurrentVersions > 0 {
 			rules = append(rules, rule)
 			continue
 		}
@ -304,17 +303,24 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 				// 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().After(ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days))) {
+				if time.Now().UTC().After(ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days))) {
 					return DeleteVersionAction
 				}
 			}
 		}

 		if !rule.NoncurrentVersionExpiration.IsDaysNull() {
+			// Skip rules with newer noncurrent versions specified.
+			// These rules are not handled at an individual 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() {
 				// 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().After(ExpectedExpiryTime(obj.SuccessorModTime, int(rule.NoncurrentVersionExpiration.NoncurrentDays))) {
+				if time.Now().UTC().After(ExpectedExpiryTime(obj.SuccessorModTime, int(rule.NoncurrentVersionExpiration.NoncurrentDays))) {
 					return DeleteVersionAction
 				}
 			}
@ -350,7 +356,7 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 					}
 				}

-				if !obj.RestoreExpires.IsZero() && time.Now().After(obj.RestoreExpires) {
+				if !obj.RestoreExpires.IsZero() && time.Now().UTC().After(obj.RestoreExpires) {
 					if obj.VersionID != "" {
 						action = DeleteRestoredVersionAction
 					} else {
@ -358,7 +364,7 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 					}
 				}
 			}
-			if !obj.RestoreExpires.IsZero() && time.Now().After(obj.RestoreExpires) {
+			if !obj.RestoreExpires.IsZero() && time.Now().UTC().After(obj.RestoreExpires) {
 				if obj.VersionID != "" {
 					action = DeleteRestoredVersionAction
 				} else {
@ -377,6 +383,9 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 //   e.g. If the object modtime is `Thu May 21 13:42:50 GMT 2020` and the object should
 //       transition in 1 day, then the expected transition time is `Fri, 23 May 2020 00:00:00 GMT`
 func ExpectedExpiryTime(modTime time.Time, days int) time.Time {
+	if days == 0 {
+		return modTime
+	}
 	t := modTime.UTC().Add(time.Duration(days+1) * 24 * time.Hour)
 	return t.Truncate(24 * time.Hour)
 }
@ -395,6 +404,11 @@ func (lc Lifecycle) PredictExpiryTime(obj ObjectOpts) (string, time.Time) {
 	// 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))
 		}
@ -477,31 +491,28 @@ func (lc Lifecycle) TransitionTier(obj ObjectOpts) string {
 	return ""
 }

-// NoncurrentVersionsExpirationLimit returns the minimum limit on number of
+// NoncurrentVersionsExpirationLimit returns the maximum limit on number of
 // noncurrent versions across rules.
-func (lc Lifecycle) NoncurrentVersionsExpirationLimit(obj ObjectOpts) int {
+func (lc Lifecycle) NoncurrentVersionsExpirationLimit(obj ObjectOpts) (string, int, int) {
 	var lim int
+	var days int
+	var ruleID string
 	for _, rule := range lc.FilterActionableRules(obj) {
-		if rule.NoncurrentVersionExpiration.MaxNoncurrentVersions == 0 {
+		if rule.NoncurrentVersionExpiration.NewerNoncurrentVersions == 0 {
 			continue
 		}
-		if lim == 0 || lim > rule.NoncurrentVersionExpiration.MaxNoncurrentVersions {
-			lim = rule.NoncurrentVersionExpiration.MaxNoncurrentVersions
+		// Pick the highest number of NewerNoncurrentVersions value
+		// 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 lim
-}
-
-// HasMaxNoncurrentVersions returns true if there exists a rule with
-// MaxNoncurrentVersions limit set.
-func (lc Lifecycle) HasMaxNoncurrentVersions() bool {
-	for _, rule := range lc.Rules {
-		if rule.Status == Disabled {
-			continue
-		}
-		if rule.NoncurrentVersionExpiration.MaxNoncurrentVersions > 0 {
-			return true
-		}
-	}
-	return false
+	return ruleID, days, lim
 }
--- a/internal/bucket/lifecycle/lifecycle_test.go
+++ b/internal/bucket/lifecycle/lifecycle_test.go
@ -114,7 +114,7 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) {
 		},
 		// Lifecycle with max noncurrent versions
 		{
-			inputConfig:           `<LifecycleConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Rule><ID>rule</ID>><Status>Enabled</Status><Filter></Filter><NoncurrentVersionExpiration><MaxNoncurrentVersions>5</MaxNoncurrentVersions></NoncurrentVersionExpiration></Rule></LifecycleConfiguration>`,
+			inputConfig:           `<LifecycleConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Rule><ID>rule</ID>><Status>Enabled</Status><Filter></Filter><NoncurrentVersionExpiration><NewerNoncurrentVersions>5</NewerNoncurrentVersions></NoncurrentVersionExpiration></Rule></LifecycleConfiguration>`,
 			expectedParsingErr:    nil,
 			expectedValidationErr: nil,
 		},
@ -414,6 +414,24 @@ func TestComputeActions(t *testing.T) {
 			objectSuccessorModTime: time.Now().Add(-1 * time.Nanosecond).UTC(),
 			versionID:              uuid.New().String(),
 		},
+		// Lifecycle rules with NewerNoncurrentVersions specified must return NoneAction.
+		{
+			inputConfig:    `<LifecycleConfiguration><Rule><Filter><Prefix>foodir/</Prefix></Filter><Status>Enabled</Status><NoncurrentVersionExpiration><NewerNoncurrentVersions>5</NewerNoncurrentVersions></NoncurrentVersionExpiration></Rule></LifecycleConfiguration>`,
+			objectName:     "foodir/fooobject",
+			versionID:      uuid.NewString(),
+			objectModTime:  time.Now().UTC().Add(-10 * 24 * time.Hour), // Created 10 days ago
+			expectedAction: NoneAction,
+		},
+		// Disabled rules with NewerNoncurrentVersions shouldn't affect outcome.
+		{
+			inputConfig:            `<LifecycleConfiguration><Rule><Filter><Prefix>foodir/</Prefix></Filter><Status>Enabled</Status><NoncurrentVersionExpiration><NoncurrentDays>5</NoncurrentDays></NoncurrentVersionExpiration></Rule><Rule><Filter><Prefix>foodir/</Prefix></Filter><Status>Disabled</Status><NoncurrentVersionExpiration><NewerNoncurrentVersions>5</NewerNoncurrentVersions></NoncurrentVersionExpiration></Rule></LifecycleConfiguration>`,
+			objectName:             "foodir/fooobject",
+			versionID:              uuid.NewString(),
+			objectModTime:          time.Now().UTC().Add(-10 * 24 * time.Hour), // Created 10 days ago
+			objectSuccessorModTime: time.Now().UTC().Add(-10 * 24 * time.Hour), // Created 10 days ago
+			isNoncurrent:           true,
+			expectedAction:         DeleteVersionAction,
+		},
 	}

 	for _, tc := range testCases {
@ -636,14 +654,55 @@ func TestNoncurrentVersionsLimit(t *testing.T) {
 			ID:     strconv.Itoa(i),
 			Status: "Enabled",
 			NoncurrentVersionExpiration: NoncurrentVersionExpiration{
-				MaxNoncurrentVersions: i,
+				NewerNoncurrentVersions: i,
+				NoncurrentDays:          ExpirationDays(i),
 			},
 		})
 	}
 	lc := Lifecycle{
 		Rules: rules,
 	}
-	if lim := lc.NoncurrentVersionsExpirationLimit(ObjectOpts{Name: "obj"}); lim != 1 {
-		t.Fatalf("Expected max noncurrent versions limit to be 1 but got %d", lim)
+	if ruleID, days, lim := lc.NoncurrentVersionsExpirationLimit(ObjectOpts{Name: "obj"}); ruleID != "1" || days != 1 || lim != 10 {
+		t.Fatalf("Expected (ruleID, days, lim) to be (\"1\", 1, 10) but got (%s, %d, %d)", ruleID, days, lim)
+	}
+}
+
+func TestMaxNoncurrentBackwardCompat(t *testing.T) {
+	testCases := []struct {
+		xml      string
+		expected NoncurrentVersionExpiration
+	}{
+		{
+			xml: `<NoncurrentVersionExpiration><NoncurrentDays>1</NoncurrentDays><NewerNoncurrentVersions>3</NewerNoncurrentVersions></NoncurrentVersionExpiration>`,
+			expected: NoncurrentVersionExpiration{
+				XMLName: xml.Name{
+					Local: "NoncurrentVersionExpiration",
+				},
+				NoncurrentDays:          1,
+				NewerNoncurrentVersions: 3,
+				set:                     true,
+			},
+		},
+		{
+			xml: `<NoncurrentVersionExpiration><NoncurrentDays>2</NoncurrentDays><MaxNoncurrentVersions>4</MaxNoncurrentVersions></NoncurrentVersionExpiration>`,
+			expected: NoncurrentVersionExpiration{
+				XMLName: xml.Name{
+					Local: "NoncurrentVersionExpiration",
+				},
+				NoncurrentDays:          2,
+				NewerNoncurrentVersions: 4,
+				set:                     true,
+			},
+		},
+	}
+	for i, tc := range testCases {
+		var got NoncurrentVersionExpiration
+		dec := xml.NewDecoder(strings.NewReader(tc.xml))
+		if err := dec.Decode(&got); err != nil || got != tc.expected {
+			if err != nil {
+				t.Fatalf("%d: Failed to unmarshal xml %v", i+1, err)
+			}
+			t.Fatalf("%d: Expected %v but got %v", i+1, tc.expected, got)
+		}
 	}
 }
--- a/internal/bucket/lifecycle/noncurrentversion.go
+++ b/internal/bucket/lifecycle/noncurrentversion.go
@ -26,7 +26,7 @@ import (
 type NoncurrentVersionExpiration struct {
 	XMLName                 xml.Name       `xml:"NoncurrentVersionExpiration"`
 	NoncurrentDays          ExpirationDays `xml:"NoncurrentDays,omitempty"`
-	MaxNoncurrentVersions int            `xml:"MaxNoncurrentVersions,omitempty"`
+	NewerNoncurrentVersions int            `xml:"NewerNoncurrentVersions,omitempty"`
 	set                     bool
 }

@ -41,20 +41,35 @@ func (n NoncurrentVersionExpiration) MarshalXML(e *xml.Encoder, start xml.StartE

 // UnmarshalXML decodes NoncurrentVersionExpiration
 func (n *NoncurrentVersionExpiration) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error {
-	type noncurrentVersionExpirationWrapper NoncurrentVersionExpiration
-	var val noncurrentVersionExpirationWrapper
+	// To handle xml with MaxNoncurrentVersions from older MinIO releases.
+	// note: only one of MaxNoncurrentVersions or NewerNoncurrentVersions would be present.
+	type noncurrentExpiration struct {
+		XMLName                 xml.Name       `xml:"NoncurrentVersionExpiration"`
+		NoncurrentDays          ExpirationDays `xml:"NoncurrentDays,omitempty"`
+		NewerNoncurrentVersions int            `xml:"NewerNoncurrentVersions,omitempty"`
+		MaxNoncurrentVersions   int            `xml:"MaxNoncurrentVersions,omitempty"`
+	}
+
+	var val noncurrentExpiration
 	err := d.DecodeElement(&val, &startElement)
 	if err != nil {
 		return err
 	}
-	*n = NoncurrentVersionExpiration(val)
+	if val.MaxNoncurrentVersions > 0 {
+		val.NewerNoncurrentVersions = val.MaxNoncurrentVersions
+	}
+	*n = NoncurrentVersionExpiration{
+		XMLName:                 val.XMLName,
+		NoncurrentDays:          val.NoncurrentDays,
+		NewerNoncurrentVersions: val.NewerNoncurrentVersions,
+	}
 	n.set = true
 	return nil
 }

 // IsNull returns if both NoncurrentDays and NoncurrentVersions are empty
 func (n NoncurrentVersionExpiration) IsNull() bool {
-	return n.IsDaysNull() && n.MaxNoncurrentVersions == 0
+	return n.IsDaysNull() && n.NewerNoncurrentVersions == 0
 }

 // IsDaysNull returns true if days field is null
@ -69,16 +84,13 @@ func (n NoncurrentVersionExpiration) Validate() error {
 	}
 	val := int(n.NoncurrentDays)
 	switch {
-	case val == 0 && n.MaxNoncurrentVersions == 0:
+	case val == 0 && n.NewerNoncurrentVersions == 0:
 		// both fields can't be zero
 		return errXMLNotWellFormed

-	case val > 0 && n.MaxNoncurrentVersions > 0:
-		// both tags can't be non-zero simultaneously
-		return errLifecycleInvalidNoncurrentExpiration
-
-	case val < 0, n.MaxNoncurrentVersions < 0:
+	case val < 0, n.NewerNoncurrentVersions < 0:
 		// negative values are not supported
+		return errXMLNotWellFormed
 	}
 	return nil
 }
--- a/internal/bucket/lifecycle/noncurrentversion_test.go
+++ b/internal/bucket/lifecycle/noncurrentversion_test.go
@ -0,0 +1,82 @@
+// Copyright (c) 2015-2021 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package lifecycle
+
+import "testing"
+
+func Test_NoncurrentVersionsExpiration_Validation(t *testing.T) {
+	testcases := []struct {
+		n   NoncurrentVersionExpiration
+		err error
+	}{
+		{
+			n: NoncurrentVersionExpiration{
+				NoncurrentDays:          0,
+				NewerNoncurrentVersions: 0,
+				set:                     true,
+			},
+			err: errXMLNotWellFormed,
+		},
+		{
+			n: NoncurrentVersionExpiration{
+				NoncurrentDays:          90,
+				NewerNoncurrentVersions: 0,
+				set:                     true,
+			},
+			err: nil,
+		},
+		{
+			n: NoncurrentVersionExpiration{
+				NoncurrentDays:          90,
+				NewerNoncurrentVersions: 2,
+				set:                     true,
+			},
+			err: nil,
+		},
+		{
+			n: NoncurrentVersionExpiration{
+				NoncurrentDays: -1,
+				set:            true,
+			},
+			err: errXMLNotWellFormed,
+		},
+		{
+			n: NoncurrentVersionExpiration{
+				NoncurrentDays:          90,
+				NewerNoncurrentVersions: -2,
+				set:                     true,
+			},
+			err: errXMLNotWellFormed,
+		},
+		// MinIO extension: supports zero NoncurrentDays when NewerNoncurrentVersions > 0
+		{
+			n: NoncurrentVersionExpiration{
+				NoncurrentDays:          0,
+				NewerNoncurrentVersions: 5,
+				set:                     true,
+			},
+			err: nil,
+		},
+	}
+
+	for i, tc := range testcases {
+		if got := tc.n.Validate(); got != tc.err {
+			t.Fatalf("%d: expected %v but got %v", i+1, tc.err, got)
+		}
+	}
+}