s3: warn when a lifecycle change leaves fast-path-stamped objects on their old TTL (#11184)

* s3: warn when a lifecycle change leaves fast-path-stamped objects on their old TTL

The per-write TTL fast path (opt-in via s3.bucket.lifecycle.fastpath)
stamps a volume TTL at PutObject time that can't be taken back. When an
operator lengthens or removes an Expiration.Days rule (or deletes the
bucket lifecycle) on a fast-path-enabled bucket, objects already written
keep their baked-in TTL and won't be rescued by the change — unlike the
default worker-driven path, which re-evaluates the current rules each
pass. This is the data-loss direction described in #11183.

Surface it: Put/DeleteBucketLifecycle now emit a glog warning and set
X-Seaweed-Lifecycle-Fastpath-Warning on the response when the change
removes, disables, lengthens, or re-scopes a fast-path-eligible rule.
Shortening a rule does not warn (old objects simply expire later, not
data loss). Tag-only and overflow-day rules are never on the fast path
and never warn.

Addresses the warning half of option 2 in #11183.

* s3: address review — emit warning after mutation succeeds, fix ID-rename false positive

Two issues raised by CodeRabbit, Greptile, and Devin reviews:

1. Failed mutations retained the warning header. The warning was set on
   the ResponseWriter before storeBucketLifecycleConfiguration /
   clearStoredBucketLifecycleConfiguration was called; if that failed,
   the error response carried a warning for a change that was never
   applied. Now the reason is computed before the mutation but the log
   and header are emitted only after it succeeds.

2. Rule renames produced false "removed" warnings. fastpathRuleKey used
   Rule.ID as the sole identity when present, so renaming a rule (same
   prefix/size/days, different ID) treated the old rule as removed.
   Replaced with two-pass matching: first by ID, then by fast-path
   predicates (prefix + size). An ID-only rename with unchanged
   predicates and days no longer warns. Greedy matching ensures each
   new rule is consumed by at most one old rule.

Added regression tests: ID-only rename (no warn), rename + lengthen
(warn), rename + shorten (no warn).

weed/s3api/s3api_bucket_handlers.go

@@ -1178,11 +1178,29 @@ func (s3a *S3ApiServer) PutBucketLifecycleConfigurationHandler(w http.ResponseWr
 		return
 	}
 
+	// The per-write TTL fast path stamps a volume TTL at write time that
+	// can't be taken back. If it's active on this bucket, warn when the new
+	// config removes or lengthens a rule: objects already written keep their
+	// baked-in TTL and won't be rescued by this change (unlike the default
+	// worker path, which re-evaluates the current rules each pass).
+	// Compute the reason before storing, but emit only after the store
+	// succeeds so a failed mutation never carries a warning for a change
+	// that was not applied.
+	var fastpathWarnReason string
+	if cfg, _ := s3a.getBucketConfig(bucket); cfg != nil && cfg.LifecycleTTL != nil {
+		fastpathWarnReason = fastpathConfigChangeLeavesStampedObjects(cfg.LifecycleXML, lifecycleXML)
+	}
+
 	if errCode := s3a.storeBucketLifecycleConfiguration(bucket, lifecycleXML, r.Header.Get(bucketLifecycleTransitionMinimumObjectSizeHeader)); errCode != s3err.ErrNone {
 		s3err.WriteErrorResponse(w, r, errCode)
 		return
 	}
 
+	if fastpathWarnReason != "" {
+		glog.Warningf("PutBucketLifecycleConfigurationHandler %s: %s", bucket, fastpathWarnReason)
+		w.Header().Set(fastpathWarningHeader, fastpathWarnReason)
+	}
+
 	writeSuccessResponseEmpty(w, r)
 }
 
@@ -1207,11 +1225,27 @@ func (s3a *S3ApiServer) DeleteBucketLifecycleHandler(w http.ResponseWriter, r *h
 		return
 	}
 
