* test(site-replication): expect MinIO sts-account IAM item type
MinIO madmin-go replicates STS credentials with SRIAMItem type
"sts-account" (SRIAMItemSTSAcc), but RustFS emits and accepts only
"sts-credential", so cross-implementation STS replication fails in
both directions (MinIO returns errSRInvalidRequest, RustFS returns
NotImplemented).
Red-light tests:
- pin the outbound AssumeRole replication item type to "sts-account"
(construction extracted into assume_role_site_replication_item so it
is testable, behavior unchanged in this commit)
- update the federated identity replication item snapshot to
"sts-account"
- inbound apply_iam_item must dispatch both "sts-account" and the
legacy "sts-credential" alias to the STS arm instead of the
unknown-type NotImplemented fallback
* fix(site-replication): use MinIO-compatible sts-account IAM item type
MinIO madmin-go replicates STS credentials with SRIAMItem type
"sts-account" (SRIAMItemSTSAcc). RustFS emitted "sts-credential" and
accepted only that value inbound, so STS credential replication with
MinIO peers failed in both directions: MinIO rejected RustFS items as
errSRInvalidRequest and RustFS answered MinIO items with
NotImplemented.
- define SR_IAM_ITEM_STS_ACC ("sts-account") and
SR_IAM_ITEM_STS_ACC_LEGACY ("sts-credential") in rustfs-madmin
- emit "sts-account" from both outbound sites (AssumeRole hook and
federated identity OIDC hook)
- accept both types inbound; the legacy alias remains permanently for
mixed-version RustFS rolling upgrades
Token verification and the retry/event mechanism are unchanged.
* test(replication): accept madmin nanosecond healthCheckDuration payloads
Red-phase TDD tests for P0-7: mc 'replicate add' sends the madmin default
healthCheckDuration=60s as a Go time.Duration nanosecond integer
(60000000000), which RustFS currently rejects as an unsupported field and
would misread as seconds. Also pins the defensive seconds-or-nanos read
for persisted bucket-targets metadata and the capability contract listing
healthCheckDuration as writable.
Currently failing (red):
- remote_target_request_accepts_go_duration_wire_values
- remote_target_request_accepts_legacy_seconds_health_check
- remote_target_health_check_duration_is_declared_writable
- bucket_target_reads_go_nanosecond_durations_defensively
- runtime_capabilities_response_reports_missing_topology_before_storage_init
* fix(replication): accept remote target healthCheckDuration nanoseconds
mc 'replicate add' always sends the madmin default healthcheck-seconds=60
serialized as a Go time.Duration nanosecond integer (60000000000), so the
default mc link-creation path (and 'mc replicate update') failed with
InvalidRequest. Move healthCheckDuration from the unsupported to the
writable remote-target field list; the capability contract in the runtime
capabilities response follows the constants automatically.
Fix the unit mismatch in both directions:
- Request parsing and persisted bucket-targets reads decode the value
defensively: below 10^7 it is legacy RustFS seconds, otherwise Go
time.Duration nanoseconds (also covers MinIO-written metadata).
totalDowntime shares the same wire shape and gets the same handling.
- The list-remote-targets admin response re-encodes only these two fields
as nanoseconds via a dedicated serialization path, leaving the persisted
seconds-based wire format untouched for existing readers.
The per-target health-check interval is accepted for mc compatibility but
not yet applied; the heartbeat keeps its global env-configured interval,
and the explicit 'healthcheck' update op stays rejected. disableProxy,
edge, and edgeSyncBeforeExpiry remain explicitly rejected.
* chore(deps): refresh mimalloc revision
Update mimalloc and libmimalloc-sys to the requested git revision after running the dependency refresh flow.
Keep ratelimit excluded while accepting compatible dependency updates from cargo update and cargo upgrade.
Harden all-feature test compilation by giving heavy integration test crates their own recursion limit and avoiding a cross-thread spawn for the embedded startup barrier future.
Co-Authored-By: heihutu <heihutu@gmail.com>
* upgrade version
---------
Co-authored-by: heihutu <heihutu@gmail.com>
PR #5719 fixed the issue #5716 per-object heal log amplification (per-object statements demoted, heal spans forced to TRACE, raw metadata dumps banned by guardrail) and superseded the demotion originally proposed here. This PR now carries only the residual cleanup on top of it:
- Convert the remaining bare-field and format-arg heal logs in crates/ecstore/src/set_disk/ops/heal.rs to the file's structured convention (event/component/subsystem + context fields): missing-object skip, disk-marked-for-healing, cannot-reconstruct errors, dangling-cleanup error, missing data_dir error, xl.meta regeneration warn, and orphan-reclaim failure warn.
- Demote the last remaining info! in the file — the per-set heal_format "set disk formats success, NoHealRequired" no-op message — to a structured debug! (error_count instead of a raw errs dump), and drop its whitelist exclusion in scripts/check_logging_guardrails.sh so the no-INFO check for set-disk heal files is strict.
No control flow or behavior changes.
Multipart upload ids embed the process-global deployment id at both create time and list time. Under plain cargo test (thread-parallel, shared process globals) a concurrently running test that re-initializes a store can swap the global between the two reads, making full-upload-id equality assertions fail spuriously (observed: core::sets::tests::list_multipart_uploads_merges_all_sets_without_pagination_loss failing when run concurrently with bucket::quota tests, passing in isolation).
Add a test-only upload_uuid_suffix helper next to deployment_upload_id and make the affected assertions compare only the decoded <uuid>x<timestamp> suffix. Where suffix normalization changes within-key ordering (base64 alphabet order is not byte order), both sides are sorted before comparison. nextest/CI is unaffected (process-per-test); this only hardens local plain cargo test runs.
initialize_local_disk_maps appended pool entries to local_disk_set_drives and inserted into local_disk_map without ever clearing previous state. Every caller (both production startup entry points and all tests) passes the FULL topology, so re-initializing the same InstanceContext left the pool/set vectors sized for the stale topology and panicked with index-out-of-bounds for wider disk indices.
This surfaced as deterministic cross-test contamination under single-process cargo test: in crates/heal heal_b920_subquorum_union_test, a 4-disk test initialized pool 0 as [None; 4] on the process-level default context, and the two 8-disk tests then panicked at disk_idx 4. Each test passed alone, and CI never caught it because cargo nextest isolates every test in its own process.
Fix: clear both registries at the start of initialize_local_disk_maps so initialization is idempotent and last-topology-wins. Add a regression unit test in ecstore (process-isolation-proof, unlike the heal integration binary) that re-initializes the same context with a wider topology; it fails with the pre-fix code.
The multipart staging namespace is one flat set of sha256(bucket/object) directories shared by every bucket, and the cross-set listing rewrite reads every upload's metadata. Two shapes poisoned the whole ListMultipartUploads response with InternalError: Corrupted format: a healthy in-flight upload belonging to another bucket (its stored owner bucket fails the guard and fell into the corrupted-format arm), and a single upload directory whose xl.meta was torn by an unclean shutdown. Docker Distribution calls ListMultipartUploads on every PATCH/commit, so either shape broke OCI registry pushes entirely (issue #5716).
Foreign-bucket uploads are now skipped silently, and directories whose metadata is affirmatively corrupt at quorum are skipped with a debug log, while every other decode failure (quorum loss from offline disks, timeouts, transport errors) keeps failing the listing so clients retry instead of silently losing entries. The degrade-vs-propagate decision is a named corrupt-family classifier with a unit test pinning both sides. FileMeta::check_xl2_v1 now classifies a missing or wrong XL2 magic as FileCorrupt instead of an anonymous io error so damage is distinguishable from transient IO faults.
Refs #5716
#5724 reclaimed the synthetic inline-rollback dir after a committed rename with delete_data_dir(recursive: true), which has no notion of object metadata: for unversioned objects the synthetic UUID is a fixed, publicly-known constant, so object/<rollback-dir> can simultaneously be a legitimate child key's directory, and recursively deleting it reopens the authorization bypass #5703 closed (PutObject on K destroying K/<uuid> without DeleteObject permission).
Replace the recursive pass with a file-precise one: after quorum commit, delete exactly object/<rollback>/xl.meta.bkp with a non-recursive delete on every disk whose rollback dir is not also the cleanup dir. The parent-rmdir walk removes the dir only when the backup was its sole content, so the BucketNotEmpty leak fix is preserved (#5724's regression test passes unchanged) while a child key at the same path keeps its metadata. The undo path's restore_metadata_backup now also reclaims the emptied synthetic dir, mirroring restore_delete_rollback.
The fallback future embeds the whole snapshot loader, and every object write nests a quota check several futures deep, so inlining it grew each write's state machine by the loader's full size — the debug-build 2MiB worker-stack overflow class fixed for bucket-config writes in #5648. Box the fallback at its call site; the allocation only happens on the degraded path.
fix(heal): demote per-object logs and cap erasure-set failure warns
Follow-up to rustfs/rustfs#5716. Per-object heal task kinds (Object/Metadata/MRF/ECDecode) queued by MRF/autoheal/scanner loops emitted info!/warn!/error! lines per object: task lifecycle (started/completed/timed_out/failed), the missing-object warn, queue admission full/drop/displacement warns, retry-admission decisions, and uncapped per-object warns in erasure-set sweeps.
Add a shared demote_to_debug_when! macro that keeps aggregate task kinds and admin/internal requests at operator-visible levels while demoting per-object occurrences to debug!, sample-cap the erasure-set transient_skip/failed warns per bucket via take_failure_log_sample (reusing the heal_bucket_objects precedent), demote the per-retry admission decision logs to debug! (covered by rustfs_heal_admission_total and the scheduler task_retrying/task_failed events), and record the previously unmetered duplicate-admission outcome.
Extend scripts/check_logging_guardrails.sh with injection-verified regression guards and run it in both quick-checks jobs (ci.yml and its ci-docs-only.yml mirror).
A failed target construction previously logged only reason=construction_failed and pushed an opaque "target construction failed" summary, discarding the underlying TargetError. Debugging rustfs#5115 showed the log repeated every 5s with no root cause, even though the error carried an actionable egress-policy rejection (RUSTFS_OUTBOUND_ALLOW_ORIGINS hint).
The Err branch now includes the error detail in both the error! log (detail field) and the returned failures summary. The detail is scrubbed against the instance's merged config via the loader's existing per-field redaction (secrets -> ***redacted***, endpoint URLs -> origin only, DSNs -> password masked) so credential-bearing values never reach logs or Admin-visible summaries.
FileInfo's derived Debug printed the full metadata map (including X-Rustfs/X-Minio-Internal-Server-Side-Encryption-Sealed-Key and -Iv values, i.e. KEK-wrapped DEK ciphertext) and the full inline data bytes (plaintext user content for non-SSE small objects), so any whole-struct log dump such as the heal_object dumps leaked user data and sealed key material into logs.
Replace the derive with a manual Debug impl that redacts encryption metadata values (keys stay visible, values print as redacted with length) under both internal prefixes, and elides data/checksum bytes to a length summary. The exhaustive destructuring forces every future field through an explicit show/redact decision. starts_with_ignore_ascii_case is made pub in rustfs-utils for reuse.
#5703 split rollback state from old-data-dir cleanup so the synthetic inline-rollback dir is reported only as rollback_data_dir and never reclaimed as if it were a real data dir. But nothing reclaims it after a successful commit either: every overwrite of an inline version by a non-inline one leaves <object>/<rollback-dir>/xl.meta.bkp behind. The residue is not referenced by any version, so it survives object deletion and DeleteBucket fails with BucketNotEmpty forever — the mass teardown cascade currently failing the S3 Implemented Tests CI lane.
Reclaim the synthetic dirs in SetDisks::rename_data once the commit holds write quorum. The quorum-failure undo inside the same function is the only consumer of the backup, so its window is closed at that point. Best-effort with the same anti-misdelete posture as commit_rename_data_dir: never touch the just-committed data dir, and residue must not fail a durable write (backlog#898).
* fix(replication): accept explicit STANDARD destination storage class
The replication engine never reads Rule.Destination.StorageClass (replica
placement comes from the bucket-target config or the source object), yet the
validator rejected any config carrying the field. The console's add-rule form
always sends StorageClass=STANDARD, so every rule created through it failed
with InvalidRequest.
Tolerate exactly STANDARD as a no-op — semantically identical to omitting
the field — and keep rejecting every other value, which would be silently
ignored rather than honored. Document the deliberate omission from the
replication capability contract.
* feat(admin): support MinIO-style partial updates for set-remote-target
set-remote-target?update=true previously replaced every stored field and
required complete credentials in the body, so flipping a target's sync mode
from the console forced operators to re-enter the secret key, and real
mc replicate update bodies (madmin Clone() strips the secret) failed to
deserialize at all.
Adopt MinIO's TargetUpdateType contract: query params creds/sync/bandwidth/
path name the field groups to overlay onto the stored target, everything
else keeps its persisted value, and unsupported groups (proxy, healthcheck,
edge, edgeSyncBeforeExpiry) fail loudly. Credentials updates are skipped for
site-replication peer targets — probed by both scheme derivations of the
stored endpoint and the stored deployment id — because an operator never
knows the site replicator's credentials, and a body-supplied deployment id
is ignored on update since it anchors peer identity. madmin JSON aliases
(bandwidthlimit, storageclass, resetID, deploymentID, sessionToken) let mc
bodies parse under deny_unknown_fields.
e2e: cover a credential-free sync-only update preserving the stored
connection and the zero-ops no-op contract; align the missing-arn assertion
with the earlier validation error.
* chore(scripts): add two-site replication lab manager
site_replication_smoke.py spawns and manages two local rustfs processes,
pairs them via the site-replication admin API (idempotent), and verifies
bidirectional object replication. Subcommands: up/down/restart/status/logs/
smoke/info/remove/clean. Stdlib-only; requests are SigV4-signed the same
way as crates/e2e_test.
* chore(scripts): rename direction-suffixed payload variables for typos check
The typos linter reads the _ba suffix in payload_ba as a misspelling of
"by"; use payload_a_to_b / payload_b_to_a instead.
---------
Co-authored-by: overtrue <anzhengchao@gmail.com>
Upgrading from a pre-v2 release leaves only the legacy .usage.json snapshot, which has no completeness marker and is demoted to non-authoritative, so every write to a quota-enabled bucket failed closed with a retryable 503 until the scanner's first complete cycle persisted .usage.v2.json — a production outage on large namespaces (issue #5716).
Quota admission now degrades to the last persisted per-bucket size: normalize_loaded_data_usage returns the pre-discard bucket sizes, the TTL-bounded snapshot cache retains them (carried forward through failed refreshes), and QuotaChecker::get_real_time_usage falls back to that baseline when the authoritative caches miss. The baseline is static between snapshot loads, so hard-quota enforcement is advisory for the duration of the degraded window — strictly tighter than beta.11 (usage treated as 0) and strictly more available than a blanket 503. Buckets absent from every persisted snapshot still fail closed, and removing a bucket's usage from the backend purges the baseline so a recreated bucket cannot inherit the dead incarnation's size.
Refs #5716
Both per-target replication methods assign the optimistic Completed status to rinfo before building put options, and the Err branch of replication_put_object_options returned rinfo unchanged. Since #5633 made the source encryption classification case-insensitive, managed SSE sources are rejected at this gate, and the rejection was reported as successful replication: the source object was marked COMPLETED, ObjectReplicationComplete was emitted, and nothing existed on the target.
Set replication_status = Failed (and record the error in the replicate_object branch) so the composite status, the OperationFailedReplication event, and MRF retries reflect the fail-closed outcome. This restores the contract pinned by test_bucket_replication_sse_kms_failure_contract, which timed out in the e2e-replication-nightly runs on 2026-08-03 and 2026-08-04.
* refactor(time): migrate audit and notify timestamps to jiff
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(ecstore): initialize heal walk decode error
Co-Authored-By: heihutu <heihutu@gmail.com>
* refactor(targets): parse MySQL event time with jiff
Preserve MySQL DATETIME(6) wall-time formatting for RFC3339 eventTime values while removing the direct chrono dependency from rustfs-targets.
Co-Authored-By: heihutu <heihutu@gmail.com>
* chore(deps): prune unused workspace dependencies
Apply cargo shear --fix to remove unused path-clean and s3select-api tempfile entries after the scoped jiff migration.
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(ecstore): remove duplicate heal walk decode error init
Remove the duplicate decode_error field from the heal walk test collector initializer so lib-test clippy compiles on CI.
Co-Authored-By: heihutu <heihutu@gmail.com>
* refactor(policy): emit OPA timestamps with jiff
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
When a returning node carries a stale object version that was deleted on
the quorum, the heal disk-walk partial callback used `resolve_union`
which picks only one entry from divergent disk entries. The minority
version was never enumerated and therefore never cleaned up.
Replace `resolve_union` + `ingest` with a new `ingest_merged` that
collects all unique versions from every partial entry across disks,
deduplicating by (name, version_id). This ensures stale data on a
returning node is surfaced for healing and can be deleted as dangling.
Fixes#5029
Classify expected metadata-missing errors separately from unknown get pipeline failures and attribute internal meta-bucket reader failures to an internal_meta path instead of legacy_duplex.
This keeps scanner/data-usage metadata probes from polluting user GET/mixed failure attribution while preserving the existing read error behavior.
Co-authored-by: heihutu <heihutu@gmail.com>
When a fresh multi-node cluster starts, the first disk detects all disks
as unformatted and initializes the format. However, `should_init_erasure_disks`
and `quorum_unformatted_disks` only counted `UnformattedDisk` errors. Remote
peers that have not yet started their gRPC server return transient network
errors (connection refused, timeout) instead of `UnformattedDisk`, causing
the first disk to miss the "all unformatted" signal and creating a deadlock:
first disk retries endlessly while non-first disks wait for it.
Add `is_unformatted_or_transient_network` that treats transient network
errors as equivalent to `UnformattedDisk` for the bootstrap decision.
A remote disk that cannot be reached during fresh-cluster startup is
indistinguishable from an unformatted disk — the peer may simply not
have started its gRPC server yet.
Fixes#5655
Large buckets with millions of objects can take longer than the default
5-second walkdir timeout to produce the first page of listing results.
This causes timeouts in the web UI and mc CLI when opening or scanning
such buckets.
Skip the walkdir total timeout for S3 ListObjects operations when no
explicit walkdir_timeout is configured. The stall timeout (5s with no
forward progress) still protects against drives that stop responding.
This matches the scanner's existing behavior for the same reason.
Fixes#5647
fix(ecstore): don't exclude pool when has_space_for is indeterminate
When `has_space_for` returns an error (not enough online disks to
reliably determine space), the old code treated this the same as
"definitely no space" via `unwrap_or_default()`, zeroing out the
pool's available capacity. During pool decommission this creates a
false "Disk full" (500) for S3 PUT requests:
- Pool 0 (decommissioning) is correctly suspended → available = 0
- Pool 1 (active) has some disks whose disk_info call fails under
heavy migration I/O → has_space_for returns Err → available forced
to 0
- get_available_pool_idx sees total = 0 → returns None → DiskFull
Fix: distinguish Ok(false) (genuinely full) from Err (indeterminate).
On Err, log a warning and fall through to compute available space from
whatever disks did respond. The actual write will enforce its own
quorum; a premature zero at the pool-selection layer is a false
rejection.
Closes#5497
Collapse per-shard blocking-pool round-trips into one spawn_blocking
pread batch when all online shards are local and mmap-read is enabled.
Co-authored-by: ba <ba@ubuntu-server.alpha30.bos16>
Co-authored-by: Zhengchao An <anzhengchao@gmail.com>
* fix: address rc.1 release blockers
* fix: route release guards through architecture boundaries
* fix: close remaining rc.1 regression gaps
* refactor: group multipart listing options
* fix: resolve rc.1 CI regressions
* fix(ecstore): keep bucket-config writes off the caller's stack
A bucket-config write nests incarnation resolution (which can drive legacy
migration and a peer fan-out), a full metadata load, and `save` — itself an
object PUT that pulls in the whole erasure write path. Every request that
mutates bucket config is already several futures deep, so inlining all of
that into one state machine overflows the 2MiB worker stack in debug builds.
Two CI lanes aborted with SIGABRT on this:
ILM Integration (serial)
rustfs app::lifecycle_transition_api_test::
compensation_driven_complete_multipart_upload_still_transitions
Test and Lint (swift)
rustfs-protocols::swift_metadata_persistence::
swift_metadata_writes_are_durable
Neither test file is touched by this branch and both lanes are green on
main. Stack-pointer probing showed ~780KiB consumed between
`metadata_sys::update` and the config read alone, with single hops of
363KiB (`update` -> `acquire_config_write_guard_for_incarnation`), 125KiB
and 105KiB.
Box the deep sub-futures on both read-modify-write paths (`update` /
`update_checked` and `update_config_with` / `update_config_with_checked`)
so each guard's own state machine stays small. Behaviour is unchanged;
`update` -> guard drops to 253KiB and both tests pass on the default stack.
* fix(lifecycle): unbreak restore under the bucket generation fence
The ILM lane aborted on a stack overflow before reaching these, so they
were never reported; with that fixed, four restore tests fail. All four
are green on main and none of their test files are touched by this branch.
1. RestoreObject and ListMultipartUploads hard-required
`opts.expected_bucket_incarnation_id`, but `apply_bucket_generation_guard`
deliberately leaves it unset when no guard extension is present — only the
S3 access layer installs one. Every direct caller therefore got
`InternalError: ... bucket generation guard is missing`. Resolve the
current generation instead, the way the copy path already does. The fence
is unaffected: RestoreObject still re-reads the incarnation from disk and
compares before admitting the restore, and the multipart listing is
filtered by the value it resolves.
2. `restore_expiry_snapshot_matches` (new on this branch) rejected every
restored-copy expiry whose `restore_expires` had not already elapsed.
Whether the restored copy is due to expire is the ILM evaluator's
decision, made when it emitted DeleteRestoredAction; re-deriving it in
the set layer only adds a way for a legitimate action to be rejected.
The stale-event risk it appears to guard is already covered by the
surrounding snapshot match — a re-restore rewrites `restore_expires`,
so a replayed event fails the equality check. Drop the clause; the
fifteen identity clauses are unchanged.
Fixed:
rustfs app::lifecycle_transition_api_test::
restore_object_usecase_accepts_exactly_one_of_two_concurrent_restores
restore_object_usecase_completes_suspended_null_version_in_place
restore_object_usecase_reports_ongoing_conflict
rustfs-scanner::lifecycle_integration_test serial_tests::
test_restore_chain_local_read_expiry_keeps_remote_and_allows_re_restore
Verification: the CI ILM lane filter now runs 53/53 green locally.
* chore: address review follow-ups on this branch
Four items from the adversarial review that were still open.
- Restore the assertion `test_bucket_replication_replayed_delete_marker_
preserves_source_mtime_without_source_restart` is named for. The branch
had replaced the backlog#867 mtime check with `assert_replication_
converged`, which any successful replication satisfies, and deleted the
two helpers it needed — so the regression the test exists to catch would
now pass. This matters here specifically because the branch changes the
flag feeding `replication_delete_remove_options` and routes replay
through a new file and ordering.
- Drop `read_config_no_lock_preserve_empty`: zero production callers (the
one real consumer calls the `_with_metadata` variant directly). Its test
stanza now exercises that variant, so the coverage moves to live code
rather than being deleted.
- Revert the `bytesize` bump. It is a no-op: `Cargo.lock` already pinned
2.7.0 before this branch and is untouched, so the caret range already
resolved there. Nothing in the diff uses the crate.
- Split the AGENTS.md "Adversarial Validation" policy change out of this
branch. The edit is defensible on its own, but it relaxes the review gate
that this branch has to pass, so it should land as its own PR reviewed on
its own merits rather than bundled with the change that benefits from it.
The reverted hunks are unchanged and ready to re-apply.
Not changed, deliberately: the missing-sidecar path still fails closed.
`missing_bucket_incarnation_sidecar_for_new_metadata_fails_closed` pins
that on purpose, and serving a non-authoritative Object Lock state would
be the wrong trade. The residual concern stands and is recorded in review
— a crash between the two writes in `persist_new_and_set` leaves the
bucket unloadable until DeleteBucket+CreateBucket, and the repair branches
in `migrate_legacy_metadata` and `make_bucket` are unreachable dead code
for that case. Resolving it needs the read path and the (transaction-lock
holding) repair path to be separated, which is more than a follow-up edit.
* test(ci): serialize the new bucket-incarnation tests
The five tests this branch adds around the incarnation / lifecycle fence
drive `init_bucket_metadata_sys` and `bucket_metadata_sys_of` — process-global
OnceLock state that `serial_test`'s `#[serial]` cannot protect across
nextest's process boundary — and they delete+recreate buckets, the shape that
raced into InsufficientWriteQuorum in backlog#937.
Add them to the `ecstore-serial-flaky` group in both the default and ci
profiles (nextest evaluates a named profile's own overrides list, so the
ci mirror is required). Preventive serialization only, no retries.
Not a full fix for the review comment: `bucket_delete_waits_for_config_
mutation_fence` still proves liveness with a fixed 200ms sleep plus
`assert!(!delete.is_finished())`. Turning that into readiness polling needs
a production-side signal to wait on — asserting "still blocked" is inherently
a negative. Serializing the group removes the parallel-load pressure that
makes the window fragile; the sleep itself is left for a follow-up.
* test(ecstore): pin that a drained bucket is actually deletable
`DeleteBucket`'s emptiness check is `has_xlmeta_files`, a raw scan of the
bucket directory on local disks — not an S3-level listing. So "the client
drained the bucket" and "the bucket is deletable" are two different
contracts, and only the first one was covered.
That gap is what the `S3 Implemented Tests` lane is failing on: 219 cases,
all `BucketNotEmpty` on `nuke_prefixed_buckets`, with every test body
passing. The first one is `test_versioning_obj_suspend_versions`, reported
by pytest as PASSED followed by ERROR at teardown.
Add the missing assertion for the unversioned path: PUT, client DELETE,
then assert no `xl.meta` survives and `DeleteBucket` succeeds. It passes —
which is itself a result: the plain delete path leaves no residue, so the
s3-tests failure is not there.
The versioning-suspended path is the remaining suspect (the client DELETE
leaves a null delete marker, and draining means purging it by
`versionId=null`). It is not covered here: `BucketVersioningSys` resolves
through the ambient `get_bucket_metadata_sys()` OnceLock, which this unit
env cannot set, so the bucket never actually reports as suspended. That
repro belongs at the e2e layer where a real server owns the versioning
state.
* fix(ecstore): let an explicit null-version delete purge its delete marker
Root cause of the `S3 Implemented Tests` lane: 219 cases, all
`BucketNotEmpty` on `nuke_prefixed_buckets`, every test body passing.
On a versioning-suspended bucket a client DELETE leaves a null delete
marker — correct S3 semantics, and an `xl.meta` on disk. Draining the
bucket therefore means purging that marker as `?versionId=null`, which is
what `nuke_bucket` does before `DeleteBucket`. That purge was rejected:
explicit null-version purge of the null delete marker must succeed,
got [Some(MethodNotAllowed)]
so the marker survived, and `DeleteBucket`'s emptiness check — a raw
`has_xlmeta_files` scan of the bucket directory, not an S3 listing — kept
reporting the bucket as non-empty.
The two sides of the version comparison in the batch delete loop are in
different namespaces. `goi.version_id` is the client-facing identity, where
`from_file_info` synthesizes `Some(Uuid::nil())` for a null version on a
versioned *or versioning-suspended* bucket. `version_id` is the storage
identity, where `delete_file_info_version_id` maps an explicit
`?versionId=null` to `None`. Comparing them raw makes the purge look like a
version mismatch, so `explicit_delete_marker` is false and the
`MethodNotAllowed` from the lookup is recorded as a delete failure.
This only became reachable on this branch: previously `check_opts` did not
carry `dobj.version_id`, so `set_disk_delete_creates_delete_marker` was
true, `object_lock_check_required` was false, and the lookup that produces
`MethodNotAllowed` never ran. Adding the version id to `check_opts` lit up
a comparison that was already wrong.
Normalize both sides through `delete_file_info_version_id`.
The regression test injects a real Suspended bucket-config snapshot — the
delete path reads versioned/suspended from that snapshot, not from `opts`,
so without it `from_file_info` never synthesizes the null version id and
the branch is not reached. Mutation-checked: restoring the raw comparison
fails the test with the exact `MethodNotAllowed` above.
* fix(app): drop the now-needless struct update
Reverting `crates/replication` to main removed the extra `MrfReplicateEntry`
fields, so this literal specifies every field again and `..Default::default()`
trips `clippy::needless_update` under `-D warnings`.
Caught by CI, not locally: I had run `cargo check --workspace --all-targets`,
which does not see clippy-only lints. Ran `cargo clippy --workspace
--all-targets -- -D warnings` here — clean.
* test(e2e): assert the fresh-volume classification
four_node_empty_legacy_volumes_start_as_fresh only started the cluster and
listed buckets — no assertion, so any classification path that still permits
startup left it green without proving the pre-created empty `.minio.sys`
directories were treated as fresh volumes.
Pin what that classification actually leaves behind: no buckets adopted into
the namespace, `.rustfs.sys/format.json` written on every drive, and the empty
legacy directory left untouched rather than migrated into.
* fix(bucket): apply the requested Object Lock to existing buckets
Site replication replays make-with-versioning against the destination,
carrying the source's `lockEnabled`. When the destination bucket already
exists it takes `force_create`, and the whole option-application block was
gated on `confirmed_missing` — so the call returned success while the replica
stayed unlocked. Replicated versions could then be deleted without the
retention the source enforces.
Object Lock enable is one-way, so applying it to an existing bucket is safe:
move it out of the creation-only gate, keeping `created` and versioning-only
options creation-scoped as before.
An existing authoritative bucket takes the `cache_bucket_metadata_in` branch,
which only caches, so the enable would have been dropped on restart. Persist
instead when the enable actually changed something.
Mutation-checked: restoring the creation-only gate fails the new
`force_create_enables_object_lock_on_an_existing_bucket` with "Object Lock
must be enabled on the existing bucket".
cargo nextest run -p rustfs-ecstore --lib: 3633 passed.
* fix(ecstore): box the generation-checked config mutation paths too
The earlier stack fix boxed `update` and `delete`, but an authorized
bucket-config mutation carrying an incarnation takes `update_if_incarnation`
/ `delete_if_incarnation` instead — which were still inlining the whole
resolve/load/save chain into an already-deep request future. Same overflow,
sibling path.
* fix(restore): keep the nil-version normalization the strip removed
Reverting the replication subsystem to main took `set_disk/replication.rs`
with it, but one line in that file was this branch's own fix rather than
replication work:
- self.version_id.filter(|v| !v.is_nil()) == fi.version_id.filter(|v| !v.is_nil())
+ self.version_id == fi.version_id
For a versioning-suspended object the expected version is `Some(Uuid::nil())`
while the read-back `FileInfo` carries `None`, so the raw compare reports
every suspended restore as "restored object changed before restore metadata
finalization" and the copy-back never commits. Same nil-vs-None mismatch as
the null delete-marker purge fixed earlier on this branch.
Caught by `Test and Lint (rio-v2)`, not by my local runs: the test lives in
`transition_commit_failure_tests`, gated behind `feature = "test-util"`, so
the 3633-test suite I had been running never included it. Re-ran with
`--features rio-v2,test-util`: 3722 passed.
* feat(replication): purge delete markers by the target's own version id
When a delete marker is replicated, the target assigns it a version id. The
purge that follows derived one from the *source* uuid instead, which is only
correct when the target mirrors source version ids. A generic S3 target does
not: the derived id addresses a version that does not exist there, so the
purge is a no-op and the replica keeps a marker the source has already
removed. Same failure class as #4401.
Record the id the target reports and address it directly on purge.
Data path, all of it driven by the object's internal metadata rather than the
`ReplicationState` wire form, which encodes positionally and cannot carry a
map:
- `rustfs-utils`: the `replication-delete-marker-version-<arn>` key family,
plus `strip_internal_prefix_preserving_case` — ARNs are case-sensitive and
the existing `strip_internal_prefix` lowercases.
- `ReplicationState` gains the map and a `..._corrupt` flag, both
`#[serde(skip)]`; `ReplicatedTargetInfo` carries the per-target id.
- `persist_target_delete_marker_versions` is merge-only. A delete arriving
over internode RPC has an empty map, so treating it as authoritative would
let a remote disk erase an id the local disk still holds.
- `delete_object_version` copies the map into `fi.metadata` before dispatch,
so the durable carrier crosses the wire even though the field does not.
- The keys are folded into the quorum hash through their normalized form:
the dual internal prefixes carrying one mapping share an identity, while a
genuine disagreement between disks still shows up as a quorum difference.
- `corrupt` (the prefixes disagreed) fails closed: skip the purge and warn
rather than guess an id and risk destroying a live version on the target.
Ported from the rc.1 branch, which cannot merge as a whole: its MRF replay
rewrite collides with #5659/#5671/#5672/#5673 and regressed
`MRF_PENDING_CAP`. main's MRF machinery is kept; only this capability moves
across. It touches no MRF code.
Two things did not survive the port, deliberately. The branch's
`missing_is_complete` purge regression does not exist here — it came from its
own HEAD-precheck rewrite, and main's simpler path never had it. And the
branch's `MrfReplicateEntry` ordering fields are MRF-redesign scope, left
behind.
Verification: cargo fmt --all --check, git diff --check,
cargo check --workspace --all-targets, and the suites for the four touched
crates — 4070 tests, 2 pre-existing failures unrelated to this change
(`system_resolver_negative_result_reaches_the_dns_allowlist`,
`test_resolve_domain_preserves_system_resolver_error_provenance`; both are
the sandbox DNS interception, they fail on a clean checkout too).
* fix(replication): keep the layer guard happy
scripts/check_architecture_migration_rules.sh matches on text, so the doc
comments naming `rustfs_filemeta::` read as a cross-layer dependency even
though nothing imports it. Reword them; the guard passes.
* fix(replication): make the target-version cap deterministic
Two defects in this PR, both found in review.
The cap was applied while iterating a `HashMap`, so *which* 1000 entries
survived depended on iteration order. Two disks decoding the same oversized
metadata could keep different subsets, hash differently, and lose quorum —
instead of both reporting the same corruption. Collect first, then truncate
in `BTreeMap` order, which is total and identical everywhere.
And `persist_target_delete_marker_versions` discarded the `corrupt` flag from
the RPC carrier, committing a delete-marker update that looked clean while the
exact remote marker identity was unknown. It now declines to merge a corrupt
carrier. Because the helper only ever inserts, declining leaves the durable
keys already on the object untouched, which is strictly safer than writing a
mapping we cannot trust.
Residual, stated rather than papered over: corruption confined to the RPC
carrier is not persisted as a sentinel, so a later reader of an object that
carried no durable keys still sees "legacy, no mapping" rather than "corrupt".
Persisting that would need a wire-format addition; the consumer already fails
closed on any corruption it can observe.
New test: `target_delete_marker_versions_cap_is_deterministic_across_decodes`
decodes the same 1050-entry map twice and asserts both the corrupt flag and
the retained subset agree.
* fix(replication): preserve multipart source mtime (#5669)
* fix(kms): repair unopenable ciphertext and cover the Vault backends (#5668)
* Add black-box behavior tests for KMS resilience and serialization
* fix(kms): repair unopenable ciphertext across backends
Black-box testing of the KMS crate surfaced several defects that make
encrypted data permanently unreadable.
Symmetric envelopes. The Local and Vault Transit backends returned raw
cipher output from `encrypt` while `decrypt` parsed a JSON envelope, so
anything sealed through the master-key path could never be opened again.
Local also discarded the AES-GCM nonce. Both now emit the same envelope
`decrypt` consumes, matching the Static backend.
Deterministic AAD. The object layer derived AEAD additional data by
serializing a `HashMap` directly. Iteration order differs per instance,
so a context rebuilt from storage produced different AAD bytes than the
one used to seal and the object stopped opening. Ordering by key removes
that dependency, matching the Static backend's existing `context_aad`.
Objects written with the default single-key context are unaffected,
since a one-entry map has only one serialization.
Cipher in the header projection. `metadata_to_headers` recorded the SSE
mode (`AES256` / `aws:kms`), which cannot represent ChaCha20-Poly1305,
so a ChaCha-sealed object came back claiming `aws:kms` and was opened
with the wrong cipher. The cipher now travels in
`x-rustfs-encryption-algorithm` — the header the storage layer already
reads but nothing ever wrote. Objects without it fall back as before.
Also: the Static backend ignored `key_spec` and always issued 256-bit
data keys; Local `list_keys` hardcoded `truncated: false`, ignored
`marker`, and paginated over unordered `read_dir`, so a paginating
client silently saw a partial key list; and Local and Vault KV2 reported
`key_id: "unknown"` from `decrypt` despite the envelope naming the
master key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(kms): cover both Vault backends and key rotation
The behavior suite ran only against Local and Static, and its own harness
documented the gap: the Vault backends had no business-capability
coverage at all. Setting `RUSTFS_KMS_VAULT_TOKEN` now adds Vault KV2 and
Vault Transit to every `for_each_backend` spec against a live server.
That lane is what surfaced the Transit envelope defect fixed in the
previous commit.
`rotate` and `versioning` are advertised only by the Vault backends, so
until now every capability-gated branch for them took the
`UnsupportedCapability` side and the working half was never asserted — a
rotation that dropped prior key versions would have gone green. The new
`behavior_rotation.rs` pins that half: material sealed before a rotation
still opens after it, repeated rotations accumulate versions rather than
overwriting a single spare, and the history survives a restart.
Two test defects fixed. `objects_round_trip_across_sizes_and_algorithms`
asserted a 1-byte object differs from its own ciphertext, which collides
once every 256 runs; the assertion now applies only where a collision is
not realistic, and small objects stay covered by the tag check and the
decrypt round-trip. `test_from_env_selects_token_file` depended on
`RUSTFS_KMS_VAULT_TOKEN` being absent from the caller's environment and
now clears it explicitly.
The snapshots directory was also removed from `.gitignore`: insta
snapshots are the assertions themselves, so leaving them untracked gives
CI nothing to compare against. Only `.snap.new` scratch files are
ignored now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(kms): adapt behavior suite to current key APIs
Rebasing onto main brought four API changes the suite predates.
`DeleteKeyRequest` gained `confirm_key_id`, and immediate deletion is now
gated on the server's `allow_immediate_deletion`. Scheduled deletions pass
`None`; the four specs that destroy a key outright echo the key id back
and opt the harness config in, which is what the gate asks of a real
caller.
`LocalBackupExportRequest` gained `sanitized_config`. These specs cover
the key-material path, so they seal no configuration and pass `None`.
`KmsCacheStats` became a named struct with real hit, miss, and eviction
counters. `cache_stats_returns_an_entry_count_and_no_hit_or_miss_data`
existed to pin the old placeholder behavior — that the second tuple
element was always zero — which main has since fixed, so it is now
`cache_stats_reports_hits_and_misses_separately` and asserts the counters
actually move.
Starting the service provisions the reserved probe key, so it shows up in
listings and backup bundles. Exact-set assertions filter it through a new
`without_probe_key` helper rather than naming it, keeping those specs
about the keys they seeded.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(kms): bind the AAD to the stored context bytes
Review caught that canonicalizing the AAD on decrypt breaks objects sealed
before canonicalization existed, and it was right. The AAD is the
*serialization* of the encryption context, and `x-rustfs-encryption-context`
stores that exact byte sequence: `encrypt_object` fed one `HashMap` to the
AEAD and then moved the same map into the metadata the header is written
from, so the stored string is byte-identical to the AAD the object was
sealed under. Those objects are therefore recoverable — but only while
nothing round-trips the value through a `HashMap` and re-serializes it.
Recomputing sorted AAD on decrypt would have turned a readable object into
a permanently unreadable one. The previous behavior was worse than the
first analysis credited: it did not merely fail intermittently, it made
the failure deterministic.
`EncryptionMetadata` now carries `context_aad`, the bytes the object was
actually sealed with. Encryption records what it fed the AEAD, the header
projection stores those bytes verbatim (and preserves a legacy ordering
across a re-projection rather than rewriting it into sorted form), and
`headers_to_metadata` carries the stored string through untouched. Both
decrypt paths, SSE-KMS and SSE-C, prefer it and fall back to canonical
serialization only when no stored serialization exists. Canonicalization
still applies to everything newly sealed, so the original ordering bug
cannot recur.
Two tests pin this: a legacy record whose sealed bytes are non-canonical
must survive a full header round trip unchanged, and a context header
rewritten to an equivalent-but-reordered serialization must fail
authentication rather than silently re-deriving a working AAD. Both were
mutation-checked against the reinstated bug on each side.
Also from review: the lifecycle churn test asserted only that every
request was accounted for, which holds whether the state gate exists or
not, so both branches are now pinned deterministically after the churn
(asserting `refused > 0` on the concurrent phase would only trade the hole
for a scheduling flake). And the Local and Vault KV2 envelopes compare
`encryption_context` without authenticating it — `DekCrypto` seals only
the plaintext — which is now documented at both sites; closing it needs a
versioned envelope, since existing ciphertext was sealed without AAD.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: ccccpj <ccccpj@outlook.com>
Co-authored-by: 唐小鸭 <tangtang1251@qq.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Add black-box behavior tests for KMS resilience and serialization
* fix(kms): repair unopenable ciphertext across backends
Black-box testing of the KMS crate surfaced several defects that make
encrypted data permanently unreadable.
Symmetric envelopes. The Local and Vault Transit backends returned raw
cipher output from `encrypt` while `decrypt` parsed a JSON envelope, so
anything sealed through the master-key path could never be opened again.
Local also discarded the AES-GCM nonce. Both now emit the same envelope
`decrypt` consumes, matching the Static backend.
Deterministic AAD. The object layer derived AEAD additional data by
serializing a `HashMap` directly. Iteration order differs per instance,
so a context rebuilt from storage produced different AAD bytes than the
one used to seal and the object stopped opening. Ordering by key removes
that dependency, matching the Static backend's existing `context_aad`.
Objects written with the default single-key context are unaffected,
since a one-entry map has only one serialization.
Cipher in the header projection. `metadata_to_headers` recorded the SSE
mode (`AES256` / `aws:kms`), which cannot represent ChaCha20-Poly1305,
so a ChaCha-sealed object came back claiming `aws:kms` and was opened
with the wrong cipher. The cipher now travels in
`x-rustfs-encryption-algorithm` — the header the storage layer already
reads but nothing ever wrote. Objects without it fall back as before.
Also: the Static backend ignored `key_spec` and always issued 256-bit
data keys; Local `list_keys` hardcoded `truncated: false`, ignored
`marker`, and paginated over unordered `read_dir`, so a paginating
client silently saw a partial key list; and Local and Vault KV2 reported
`key_id: "unknown"` from `decrypt` despite the envelope naming the
master key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(kms): cover both Vault backends and key rotation
The behavior suite ran only against Local and Static, and its own harness
documented the gap: the Vault backends had no business-capability
coverage at all. Setting `RUSTFS_KMS_VAULT_TOKEN` now adds Vault KV2 and
Vault Transit to every `for_each_backend` spec against a live server.
That lane is what surfaced the Transit envelope defect fixed in the
previous commit.
`rotate` and `versioning` are advertised only by the Vault backends, so
until now every capability-gated branch for them took the
`UnsupportedCapability` side and the working half was never asserted — a
rotation that dropped prior key versions would have gone green. The new
`behavior_rotation.rs` pins that half: material sealed before a rotation
still opens after it, repeated rotations accumulate versions rather than
overwriting a single spare, and the history survives a restart.
Two test defects fixed. `objects_round_trip_across_sizes_and_algorithms`
asserted a 1-byte object differs from its own ciphertext, which collides
once every 256 runs; the assertion now applies only where a collision is
not realistic, and small objects stay covered by the tag check and the
decrypt round-trip. `test_from_env_selects_token_file` depended on
`RUSTFS_KMS_VAULT_TOKEN` being absent from the caller's environment and
now clears it explicitly.
The snapshots directory was also removed from `.gitignore`: insta
snapshots are the assertions themselves, so leaving them untracked gives
CI nothing to compare against. Only `.snap.new` scratch files are
ignored now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(kms): adapt behavior suite to current key APIs
Rebasing onto main brought four API changes the suite predates.
`DeleteKeyRequest` gained `confirm_key_id`, and immediate deletion is now
gated on the server's `allow_immediate_deletion`. Scheduled deletions pass
`None`; the four specs that destroy a key outright echo the key id back
and opt the harness config in, which is what the gate asks of a real
caller.
`LocalBackupExportRequest` gained `sanitized_config`. These specs cover
the key-material path, so they seal no configuration and pass `None`.
`KmsCacheStats` became a named struct with real hit, miss, and eviction
counters. `cache_stats_returns_an_entry_count_and_no_hit_or_miss_data`
existed to pin the old placeholder behavior — that the second tuple
element was always zero — which main has since fixed, so it is now
`cache_stats_reports_hits_and_misses_separately` and asserts the counters
actually move.
Starting the service provisions the reserved probe key, so it shows up in
listings and backup bundles. Exact-set assertions filter it through a new
`without_probe_key` helper rather than naming it, keeping those specs
about the keys they seeded.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(kms): bind the AAD to the stored context bytes
Review caught that canonicalizing the AAD on decrypt breaks objects sealed
before canonicalization existed, and it was right. The AAD is the
*serialization* of the encryption context, and `x-rustfs-encryption-context`
stores that exact byte sequence: `encrypt_object` fed one `HashMap` to the
AEAD and then moved the same map into the metadata the header is written
from, so the stored string is byte-identical to the AAD the object was
sealed under. Those objects are therefore recoverable — but only while
nothing round-trips the value through a `HashMap` and re-serializes it.
Recomputing sorted AAD on decrypt would have turned a readable object into
a permanently unreadable one. The previous behavior was worse than the
first analysis credited: it did not merely fail intermittently, it made
the failure deterministic.
`EncryptionMetadata` now carries `context_aad`, the bytes the object was
actually sealed with. Encryption records what it fed the AEAD, the header
projection stores those bytes verbatim (and preserves a legacy ordering
across a re-projection rather than rewriting it into sorted form), and
`headers_to_metadata` carries the stored string through untouched. Both
decrypt paths, SSE-KMS and SSE-C, prefer it and fall back to canonical
serialization only when no stored serialization exists. Canonicalization
still applies to everything newly sealed, so the original ordering bug
cannot recur.
Two tests pin this: a legacy record whose sealed bytes are non-canonical
must survive a full header round trip unchanged, and a context header
rewritten to an equivalent-but-reordered serialization must fail
authentication rather than silently re-deriving a working AAD. Both were
mutation-checked against the reinstated bug on each side.
Also from review: the lifecycle churn test asserted only that every
request was accounted for, which holds whether the state gate exists or
not, so both branches are now pinned deterministically after the churn
(asserting `refused > 0` on the concurrent phase would only trade the hole
for a scheduling flake). And the Local and Vault KV2 envelopes compare
`encryption_context` without authenticating it — `DekCrypto` seals only
the plaintext — which is now documented at both sites; closing it needs a
versioned envelope, since existing ciphertext was sealed without AAD.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
唐小鸭