mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-30 08:49:26 +00:00
a65b306fb04e43db94fcc176f9e31ac6397e05a0
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
4ddc728c9d |
fix(replication): deny non-owner replication config edits under site replication (#6375)
* fix(replication): deny non-owner replication config edits under site replication Under site replication a user holding only bucket-scoped s3:PutReplicationConfiguration could rewrite or erase the operator-managed site-repl-* rules, with the change broadcast to every peer (backlog#1948, audit A1/P2-17). - Gate PutBucketReplication/DeleteBucketReplication in the S3 handlers: when site replication is enabled and the requester is not the owner, return MinIO-parity XMinioReplicationDenyEdit (HTTP 400). The gate runs after policy authorization and only on the external S3 path; the reconciler and peer bucket-meta ingestion are unaffected. - Defense in depth in the bucket usecase: PUT merges the incoming config with the stored site-repl-* rules (same merge as peer ingestion) instead of overwriting verbatim; DELETE keeps the site-repl-* rules and never garbage-collects a bucket target a surviving site-replication rule still references. - Move is_site_replication_rule / merge_incoming_replication_config / replication_target_arn_deployment_id from the admin site-replication handler down to rustfs-replication so the app layer can reuse them without new layering violations. * fix(replication): scope site-owned rule detection to reconciler-derived rules The `site-repl-*` prefix alone classified any rule as site-owned, so on a bucket outside site replication an owner's `site-repl-user` rule survived DeleteBucketReplication (rule and target kept, success returned). Rule ids do not reserve that namespace. A rule is reconciler-owned only when it matches what the reconciler derives: id `site-repl-<deployment id>` for a current remote site replication peer and a destination ARN naming that same deployment id. The S3 put/delete path reads the remote peer set (empty when site replication is disabled) and keeps exactly those rules; everything else is operator state the request replaces or deletes. An incoming rule that claims a current peer's id is dropped so the reconciler rule's id stays unique. The peer ingestion path and the reconciler keep their prefix predicate unchanged. * fix(replication): keep operator rule priorities across site rule merges Merging stored site-replication rules into a PutBucketReplication body renumbered every rule 1..n in list order, rewriting the submitted policy: overlapping same-target rules submitted as priority 5 then 1 became 1 then 2, so the delete-marker-disabled rule won the replication decision. The reconciler and the peer-removal prune renumbered the same way. Operator priorities now stay verbatim everywhere; only the reconciler's derived rules move, to the lowest priorities no operator rule uses, via one pure helper shared by the S3 edit merge, the peer ingestion merge, the reconciler pass and the prune. Being a pure function of the rule list it is idempotent, so the reconciler's no-op check still holds after a merged write, and an on-disk config in the historical layout (operator rules 1..k, site rules k+1..n) yields the same bytes, so nothing is rewritten on upgrade. * fix(replication): pass site peer ids into the bucket usecase from the interface layer The review fix made the bucket usecase read the site-replication peer set through the admin handlers, an app->interface import the layer guard rejects. The S3 handlers (interface) now read the peer set and pass it in, so the usecase stays a pure function of its inputs; a state-read failure still fails the edit closed, just one layer up. * fix(replication): classify peer-ingested rules by the derived id/ARN contract The peer ingestion merge still treated every incoming `site-repl-*` id as reconciler-owned, so an owner-authored `site-repl-user` rule that the S3 merge now keeps on the editing site was dropped on every peer and the sites persisted different operator configs. The ingestion merge now classifies by the same derived contract as the S3 merge: a rule is the reconciler's only when its `site-repl-<id>` names the deployment its destination ARN targets and that deployment is a site of the cluster (the receiver's own id included, since the sender's rule towards the receiver names it). The reconciler, the peer-removal prune and the target-online probe switch from the id prefix to the derived shape as well, so the rule survives their passes too; rules in the derived shape that name a removed peer or this site are still rebuilt away. Regression: a PutBucketReplication merged on site A and ingested on site B keeps `site-repl-user` on both and the operator rule sets agree. * fix(replication): keep an operator role target through site rule merges The S3 and peer-ingestion merges cleared `Role` whenever it parsed as a site-replication ARN, which an owner-submitted remote target with an empty region (`arn:minio:replication::<id>:<bucket>`) also does. The merged config then selected the rule destination ARNs instead of the validated role target. Only a role naming a current site of the cluster is the holder's identity (the reconciler's per-peer target lookup reads it); every other role passed target validation and stays. The reconciler's repair pass applies the same rule. Regression: an owner role target survives both merges and `filter_target_arns` / `replication_target_arns` select it; a role naming a current peer is still cleared. * fix(replication): gate operator priority preservation on a peer contract probe Keeping operator rule priorities verbatim is not rolling-upgrade safe: a peer still running the pre-contract code renumbers every rule 1..n in list order on ingest and on each reconciler pass, so an upgraded site broadcasting `5,1` leaves that peer on `1,2` — which can select the other overlapping rule — and the sites never reconverge. Operator rules now merge under an explicit contract: - `OperatorRuleContract::Derived`: site rules are the derived id/ARN shape, operator priorities stay verbatim (the behavior of the previous commits). - `OperatorRuleContract::Legacy`: byte-for-byte what a pre-contract peer does — `site-repl-*` ids are all site rules, a site-replication-shaped `Role` is dropped, every rule is renumbered 1..n in list order. The S3 merge additionally lists the operator rules in priority order first, so the renumbering keeps their relative order and the winning rule per target is the one the operator submitted. The S3 PutBucketReplication/DeleteBucketReplication path probes every remote peer through the existing `peer/edit-capabilities` endpoint (capability `derived-rule-contract`; pre-contract peers answer `success:false` or 404) and merges under Derived only when every peer supports it; any refusal or probe failure pins that edit to Legacy. Every bucket-meta item this site sends (S3 hooks, bootstrap plan, retry snapshots, tombstones) carries `derivedRuleContract: true`; a receiver merges a payload without the marker the Legacy way, so an item from a pre-contract sender is handled exactly as its own peers handle it. Rolling upgrade: while any site runs the older code every edit is canonicalized cluster-wide (numbers lost, order kept); once the last site is upgraded the next edit keeps its priorities. Configs canonicalized during the mixed period are not renumbered back — the derived priority assignment is a no-op on the canonical layout — so an operator who wants the original values re-submits the config after the upgrade completes. Adding a site that runs the older code after priorities were preserved is not gated and would desynchronize that bucket until the next edit. --------- Co-authored-by: houseme <housemecn@gmail.com> |
||
|
|
3792fed827 |
fix(replication): madmin reset/diff wire compat and config validation (#5799)
* fix(admin): align replication-reset responses with madmin ResyncTargetsInfo shape
The replication-reset and replication-reset-status responses serialized
their shell as "Targets" and per-target fields in PascalCase, while
madmin-go ResyncTargetsInfo/ResyncTarget expect the "target" shell key
and lowercase field tags (arn/resetid/resyncStatus/replicationCount/
completedReplicationSize/failedReplicationCount/failedReplicationSize).
Go json decoding is case-insensitive per field, but Targets vs target,
Status vs resyncStatus and the size/count key names cannot match, so
mc replicate resync decoded empty results.
Rename the serde tags to the exact madmin wire shape, keep the
ResetBeforeDate/Error RustFS extension keys (unknown keys are ignored
by Go decoders), pin the shape with a snapshot unit test, and update
the e2e client DTO to decode the madmin shape.
* fix(admin): stream bare madmin DiffInfo documents from replication diff
POST /v3/replication/diff returned a single enveloped object
({Entries, IsTruncated, ScannedVersions}) while madmin-go
BucketReplicationDiff decodes the body with a json.Decoder loop over
bare DiffInfo documents. The envelope decoded as exactly one DiffInfo
with an empty object, so mc replicate diff printed a phantom empty row
instead of the real backlog.
Emit one DiffInfo JSON document per line by default, using the exact
madmin json tags (object/versionId/rStatus/deletemarker/lastModified;
Size stays as a RustFS extension key that Go decoders ignore). The
enveloped shape moves to the opt-in ?aggregate=true RustFS extension,
which remains the only carrier of scan-coverage metadata; a truncated
default-mode scan is surfaced via a warn tracing event instead of
in-stream. Pin both shapes with unit tests and tighten the e2e helper
to reject any envelope in the stream.
* feat(replication): validate replication config structure before persisting
PutBucketReplication accepted structurally invalid configurations that
MinIO's replication.Config.Validate rejects: empty or oversized rule
lists, duplicate or negative rule priorities, over-long rule IDs,
filters carrying more than one of Prefix/Tag/And, and delete marker
replication enabled on tag-filtered rules. Such configs persisted
silently and later produced undefined routing (e.g. ambiguous priority
ties) instead of failing the PUT.
Add validate_replication_config_structure as a pure function in
rustfs-replication (limits documented as constants), surface it through
the ecstore api facade, and run it first in the PUT capability gate so
defects are named before any metadata write. Missing Priority counts as
zero for the uniqueness check, matching Go's zero-value semantics. The
self-target rejection deliberately stays at set-remote-target, where the
endpoint is known; a config can never reference a self-pointing ARN.
Document the rule-level Destination.StorageClass contract (use the
remote target's storage_class instead) and renumber the acceptance
matrix e2e to unique priorities, which MinIO would also require.
* test(replication): pin duplicated wire types with boundary reconciliation tests
rustfs-filemeta (xl.meta disk format) and rustfs-replication (MRF/resync
persistence format) deliberately each own ReplicationStatusType,
VersionPurgeStatusType and ReplicationState; the boundary converts
between them via as_str(), whose From<&str> impls fall back to Empty on
unknown tokens — a variant added on one side silently degrades to Empty
on the other.
Add reconciliation tests in replication_filemeta_boundary: exhaustive
matches with no wildcard arm on both sides of both enums (a new variant
fails compilation until the mapping is reconsidered), string-token
round-trip asserts (a token the other side does not recognize fails
instead of quietly becoming Empty), and a full-field ReplicationState
round-trip. Cross-reference the tests from both type definitions.
Struct drift was already compile-guarded by the exhaustive struct
literals in the conversion functions.
* docs(replication): define split completion criteria and milestone sequence
The ecstore replication split plan had no completion measure — the
boundary scaffolding risked ossifying because nothing said when the
migration counts as done. Record the criteria in the module inventory:
done means the Required Contracts table's 'Current dependency to
remove' column is empty; the end state moves pool/resyncer/state into
crates/replication, with the boundary micro-files dissolving as code
crosses the crate line (batch-merging them beforehand is explicitly
rejected — the guard scripts anchor on their file names, so merging is
churn with zero functional gain; only datatypes.rs can retire early).
Sequence the remaining work as M2 (resyncer pure decision logic, after
the oversized function splits) → M3 (worker runtime, highest risk,
last) → M4 (retire boundaries and guard entries). Refresh the stale
first-step text — the event sink / runtime contracts already landed —
and update the split-plan status table accordingly.
* fix(replication): align structural validator with MinIO semantics after adversarial review
Three interop corrections found by adversarial review of the new
structural validator, plus review fallout fixes:
- Delete-marker replication is now rejected only for a direct Filter.Tag,
not for tags inside Filter.And — MinIO's validator only inspects the
direct tag, and mc replicate add --tags "k1=v1&k2=v2" (delete-marker
replication on by default) puts multiple tags into And.Tags, so the
stricter check rejected mc-generated configs MinIO accepts.
- Rule ID length is measured in bytes (Go len semantics), not chars —
a 255-char multibyte ID must not round-trip into a config MinIO
rejects.
- An empty <Tag/> element (no key) counts as absent, matching MinIO's
Tag.IsEmpty(); console form serializers emit empty tags, which would
otherwise trip the exactly-one-of and delete-marker checks.
Also: repair the store-uninitialized PUT test whose empty-rules fixture
now (correctly) fails structural validation before reaching the store
lookup; pin the previously untested startTime madmin key in the
reset-status shape test; and signal a truncated default-mode diff scan
via the x-rustfs-replication-diff-truncated response header — the bare
madmin stream has no envelope, so a truncated scan was otherwise
indistinguishable from a complete healthy one (madmin/mc ignore unknown
headers).
* test(e2e): activate SSE-S3 replication contract and pin resync fail-closed path
The SSE-S3 replication contract e2e was ignored under backlog#1291
(silent plaintext replication); the fail-closed gate in
replication_target_boundary.rs closed that hole, so the ignore reason
expired. Un-ignore the test — it now pins the current fail-closed
contract (FAILED status, failure event, readable encrypted source,
stable absence of all target versions), verified green.
Add test_bucket_replication_sse_s3_resync_stays_fail_closed: drives the
existing-object resync path (PUT ?replication-reset) over a FAILED
SSE-S3 object and asserts the resync generation reaches a terminal
state without ever materializing a target version, with the
stays-absent window also spanning fast-scanner heal cycles. The new
start_bucket_replication_reset helper doubles as the madmin
ResyncTargetsInfo shape assertion (target[0].arn/resetid) for the
reset-start response.
Refresh the stale nextest count commentary (the module is at 20 fast +
36 nightly = 56 tests by cargo nextest list; the SSE-S3-ignored note no
longer holds).
|
||
|
|
114b2420a2 |
feat(admin): expose versioned replication capabilities (#5631)
* feat(admin): expose replication capabilities * fix(admin): route replication capabilities through facades |
||
|
|
c1955a8498 | fix(replication): harden live delete admission (#5599) | ||
|
|
c8016cbcdb |
fix(replication): enforce bucket replication switches (#5449)
* fix(replication): enforce bucket replication switches * fix(replication): satisfy delete admission clippy lint * fix(replication): restore MinIO tag filter behavior --------- Co-authored-by: Zhengchao An <anzhengchao@gmail.com> Co-authored-by: cxymds <cxymds@gmail.com> |
||
|
|
8a0617865b | refactor(replication): route app storage through ecstore (#4251) | ||
|
|
7cc470cb08 | refactor(replication): isolate ecstore boundary imports (#4247) |