lifecycle: Accept document without expiration (#10348)

pkg/bucket/lifecycle/expiration.go

@@ -98,36 +98,76 @@ func (eDate ExpirationDate) MarshalXML(e *xml.Encoder, startElement xml.StartEle
 }
 
 // ExpireDeleteMarker represents value of ExpiredObjectDeleteMarker field in Expiration XML element.
-type ExpireDeleteMarker bool
+type ExpireDeleteMarker struct {
+	val bool
+	set bool
+}
 
 // Expiration - expiration actions for a rule in lifecycle configuration.
 type Expiration struct {
 	XMLName      xml.Name           `xml:"Expiration"`
 	Days         ExpirationDays     `xml:"Days,omitempty"`
 	Date         ExpirationDate     `xml:"Date,omitempty"`
-	DeleteMarker ExpireDeleteMarker `xml:"ExpiredObjectDeleteMarker,omitempty"`
+	DeleteMarker ExpireDeleteMarker `xml:"ExpiredObjectDeleteMarker"`
+
+	set bool
 }
 
 // MarshalXML encodes delete marker boolean into an XML form.
 func (b ExpireDeleteMarker) MarshalXML(e *xml.Encoder, startElement xml.StartElement) error {
-	if !b {
+	if !b.set {
+		return nil
+	}
+	return e.EncodeElement(b.val, startElement)
+}
+
+// UnmarshalXML decodes delete marker boolean from the XML form.
+func (b *ExpireDeleteMarker) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error {
+	var exp bool
+	err := d.DecodeElement(&exp, &startElement)
+	if err != nil {
+		return err
+	}
+	b.val = exp
+	b.set = true
+	return nil
+}
+
+// MarshalXML encodes expiration field into an XML form.
+func (e Expiration) MarshalXML(enc *xml.Encoder, startElement xml.StartElement) error {
+	if !e.set {
 		return nil
 	}
-	type expireDeleteMarkerWrapper ExpireDeleteMarker
+	type expirationWrapper Expiration
-	return e.EncodeElement(expireDeleteMarkerWrapper(b), startElement)
+	return enc.EncodeElement(expirationWrapper(e), startElement)
+}
+
+// UnmarshalXML decodes expiration field from the XML form.
+func (e *Expiration) UnmarshalXML(d *xml.Decoder, startElement xml.StartElement) error {
+	type expirationWrapper Expiration
+	var exp expirationWrapper
+	err := d.DecodeElement(&exp, &startElement)
+	if err != nil {
+		return err
+	}
+	*e = Expiration(exp)
+	e.set = true
+	return nil
 }
 
 // Validate - validates the "Expiration" element
 func (e Expiration) Validate() error {
+	if !e.set {
+		return nil
+	}
+
 	// DeleteMarker cannot be specified if date or dates are specified.
-	if (!e.IsDateNull() || !e.IsDateNull()) && bool(e.DeleteMarker) {
+	if (!e.IsDaysNull() || !e.IsDateNull()) && e.DeleteMarker.set {
 		return errLifecycleInvalidDeleteMarker
 	}
 
-	// Neither expiration days or date is specified
+	if !e.DeleteMarker.set && e.IsDaysNull() && e.IsDateNull() {
-	// if delete marker is false one of them should be specified
+		return errXMLNotWellFormed
-	if !bool(e.DeleteMarker) && e.IsDaysNull() && e.IsDateNull() {
-		return errLifecycleInvalidExpiration
 	}
 
 	// Both expiration days and date are specified

pkg/bucket/lifecycle/expiration_test.go

@@ -78,7 +78,7 @@ func TestInvalidExpiration(t *testing.T) {
 		{ // Expiration with neither number of days nor a date
 			inputXML: `<Expiration>
                                     </Expiration>`,
-			expectedErr: errLifecycleInvalidExpiration,
+			expectedErr: errXMLNotWellFormed,
 		},
 		{ // Expiration with both number of days and a date
 			inputXML: `<Expiration>
@@ -87,6 +87,13 @@ func TestInvalidExpiration(t *testing.T) {
                                     </Expiration>`,
 			expectedErr: errLifecycleInvalidExpiration,
 		},
+		{ // Expiration with both ExpiredObjectDeleteMarker and days
+			inputXML: `<Expiration>
+                                    <Days>3</Days>
+			            <ExpiredObjectDeleteMarker>false</ExpiredObjectDeleteMarker>
+                                    </Expiration>`,
+			expectedErr: errLifecycleInvalidDeleteMarker,
+		},
 	}
 	for i, tc := range validationTestCases {
 		t.Run(fmt.Sprintf("Test %d", i+1), func(t *testing.T) {
@@ -98,7 +105,7 @@ func TestInvalidExpiration(t *testing.T) {
 
 			err = expiration.Validate()
 			if err != tc.expectedErr {
-				t.Fatalf("%d: %v", i+1, err)
+				t.Fatalf("%d: got: %v, expected: %v", i+1, err, tc.expectedErr)
 			}
 		})
 	}

pkg/bucket/lifecycle/lifecycle.go

@@ -27,6 +27,7 @@ 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")
 )
 
 // Action represents a delete action or other transition
@@ -154,7 +155,7 @@ func (lc Lifecycle) FilterActionableRules(obj ObjectOpts) []Rule {
 		// be expired; if set to false the policy takes no action. This
 		// cannot be specified with Days or Date in a Lifecycle
 		// Expiration Policy.
-		if rule.Expiration.DeleteMarker {
+		if rule.Expiration.DeleteMarker.val {
 			rules = append(rules, rule)
 			continue
 		}
@@ -193,7 +194,7 @@ func (lc Lifecycle) ComputeAction(obj ObjectOpts) Action {
 	}
 
 	for _, rule := range lc.FilterActionableRules(obj) {
-		if obj.DeleteMarker && obj.NumVersions == 1 && bool(rule.Expiration.DeleteMarker) {
+		if obj.DeleteMarker && obj.NumVersions == 1 && rule.Expiration.DeleteMarker.val {
 			// Indicates whether MinIO will remove a delete marker with no noncurrent versions.
 			// Only latest marker is removed. If set to true, the delete marker will be expired;
 			// if set to false the policy takes no action. This cannot be specified with Days or

pkg/bucket/lifecycle/rule_test.go

@@ -62,13 +62,6 @@ func TestInvalidRules(t *testing.T) {
 		inputXML    string
 		expectedErr error
 	}{
-		{ // Rule without expiration action
-			inputXML: ` <Rule>
-			                <ID>rule without expiration</ID>
-                            <Status>Enabled</Status>
-	                    </Rule>`,
-			expectedErr: errLifecycleInvalidExpiration,
-		},
 		{ // Rule with ID longer than 255 characters
 			inputXML: ` <Rule>
 	                    <ID> babababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababababab </ID>