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).
This commit is contained in:
唐小鸭
2026-08-07 22:30:12 +08:00
committed by GitHub
parent 7553715f62
commit 3792fed827
16 changed files with 861 additions and 97 deletions
+7 -7
View File
@@ -185,19 +185,19 @@ pub mod bucket {
MustReplicateOptions, ObjectOpts, REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION, REMOTE_TARGET_UNSUPPORTED_FIELDS,
REMOTE_TARGET_WRITABLE_FIELDS, REPLICATE_INCOMING_DELETE, REPLICATION_CAPABILITY_CONTRACT_VERSION,
REPLICATION_READ_ONLY_HISTORICAL_FIELDS, REPLICATION_WRITABLE_FIELDS, ReplicateDecision, ReplicateObjectInfo,
ReplicationBatchAdmission, ReplicationConfig, ReplicationConfigurationExt, ReplicationDeleteScheduleInput,
ReplicationDeleteStateSource, ReplicationHealQueueResult, ReplicationObjectBridge, ReplicationObjectIO,
ReplicationOperation, ReplicationPoolTrait, ReplicationPriority, ReplicationQueueAdmission, ReplicationScannerBridge,
ReplicationState, ReplicationStats, ReplicationStatusType, ReplicationStorage, ReplicationTargetValidationError,
ReplicationType, ResyncOpts, ResyncStatusType, RuntimeReplicationTargetBacklog, TargetReplicationResyncStatus,
VersionPurgeStatusType, commit_force_delete_intent, complete_force_delete_intent,
ReplicationBatchAdmission, ReplicationConfig, ReplicationConfigStructureError, ReplicationConfigurationExt,
ReplicationDeleteScheduleInput, ReplicationDeleteStateSource, ReplicationHealQueueResult, ReplicationObjectBridge,
ReplicationObjectIO, ReplicationOperation, ReplicationPoolTrait, ReplicationPriority, ReplicationQueueAdmission,
ReplicationScannerBridge, ReplicationState, ReplicationStats, ReplicationStatusType, ReplicationStorage,
ReplicationTargetValidationError, ReplicationType, ResyncOpts, ResyncStatusType, RuntimeReplicationTargetBacklog,
TargetReplicationResyncStatus, VersionPurgeStatusType, commit_force_delete_intent, complete_force_delete_intent,
delete_replication_state_from_config, delete_replication_version_id, get_global_replication_pool,
get_global_replication_stats, init_background_replication, invalid_replication_config_status_field,
persist_force_delete_intent, read_durable_mrf_backlog, replication_state_to_filemeta, replication_status_to_filemeta,
replication_statuses_map, replication_target_arns, resync_start_conflict_id, should_remove_replication_target,
should_schedule_delete_replication, should_use_existing_delete_replication_info,
should_use_existing_delete_replication_source, unsupported_replication_config_field,
validate_replication_config_target_arns, version_purge_status_to_filemeta,
validate_replication_config_structure, validate_replication_config_target_arns, version_purge_status_to_filemeta,
};
}
@@ -100,11 +100,41 @@ paths.
behind the ECStore replication facade; only `rustfs/src/app/storage_api.rs`
may retain direct object/delete replication helper calls.
## First Code-Bearing Step
## Completion Criteria
Start with `ReplicationRuntime` or `ReplicationEventSink`. Both can be added as
narrow internal contracts while keeping current queue, MRF, resync, and target
behavior unchanged. Do not start with a crate move.
The split is complete when the "Current dependency to remove" column in the
Required Contracts table above is empty: every row is either deleted because
the dependency is gone, or reduced to "none". No other signal — file count,
boundary count, line count — measures completion.
Target end state:
- `replication_pool.rs`, `replication_resyncer.rs`, and `replication_state.rs`
move into `crates/replication` behind the contracts above;
- the `*_boundary.rs` and `*_bridge.rs` micro-files dissolve naturally as the
code they fence moves across the crate boundary. They are the mechanical
seams of the migration ratchet — the architecture guard scripts anchor on
their file names — so batch-merging them beforehand is explicitly rejected:
it forces synchronized guard-script/mod/import churn with zero functional
gain;
- the only module that can retire early is `datatypes.rs`: delete it once its
facade consumers import the resync status enums through `rustfs-replication`
directly.
## Milestones
| Milestone | Scope | Status |
|---|---|---|
| M0 | Record the completion criteria and end state (this section). | Done |
| M1 | Contract extraction: resync/queue/stats/object-decision/filemeta/storage wire contracts owned by `crates/replication`; ECStore imports concentrated in `*_boundary.rs`; event sink and runtime access behind local contracts. | Done — see Required Contracts |
| M2 | Move resyncer pure decision logic (no IO) into `crates/replication`. | Pending; sequence after splitting the oversized resyncer/pool functions (`resync_bucket`, `replicate_all`, `start_mrf_processor`) so moves stay mechanical |
| M3 | Move the worker runtime (`replication_pool.rs`, the IO paths of `replication_resyncer.rs`, `replication_state.rs`) once the contract traits are stable. Highest-risk step of the whole plan; do it last. | Pending |
| M4 | Retire the boundary modules together with their guard-script entries; delete `datatypes.rs`. | Pending |
The original first code-bearing step (narrow `ReplicationEventSink` /
`ReplicationRuntime` contracts) has landed — `replication_event_sink.rs`
exists and runtime access goes through local boundary aliases — so new work
starts from M2.
Current compatibility guard: `crates/ecstore/tests/replication_facade_compat_test.rs`
keeps the ECStore replication facade types covered while architecture rules
+3 -3
View File
@@ -47,9 +47,9 @@ pub use datatypes::ResyncStatusType;
pub use replication_config_boundary::{
ObjectOpts, REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION, REMOTE_TARGET_UNSUPPORTED_FIELDS, REMOTE_TARGET_WRITABLE_FIELDS,
REPLICATION_CAPABILITY_CONTRACT_VERSION, REPLICATION_READ_ONLY_HISTORICAL_FIELDS, REPLICATION_WRITABLE_FIELDS,
ReplicationConfigurationExt, ReplicationTargetValidationError, invalid_replication_config_status_field,
replication_target_arns, should_remove_replication_target, unsupported_replication_config_field,
validate_replication_config_target_arns,
ReplicationConfigStructureError, ReplicationConfigurationExt, ReplicationTargetValidationError,
invalid_replication_config_status_field, replication_target_arns, should_remove_replication_target,
unsupported_replication_config_field, validate_replication_config_structure, validate_replication_config_target_arns,
};
pub(crate) use replication_filemeta_boundary::version_purge_statuses_map;
pub use replication_filemeta_boundary::{
@@ -15,7 +15,7 @@
pub use rustfs_replication::{
ObjectOpts, REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION, REMOTE_TARGET_UNSUPPORTED_FIELDS, REMOTE_TARGET_WRITABLE_FIELDS,
REPLICATION_CAPABILITY_CONTRACT_VERSION, REPLICATION_READ_ONLY_HISTORICAL_FIELDS, REPLICATION_WRITABLE_FIELDS,
ReplicationConfigurationExt, ReplicationRuleExt, ReplicationTargetValidationError, invalid_replication_config_status_field,
replication_target_arns, should_remove_replication_target, unsupported_replication_config_field,
validate_replication_config_target_arns,
ReplicationConfigStructureError, ReplicationConfigurationExt, ReplicationRuleExt, ReplicationTargetValidationError,
invalid_replication_config_status_field, replication_target_arns, should_remove_replication_target,
unsupported_replication_config_field, validate_replication_config_structure, validate_replication_config_target_arns,
};
@@ -89,3 +89,116 @@ pub fn replication_state_to_filemeta(state: &ReplicationState) -> rustfs_filemet
target_delete_marker_version_ids_corrupt: state.target_delete_marker_version_ids_corrupt,
}
}
// Reconciliation tests for the deliberately duplicated wire types.
//
// `rustfs-filemeta` (xl.meta disk format) and `rustfs-replication` (MRF/resync
// persistence format) each own a copy of `ReplicationStatusType`,
// `VersionPurgeStatusType` and `ReplicationState`; the conversions above hop
// between them via `as_str()`, whose `From<&str>` impls fall back to `Empty`
// on any unknown token. That fallback silently degrades data the moment one
// side gains a variant the other lacks, so these tests pin the two sides
// together:
//
// - the `match` statements are exhaustive with no `_` arm — adding a variant
// on either side fails compilation here until the mapping is reconsidered;
// - the round-trips assert the string token survives both directions — a
// variant whose token the other side does not recognize fails the assert
// instead of quietly becoming `Empty`.
//
// Struct-shaped drift on `ReplicationState` is already compile-guarded by the
// exhaustive struct literals in the two conversion functions above; the
// round-trip test below additionally pins value fidelity for every field.
#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashMap;
#[test]
fn replication_status_variants_round_trip_across_boundary() {
use rustfs_replication::ReplicationStatusType as Repl;
let all = [
Repl::Pending,
Repl::Completed,
Repl::CompletedLegacy,
Repl::Failed,
Repl::Replica,
Repl::Empty,
];
for status in all {
// Exhaustive on the replication side: a new variant breaks this match.
match status {
Repl::Pending | Repl::Completed | Repl::CompletedLegacy | Repl::Failed | Repl::Replica | Repl::Empty => {}
}
let filemeta = replication_status_to_filemeta(status.clone());
assert_eq!(
filemeta.as_str(),
status.as_str(),
"replication->filemeta conversion must not degrade {status:?} (unknown tokens fall back to Empty)"
);
assert_eq!(replication_status_from_filemeta(filemeta), status);
}
// Exhaustive on the filemeta side: a new variant breaks this match.
fn _filemeta_side_is_covered(status: rustfs_filemeta::ReplicationStatusType) {
use rustfs_filemeta::ReplicationStatusType as Meta;
match status {
Meta::Pending | Meta::Completed | Meta::CompletedLegacy | Meta::Failed | Meta::Replica | Meta::Empty => {}
}
}
}
#[test]
fn version_purge_status_variants_round_trip_across_boundary() {
use rustfs_replication::VersionPurgeStatusType as Repl;
let all = [Repl::Pending, Repl::Complete, Repl::Failed, Repl::Empty];
for status in all {
// Exhaustive on the replication side: a new variant breaks this match.
match status {
Repl::Pending | Repl::Complete | Repl::Failed | Repl::Empty => {}
}
let filemeta = version_purge_status_to_filemeta(status.clone());
assert_eq!(
filemeta.as_str(),
status.as_str(),
"replication->filemeta conversion must not degrade {status:?} (unknown tokens fall back to Empty)"
);
assert_eq!(version_purge_status_from_filemeta(filemeta), status);
}
// Exhaustive on the filemeta side: a new variant breaks this match.
fn _filemeta_side_is_covered(status: rustfs_filemeta::VersionPurgeStatusType) {
use rustfs_filemeta::VersionPurgeStatusType as Meta;
match status {
Meta::Pending | Meta::Complete | Meta::Failed | Meta::Empty => {}
}
}
}
#[test]
fn replication_state_round_trips_every_field_across_boundary() {
let timestamp = time::OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("valid timestamp");
let state = ReplicationState {
replica_timestamp: Some(timestamp),
replica_status: ReplicationStatusType::Replica,
delete_marker: true,
replication_timestamp: Some(timestamp),
replication_status_internal: Some("arn:a=PENDING;".to_string()),
version_purge_status_internal: Some("arn:a=FAILED;".to_string()),
replicate_decision_str: "arn:a=true;false;;".to_string(),
targets: HashMap::from([
("arn:a".to_string(), ReplicationStatusType::Completed),
("arn:b".to_string(), ReplicationStatusType::Failed),
]),
purge_targets: HashMap::from([("arn:a".to_string(), VersionPurgeStatusType::Pending)]),
reset_statuses_map: HashMap::from([("reset-arn:a".to_string(), "reset-id;ts".to_string())]),
target_delete_marker_version_ids: HashMap::from([("arn:a".to_string(), "version-1".to_string())]),
target_delete_marker_version_ids_corrupt: true,
};
let round_tripped = replication_state_from_filemeta(&replication_state_to_filemeta(&state));
assert_eq!(round_tripped, state);
}
}