Allow Days=0 in lifecycle rules (relax day-based actions to nonnegative)#2648
Allow Days=0 in lifecycle rules (relax day-based actions to nonnegative)#2648delthas wants to merge 3 commits into
Conversation
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/8.4 #2648 +/- ##
===================================================
+ Coverage 74.00% 74.04% +0.03%
===================================================
Files 229 229
Lines 18503 18504 +1
Branches 3820 3837 +17
===================================================
+ Hits 13694 13702 +8
+ Misses 4804 4797 -7
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
delthas
force-pushed
the
improvement/ARSN-597/allow-days-0-lifecycle-rules
branch
from
f5d43ab to
72e5d59
Compare
|
LGTM |
There was a problem hiding this comment.
to be confirmed by product team : can this be allowed everywhere, or do we need to make this opt-in so it can be selectively allowed?
| const error = this._checkDays({ | ||
| days: daysInt, | ||
| field: 'NoncurrentDays', | ||
| ancestor: 'NoncurrentVersionExpiration', |
There was a problem hiding this comment.
I don't remember exactly the design but should we allow everything to be 0 days ? My point is : if we want to empty a bucket, maybe we should only allow to expire "current version" and first clean everything else after one day ? It's a matter of safety but allowing 0 everywhere give more flexibility to our customer.
There was a problem hiding this comment.
I'd say more flexibility, and safety can be done with a UI warning etc. Currently users can mess up their install badly if they do weird stuff with the API anyway.
delthas
force-pushed
the
improvement/ARSN-597/allow-days-0-lifecycle-rules
branch
from
72e5d59 to
4f0ac7f
Compare
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
Pure formatting pass (no logic change) on the files touched by the Days=0 change, so the prettier CI check (which runs on whole files modified by the PR) passes. Isolating the reformat here keeps the functional commit's diff reviewable. Issue: ARSN-597
S3 lifecycle day-based actions previously required a value >= 1. Accept 0 (nonnegative) for Expiration Days, NoncurrentVersionExpiration NoncurrentDays, and AbortIncompleteMultipartUpload DaysAfterInitiation by routing them through the existing _checkDays() helper. This also adds the MAX_DAYS upper bound these three actions lacked, consistent with Transitions. Days=0 is an explicit "empty this bucket" intent: because it was rejected before, it is an unambiguous signal (unlike Days=1, which legitimate short-retention buckets use) that backbeat can key on to skip building costly lifecycle indexes (BB-779). Also fix falsy-zero handling so a Days=0 rule is preserved end to end: - _parseAction copied action times with `if (action[t])`, dropping days=0 - getConfigXml serialized Expiration Days with `days ? ...`, dropping 0 - _validateRules skipped the type assertion for days=0 - LifecycleUtils.getApplicableRules guards/comparisons dropped 0 before it could be selected as the earliest applicable action Issue: ARSN-597
A 0-day action time (now accepted) triggers immediate processing, e.g. emptying a bucket. Emit an audit log when one is configured so an accidental or malformed rule is traceable. Thread a werelogs logger into the LifecycleConfiguration constructor (mirroring ReplicationConfiguration) and log from the shared _checkDays helper, which covers Expiration, NoncurrentVersionExpiration, AbortIncompleteMultipartUpload and Transition day actions. Callers must now pass a logger; the CloudServer caller is updated separately. Issue: ARSN-597
delthas
force-pushed
the
improvement/ARSN-597/allow-days-0-lifecycle-rules
branch
from
4f0ac7f to
7d3bb94
Compare
4 participants
What
Allow
Days = 0in S3 lifecycle rules. Previously the day-based actions required a value>= 1; this accepts0(nonnegative) for:Expiration→DaysNoncurrentVersionExpiration→NoncurrentDaysAbortIncompleteMultipartUpload→DaysAfterInitiationAll three now go through the existing
_checkDays()helper (already used byTransition/NoncurrentVersionTransition), which accepts0..MAX_DAYSand returnsmust be nonnegative/must not exceederrors. As a side effect this adds theMAX_DAYS(2³¹−1) upper bound that these three actions previously lacked, consistent with the transition actions.Why
Days = 0is meant as an explicit "empty this bucket" intent. Backbeat (BB-779) keys on it to skip building the v2 lifecycle index, which is expensive (~5–10× its size in transient disk during the merge-sort) and pointless when a bucket is being drained.Days = 1can't serve this purpose because legitimate short-retention buckets (logs, etc.) use it. SinceDays = 0was rejected everywhere until now, it is an unambiguous marker that cannot collide with any previously-valid configuration.This intentionally diverges from AWS S3 (which requires
Days >= 1) — that divergence is exactly what makes0a safe internal signal.Falsy-zero fixes
Accepting
0exposed several spots that treated the day value as falsy and would have silently dropped aDays = 0rule. Fixed so the rule is preserved end to end:_parseActioncopied action times withif (action[t])→ now!== undefinedgetConfigXmlserializedExpirationDayswithdays ? …→ nowdays !== undefined(otherwise GetBucketLifecycle returned anExpirationwith no action time)_validateRulesskipped the type assertion whendays === 0LifecycleUtils.getApplicableRulesguards/comparisons dropped0before it could be selected as the earliest applicable action — this is the path that feeds expiration decisionsScope
NewerNoncurrentVersionskeeps its>= 1rule (it is a count, not a number of days).Issue: ARSN-597