From 297cdef1a334829b04f1f1bd5e11e1a1f50a7bb5 Mon Sep 17 00:00:00 2001
From: Chris Lu <chrislusf@users.noreply.github.com>
Date: Sat, 28 Mar 2026 19:39:21 -0700
Subject: [PATCH] s3api: accept all supported lifecycle rule types (#8813)

* s3api: accept NoncurrentVersionExpiration, AbortIncompleteMultipartUpload, Expiration.Date

Update PutBucketLifecycleConfigurationHandler to accept all newly-supported
lifecycle rule types. Only Transition and NoncurrentVersionTransition are
still rejected (require storage class tier infrastructure).

Changes:
- Remove ErrNotImplemented for Expiration.Date (handled by worker at scan time)
- Only reject rules with Transition.set or NoncurrentVersionTransition.set
- Extract prefix from Filter.And when present
- Add comment explaining that non-Expiration.Days rules are evaluated by
  the lifecycle worker from stored lifecycle XML, not via filer.conf TTL

The lifecycle XML is already stored verbatim in bucket metadata, so new
rule types are preserved on Get even without explicit handler support.
Filer.conf TTL entries are only created for Expiration.Days (fast path).

* s3api: skip TTL fast path for rules with tag or size filters

Rules with tag or size constraints (Filter.Tag, Filter.And with tags
or size bounds, Filter.ObjectSizeGreaterThan/LessThan) must not be
lowered to filer.conf TTL entries, because TTL applies unconditionally
to all objects under the prefix. These rules are evaluated at scan
time by the lifecycle worker which checks each object's tags and size.

Only simple Expiration.Days rules with prefix-only filters use the
TTL fast path (RocksDB compaction filter).

---------

Co-authored-by: Copilot <copilot@github.com>
---
 weed/s3api/s3api_bucket_handlers.go | 24 +++++++++++++--
 weed/s3api/s3api_policy_test.go     | 45 +++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go
index a88ef563e..c5833dacd 100644
--- a/weed/s3api/s3api_bucket_handlers.go
+++ b/weed/s3api/s3api_bucket_handlers.go
@@ -956,20 +956,38 @@ func (s3a *S3ApiServer) PutBucketLifecycleConfigurationHandler(w http.ResponseWr
 		if rule.Status != Enabled {
 			continue
 		}
+		// Reject Transition rules — they require storage class migration
+		// infrastructure that does not exist yet.
+		if rule.Transition.set || rule.NoncurrentVersionTransition.set {
+			s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented)
+			return
+		}
+
 		var rulePrefix string
 		switch {
+		case rule.Filter.andSet:
+			rulePrefix = rule.Filter.And.Prefix.val
 		case rule.Filter.Prefix.set:
 			rulePrefix = rule.Filter.Prefix.val
 		case rule.Prefix.set:
 			rulePrefix = rule.Prefix.val
-		case !rule.Expiration.Date.IsZero() || rule.Transition.Days > 0 || !rule.Transition.Date.IsZero():
-			s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented)
-			return
 		}
 
+		// Only create filer.conf TTL entries for simple Expiration.Days rules
+		// with prefix-only filters (the fast path handled by RocksDB compaction
+		// filter). Rules with tag or size filters must be evaluated at scan time
+		// by the lifecycle worker, because TTL applies to all objects under the
+		// prefix regardless of tags or size.
 		if rule.Expiration.Days == 0 {
 			continue
 		}
+		hasTagOrSizeFilter := rule.Filter.tagSet ||
+			rule.Filter.ObjectSizeGreaterThan > 0 || rule.Filter.ObjectSizeLessThan > 0 ||
+			(rule.Filter.andSet && (len(rule.Filter.And.Tags) > 0 ||
+				rule.Filter.And.ObjectSizeGreaterThan > 0 || rule.Filter.And.ObjectSizeLessThan > 0))
+		if hasTagOrSizeFilter {
+			continue // evaluated by lifecycle worker at scan time
+		}
 		locationPrefix := fmt.Sprintf("%s/%s/%s", s3a.option.BucketsPath, bucket, rulePrefix)
 		locConf := &filer_pb.FilerConf_PathConf{
 			LocationPrefix:    locationPrefix,
diff --git a/weed/s3api/s3api_policy_test.go b/weed/s3api/s3api_policy_test.go
index b14b4f824..f6cc38f60 100644
--- a/weed/s3api/s3api_policy_test.go
+++ b/weed/s3api/s3api_policy_test.go
@@ -177,6 +177,51 @@ func TestLifecycleXMLRoundTrip_FilterWithSizeOnly(t *testing.T) {
 	}
 }
 
+func TestLifecycleXML_TransitionSetFlag(t *testing.T) {
+	// Verify that Transition.set is true after unmarshaling.
+	input := `<LifecycleConfiguration>
+  <Rule>
+    <ID>transition</ID>
+    <Status>Enabled</Status>
+    <Filter><Prefix></Prefix></Filter>
+    <Transition>
+      <Days>30</Days>
+      <StorageClass>GLACIER</StorageClass>
+    </Transition>
+  </Rule>
+</LifecycleConfiguration>`
+
+	var lc Lifecycle
+	if err := xml.Unmarshal([]byte(input), &lc); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if !lc.Rules[0].Transition.set {
+		t.Error("expected Transition.set=true after unmarshal")
+	}
+}
+
+func TestLifecycleXML_NoncurrentVersionTransitionSetFlag(t *testing.T) {
+	input := `<LifecycleConfiguration>
+  <Rule>
+    <ID>nv-transition</ID>
+    <Status>Enabled</Status>
+    <Filter><Prefix></Prefix></Filter>
+    <NoncurrentVersionTransition>
+      <NoncurrentDays>60</NoncurrentDays>
+      <StorageClass>GLACIER</StorageClass>
+    </NoncurrentVersionTransition>
+  </Rule>
+</LifecycleConfiguration>`
+
+	var lc Lifecycle
+	if err := xml.Unmarshal([]byte(input), &lc); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if !lc.Rules[0].NoncurrentVersionTransition.set {
+		t.Error("expected NoncurrentVersionTransition.set=true after unmarshal")
+	}
+}
+
 func TestLifecycleXMLRoundTrip_CompleteRule(t *testing.T) {
 	// A complete lifecycle config similar to what Terraform sends.
 	input := `<LifecycleConfiguration>