Skip to content

Commit

Permalink
ilm: Handle DeleteAllVersions action differently for DEL markers (#19481
Browse files Browse the repository at this point in the history
)

i.e., this rule element doesn't apply to DEL markers.

This is a breaking change to how ExpiredObejctDeleteAllVersions
functions today. This is necessary to avoid the following highly probable
footgun scenario in the future.

Scenario:
The user uses tags-based filtering to select an object's time to live(TTL). 
The application sometimes deletes objects, too, making its latest
version a DEL marker. The previous implementation skipped tag-based filters
if the newest version was DEL marker, voiding the tag-based TTL. The user is
surprised to find objects that have expired sooner than expected.

* Add DelMarkerExpiration action

This ILM action removes all versions of an object if its
the latest version is a DEL marker.

```xml
<DelMarkerObjectExpiration>
    <Days> 10 </Days>
</DelMarkerObjectExpiration>
```

1. Applies only to objects whose,
  • The latest version is a DEL marker.
  • satisfies the number of days criteria
2. Deletes all versions of this object
3. Associated rule can't have tag-based filtering

Includes,
- New bucket event type for deletion due to DelMarkerExpiration
  • Loading branch information
krisis committed May 1, 2024
1 parent 8161411 commit 7926401
Show file tree
Hide file tree
Showing 11 changed files with 471 additions and 89 deletions.
13 changes: 8 additions & 5 deletions cmd/data-scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ func (i *scannerItem) applyLifecycle(ctx context.Context, o ObjectLayer, oi Obje
// This can happen when,
// - ExpireObjectAllVersions flag is enabled
// - NoncurrentVersionExpiration is applicable
case lifecycle.DeleteVersionAction, lifecycle.DeleteAllVersionsAction:
case lifecycle.DeleteVersionAction, lifecycle.DeleteAllVersionsAction, lifecycle.DelMarkerDeleteAllVersionsAction:
size = 0
case lifecycle.DeleteAction:
// On a non-versioned bucket, DeleteObject removes the only version permanently.
Expand Down Expand Up @@ -1162,7 +1162,7 @@ func (i *scannerItem) applyActions(ctx context.Context, o ObjectLayer, oi Object

// Note: objDeleted is true if and only if action ==
// lifecycle.DeleteAllVersionsAction
if action == lifecycle.DeleteAllVersionsAction {
if action.DeleteAll() {
return true, 0
}

Expand Down Expand Up @@ -1292,7 +1292,7 @@ func applyExpiryOnNonTransitionedObjects(ctx context.Context, objLayer ObjectLay

if lcEvent.Action != lifecycle.NoneAction {
numVersions := uint64(1)
if lcEvent.Action == lifecycle.DeleteAllVersionsAction {
if lcEvent.Action.DeleteAll() {
numVersions = uint64(obj.NumVersions)
}
globalScannerMetrics.timeILM(lcEvent.Action)(numVersions)
Expand Down Expand Up @@ -1320,8 +1320,11 @@ func applyExpiryOnNonTransitionedObjects(ctx context.Context, objLayer ObjectLay
if obj.DeleteMarker {
eventName = event.ObjectRemovedDeleteMarkerCreated
}
if lcEvent.Action.DeleteAll() {
switch lcEvent.Action {
case lifecycle.DeleteAllVersionsAction:
eventName = event.ObjectRemovedDeleteAllVersions
case lifecycle.DelMarkerDeleteAllVersionsAction:
eventName = event.ILMDelMarkerExpirationDelete
}
// Notify object deleted event.
sendEvent(eventArgs{
Expand All @@ -1346,7 +1349,7 @@ func applyLifecycleAction(event lifecycle.Event, src lcEventSrc, obj ObjectInfo)
switch action := event.Action; action {
case lifecycle.DeleteVersionAction, lifecycle.DeleteAction,
lifecycle.DeleteRestoredAction, lifecycle.DeleteRestoredVersionAction,
lifecycle.DeleteAllVersionsAction:
lifecycle.DeleteAllVersionsAction, lifecycle.DelMarkerDeleteAllVersionsAction:
success = applyExpiryRule(event, src, obj)
case lifecycle.TransitionAction, lifecycle.TransitionVersionAction:
success = applyTransitionRule(event, src, obj)
Expand Down
9 changes: 5 additions & 4 deletions cmd/erasure-object.go
Original file line number Diff line number Diff line change
Expand Up @@ -1886,12 +1886,13 @@ func (er erasureObjects) DeleteObject(ctx context.Context, bucket, object string
// based on the latest objectInfo and see if the object still
// qualifies for deletion.
if gerr == nil {
evt := evalActionFromLifecycle(ctx, *lc, rcfg, replcfg, goi)
var isErr bool
evt := evalActionFromLifecycle(ctx, *lc, rcfg, replcfg, goi)
switch evt.Action {
case lifecycle.NoneAction:
isErr = true
case lifecycle.TransitionAction, lifecycle.TransitionVersionAction:
case lifecycle.DeleteAllVersionsAction, lifecycle.DelMarkerDeleteAllVersionsAction:
// opts.DeletePrefix is used only in the above lifecycle Expiration actions.
default:
// object has been modified since lifecycle action was previously evaluated
isErr = true
}
if isErr {
Expand Down
7 changes: 4 additions & 3 deletions internal/bucket/lifecycle/action_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

74 changes: 74 additions & 0 deletions internal/bucket/lifecycle/delmarker-expiration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// Copyright (c) 2024 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 (
"encoding/xml"
"time"
)

var errInvalidDaysDelMarkerExpiration = Errorf("Days must be a positive integer with DelMarkerExpiration")

// DelMarkerExpiration used to xml encode/decode ILM action by the same name
type DelMarkerExpiration struct {
XMLName xml.Name `xml:"DelMarkerExpiration"`
Days int `xml:"Days,omitempty"`
}

// Empty returns if a DelMarkerExpiration XML element is empty.
// Used to detect if lifecycle.Rule contained a DelMarkerExpiration element.
func (de DelMarkerExpiration) Empty() bool {
return de.Days == 0
}

// UnmarshalXML decodes a single XML element into a DelMarkerExpiration value
func (de *DelMarkerExpiration) UnmarshalXML(dec *xml.Decoder, start xml.StartElement) error {
type delMarkerExpiration DelMarkerExpiration
var dexp delMarkerExpiration
err := dec.DecodeElement(&dexp, &start)
if err != nil {
return err
}

if dexp.Days <= 0 {
return errInvalidDaysDelMarkerExpiration
}

*de = DelMarkerExpiration(dexp)
return nil
}

// MarshalXML encodes a DelMarkerExpiration value into an XML element
func (de DelMarkerExpiration) MarshalXML(enc *xml.Encoder, start xml.StartElement) error {
if de.Empty() {
return nil
}

type delMarkerExpiration DelMarkerExpiration
return enc.EncodeElement(delMarkerExpiration(de), start)
}

// NextDue returns upcoming DelMarkerExpiration date for obj if
// applicable, returns false otherwise.
func (de DelMarkerExpiration) NextDue(obj ObjectOpts) (time.Time, bool) {
if !obj.IsLatest || !obj.DeleteMarker {
return time.Time{}, false
}

return ExpectedExpiryTime(obj.ModTime, de.Days), true
}
63 changes: 63 additions & 0 deletions internal/bucket/lifecycle/delmarker-expiration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright (c) 2024 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 (
"encoding/xml"
"fmt"
"testing"
)

func TestDelMarkerExpParseAndValidate(t *testing.T) {
tests := []struct {
xml string
err error
}{
{
xml: `<DelMarkerExpiration> <Days> 1 </Days> </DelMarkerExpiration>`,
err: nil,
},
{
xml: `<DelMarkerExpiration> <Days> -1 </Days> </DelMarkerExpiration>`,
err: errInvalidDaysDelMarkerExpiration,
},
}

for i, test := range tests {
t.Run(fmt.Sprintf("TestDelMarker-%d", i), func(t *testing.T) {
var dexp DelMarkerExpiration
var fail bool
err := xml.Unmarshal([]byte(test.xml), &dexp)
if test.err == nil {
if err != nil {
fail = true
}
} else {
if err == nil {
fail = true
}
if test.err.Error() != err.Error() {
fail = true
}
}
if fail {
t.Fatalf("Expected %v but got %v", test.err, err)
}
})
}
}
82 changes: 46 additions & 36 deletions internal/bucket/lifecycle/lifecycle.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2015-2021 MinIO, Inc.
// Copyright (c) 2015-2024 MinIO, Inc.
//
// This file is part of MinIO Object Storage stack
//
Expand All @@ -22,7 +22,7 @@ import (
"fmt"
"io"
"net/http"
"sort"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -67,7 +67,8 @@ const (
DeleteRestoredVersionAction
// DeleteAllVersionsAction deletes all versions when an object expires
DeleteAllVersionsAction

// DelMarkerDeleteAllVersionsAction deletes all versions when an object with delete marker as latest version expires
DelMarkerDeleteAllVersionsAction
// ActionCount must be the last action and shouldn't be used as a regular action.
ActionCount
)
Expand All @@ -84,15 +85,15 @@ func (a Action) DeleteVersioned() bool {

// DeleteAll - Returns true if the action demands deleting all versions of an object
func (a Action) DeleteAll() bool {
return a == DeleteAllVersionsAction
return a == DeleteAllVersionsAction || a == DelMarkerDeleteAllVersionsAction
}

// Delete - Returns true if action demands delete on all objects (including restored)
func (a Action) Delete() bool {
if a.DeleteRestored() {
return true
}
return a == DeleteVersionAction || a == DeleteAction || a == DeleteAllVersionsAction
return a == DeleteVersionAction || a == DeleteAction || a == DeleteAllVersionsAction || a == DelMarkerDeleteAllVersionsAction
}

// Lifecycle - Configuration for bucket lifecycle.
Expand Down Expand Up @@ -279,7 +280,7 @@ func (lc Lifecycle) FilterRules(obj ObjectOpts) []Rule {
if !strings.HasPrefix(obj.Name, rule.GetPrefix()) {
continue
}
if !obj.DeleteMarker && !rule.Filter.TestTags(obj.UserTags) {
if !rule.Filter.TestTags(obj.UserTags) {
continue
}
if !obj.DeleteMarker && !rule.Filter.BySize(obj.Size) {
Expand Down Expand Up @@ -353,23 +354,6 @@ func (lc Lifecycle) eval(obj ObjectOpts, now time.Time) Event {
}

for _, rule := range lc.FilterRules(obj) {
if obj.IsLatest && rule.Expiration.DeleteAll.val {
if !rule.Expiration.IsDaysNull() {
// Specifying the Days tag will automatically perform all versions cleanup
// once the latest object is old enough to satisfy the age criteria.
// This is a MinIO only extension.
if expectedExpiry := ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days)); now.IsZero() || now.After(expectedExpiry) {
events = append(events, Event{
Action: DeleteAllVersionsAction,
RuleID: rule.ID,
Due: expectedExpiry,
})
// No other conflicting actions apply to an all version expired object.
break
}
}
}

if obj.ExpiredObjectDeleteMarker() {
if rule.Expiration.DeleteMarker.val {
// Indicates whether MinIO will remove a delete marker with no noncurrent versions.
Expand Down Expand Up @@ -401,6 +385,21 @@ func (lc Lifecycle) eval(obj ObjectOpts, now time.Time) Event {
}
}

// DelMarkerExpiration
if obj.IsLatest && obj.DeleteMarker && !rule.DelMarkerExpiration.Empty() {
if due, ok := rule.DelMarkerExpiration.NextDue(obj); ok && (now.IsZero() || now.After(due)) {
events = append(events, Event{
Action: DelMarkerDeleteAllVersionsAction,
RuleID: rule.ID,
Due: due,
})
}
// No other conflicting actions in this rule can apply to an object with current version as DEL marker
// Note: There could be other rules with earlier expiration which need to be considered.
// See TestDelMarkerExpiration
continue
}

// Skip rules with newer noncurrent versions specified. These rules are
// not handled at an individual version level. eval applies only to a
// specific version.
Expand Down Expand Up @@ -448,11 +447,17 @@ func (lc Lifecycle) eval(obj ObjectOpts, now time.Time) Event {
}
case !rule.Expiration.IsDaysNull():
if expectedExpiry := ExpectedExpiryTime(obj.ModTime, int(rule.Expiration.Days)); now.IsZero() || now.After(expectedExpiry) {
events = append(events, Event{
event := Event{
Action: DeleteAction,
RuleID: rule.ID,
Due: expectedExpiry,
})
}
if rule.Expiration.DeleteAll.val {
// Expires all versions of this object once the latest object is old enough.
// This is a MinIO only extension.
event.Action = DeleteAllVersionsAction
}
events = append(events, event)
}
}

Expand All @@ -470,25 +475,30 @@ func (lc Lifecycle) eval(obj ObjectOpts, now time.Time) Event {
}

if len(events) > 0 {
sort.Slice(events, func(i, j int) bool {
slices.SortFunc(events, func(a, b Event) int {
// Prefer Expiration over Transition for both current
// and noncurrent versions when,
// - now is past the expected time to action
// - expected time to action is the same for both actions
if now.After(events[i].Due) && now.After(events[j].Due) || events[i].Due.Equal(events[j].Due) {
switch events[i].Action {
case DeleteAction, DeleteVersionAction:
return true
if now.After(a.Due) && now.After(b.Due) || a.Due.Equal(b.Due) {
switch a.Action {
case DeleteAllVersionsAction, DelMarkerDeleteAllVersionsAction,
DeleteAction, DeleteVersionAction:
return -1
}
switch events[j].Action {
case DeleteAction, DeleteVersionAction:
return false
switch b.Action {
case DeleteAllVersionsAction, DelMarkerDeleteAllVersionsAction,
DeleteAction, DeleteVersionAction:
return 1
}
return true
return -1
}

// Prefer earlier occurring event
return events[i].Due.Before(events[j].Due)
if a.Due.Before(b.Due) {
return -1
}
return 1
})
return events[0]
}
Expand Down Expand Up @@ -517,7 +527,7 @@ func ExpectedExpiryTime(modTime time.Time, days int) time.Time {
func (lc Lifecycle) SetPredictionHeaders(w http.ResponseWriter, obj ObjectOpts) {
event := lc.eval(obj, time.Time{})
switch event.Action {
case DeleteAction, DeleteVersionAction, DeleteAllVersionsAction:
case DeleteAction, DeleteVersionAction, DeleteAllVersionsAction, DelMarkerDeleteAllVersionsAction:
w.Header()[xhttp.AmzExpiration] = []string{
fmt.Sprintf(`expiry-date="%s", rule-id="%s"`, event.Due.Format(http.TimeFormat), event.RuleID),
}
Expand Down

0 comments on commit 7926401

Please sign in to comment.