lifecycle: Assign unique id to rules with empty id (#15731)

cmd/bucket-lifecycle-handlers.go

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

internal/bucket/lifecycle/lifecycle.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/google/uuid"
 	xhttp "github.com/minio/minio/internal/http"
 )
 
@@ -176,6 +177,22 @@ func (lc Lifecycle) HasActiveRules(prefix string, recursive bool) bool {
 	return false
 }
 
+// ParseLifecycleConfigWithID - parses for a Lifecycle config and assigns
+// unique id to rules with empty ID.
+func ParseLifecycleConfigWithID(r io.Reader) (*Lifecycle, error) {
+	var lc Lifecycle
+	if err := xml.NewDecoder(r).Decode(&lc); err != nil {
+		return nil, err
+	}
+	// assign a unique id for rules with empty ID
+	for i := range lc.Rules {
+		if lc.Rules[i].ID == "" {
+			lc.Rules[i].ID = uuid.New().String()
+		}
+	}
+	return &lc, nil
+}
+
 // ParseLifecycleConfig - parses data in given reader to Lifecycle.
 func ParseLifecycleConfig(reader io.Reader) (*Lifecycle, error) {
 	var lc Lifecycle

internal/bucket/lifecycle/lifecycle_test.go

@@ -712,3 +712,32 @@ func TestMaxNoncurrentBackwardCompat(t *testing.T) {
 		}
 	}
 }
+
+func TestParseLifecycleConfigWithID(t *testing.T) {
+	r := bytes.NewReader([]byte(`<LifecycleConfiguration>
+								  <Rule>
+	                              <ID>rule-1</ID>
+		                          <Filter>
+		                             <Prefix>prefix</Prefix>
+		                          </Filter>
+		                          <Status>Enabled</Status>
+		                          <Expiration><Days>3</Days></Expiration>
+		                          </Rule>
+		                          <Rule>
+		                          <Filter>
+		                             <Prefix>another-prefix</Prefix>
+		                          </Filter>
+		                          <Status>Enabled</Status>
+		                          <Expiration><Days>3</Days></Expiration>
+		                          </Rule>
+		                          </LifecycleConfiguration>`))
+	lc, err := ParseLifecycleConfigWithID(r)
+	if err != nil {
+		t.Fatalf("Expected parsing to succeed but failed with %v", err)
+	}
+	for _, rule := range lc.Rules {
+		if rule.ID == "" {
+			t.Fatalf("Expected all rules to have a unique id assigned %#v", rule)
+		}
+	}
+}

internal/bucket/lifecycle/rule.go

@@ -20,8 +20,6 @@ package lifecycle
 import (
 	"bytes"
 	"encoding/xml"
-
-	"github.com/google/uuid"
 )
 
 // Status represents lifecycle configuration status
@@ -53,28 +51,9 @@ var (
 	errInvalidRuleStatus = Errorf("Status must be set to either Enabled or Disabled")
 )
 
-// generates random UUID
-func getNewUUID() (string, error) {
-	u, err := uuid.NewRandom()
-	if err != nil {
-		return "", err
-	}
-
-	return u.String(), nil
-}
-
 // validateID - checks if ID is valid or not.
 func (r Rule) validateID() error {
-	IDLen := len(r.ID)
-	// generate new ID when not provided
-	// cannot be longer than 255 characters
-	if IDLen == 0 {
-		if newID, err := getNewUUID(); err == nil {
-			r.ID = newID
-		} else {
-			return err
-		}
-	} else if IDLen > 255 {
+	if len(r.ID) > 255 {
 		return errInvalidRuleID
 	}
 	return nil