From 44a9339c0ac2d43b51cf78cb701744f033ea032e Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Tue, 14 Dec 2021 09:41:44 -0800 Subject: [PATCH] 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 --- cmd/bucket-lifecycle-handlers.go | 12 --- cmd/bucket-lifecycle.go | 30 +++---- cmd/data-scanner.go | 23 ++++-- internal/bucket/lifecycle/lifecycle.go | 77 +++++++++-------- internal/bucket/lifecycle/lifecycle_test.go | 67 ++++++++++++++- .../bucket/lifecycle/noncurrentversion.go | 40 +++++---- .../lifecycle/noncurrentversion_test.go | 82 +++++++++++++++++++ 7 files changed, 248 insertions(+), 83 deletions(-) create mode 100644 internal/bucket/lifecycle/noncurrentversion_test.go diff --git a/cmd/bucket-lifecycle-handlers.go b/cmd/bucket-lifecycle-handlers.go index bcc32cc96..a1673324b 100644 --- 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) diff --git a/cmd/bucket-lifecycle.go b/cmd/bucket-lifecycle.go index 4e4e13c8a..d6eee67ea 100644 --- a/cmd/bucket-lifecycle.go +++ b/cmd/bucket-lifecycle.go @@ -81,21 +81,21 @@ type expiryTask struct { } type expiryState struct { - once sync.Once - byDaysCh chan expiryTask - byMaxNoncurrentCh chan maxNoncurrentTask + once sync.Once + byDaysCh chan expiryTask + 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: } } @@ -124,8 +124,8 @@ var globalExpiryState *expiryState func newExpiryState() *expiryState { return &expiryState{ - byDaysCh: make(chan expiryTask, 10000), - byMaxNoncurrentCh: make(chan maxNoncurrentTask, 10000), + byDaysCh: make(chan expiryTask, 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 } diff --git a/cmd/data-scanner.go b/cmd/data-scanner.go index 519fb5228..e77539b1a 100644 --- 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. diff --git a/internal/bucket/lifecycle/lifecycle.go b/internal/bucket/lifecycle/lifecycle.go index c622430f3..02bd93140 100644 --- a/internal/bucket/lifecycle/lifecycle.go +++ b/internal/bucket/lifecycle/lifecycle.go @@ -29,11 +29,10 @@ import ( ) var ( - errLifecycleTooManyRules = Errorf("Lifecycle configuration allows a maximum of 1000 rules") - 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.") + errLifecycleTooManyRules = Errorf("Lifecycle configuration allows a maximum of 1000 rules") + 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") ) 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 } diff --git a/internal/bucket/lifecycle/lifecycle_test.go b/internal/bucket/lifecycle/lifecycle_test.go index 6b4133ff3..9519e645a 100644 --- 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: `rule>Enabled5`, + inputConfig: `rule>Enabled5`, 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: `foodir/Enabled5`, + 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: `foodir/Enabled5foodir/Disabled5`, + 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: `13`, + expected: NoncurrentVersionExpiration{ + XMLName: xml.Name{ + Local: "NoncurrentVersionExpiration", + }, + NoncurrentDays: 1, + NewerNoncurrentVersions: 3, + set: true, + }, + }, + { + xml: `24`, + 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) + } } } diff --git a/internal/bucket/lifecycle/noncurrentversion.go b/internal/bucket/lifecycle/noncurrentversion.go index cf9ff6a04..23d03c845 100644 --- a/internal/bucket/lifecycle/noncurrentversion.go +++ b/internal/bucket/lifecycle/noncurrentversion.go @@ -24,10 +24,10 @@ import ( // NoncurrentVersionExpiration - an action for lifecycle configuration rule. type NoncurrentVersionExpiration struct { - XMLName xml.Name `xml:"NoncurrentVersionExpiration"` - NoncurrentDays ExpirationDays `xml:"NoncurrentDays,omitempty"` - MaxNoncurrentVersions int `xml:"MaxNoncurrentVersions,omitempty"` - set bool + XMLName xml.Name `xml:"NoncurrentVersionExpiration"` + NoncurrentDays ExpirationDays `xml:"NoncurrentDays,omitempty"` + NewerNoncurrentVersions int `xml:"NewerNoncurrentVersions,omitempty"` + set bool } // MarshalXML if non-current days not set to non zero value @@ -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 } diff --git a/internal/bucket/lifecycle/noncurrentversion_test.go b/internal/bucket/lifecycle/noncurrentversion_test.go new file mode 100644 index 000000000..fc6d1b343 --- /dev/null +++ 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 . + +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) + } + } +}