diff --git a/pkg/bucket/lifecycle/and.go b/pkg/bucket/lifecycle/and.go index c1ef17ca2..36f7cd833 100644 --- a/pkg/bucket/lifecycle/and.go +++ b/pkg/bucket/lifecycle/and.go @@ -20,22 +20,33 @@ import ( "encoding/xml" ) +var errDuplicateTagKey = Errorf("Duplicate Tag Keys are not allowed") + // And - a tag to combine a prefix and multiple tags for lifecycle configuration rule. type And struct { XMLName xml.Name `xml:"And"` - Prefix string `xml:"Prefix,omitempty"` + Prefix Prefix `xml:"Prefix,omitempty"` Tags []Tag `xml:"Tag,omitempty"` } -var errDuplicateTagKey = Errorf("Duplicate Tag Keys are not allowed") - // isEmpty returns true if Tags field is null func (a And) isEmpty() bool { - return len(a.Tags) == 0 && a.Prefix == "" + return len(a.Tags) == 0 && !a.Prefix.set } // Validate - validates the And field func (a And) Validate() error { + emptyPrefix := !a.Prefix.set + emptyTags := len(a.Tags) == 0 + + if emptyPrefix && emptyTags { + return nil + } + + if emptyPrefix && !emptyTags || !emptyPrefix && emptyTags { + return errXMLNotWellFormed + } + if a.ContainsDuplicateTag() { return errDuplicateTagKey } diff --git a/pkg/bucket/lifecycle/filter.go b/pkg/bucket/lifecycle/filter.go index e93458723..fe1d1f661 100644 --- a/pkg/bucket/lifecycle/filter.go +++ b/pkg/bucket/lifecycle/filter.go @@ -18,6 +18,7 @@ package lifecycle import ( "encoding/xml" + "io" ) var ( @@ -27,10 +28,14 @@ var ( // Filter - a filter for a lifecycle configuration Rule. type Filter struct { XMLName xml.Name `xml:"Filter"` - Prefix string - And And - Tag Tag + Prefix Prefix + + And And + andSet bool + + Tag Tag + tagSet bool // Caching tags, only once cachedTags []string } @@ -61,11 +66,62 @@ func (f Filter) MarshalXML(e *xml.Encoder, start xml.StartElement) error { return e.EncodeToken(xml.EndElement{Name: start.Name}) } +// UnmarshalXML - decodes XML data. +func (f *Filter) UnmarshalXML(d *xml.Decoder, start xml.StartElement) (err error) { + 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: + switch se.Name.Local { + case "Prefix": + var p Prefix + if err = d.DecodeElement(&p, &se); err != nil { + return err + } + f.Prefix = p + case "And": + var and And + if err = d.DecodeElement(&and, &se); err != nil { + return err + } + f.And = and + f.andSet = true + case "Tag": + var tag Tag + if err = d.DecodeElement(&tag, &se); err != nil { + return err + } + f.Tag = tag + f.tagSet = true + default: + return errUnknownXMLTag + } + } + } + return nil +} + +// IsEmpty returns true if Filter is not specified in the XML +func (f Filter) IsEmpty() bool { + return !f.Prefix.set && !f.andSet && !f.tagSet +} + // Validate - validates the filter element func (f Filter) Validate() error { + if !f.Prefix.set && !f.andSet && !f.tagSet { + return errXMLNotWellFormed + } // A Filter must have exactly one of Prefix, Tag, or And specified. if !f.And.isEmpty() { - if f.Prefix != "" { + if f.Prefix.set { return errInvalidFilter } if !f.Tag.IsEmpty() { @@ -75,12 +131,15 @@ func (f Filter) Validate() error { return err } } - if f.Prefix != "" { + if f.Prefix.set { if !f.Tag.IsEmpty() { return errInvalidFilter } } if !f.Tag.IsEmpty() { + if f.Prefix.set { + return errInvalidFilter + } if err := f.Tag.Validate(); err != nil { return err } diff --git a/pkg/bucket/lifecycle/filter_test.go b/pkg/bucket/lifecycle/filter_test.go index a398fce3d..d9cd5959e 100644 --- a/pkg/bucket/lifecycle/filter_test.go +++ b/pkg/bucket/lifecycle/filter_test.go @@ -35,7 +35,7 @@ func TestUnsupportedFilters(t *testing.T) { key-prefix `, - expectedErr: nil, + expectedErr: errXMLNotWellFormed, }, { // Filter with Tag tags inputXML: ` @@ -85,6 +85,7 @@ func TestUnsupportedFilters(t *testing.T) { { // Filter with And and multiple Tag tags inputXML: ` + key1 value1 diff --git a/pkg/bucket/lifecycle/lifecycle.go b/pkg/bucket/lifecycle/lifecycle.go index c9fbad14f..eef08bd21 100644 --- a/pkg/bucket/lifecycle/lifecycle.go +++ b/pkg/bucket/lifecycle/lifecycle.go @@ -79,16 +79,16 @@ func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool { continue } - if len(prefix) > 0 && len(rule.Filter.Prefix) > 0 { + if len(prefix) > 0 && len(rule.GetPrefix()) > 0 { if !recursive { // If not recursive, incoming prefix must be in rule prefix - if !strings.HasPrefix(prefix, rule.Filter.Prefix) { + if !strings.HasPrefix(prefix, rule.GetPrefix()) { continue } } if recursive { // If recursive, we can skip this rule if it doesn't match the tested prefix. - if !strings.HasPrefix(prefix, rule.Filter.Prefix) && !strings.HasPrefix(rule.Filter.Prefix, prefix) { + if !strings.HasPrefix(prefix, rule.GetPrefix()) && !strings.HasPrefix(rule.GetPrefix(), prefix) { continue } } @@ -167,7 +167,7 @@ func (lc Lifecycle) FilterActionableRules(obj ObjectOpts) []Rule { if rule.Status == Disabled { continue } - if !strings.HasPrefix(obj.Name, rule.Prefix()) { + if !strings.HasPrefix(obj.Name, rule.GetPrefix()) { continue } // Indicates whether MinIO will remove a delete marker with no diff --git a/pkg/bucket/lifecycle/lifecycle_test.go b/pkg/bucket/lifecycle/lifecycle_test.go index e4ad92dd5..101336d98 100644 --- a/pkg/bucket/lifecycle/lifecycle_test.go +++ b/pkg/bucket/lifecycle/lifecycle_test.go @@ -25,72 +25,6 @@ import ( ) func TestParseAndValidateLifecycleConfig(t *testing.T) { - // Test for lifecycle config with more than 1000 rules - var manyRules []Rule - for i := 0; i < 1001; i++ { - rule := Rule{ - ID: fmt.Sprintf("toManyRule%d", i), - Status: "Enabled", - Expiration: Expiration{Days: ExpirationDays(i)}, - } - manyRules = append(manyRules, rule) - } - - manyRuleLcConfig, err := xml.Marshal(Lifecycle{Rules: manyRules}) - if err != nil { - t.Fatal("Failed to marshal lifecycle config with more than 1000 rules") - } - - // Test for lifecycle config with rules containing overlapping prefixes - rule1 := Rule{ - ID: "rule1", - Status: "Enabled", - Expiration: Expiration{Days: ExpirationDays(3)}, - Filter: Filter{ - Prefix: "/a/b", - }, - } - rule2 := Rule{ - ID: "rule2", - Status: "Enabled", - Expiration: Expiration{Days: ExpirationDays(3)}, - Filter: Filter{ - And: And{ - Prefix: "/a/b/c", - }, - }, - } - overlappingRules := []Rule{rule1, rule2} - overlappingLcConfig, err := xml.Marshal(Lifecycle{Rules: overlappingRules}) - if err != nil { - t.Fatal("Failed to marshal lifecycle config with rules having overlapping prefix") - } - - // Test for lifecycle rules with duplicate IDs - rule3 := Rule{ - ID: "duplicateID", - Status: "Enabled", - Expiration: Expiration{Days: ExpirationDays(3)}, - Filter: Filter{ - Prefix: "/a/b", - }, - } - rule4 := Rule{ - ID: "duplicateID", - Status: "Enabled", - Expiration: Expiration{Days: ExpirationDays(4)}, - Filter: Filter{ - And: And{ - Prefix: "/x/z", - }, - }, - } - duplicateIDRules := []Rule{rule3, rule4} - duplicateIDLcConfig, err := xml.Marshal(Lifecycle{Rules: duplicateIDRules}) - if err != nil { - t.Fatal("Failed to marshal lifecycle config of rules with duplicate ID.") - } - testCases := []struct { inputConfig string expectedParsingErr error @@ -136,21 +70,28 @@ func TestParseAndValidateLifecycleConfig(t *testing.T) { expectedParsingErr: nil, expectedValidationErr: errLifecycleNoRule, }, - { // lifecycle config with more than 1000 rules - inputConfig: string(manyRuleLcConfig), - expectedParsingErr: nil, - expectedValidationErr: errLifecycleTooManyRules, - }, { // lifecycle config with rules having overlapping prefix - inputConfig: string(overlappingLcConfig), + inputConfig: `rule1Enabled/a/b3rule2Enabled/a/b/ckey1val13 `, expectedParsingErr: nil, expectedValidationErr: nil, }, { // lifecycle config with rules having duplicate ID - inputConfig: string(duplicateIDLcConfig), + inputConfig: `duplicateIDEnabled/a/b3duplicateIDEnabled/x/zkey1val14`, expectedParsingErr: nil, expectedValidationErr: errLifecycleDuplicateID, }, + // Missing in + { + inputConfig: `sample-rule-2/a/b/cEnabled1`, + expectedParsingErr: nil, + expectedValidationErr: errXMLNotWellFormed, + }, + // Legitimate lifecycle + { + inputConfig: `ruleEnabled1`, + expectedParsingErr: nil, + expectedValidationErr: nil, + }, } for i, tc := range testCases { @@ -181,17 +122,17 @@ func TestMarshalLifecycleConfig(t *testing.T) { Rules: []Rule{ { Status: "Enabled", - Filter: Filter{Prefix: "prefix-1"}, + Filter: Filter{Prefix: Prefix{string: "prefix-1", set: true}}, Expiration: Expiration{Days: ExpirationDays(3)}, }, { Status: "Enabled", - Filter: Filter{Prefix: "prefix-1"}, + Filter: Filter{Prefix: Prefix{string: "prefix-1", set: true}}, Expiration: Expiration{Date: ExpirationDate(midnightTS)}, }, { Status: "Enabled", - Filter: Filter{Prefix: "prefix-1"}, + Filter: Filter{Prefix: Prefix{string: "prefix-1", set: true}}, Expiration: Expiration{Date: ExpirationDate(midnightTS)}, NoncurrentVersionTransition: NoncurrentVersionTransition{NoncurrentDays: 2, StorageClass: "TEST"}, }, diff --git a/pkg/bucket/lifecycle/noncurrentversion.go b/pkg/bucket/lifecycle/noncurrentversion.go index d1912cdfc..acbaf24ac 100644 --- a/pkg/bucket/lifecycle/noncurrentversion.go +++ b/pkg/bucket/lifecycle/noncurrentversion.go @@ -1,5 +1,5 @@ /* - * MinIO Cloud Storage, (C) 2019 MinIO, Inc. + * MinIO Cloud Storage, (C) 2019-2020 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,12 +24,7 @@ import ( type NoncurrentVersionExpiration struct { XMLName xml.Name `xml:"NoncurrentVersionExpiration"` NoncurrentDays ExpirationDays `xml:"NoncurrentDays,omitempty"` -} - -// NoncurrentVersionTransition - an action for lifecycle configuration rule. -type NoncurrentVersionTransition struct { - NoncurrentDays ExpirationDays `xml:"NoncurrentDays"` - StorageClass string `xml:"StorageClass"` + set bool } // MarshalXML if non-current days not set to non zero value @@ -41,11 +36,43 @@ func (n NoncurrentVersionExpiration) MarshalXML(e *xml.Encoder, start xml.StartE return e.EncodeElement(noncurrentVersionExpirationWrapper(n), start) } +// UnmarshalXML decodes NoncurrentVersionExpiration +func (n *NoncurrentVersionExpiration) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error { + type noncurrentVersionExpirationWrapper NoncurrentVersionExpiration + var val noncurrentVersionExpirationWrapper + err := d.DecodeElement(&val, &startElement) + if err != nil { + return err + } + *n = NoncurrentVersionExpiration(val) + n.set = true + return nil +} + // IsDaysNull returns true if days field is null func (n NoncurrentVersionExpiration) IsDaysNull() bool { return n.NoncurrentDays == ExpirationDays(0) } +// Validate returns an error with wrong value +func (n NoncurrentVersionExpiration) Validate() error { + if !n.set { + return nil + } + val := int(n.NoncurrentDays) + if val <= 0 { + return errXMLNotWellFormed + } + return nil +} + +// NoncurrentVersionTransition - an action for lifecycle configuration rule. +type NoncurrentVersionTransition struct { + NoncurrentDays ExpirationDays `xml:"NoncurrentDays"` + StorageClass string `xml:"StorageClass"` + set bool +} + // MarshalXML is extended to leave out // tags func (n NoncurrentVersionTransition) MarshalXML(e *xml.Encoder, start xml.StartElement) error { @@ -60,3 +87,27 @@ func (n NoncurrentVersionTransition) MarshalXML(e *xml.Encoder, start xml.StartE func (n NoncurrentVersionTransition) IsDaysNull() bool { return n.NoncurrentDays == ExpirationDays(0) } + +// UnmarshalXML decodes NoncurrentVersionExpiration +func (n *NoncurrentVersionTransition) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error { + type noncurrentVersionTransitionWrapper NoncurrentVersionTransition + var val noncurrentVersionTransitionWrapper + err := d.DecodeElement(&val, &startElement) + if err != nil { + return err + } + *n = NoncurrentVersionTransition(val) + n.set = true + return nil +} + +// Validate returns an error with wrong value +func (n NoncurrentVersionTransition) Validate() error { + if !n.set { + return nil + } + if int(n.NoncurrentDays) <= 0 || n.StorageClass == "" { + return errXMLNotWellFormed + } + return nil +} diff --git a/pkg/bucket/lifecycle/prefix.go b/pkg/bucket/lifecycle/prefix.go new file mode 100644 index 000000000..4a7de225b --- /dev/null +++ b/pkg/bucket/lifecycle/prefix.go @@ -0,0 +1,50 @@ +/* + * MinIO Cloud Storage, (C) 2020 MinIO, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package lifecycle + +import ( + "encoding/xml" +) + +// Prefix holds the prefix xml tag in and +type Prefix struct { + string + set bool +} + +// UnmarshalXML - decodes XML data. +func (p *Prefix) UnmarshalXML(d *xml.Decoder, start xml.StartElement) (err error) { + var s string + if err = d.DecodeElement(&s, &start); err != nil { + return err + } + *p = Prefix{string: s, set: true} + return nil +} + +// MarshalXML - decodes XML data. +func (p Prefix) MarshalXML(e *xml.Encoder, startElement xml.StartElement) error { + if !p.set { + return nil + } + return e.EncodeElement(p.string, startElement) +} + +// String returns the prefix string +func (p Prefix) String() string { + return p.string +} diff --git a/pkg/bucket/lifecycle/rule.go b/pkg/bucket/lifecycle/rule.go index 66c90e490..7676b1161 100644 --- a/pkg/bucket/lifecycle/rule.go +++ b/pkg/bucket/lifecycle/rule.go @@ -38,6 +38,7 @@ type Rule struct { ID string `xml:"ID,omitempty"` Status Status `xml:"Status"` Filter Filter `xml:"Filter,omitempty"` + Prefix Prefix `xml:"Prefix,omitempty"` Expiration Expiration `xml:"Expiration,omitempty"` Transition Transition `xml:"Transition,omitempty"` // FIXME: add a type to catch unsupported AbortIncompleteMultipartUpload AbortIncompleteMultipartUpload `xml:"AbortIncompleteMultipartUpload,omitempty"` @@ -92,27 +93,44 @@ func (r Rule) validateStatus() error { return nil } -func (r Rule) validateAction() error { +func (r Rule) validateExpiration() error { return r.Expiration.Validate() } -func (r Rule) validateFilter() error { - return r.Filter.Validate() +func (r Rule) validateNoncurrentExpiration() error { + return r.NoncurrentVersionExpiration.Validate() +} + +func (r Rule) validatePrefixAndFilter() error { + if !r.Prefix.set && r.Filter.IsEmpty() || r.Prefix.set && !r.Filter.IsEmpty() { + return errXMLNotWellFormed + } + if !r.Prefix.set { + return r.Filter.Validate() + } + return nil } func (r Rule) validateTransition() error { return r.Transition.Validate() } -// Prefix - a rule can either have prefix under or under -// . This method returns the prefix from the -// location where it is available -func (r Rule) Prefix() string { - if r.Filter.Prefix != "" { - return r.Filter.Prefix +func (r Rule) validateNoncurrentTransition() error { + return r.NoncurrentVersionTransition.Validate() +} + +// GetPrefix - a rule can either have prefix under , +// or under . This method returns the prefix from the +// location where it is available. +func (r Rule) GetPrefix() string { + if p := r.Prefix.String(); p != "" { + return p } - if r.Filter.And.Prefix != "" { - return r.Filter.And.Prefix + if p := r.Filter.Prefix.String(); p != "" { + return p + } + if p := r.Filter.And.Prefix.String(); p != "" { + return p } return "" } @@ -145,14 +163,23 @@ func (r Rule) Validate() error { if err := r.validateStatus(); err != nil { return err } - if err := r.validateAction(); err != nil { + if err := r.validateExpiration(); err != nil { return err } - if err := r.validateFilter(); err != nil { + if err := r.validateNoncurrentExpiration(); err != nil { + return err + } + if err := r.validatePrefixAndFilter(); err != nil { return err } if err := r.validateTransition(); err != nil { return err } + if err := r.validateNoncurrentTransition(); err != nil { + return err + } + if !r.Expiration.set && !r.Transition.set && !r.NoncurrentVersionExpiration.set && !r.NoncurrentVersionTransition.set { + return errXMLNotWellFormed + } return nil } diff --git a/pkg/bucket/lifecycle/rule_test.go b/pkg/bucket/lifecycle/rule_test.go index c3e7a1e48..bb805b3cb 100644 --- a/pkg/bucket/lifecycle/rule_test.go +++ b/pkg/bucket/lifecycle/rule_test.go @@ -38,6 +38,7 @@ func TestInvalidRules(t *testing.T) { { // Rule with empty ID inputXML: ` + 365