From e906b511e9f427d14ea71494c4d23d7ef76b8121 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Fri, 5 Jun 2020 18:30:10 +0100 Subject: [PATCH] lifecycle: Consider multiple tags filtering (#9775) * lifecycle: Consider multiple tags filtering * lifecycle: Disallow duplicated Key/Value xml in --- pkg/bucket/lifecycle/filter.go | 30 +++++++++++++++++ pkg/bucket/lifecycle/lifecycle.go | 15 ++++----- pkg/bucket/lifecycle/lifecycle_test.go | 30 +++++++++++++++-- pkg/bucket/lifecycle/tag.go | 45 ++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 11 deletions(-) diff --git a/pkg/bucket/lifecycle/filter.go b/pkg/bucket/lifecycle/filter.go index 6191d5ee3..e93458723 100644 --- a/pkg/bucket/lifecycle/filter.go +++ b/pkg/bucket/lifecycle/filter.go @@ -30,6 +30,9 @@ type Filter struct { Prefix string And And Tag Tag + + // Caching tags, only once + cachedTags []string } // MarshalXML - produces the xml representation of the Filter struct @@ -84,3 +87,30 @@ func (f Filter) Validate() error { } return nil } + +// TestTags tests if the object tags satisfy the Filter tags requirement, +// it returns true if there is no tags in the underlying Filter. +func (f Filter) TestTags(tags []string) bool { + if f.cachedTags == nil { + tags := make([]string, 0) + for _, t := range append(f.And.Tags, f.Tag) { + if !t.IsEmpty() { + tags = append(tags, t.String()) + } + } + f.cachedTags = tags + } + for _, ct := range f.cachedTags { + foundTag := false + for _, t := range tags { + if ct == t { + foundTag = true + break + } + } + if !foundTag { + return false + } + } + return true +} diff --git a/pkg/bucket/lifecycle/lifecycle.go b/pkg/bucket/lifecycle/lifecycle.go index f7f0af66d..925b4ebcb 100644 --- a/pkg/bucket/lifecycle/lifecycle.go +++ b/pkg/bucket/lifecycle/lifecycle.go @@ -104,15 +104,12 @@ func (lc Lifecycle) FilterActionableRules(objName, objTags string) []Rule { if rule.Status == Disabled { continue } - if strings.HasPrefix(objName, rule.Prefix()) { - tags := rule.Tags() - if tags != "" { - if strings.Contains(objTags, tags) { - rules = append(rules, rule) - } - } else { - rules = append(rules, rule) - } + if !strings.HasPrefix(objName, rule.Prefix()) { + continue + } + tags := strings.Split(objTags, "&") + if rule.Filter.TestTags(tags) { + rules = append(rules, rule) } } return rules diff --git a/pkg/bucket/lifecycle/lifecycle_test.go b/pkg/bucket/lifecycle/lifecycle_test.go index 367075d3e..35b243841 100644 --- a/pkg/bucket/lifecycle/lifecycle_test.go +++ b/pkg/bucket/lifecycle/lifecycle_test.go @@ -88,6 +88,18 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) { expectedParsingErr: nil, expectedValidationErr: nil, }, + { // Valid lifecycle config + inputConfig: ` + + + key1val1key2val2 + + 3 + + `, + expectedParsingErr: errDuplicatedXMLTag, + expectedValidationErr: nil, + }, { // lifecycle config with no rules inputConfig: ` `, @@ -112,6 +124,11 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) { if err != tc.expectedParsingErr { t.Fatalf("%d: Expected %v during parsing but got %v", i+1, tc.expectedParsingErr, err) } + if tc.expectedParsingErr != nil { + // We already expect a parsing error, + // no need to continue this test. + return + } err = lc.Validate() if err != tc.expectedValidationErr { t.Fatalf("%d: Expected %v during parsing but got %v", i+1, tc.expectedValidationErr, err) @@ -268,7 +285,7 @@ func TestComputeActions(t *testing.T) { }, // Should remove (Multiple Rules, Tags match) { - inputConfig: `foodir/tag1value1tag2value2Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + `abc/tag2valueEnabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + inputConfig: `foodir/tag1value1tag2value2Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + `abc/tag2valueEnabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, objectName: "foodir/fooobject", objectTags: "tag1=value1&tag2=value2", objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago @@ -276,12 +293,21 @@ func TestComputeActions(t *testing.T) { }, // Should remove (Tags match) { - inputConfig: `foodir/tag1value1tag2value2Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + inputConfig: `foodir/tag1value1tag2value2Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, objectName: "foodir/fooobject", objectTags: "tag1=value1&tag2=value2", objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago expectedAction: DeleteAction, }, + // Should remove (Tags match with inverted order) + { + inputConfig: `factorytruestoreforeverfalseEnabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, + objectName: "fooobject", + objectTags: "storeforever=false&factory=true", + objectModTime: time.Now().UTC().Add(-24 * time.Hour), // Created 1 day ago + expectedAction: DeleteAction, + }, + // Should not remove (Tags don't match) { inputConfig: `foodir/tagvalue1Enabled` + time.Now().Truncate(24*time.Hour).UTC().Add(-24*time.Hour).Format(time.RFC3339) + ``, diff --git a/pkg/bucket/lifecycle/tag.go b/pkg/bucket/lifecycle/tag.go index ada5a4f4f..d183e0224 100644 --- a/pkg/bucket/lifecycle/tag.go +++ b/pkg/bucket/lifecycle/tag.go @@ -18,6 +18,7 @@ package lifecycle import ( "encoding/xml" + "io" "unicode/utf8" ) @@ -31,8 +32,52 @@ type Tag struct { var ( errInvalidTagKey = Errorf("The TagKey you have provided is invalid") errInvalidTagValue = Errorf("The TagValue you have provided is invalid") + + errDuplicatedXMLTag = Errorf("duplicated XML Tag") + errUnknownXMLTag = Errorf("unknown XML Tag") ) +// UnmarshalXML - decodes XML data. +func (tag *Tag) UnmarshalXML(d *xml.Decoder, start xml.StartElement) (err error) { + var keyAlreadyParsed, valueAlreadyParsed bool + for { + // Read tokens from the XML document in a stream. + t, err := d.Token() + if err != nil { + if err == io.EOF { + break + } + return err + } + + switch se := t.(type) { + case xml.StartElement: + var s string + if err = d.DecodeElement(&s, &se); err != nil { + return err + } + switch se.Name.Local { + case "Key": + if keyAlreadyParsed { + return errDuplicatedXMLTag + } + tag.Key = s + keyAlreadyParsed = true + case "Value": + if valueAlreadyParsed { + return errDuplicatedXMLTag + } + tag.Value = s + valueAlreadyParsed = true + default: + return errUnknownXMLTag + } + } + } + + return nil +} + func (tag Tag) String() string { return tag.Key + "=" + tag.Value }