+	// If the per-write TTL fast path is active, every previously-stamped
+	// object keeps its baked-in volume TTL after the config is removed —
+	// deleting the rules does not rescue them (unlike the default worker
+	// path). Compute the reason before clearing, but emit only after the
+	// clear succeeds so a failed mutation never carries a warning for a
+	// change that was not applied.
+	var fastpathWarnReason string
+	if cfg, _ := s3a.getBucketConfig(bucket); cfg != nil && cfg.LifecycleTTL != nil {
+		fastpathWarnReason = fastpathConfigChangeLeavesStampedObjects(cfg.LifecycleXML, nil)
+	}
+
 	if errCode := s3a.clearStoredBucketLifecycleConfiguration(bucket); errCode != s3err.ErrNone {
 		s3err.WriteErrorResponse(w, r, errCode)
 		return
 	}
 
+	if fastpathWarnReason != "" {
+		glog.Warningf("DeleteBucketLifecycleHandler %s: %s", bucket, fastpathWarnReason)
+		w.Header().Set(fastpathWarningHeader, fastpathWarnReason)
+	}
+
 	s3err.WriteEmptyResponse(w, r, http.StatusNoContent)
 }
 

weed/s3api/s3api_bucket_lifecycle_fastpath_warn.go

@@ -0,0 +1,161 @@
+package s3api
+
+import (
+	"fmt"
+	"math"
+	"sort"
+	"strings"
+
+	"github.com/seaweedfs/seaweedfs/weed/s3api/lifecycle_xml"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle"
+)
+
+// fastpathWarningHeader is set on Put/DeleteBucketLifecycle responses when
+// the bucket has the lifecycle TTL fast path enabled and the configuration
+// change leaves objects already written under the previous rules on their
+// baked-in volume TTL. A volume TTL is decided once, at write time, and
+// can't be taken back: removing or lengthening a rule does not retroactively
+// apply to objects already stamped, unlike the default worker-driven path
+// which re-evaluates the current rules each pass. The header lets an
+// operator notice (and act before the old deadline) that "extend retention
+// / remove the rule" did not protect already-recorded objects.
+const fastpathWarningHeader = "X-Seaweed-Lifecycle-Fastpath-Warning"
+
+// fastpathConfigChangeLeavesStampedObjects reports whether a lifecycle
+// configuration change from oldXML to newXML on a fastpath-enabled bucket
+// leaves previously-stamped objects on a volume TTL that no longer matches
+// the new policy. Returns a short human-readable reason when it does, and
+// an empty string when it does not.
+//
+// A change "leaves stamped objects" when, for any rule that was on the
+// fast path before (Enabled, ExpirationDays>0, no tag filter, fits int32
+// seconds — the same eligibility NewLifecycleTTLResolver applies):
+//
+//   - it is absent or Disabled in the new config (removed/disabled), or
+//   - its ExpirationDays increased (lengthened — old objects keep the
+//     shorter baked-in TTL), or
+//   - its prefix or size filter changed (objects that matched before may
+//     keep an expiry the new rule no longer describes).
+//
+// Rule identity is matched in two passes: first by ID, then by fast-path
+// predicates (prefix + size filters). An ID-only rename with unchanged
+// predicates and days is NOT a warning — the policy is equivalent, only
+// the label moved.
+//
+// Shortening a rule (e.g. 30d -> 7d) does NOT warn: old objects simply
+// expire later than the new shorter rule, which is not the data-loss
+// direction. Tag-filtered rules are never on the fast path, so their
+// removal/change does not warn here (the worker re-evaluates tags).
+//
+// newXML may be nil/empty, which represents DeleteBucketLifecycle: every
+// previously-eligible rule is removed.
+func fastpathConfigChangeLeavesStampedObjects(oldXML, newXML []byte) string {
+	oldRules := fastpathEligibleRules(oldXML)
+	if len(oldRules) == 0 {
+		return ""
+	}
+	newRules := fastpathEligibleRules(newXML)
+
+	// Match old rules to new rules in two passes so an ID-only rename
+	// (same prefix/size/days, different ID) does not produce a false
+	// "removed" warning:
+	//   1. Match by ID. A rule that kept its ID is the same rule; check
+	//      for filter or days changes.
+	//   2. For unmatched old rules, try matching by fast-path predicates
+	//      (prefix + size). A predicate match with different days means
+	//      the rule was renamed and possibly lengthened; same days means
+	//      a pure rename — no warning.
+	// Greedy: each new rule is consumed by at most one old rule so
+	// overlapping rules can't be double-matched.
+	used := make([]bool, len(newRules))
+	newByID := make(map[string]int, len(newRules))
+	for i, r := range newRules {
+		if r.ID != "" {
+			newByID[r.ID] = i
+		}
+	}
+
+	var reasons []string
+	for _, r := range oldRules {
+		// Pass 1: ID match.
+		if r.ID != "" {
+			if idx, ok := newByID[r.ID]; ok && !used[idx] {
+				used[idx] = true
+				nr := newRules[idx]
+				if nr.Prefix != r.Prefix || nr.FilterSizeGreaterThan != r.FilterSizeGreaterThan || nr.FilterSizeLessThan != r.FilterSizeLessThan {
+					reasons = append(reasons, fmt.Sprintf("rule %q filter changed", ruleName(r)))
+				} else if nr.ExpirationDays > r.ExpirationDays {
+					reasons = append(reasons, fmt.Sprintf("rule %q lengthened %d -> %d days", ruleName(r), r.ExpirationDays, nr.ExpirationDays))
+				}
+				continue
+			}
+		}
+		// Pass 2: predicate match (prefix + size filters).
+		matched := false
+		for i, nr := range newRules {
+			if used[i] {
+				continue
+			}
+			if nr.Prefix == r.Prefix && nr.FilterSizeGreaterThan == r.FilterSizeGreaterThan && nr.FilterSizeLessThan == r.FilterSizeLessThan {
+				used[i] = true
+				// Same coverage; only days can differ. An ID-only
+				// rename with unchanged days is not a warning.
+				if nr.ExpirationDays > r.ExpirationDays {
+					reasons = append(reasons, fmt.Sprintf("rule %q lengthened %d -> %d days", ruleName(r), r.ExpirationDays, nr.ExpirationDays))
+				}
+				matched = true
+				break
+			}
+		}
+		if !matched {
+			reasons = append(reasons, fmt.Sprintf("rule %q removed or disabled", ruleName(r)))
+		}
+	}
+	if len(reasons) == 0 {
+		return ""
+	}
+	sort.Strings(reasons)
+	return "fast path enabled; objects already written keep their baked-in volume TTL: " + strings.Join(reasons, "; ")
+}
+
+// fastpathEligibleRules parses lifecycle XML and returns the subset of
+// rules the per-write fast path would stamp: Enabled, ExpirationDays>0,
+// no tag filter, and ExpirationDays fits int32 seconds. Mirrors the
+// filter in NewLifecycleTTLResolver so the warning fires exactly for
+// rules that could have stamped objects. A nil/empty XML yields nil.
+func fastpathEligibleRules(xmlBytes []byte) []*s3lifecycle.Rule {
+	if len(xmlBytes) == 0 {
+		return nil
+	}
+	rules, err := lifecycle_xml.ParseCanonical(xmlBytes)
+	if err != nil {
+		return nil
+	}
+	out := make([]*s3lifecycle.Rule, 0, len(rules))
+	for _, r := range rules {
+		if r == nil || r.Status != s3lifecycle.StatusEnabled {
+			continue
+		}
+		if r.ExpirationDays <= 0 {
+			continue
+		}
+		if len(r.FilterTags) > 0 {
+			continue
+		}
+		if int64(r.ExpirationDays)*secondsPerDay > math.MaxInt32 {
+			continue
+		}
+		out = append(out, r)
+	}
+	return out
+}
+
+func ruleName(r *s3lifecycle.Rule) string {
+	if r.ID != "" {
+		return r.ID
+	}
+	if r.Prefix != "" {
+		return "prefix=" + r.Prefix
+	}
+	return "(whole-bucket)"
+}

