fix(s3): read every lifecycle rule-level key and translate the noncurrent transition day field

[go-to-k]

Closes #1388
Closes #1424

Summary

The CFn AWS::S3::Bucket lifecycle Rule has no Filter member at all —
every scope component and several actions sit at the rule level, and the schema
still accepts legacy singular action forms next to the modern plural ones.
applyLifecycleConfiguration read a subset. Enumerating all 16 Rule
members (from aws-cdk-lib's generated CfnBucket.RuleProperty) against what
the provider actually reads found 6 defects, not the 3 #1388 reported — and
the two worst are on the CURRENT CDK L2 path, not the legacy one the issue
describes.

The severe one: rule-level TagFilters (#1424)

gatherScope had rule-level fallbacks for Prefix, ObjectSizeGreaterThan
and ObjectSizeLessThan — but for tags it read only Filter.TagFilters:

prefix: (filter?.['Prefix'] ?? rule['Prefix']),
tagFilters: filter?.['TagFilters'],            // <- no `?? rule['TagFilters']`

An omission in an otherwise-consistent pattern (Filter is cdkd's own
accommodation for SDK-shaped / imported input).

A real cdk synth (aws-cdk-lib 2.244.0) of
lifecycleRules: [{ tagFilters: { env: 'prod', team: 'core' }, expiration: Duration.days(30) }]
emits TagFilters at the rule level. cdkd gathered no scope for that rule, so
it fell through to the catch-all Filter: { Prefix: '' } — a 30-day
expiration sent against every object in the bucket instead of the tagged
subset. Lifecycle expiration deletes objects, and cdkd reported success. This
is the CURRENT CDK L2 path, so the impact is not limited to the legacy /
hand-written templates #1388 describes.

Fixing the gather alone would have introduced a second silent drop. The
single-component branch was Filter: { Tag: tagFilters[0] }, and the SDK's
Tag member holds exactly one tag; multi-tag rules now take
And: { Tags: [...] }. That branch was unreachable before this PR precisely
because tags were never gathered, so it had to land in the same change.

Rule-level ExpiredObjectDeleteMarker (#1424)

expiredObjectDeleteMarker: true synthesizes to the rule level, but the
provider only read it from a nested Expiration object. A rule whose only
action is the delete-marker cleanup produced no Expiration at all, and S3
rejects the action-less rule. Now fills an otherwise-empty Expiration, and
deliberately does not override an explicit Days / Date (S3 forbids
combining them).

The plural path was broken too: TransitionInDays

Found while extending the integ fixture. CFn spells the noncurrent transition
day count TransitionInDays on both the singular and the plural
NoncurrentVersionTransitions[]; the SDK member is NoncurrentDays. The
provider read only the SDK spelling:

NoncurrentDays: nvt['NoncurrentDays'] as number | undefined,   // always undefined

So every CDK noncurrentVersionTransitions reached AWS with no schedule.
Its sibling Transitions already did TransitionInDays ?? Days; this makes the
pair consistent. Confirmed in aws-cdk-lib's
NoncurrentVersionTransitionProperty, in the SDK model, and in a real synth.

The legacy singular forms (#1388, as filed)

Transition, NoncurrentVersionTransition, and the scalar
NoncurrentVersionExpirationInDays. Singular and plural are concatenated
rather than treated as alternatives, since the schema permits both on one rule;
for the expiration pair the modern object wins over the legacy scalar.

Full 16-member comparison

CFn Rule member	read before
AbortIncompleteMultipartUpload	yes
ExpirationDate / ExpirationInDays	yes
ExpiredObjectDeleteMarker	no (#1424)
Id / Status / Prefix	yes
NoncurrentVersionExpiration	yes
NoncurrentVersionExpirationInDays	no (#1388)
NoncurrentVersionTransition	no (#1388)
NoncurrentVersionTransitions	name yes, day field no (TransitionInDays)
ObjectSizeGreaterThan / ObjectSizeLessThan	yes
TagFilters	no (#1424)
Transition	no (#1388)
Transitions	yes

Test plan

tests/unit/provisioning/s3-bucket-provider-lifecycle-rule-keys.test.ts — 14
cases. The fixtures are copied verbatim from the real cdk synth output above
rather than hand-authored, so they pin the shape CDK actually emits (no
Filter wrapper anywhere).

Coverage: multi-tag And.Tags scoping and the explicit assertion that the
catch-all Filter: { Prefix: '' } is NOT what we send; every tag preserved;
single-tag Tag form; tags combined with a prefix; delete-marker rule not
action-less; delete-marker not overriding explicit Days; each legacy singular
form; singular+plural concatenation; modern object beating the legacy scalar;
plus two no-regression cases (prefix-only stays V1 with a bare top-level
Prefix; an explicit Filter.TagFilters still works for imported input).

Plus the two NoncurrentVersionTransitions cases: the CFn TransitionInDays
spelling a real synth emits, and the SDK NoncurrentDays spelling that must
keep working for imported input.

Revert-proof: restoring s3-bucket-provider.ts from origin/main fails
exactly 9 of the 14. The 5 that still pass are the two no-regression cases,
the two "already worked" cases, and the SDK-spelling fallback — the expected
split. The restore was verified byte-identical to the commit afterwards.

Full local gate: typecheck, lint, build, 522 files / 8919 tests,
vp run gen:all-matrices clean (the fixture change regenerated
integ-coverage, included here).

Real-AWS verification

s3-lifecycle extended and run end to end (PASS, 58s, 2 deleted / 0 errors /
0 orphans). The fixture gained the tag-scoped rule, a plural noncurrent
transition, and an L1 CfnBucket carrying the legacy singular forms plus a
rule-level ExpiredObjectDeleteMarker — shapes the L2 construct cannot emit.
verify.sh asserts each against a real
get-bucket-lifecycle-configuration readback (tags sorted, since AWS does not
preserve list order):

tag-scoped rule: both tags applied via Filter.And.Tags, no whole-bucket fallback
noncurrent-version transition schedule reached AWS (NoncurrentDays=15)
legacy singular Transition + NoncurrentVersionTransition
  + NoncurrentVersionExpirationInDays + rule-level ExpiredObjectDeleteMarker all applied

The first run failed, and the failure is the strongest evidence in this PR:

'NoncurrentDays' in the NoncurrentVersionExpiration action must be greater than
'NoncurrentDays' in the NoncurrentVersionTransition action

AWS could only say that once the day count actually reached it — the pre-fix
binary sent nothing there to compare. The fixture's values were genuinely
invalid and were corrected.

Also hardens the post-destroy bucket probe with a bounded retry: S3 propagates
DeleteBucket to HeadBucket asynchronously and a single probe raced it
(both buckets were in fact gone). A bucket that never disappears still FAILs,
so leak detection is unchanged.

Review fix-backs

A 3-axis review ran on the first version. No blockers; every finding is
addressed here, and the integ was re-run afterwards (PASS, 2 deleted / 0 errors
/ 0 orphans). Independently confirmed by the reviewers: the 16-member
enumeration is complete (no 7th gap), the unit fixtures match a real synth
byte-for-byte, and the 9-of-14 revert-proof split holds.

• Plural transitions now WIN over the legacy singular instead of being
  concatenated. Concatenating can emit two transitions with the same
  StorageClass, which S3 rejects (Found two transitions with the same storage class), failing the whole PutBucketLifecycleConfiguration — a regression,
  since pre-fix such a template deployed with the singular simply ignored. It
  also now matches the NoncurrentVersionExpiration policy chosen 30 lines
  above.
• ExpiredObjectDeleteMarker gates on Days/Date, not on Expiration existing.
  The nested branch emits an all-undefined object for an empty Expiration, so
  the existence check dropped the marker AND left the rule action-less — the
  exact failure the block exists to prevent. A genuine Days + marker conflict
  now warns instead of dropping silently.
• readLifecycle reverse-maps to CFn's TransitionInDays, matching its
  Transitions sibling. Emitting the SDK's NoncurrentDays made cdkd drift
  report a permanent phantom diff on every versioned bucket with a noncurrent
  transition. Latent until this PR: the write side never delivered the value, so
  both sides were empty and agreed by accident.
• isPlainObject / coerceCfnNumber / coerceCfnBoolean. typeof x === 'object' accepts arrays and null, so a Transition: [] became an entry
  with no StorageClass and S3 answered MalformedXML for the whole config;
  and CFn is stringly typed, so "365" / "true" is exactly what the
  hand-written templates these legacy branches serve actually carry.

8 further unit tests (22 total).

Deliberately out of scope — filed as #1430 and #1423

#1430 — AWS::S3::Bucket is not in NESTED_KEY_TARGETS, so the nested-key
critic does not guard this provider; it would have caught 3 of the 6 defects
mechanically. Deferred because the schema fixture must be re-captured first
(it predates the nestedProperties capture) and because the first run audits
every nested blob in a ~2,500-line provider — its own work, with its own review
surface. Same call as the EMR deferral in #1393.

#1423 —

removing a per-GSI on-demand limit from a template silently no-ops (the old
value stays in AWS; CFn would reset it). Same absent-field-reset class as
#1160. The blocking unknown there — whether -1 is the reset sentinel for the
per-GSI Update action — has since been settled by a live AWS probe and
recorded on the issue, so it is ready to implement. Unrelated file
(dynamodb-globaltable-provider.ts).

[github-actions]

🎉 This PR is included in version 0.278.13 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