Add support for Object Tagging in LifeCycle configuration (minio#8880)

Fixes minio#8870

Co-Authored-By: Krishnan Parthasarathi <[email protected]>

cmd/api-errors.go

@@ -32,6 +32,7 @@ import (
 	"github.com/minio/minio/cmd/crypto"
 	"github.com/minio/minio/cmd/logger"
 	"github.com/minio/minio/pkg/auth"
+	"github.com/minio/minio/pkg/bucket/lifecycle"
 	objectlock "github.com/minio/minio/pkg/bucket/object/lock"
 	"github.com/minio/minio/pkg/bucket/object/tagging"
 	"github.com/minio/minio/pkg/bucket/policy"
@@ -1795,6 +1796,12 @@ func toAPIError(ctx context.Context, err error) APIError {
 		// their internal error types. This code is only
 		// useful with gateway implementations.
 		switch e := err.(type) {
+		case lifecycle.Error:
+			apiErr = APIError{
+				Code:           "InvalidRequest",
+				Description:    e.Error(),
+				HTTPStatusCode: http.StatusBadRequest,
+			}
 		case tagging.Error:
 			apiErr = APIError{
 				Code:           "InvalidTag",

cmd/bucket-lifecycle-handler.go

@@ -67,7 +67,7 @@ func (api objectAPIHandlers) PutBucketLifecycleHandler(w http.ResponseWriter, r
 
 	bucketLifecycle, err := lifecycle.ParseLifecycleConfig(io.LimitReader(r.Body, r.ContentLength))
 	if err != nil {
-		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMalformedXML), r.URL, guessIsBrowserReq(r))
+		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
 		return
 	}
 

cmd/daily-lifecycle-ops.go

@@ -129,7 +129,7 @@ func lifecycleRound(ctx context.Context, objAPI ObjectLayer) error {
 		// Calculate the common prefix of all lifecycle rules
 		var prefixes []string
 		for _, rule := range l.Rules {
-			prefixes = append(prefixes, rule.Filter.Prefix)
+			prefixes = append(prefixes, rule.Prefix())
 		}
 		commonPrefix := lcp(prefixes)
 
@@ -143,7 +143,7 @@ func lifecycleRound(ctx context.Context, objAPI ObjectLayer) error {
 			var objects []string
 			for _, obj := range res.Objects {
 				// Find the action that need to be executed
-				action := l.ComputeAction(obj.Name, obj.ModTime)
+				action := l.ComputeAction(obj.Name, obj.UserTags, obj.ModTime)
 				switch action {
 				case lifecycle.DeleteAction:
 					objects = append(objects, obj.Name)

cmd/object-handlers.go

@@ -2877,7 +2877,6 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
 	}
 
 	tagging, err := tagging.ParseTagging(io.LimitReader(r.Body, r.ContentLength))
-
 	if err != nil {
 		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
 		return

pkg/bucket/lifecycle/and.go

@@ -18,25 +18,47 @@ package lifecycle
 
 import (
 	"encoding/xml"
-	"errors"
+
+	"github.com/minio/minio/pkg/bucket/object/tagging"
 )
 
 // 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"`
-	Tags    []Tag    `xml:"Tag,omitempty"`
+	XMLName xml.Name      `xml:"And"`
+	Prefix  string        `xml:"Prefix,omitempty"`
+	Tags    []tagging.Tag `xml:"Tag,omitempty"`
 }
 
-var errAndUnsupported = errors.New("Specifying <And></And> tag is not supported")
+var errDuplicateTagKey = Errorf("Duplicate Tag Keys are not allowed")
 
-// UnmarshalXML is extended to indicate lack of support for And xml
-// tag in object lifecycle configuration
-func (a And) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
-	return errAndUnsupported
+// isEmpty returns true if Tags field is null
+func (a And) isEmpty() bool {
+	return len(a.Tags) == 0 && a.Prefix == ""
 }
 
-// MarshalXML is extended to leave out <And></And> tags
-func (a And) MarshalXML(e *xml.Encoder, start xml.StartElement) error {
+// Validate - validates the And field
+func (a And) Validate() error {
+	if a.ContainsDuplicateTag() {
+		return errDuplicateTagKey
+	}
+	for _, t := range a.Tags {
+		if err := t.Validate(); err != nil {
+			return err
+		}
+	}
 	return nil
 }
+
+// ContainsDuplicateTag - returns true if duplicate keys are present in And
+func (a And) ContainsDuplicateTag() bool {
+	x := make(map[string]struct{}, len(a.Tags))
+
+	for _, t := range a.Tags {
+		if _, has := x[t.Key]; has {
+			return true
+		}
+		x[t.Key] = struct{}{}
+	}
+
+	return false
+}

pkg/bucket/lifecycle/error.go

@@ -0,0 +1,44 @@
+/*
+ * 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 (
+	"fmt"
+)
+
+// Error is the generic type for any error happening during tag
+// parsing.
+type Error struct {
+	err error
+}
+
+// Errorf - formats according to a format specifier and returns
+// the string as a value that satisfies error of type tagging.Error
+func Errorf(format string, a ...interface{}) error {
+	return Error{err: fmt.Errorf(format, a...)}
+}
+
+// Unwrap the internal error.
+func (e Error) Unwrap() error { return e.err }
+
+// Error 'error' compatible method.
+func (e Error) Error() string {
+	if e.err == nil {
+		return "lifecycle: cause <nil>"
+	}
+	return e.err.Error()
+}

pkg/bucket/lifecycle/expiration.go

@@ -18,15 +18,14 @@ package lifecycle
 
 import (
 	"encoding/xml"
-	"errors"
 	"time"
 )
 
 var (
-	errLifecycleInvalidDate       = errors.New("Date must be provided in ISO 8601 format")
-	errLifecycleInvalidDays       = errors.New("Days must be positive integer when used with Expiration")
-	errLifecycleInvalidExpiration = errors.New("At least one of Days or Date should be present inside Expiration")
-	errLifecycleDateNotMidnight   = errors.New(" 'Date' must be at midnight GMT")
+	errLifecycleInvalidDate       = Errorf("Date must be provided in ISO 8601 format")
+	errLifecycleInvalidDays       = Errorf("Days must be positive integer when used with Expiration")
+	errLifecycleInvalidExpiration = Errorf("At least one of Days or Date should be present inside Expiration")
+	errLifecycleDateNotMidnight   = Errorf("'Date' must be at midnight GMT")
 )
 
 // ExpirationDays is a type alias to unmarshal Days in Expiration
@@ -121,7 +120,6 @@ func (e Expiration) Validate() error {
 // IsDaysNull returns true if days field is null
 func (e Expiration) IsDaysNull() bool {
 	return e.Days == ExpirationDays(0)
-
 }
 
 // IsDateNull returns true if date field is null

pkg/bucket/lifecycle/filter.go

@@ -16,17 +16,52 @@
 
 package lifecycle
 
-import "encoding/xml"
+import (
+	"encoding/xml"
+
+	"github.com/minio/minio/pkg/bucket/object/tagging"
+)
 
 // Filter - a filter for a lifecycle configuration Rule.
 type Filter struct {
-	XMLName xml.Name `xml:"Filter"`
-	And     And      `xml:"And,omitempty"`
-	Prefix  string   `xml:"Prefix"`
-	Tag     Tag      `xml:"Tag,omitempty"`
+	XMLName xml.Name    `xml:"Filter"`
+	Prefix  string      `xml:"Prefix,omitempty"`
+	And     And         `xml:"And,omitempty"`
+	Tag     tagging.Tag `xml:"Tag,omitempty"`
 }
 
+var (
+	errInvalidFilter = Errorf("Filter must have exactly one of Prefix, Tag, or And specified")
+)
+
 // Validate - validates the filter element
 func (f Filter) Validate() error {
+	// A Filter must have exactly one of Prefix, Tag, or And specified.
+	if !f.And.isEmpty() {
+		if f.Prefix != "" {
+			return errInvalidFilter
+		}
+		if !f.Tag.IsEmpty() {
+			return errInvalidFilter
+		}
+		if err := f.And.Validate(); err != nil {
+			return err
+		}
+	}
+	if f.Prefix != "" {
+		if !f.Tag.IsEmpty() {
+			return errInvalidFilter
+		}
+	}
+	if !f.Tag.IsEmpty() {
+		if err := f.Tag.Validate(); err != nil {
+			return err
+		}
+	}
 	return nil
 }
+
+// isEmpty - returns true if Filter tag is empty
+func (f Filter) isEmpty() bool {
+	return f.And.isEmpty() && f.Prefix == "" && f.Tag == tagging.Tag{}
+}

pkg/bucket/lifecycle/filter_test.go

@@ -31,23 +31,91 @@ func TestUnsupportedFilters(t *testing.T) {
 	}{
 		{ // Filter with And tags
 			inputXML: ` <Filter>
-	                     <And>
-	                     <Prefix></Prefix>
-	                     </And>
-	                    </Filter>`,
-			expectedErr: errAndUnsupported,
+						<And>
+							<Prefix>key-prefix</Prefix>
+						</And>
+						</Filter>`,
+			expectedErr: nil,
 		},
 		{ // Filter with Tag tags
 			inputXML: ` <Filter>
-	                     <Tag></Tag>
-	                    </Filter>`,
-			expectedErr: errTagUnsupported,
+						<Tag>
+							<Key>key1</Key>
+							<Value>value1</Value>
+						</Tag>
+						</Filter>`,
+			expectedErr: nil,
+		},
+		{ // Filter with Prefix tag
+			inputXML: ` <Filter>
+							<Prefix>key-prefix</Prefix>
+						</Filter>`,
+			expectedErr: nil,
+		},
+		{ // Filter without And and multiple Tag tags
+			inputXML: ` <Filter>
+							<Prefix>key-prefix</Prefix>
+							<Tag>
+								<Key>key1</Key>
+								<Value>value1</Value>
+							</Tag>
+							<Tag>
+								<Key>key2</Key>
+								<Value>value2</Value>
+							</Tag>
+						</Filter>`,
+			expectedErr: errInvalidFilter,
+		},
+		{ // Filter with And, Prefix & multiple Tag tags
+			inputXML: ` <Filter>
+							<And>
+							<Prefix>key-prefix</Prefix>
+							<Tag>
+								<Key>key1</Key>
+								<Value>value1</Value>
+							</Tag>
+							<Tag>
+								<Key>key2</Key>
+								<Value>value2</Value>
+							</Tag>
+							</And>
+						</Filter>`,
+			expectedErr: nil,
+		},
+		{ // Filter with And and multiple Tag tags
+			inputXML: ` <Filter>
+							<And>
+							<Tag>
+								<Key>key1</Key>
+								<Value>value1</Value>
+							</Tag>
+							<Tag>
+								<Key>key2</Key>
+								<Value>value2</Value>
+							</Tag>
+							</And>
+						</Filter>`,
+			expectedErr: nil,
+		},
+		{ // Filter without And and single Tag tag
+			inputXML: ` <Filter>
+							<Prefix>key-prefix</Prefix>
+							<Tag>
+								<Key>key1</Key>
+								<Value>value1</Value>
+							</Tag>
+						</Filter>`,
+			expectedErr: errInvalidFilter,
 		},
 	}
 	for i, tc := range testCases {
 		t.Run(fmt.Sprintf("Test %d", i+1), func(t *testing.T) {
 			var filter Filter
 			err := xml.Unmarshal([]byte(tc.inputXML), &filter)
+			if err != nil {
+				t.Fatalf("%d: Expected no error but got %v", i+1, err)
+			}
+			err = filter.Validate()
 			if err != tc.expectedErr {
 				t.Fatalf("%d: Expected %v but got %v", i+1, tc.expectedErr, err)
 			}