The LoadBucketMetadata peer-notification handler loaded bucket metadata
with the fabricating loader (ConfigNotFound -> BucketMetadata::new) and
unconditionally cached the result. On a transient read-quorum dip during
a reload notification, a peer cached an authoritative "no Object Lock"
default for a lock-enabled bucket, disabling the batch-delete retention
gate (object_lock_delete_check_required) on that node until the next
refresh, and wiping its bucket-target/durability sync state.
Production changes:
- New BucketMetadataSys::reload_from_store (metadata_sys::
reload_bucket_metadata): the peer reload path uses the presence-aware
loader and installs only metadata actually read from persisted
storage. A load miss returns an error (surfaced to the notifying peer
as success=false) and leaves the cache untouched; deletion still
propagates only through the dedicated DeleteBucketMetadata
notification.
- The reload runs under the outer metadata-sys write guard, load
included, mirroring update(): every other cache installer holds that
lock, so a reload snapshot can never land after - and roll back - a
newer concurrent install (the stale-load lost-update from the
review), and the install-plus-registry-sync sequence stays atomic
against concurrent removes and reloads (previously only the set call
was write-guarded, with the load outside any lock).
- The peer-visible miss error is a fixed string: the notifying peer
substring-matches error text against network-failure needles
(is_network_like_error), so interpolating a bucket name (e.g. a legal
bucket literally named "unavailable") could mark a healthy peer
offline.
- get_config's lazy insert routes through set(), picking up the
negative-cache invalidation.
An earlier draft instead guarded set() with a per-config updated_at
freshness comparison. Adversarial validation rejected it (three roles
independently): update_config stamps with the handling node's wall
clock, so within the skew the cluster already tolerates (+/-300s RPC
auth window) a config rewritten with an earlier stamp - e.g. revoking a
public-read policy through a second node, or any same-field rewrite
after an NTP step-back - would be skipped by every peer forever,
silently pinning the revoked permissive config with no re-convergence
path (the 15-minute refresh also routed through the guard). Race
staleness is second-scale while skew is minute-scale, so no tolerance
bound can separate them; the write-guard serialization closes the same
race without clocks and preserves the refresh loop's unconditional
converge-to-disk property, which is the cluster's self-healing
mechanism.
Startup audit (BucketMetadataSys::init): concurrent_load's
insert-if-vacant still installs a fabricated default when a transient
miss hits at boot - indistinguishable from a legacy bucket without a
metadata file at this layer - bounded by the next successful persisted
load. Making the object-lock gate fail closed on such entries is filed
as a follow-up, alongside the bare "unavailable" needle in
is_network_like_error and the Swift cache-only metadata writes.
Tests:
- bucket::metadata_sys::tests::
peer_reload_never_caches_fabricated_defaults_as_authoritative:
miss installs nothing / miss keeps the existing entry intact
(asserting the dedicated non-persisted error) / persisted reload
converges the cache over a stale entry.
- node_service::tests::
test_load_bucket_metadata_failure_skips_scanner_maintenance:
a failed reload reports failure and does not advance scanner
maintenance activity (previously recorded even on a miss).
- The handler success path stays uncovered at the RPC layer (needs an
isolated global object layer, like the pre-existing ignored test);
the composition is pinned at the sys level instead.
Verification:
- cargo fmt --check and cargo clippy --lib --tests clean on
rustfs-ecstore and rustfs.
- Targeted suites green; full cargo test -p rustfs-ecstore --lib:
3198/3200 with two parallelism-sensitive lock-test flakes from the
known baseline (pass in isolation; a different pair flakes per run).
- Adversarial validation (high-risk tier, all seven roles as
independent parallel reviewers) run per AGENTS.md; all findings
fixed or rebutted with evidence, three out-of-scope findings filed
as follow-up tasks.
Co-authored-by: houseme <housemecn@gmail.com>
Swift container and account metadata handlers cloned the cached
BucketMetadata, set the tagging fields, and called set_bucket_metadata,
which only updates the in-memory cache map. Nothing reached
.metadata.bin, so every Swift metadata POST was lost on restart and
silently overwritten by the next disk-truth reload (a peer
LoadBucketMetadata notification or the 15-minute refresh loop) — while
the client had already been told 2xx.
Route these writes through a new metadata_sys::update_config_with: a
read-modify-write that loads the on-disk metadata and persists the
result under the same write guard metadata_sys::update uses, so the
rewrite merges against disk truth instead of a possibly stale cache and
cannot clobber a concurrent update to another config file. Peers are
notified afterwards, matching the S3 config handlers.
Persisting these writes required hardening the paths that now produce
durable state:
- Account metadata writes validate account ownership. This metadata
holds the account's TempURL signing key, so an unauthenticated write
for someone else's account would have become a durable, cluster-wide
takeover of that account's pre-signed URLs. Reads stay open because
TempURL signature validation runs before credentials exist.
- disable_versioning verifies the container exists. Without it the
metadata loader's "no metadata on disk" default would be persisted,
creating an orphan metadata file and caching a fabricated default as
authoritative.
- Container and account metadata are size- and count-limited, reusing
the Swift limits object metadata already enforces; these tags land in
the bucket metadata file that every later config write rewrites whole.
- A rewrite refuses to run when the persisted tagging config is
unreadable, instead of merging onto an empty set and wiping the
container ACL and versioning tags. It reports 409 naming the remedy.
- Storage errors are logged in full and reported generically, since they
now carry real disk and quorum detail.
The tagging arm of BucketMetadata::update_config also clears the parsed
config, as the lifecycle arm does: parse_all_configs skips empty XML
rather than clearing, so a cleared config kept serving the old tags.
Tagging is serialized with the S3 XML serializer the loader can parse
back, not quick_xml, whose output was never round-trippable.
* feat(kms): implement secure handling of static KMS secret keys and enhance encryption context validation
* feat: enhance local SSE DEK handling with JSON envelope format and versioning
Align inline fast path cluster tests with the current tier mutation and manual transition contracts while keeping msgpack compat assertions focused on observed traffic.
Co-authored-by: heihutu <heihutu@gmail.com>
cxymds