weed/s3api/s3api_bucket_lifecycle_fastpath_warn_test.go

@@ -0,0 +1,187 @@
+package s3api
+
+import (
+	"strings"
+	"testing"
+)
+
+// lcXML builds a minimal lifecycle config XML with one Enabled
+// Expiration.Days rule. prefix may be "" for a whole-bucket rule.
+func lcXML(id, prefix string, days int) []byte {
+	var b strings.Builder
+	b.WriteString("<LifecycleConfiguration><Rule>")
+	if id != "" {
+		b.WriteString("<ID>" + id + "</ID>")
+	}
+	b.WriteString("<Status>Enabled</Status>")
+	if prefix != "" {
+		b.WriteString("<Filter><Prefix>" + prefix + "</Prefix></Filter>")
+	}
+	b.WriteString("<Expiration><Days>")
+	// days is small in tests; fmt-free int->string
+	b.WriteString(itoa(days))
+	b.WriteString("</Days></Expiration></Rule></LifecycleConfiguration>")
+	return []byte(b.String())
+}
+
+func itoa(n int) string {
+	if n == 0 {
+		return "0"
+	}
+	var buf [12]byte
+	i := len(buf)
+	neg := n < 0
+	if neg {
+		n = -n
+	}
+	for n > 0 {
+		i--
+		buf[i] = byte('0' + n%10)
+		n /= 10
+	}
+	if neg {
+		i--
+		buf[i] = '-'
+	}
+	return string(buf[i:])
+}
+
+func TestFastpathWarn_Lengthened(t *testing.T) {
+	old := lcXML("r1", "logs/", 7)
+	new := lcXML("r1", "logs/", 30)
+	got := fastpathConfigChangeLeavesStampedObjects(old, new)
+	if got == "" {
+		t.Fatalf("lengthen 7->30 must warn")
+	}
+	if !strings.Contains(got, "lengthened") || !strings.Contains(got, "7 -> 30") {
+		t.Fatalf("unexpected reason: %q", got)
+	}
+}
+
+func TestFastpathWarn_Shortened_NoWarn(t *testing.T) {
+	old := lcXML("r1", "logs/", 30)
+	new := lcXML("r1", "logs/", 7)
+	if got := fastpathConfigChangeLeavesStampedObjects(old, new); got != "" {
+		t.Fatalf("shorten 30->7 must not warn, got %q", got)
+	}
+}
+
+func TestFastpathWarn_Removed(t *testing.T) {
+	old := lcXML("r1", "logs/", 7)
+	// new config has a different rule; r1 is gone
+	new := lcXML("r2", "data/", 30)
+	got := fastpathConfigChangeLeavesStampedObjects(old, new)
+	if got == "" {
+		t.Fatalf("removing an eligible rule must warn")
+	}
+	if !strings.Contains(got, "removed or disabled") || !strings.Contains(got, "r1") {
+		t.Fatalf("unexpected reason: %q", got)
+	}
+}
+
+func TestFastpathWarn_Delete(t *testing.T) {
+	old := lcXML("r1", "logs/", 7)
+	got := fastpathConfigChangeLeavesStampedObjects(old, nil)
+	if got == "" {
+		t.Fatalf("delete with an eligible rule must warn")
+	}
+	if !strings.Contains(got, "removed or disabled") {
+		t.Fatalf("unexpected reason: %q", got)
+	}
+}
+
+func TestFastpathWarn_Unchanged_NoWarn(t *testing.T) {
+	old := lcXML("r1", "logs/", 7)
+	new := lcXML("r1", "logs/", 7)
+	if got := fastpathConfigChangeLeavesStampedObjects(old, new); got != "" {
+		t.Fatalf("identical config must not warn, got %q", got)
+	}
+}
+
+func TestFastpathWarn_NoOldRules_NoWarn(t *testing.T) {
+	// fastpath off path: old had no eligible rules -> no warning even on delete
+	if got := fastpathConfigChangeLeavesStampedObjects(nil, nil); got != "" {
+		t.Fatalf("no old rules must not warn, got %q", got)
+	}
+	new := lcXML("r1", "logs/", 7)
+	if got := fastpathConfigChangeLeavesStampedObjects(nil, new); got != "" {
+		t.Fatalf("adding a rule (no old eligible) must not warn, got %q", got)
+	}
+}
+
+func TestFastpathWarn_TagFilteredRule_NoWarn(t *testing.T) {
+	// Tag-filtered rules are never on the fast path; removing one must not warn.
+	old := []byte(`<LifecycleConfiguration><Rule><ID>rt</ID><Status>Enabled</Status>` +
+		`<Filter><Tag><Key>k</Key><Value>v</Value></Tag></Filter>` +
+		`<Expiration><Days>7</Days></Expiration></Rule></LifecycleConfiguration>`)
+	if got := fastpathConfigChangeLeavesStampedObjects(old, nil); got != "" {
+		t.Fatalf("tag-only rule removal must not warn, got %q", got)
+	}
+}
+
+func TestFastpathWarn_DisabledInNew_Warns(t *testing.T) {
+	old := lcXML("r1", "logs/", 7)
+	new := []byte(`<LifecycleConfiguration><Rule><ID>r1</ID><Status>Disabled</Status>` +
+		`<Filter><Prefix>logs/</Prefix></Filter>` +
+		`<Expiration><Days>7</Days></Expiration></Rule></LifecycleConfiguration>`)
+	got := fastpathConfigChangeLeavesStampedObjects(old, new)
+	if got == "" {
+		t.Fatalf("disabling an eligible rule must warn")
+	}
+}
+
+func TestFastpathWarn_PrefixChanged_Warns(t *testing.T) {
+	// Same ID, narrower prefix: old objects under "logs/" keep the old TTL
+	// but the new rule only covers "logs/2026/". Treated as removed for the
+	// old coverage -> warn.
+	old := lcXML("r1", "logs/", 7)
+	new := lcXML("r1", "logs/2026/", 7)
+	got := fastpathConfigChangeLeavesStampedObjects(old, new)
+	if got == "" {
+		t.Fatalf("prefix change must warn")
+	}
+}
+
+func TestFastpathWarn_OverflowDays_NoWarn(t *testing.T) {
+	// ~68 years overflows int32 seconds; such a rule is never on the fast
+	// path, so changing it must not warn.
+	big := []byte(`<LifecycleConfiguration><Rule><ID>rb</ID><Status>Enabled</Status>` +
+		`<Filter><Prefix>logs/</Prefix></Filter>` +
+		`<Expiration><Days>30000</Days></Expiration></Rule></LifecycleConfiguration>`)
+	if got := fastpathConfigChangeLeavesStampedObjects(big, nil); got != "" {
+		t.Fatalf("overflow rule must not warn, got %q", got)
+	}
+}
+
+func TestFastpathWarn_IDOnlyRename_NoWarn(t *testing.T) {
+	// Renaming a rule while keeping the same prefix, size filters, and
+	// days must not warn: the policy is unchanged, only the label moved.
+	old := lcXML("r1", "logs/", 7)
+	new := lcXML("r2", "logs/", 7)
+	if got := fastpathConfigChangeLeavesStampedObjects(old, new); got != "" {
+		t.Fatalf("ID-only rename must not warn, got %q", got)
+	}
+}
+
+func TestFastpathWarn_IDRenameAndLengthen_Warns(t *testing.T) {
+	// Renaming AND lengthening: the predicate match finds the successor
+	// and the days increase triggers a lengthened warning.
+	old := lcXML("r1", "logs/", 7)
+	new := lcXML("r2", "logs/", 30)
+	got := fastpathConfigChangeLeavesStampedObjects(old, new)
+	if got == "" {
+		t.Fatalf("rename + lengthen must warn")
+	}
+	if !strings.Contains(got, "lengthened") || !strings.Contains(got, "7 -> 30") {
+		t.Fatalf("unexpected reason: %q", got)
+	}
+}
+
+func TestFastpathWarn_IDRenameAndShorten_NoWarn(t *testing.T) {
+	// Renaming AND shortening: not the data-loss direction.
+	old := lcXML("r1", "logs/", 30)
+	new := lcXML("r2", "logs/", 7)
+	if got := fastpathConfigChangeLeavesStampedObjects(old, new); got != "" {
+		t.Fatalf("rename + shorten must not warn, got %q", got)
+	}
+}