Compare commits

...

2 Commits

Author SHA1 Message Date
唐小鸭 cddc003c16 fix(admin): emit madmin bandwidthlimit tag in list-remote-targets
madmin-go v3.0.109 `BucketTarget` (bucket-targets.go) tags the limit
`json:"bandwidthlimit,omitempty"`, not `bandwidth`, so the previous rename
still left mc decoding a zero bandwidth limit for every nonzero target.
Emit the exact tag in remote_target_admin_json, and add the same
`bandwidthlimit` alias on the ecstore `BucketTarget` reader so a
MinIO-written bucket-targets.json keeps its limit on migration (the legacy
`bandwidth` alias stays for compatibility). The persisted snake_case wire
key and the inbound RemoteTargetRequest aliases are unchanged.

Cover it with a madmin-shaped decode regression: the admin response is
decoded through a mirror of the v3.0.109 json tags and must yield the
nonzero bandwidth limit plus the other renamed fields.
2026-08-23 00:59:45 +08:00
唐小鸭 fbecd38e2d fix(admin): use madmin key names in list-remote-targets response
The admin list-remote-targets response serialized five fields under the
persisted snake_case keys (bandwidth_limit, storage_class, reset_id,
deployment_id, credentials.session_token), which madmin/mc decode to zero
values, blanking the bandwidth and reset-id columns of mc replicate ls.
Rename them to the madmin JSON tags (bandwidth, storageclass, resetID,
deploymentID, sessionToken) in remote_target_admin_json only, following
the existing Go-duration overwrite precedent there.

Symmetrically, BucketTarget deserialization only aliased bandwidth, so a
MinIO-written bucket-targets.json silently dropped the other four fields
to defaults on migration; add serde aliases for them. Aliases and the
response-layer rename leave the persisted bucket-targets.json and the
msgpack struct-map wire format untouched, pinned by new tests.

Refs rustfs/backlog#1951, spec rustfs/backlog#1946.
2026-08-21 18:53:48 +08:00
2 changed files with 228 additions and 6 deletions
@@ -32,6 +32,10 @@ pub struct Credentials {
pub access_key: String, pub access_key: String,
#[serde(rename = "secretKey")] #[serde(rename = "secretKey")]
pub secret_key: String, pub secret_key: String,
// The aliases accept madmin's JSON tags (MinIO-written bucket-targets
// metadata and mc request bodies) without changing the snake_case
// persisted/peer wire format this struct serializes to.
#[serde(alias = "sessionToken")]
pub session_token: Option<String>, pub session_token: Option<String>,
pub expiration: Option<Timestamp>, pub expiration: Option<Timestamp>,
} }
@@ -202,12 +206,14 @@ pub struct BucketTarget {
#[serde(default)] #[serde(default)]
pub region: String, pub region: String,
#[serde(alias = "bandwidth", default)] // madmin-go v3.0.109 tags this `bandwidthlimit`; `bandwidth` is a legacy
// alias kept for inputs written before the madmin tag was verified.
#[serde(alias = "bandwidthlimit", alias = "bandwidth", default)]
pub bandwidth_limit: i64, pub bandwidth_limit: i64,
#[serde(rename = "replicationSync", default)] #[serde(rename = "replicationSync", default)]
pub replication_sync: bool, pub replication_sync: bool,
#[serde(default)] #[serde(alias = "storageclass", default)]
pub storage_class: String, pub storage_class: String,
#[serde(rename = "skipTlsVerify", default)] #[serde(rename = "skipTlsVerify", default)]
pub skip_tls_verify: bool, pub skip_tls_verify: bool,
@@ -220,7 +226,7 @@ pub struct BucketTarget {
#[serde(rename = "resetBeforeDate", with = "time::serde::rfc3339::option", default)] #[serde(rename = "resetBeforeDate", with = "time::serde::rfc3339::option", default)]
pub reset_before_date: Option<OffsetDateTime>, pub reset_before_date: Option<OffsetDateTime>,
#[serde(default)] #[serde(alias = "resetID", default)]
pub reset_id: String, pub reset_id: String,
#[serde(rename = "totalDowntime", with = "duration_seconds", default)] #[serde(rename = "totalDowntime", with = "duration_seconds", default)]
pub total_downtime: Duration, pub total_downtime: Duration,
@@ -233,7 +239,7 @@ pub struct BucketTarget {
#[serde(default)] #[serde(default)]
pub latency: LatencyStat, pub latency: LatencyStat,
#[serde(default)] #[serde(alias = "deploymentID", default)]
pub deployment_id: String, pub deployment_id: String,
#[serde(default)] #[serde(default)]
@@ -531,6 +537,85 @@ mod tests {
assert_eq!(value["totalDowntime"], 90); assert_eq!(value["totalDowntime"], 90);
} }
#[test]
fn bucket_target_persisted_wire_keys_stay_snake_case() {
// bucket-targets.json (persisted via `serde_json::to_vec(&BucketTargets)`
// in the admin set/remove handlers) and the msgpack struct-map form
// (`BucketTargets::marshal_msg`) both come straight from this struct's
// serde field names. madmin naming is applied only in the admin
// response layer (`remote_target_admin_json`); renaming here would
// silently break every existing deployment's persisted metadata.
let targets = BucketTargets {
targets: vec![BucketTarget {
credentials: Some(Credentials {
access_key: "ak".to_string(),
secret_key: "sk".to_string(),
session_token: Some("token".to_string()),
expiration: None,
}),
bandwidth_limit: 5,
storage_class: "STANDARD".to_string(),
reset_id: "reset-1".to_string(),
deployment_id: "deploy-1".to_string(),
..Default::default()
}],
};
let json = serde_json::to_value(&targets).expect("targets should serialize to JSON");
let msgpack: serde_json::Value =
rmp_serde::from_slice(&targets.marshal_msg().expect("targets should marshal to msgpack"))
.expect("msgpack struct map should decode into a JSON value");
for (wire, entry) in [("JSON", &json["targets"][0]), ("msgpack", &msgpack["targets"][0])] {
assert_eq!(entry["bandwidth_limit"], 5, "{wire} key `bandwidth_limit` must stay");
assert_eq!(entry["storage_class"], "STANDARD", "{wire} key `storage_class` must stay");
assert_eq!(entry["reset_id"], "reset-1", "{wire} key `reset_id` must stay");
assert_eq!(entry["deployment_id"], "deploy-1", "{wire} key `deployment_id` must stay");
assert_eq!(entry["credentials"]["session_token"], "token", "{wire} key `session_token` must stay");
}
}
#[test]
fn minio_written_bucket_targets_json_populates_madmin_named_fields() {
// A MinIO-written bucket-targets.json carries madmin's JSON tags
// (`bandwidthlimit`, `storageclass`, `resetID`, `deploymentID`,
// `credentials.sessionToken` — madmin-go v3.0.109 bucket-targets.go).
// On migration these must land in the matching fields instead of
// silently defaulting (backlog#1951).
let targets: BucketTargets = serde_json::from_value(serde_json::json!({
"targets": [{
"sourcebucket": "src",
"endpoint": "minio.example:9000",
"credentials": {
"accessKey": "ak",
"secretKey": "sk",
"sessionToken": "minio-session-token"
},
"targetbucket": "dst",
"type": "replication",
"replicationSync": true,
"bandwidthlimit": 107374182400i64,
"storageclass": "STANDARD",
"resetID": "reset-789",
"deploymentID": "deploy-123"
}]
}))
.expect("MinIO-written bucket-targets.json must deserialize");
let target = &targets.targets[0];
assert_eq!(target.bandwidth_limit, 107374182400);
assert_eq!(target.storage_class, "STANDARD");
assert_eq!(target.reset_id, "reset-789");
assert_eq!(target.deployment_id, "deploy-123");
assert_eq!(
target
.credentials
.as_ref()
.and_then(|credentials| credentials.session_token.as_deref()),
Some("minio-session-token")
);
}
#[test] #[test]
fn test_bucket_target_debug_redacts_credentials() { fn test_bucket_target_debug_redacts_credentials() {
let target = BucketTarget { let target = BucketTarget {
+139 -2
View File
@@ -374,7 +374,10 @@ impl RemoteTargetRequest {
/// Admin-response encoding of a remote target: the persisted bucket-targets /// Admin-response encoding of a remote target: the persisted bucket-targets
/// format keeps `healthCheckDuration`/`totalDowntime` in seconds and the /// format keeps `healthCheckDuration`/`totalDowntime` in seconds and the
/// `latency` stats in milliseconds, but madmin decodes all of them as Go /// `latency` stats in milliseconds, but madmin decodes all of them as Go
/// `time.Duration` (nanoseconds) — re-encode just those fields without /// `time.Duration` (nanoseconds) — and it looks the fields up under its own
/// JSON tags (`bandwidthlimit`, `storageclass`, `resetID`, `deploymentID`,
/// `credentials.sessionToken` — madmin-go v3.0.109 `bucket-targets.go`), not
/// the persisted snake_case keys. Re-encode just those fields here without
/// touching the persistence wire format. /// touching the persistence wire format.
fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value, serde_json::Error> { fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value, serde_json::Error> {
fn go_duration_nanos(duration: Duration) -> serde_json::Value { fn go_duration_nanos(duration: Duration) -> serde_json::Value {
@@ -383,6 +386,12 @@ fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value,
u64::try_from(duration.as_nanos()).unwrap_or(u64::MAX).into() u64::try_from(duration.as_nanos()).unwrap_or(u64::MAX).into()
} }
fn rename_key(value: &mut serde_json::Value, from: &str, to: &str) {
if let Some(moved) = value.as_object_mut().and_then(|object| object.remove(from)) {
value[to] = moved;
}
}
let mut value = serde_json::to_value(target)?; let mut value = serde_json::to_value(target)?;
value["healthCheckDuration"] = go_duration_nanos(target.health_check_duration); value["healthCheckDuration"] = go_duration_nanos(target.health_check_duration);
value["totalDowntime"] = go_duration_nanos(target.total_downtime); value["totalDowntime"] = go_duration_nanos(target.total_downtime);
@@ -391,6 +400,11 @@ fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value,
"avg": go_duration_nanos(target.latency.avg), "avg": go_duration_nanos(target.latency.avg),
"max": go_duration_nanos(target.latency.max), "max": go_duration_nanos(target.latency.max),
}); });
rename_key(&mut value, "bandwidth_limit", "bandwidthlimit");
rename_key(&mut value, "storage_class", "storageclass");
rename_key(&mut value, "reset_id", "resetID");
rename_key(&mut value, "deployment_id", "deploymentID");
rename_key(&mut value["credentials"], "session_token", "sessionToken");
Ok(value) Ok(value)
} }
@@ -1473,7 +1487,7 @@ mod tests {
parse_remote_target_update_ops, render_mrf_backlog, render_replication_diff, unique_replication_peers, parse_remote_target_update_ops, render_mrf_backlog, render_replication_diff, unique_replication_peers,
validate_remote_target_tls_settings, validate_remote_target_tls_settings,
}; };
use crate::admin::storage_api::bucket::target::{BucketTarget, LatencyStat}; use crate::admin::storage_api::bucket::target::{BucketTarget, Credentials as TargetCredentials, LatencyStat};
use crate::admin::storage_api::replication::{BucketStats, DurableMrfBacklog, MrfOpKind, MrfReplicateEntry}; use crate::admin::storage_api::replication::{BucketStats, DurableMrfBacklog, MrfOpKind, MrfReplicateEntry};
use http::Uri; use http::Uri;
@@ -2268,6 +2282,129 @@ mod tests {
assert_eq!(persisted["latency"]["max"], 250); assert_eq!(persisted["latency"]["max"], 250);
} }
#[test]
fn list_remote_targets_response_uses_madmin_key_names() {
// madmin-go v3.0.109 BucketTarget JSON tags are `bandwidthlimit`,
// `storageclass`, `resetID`, `deploymentID`, and
// `credentials.sessionToken` (backlog#1951); the persisted snake_case
// keys decode to zero values in mc, blanking the bandwidth and
// reset-id columns of `mc replicate ls`.
let target = BucketTarget {
endpoint: "192.168.1.10:9000".to_string(),
target_bucket: "target".to_string(),
credentials: Some(TargetCredentials {
access_key: "access".to_string(),
secret_key: String::new(),
session_token: Some("session-token".to_string()),
expiration: None,
}),
bandwidth_limit: 107_374_182_400,
storage_class: "STANDARD".to_string(),
reset_id: "reset-123".to_string(),
deployment_id: "deploy-456".to_string(),
..Default::default()
};
let value = super::remote_target_admin_json(&target).expect("admin response should serialize");
assert_eq!(value["bandwidthlimit"], 107_374_182_400i64);
assert_eq!(value["storageclass"], "STANDARD");
assert_eq!(value["resetID"], "reset-123");
assert_eq!(value["deploymentID"], "deploy-456");
assert_eq!(value["credentials"]["sessionToken"], "session-token");
// The madmin keys replace the snake_case ones rather than duplicating
// them next to each other.
for stale in ["bandwidth_limit", "bandwidth", "storage_class", "reset_id", "deployment_id"] {
assert!(value.get(stale).is_none(), "admin response must not carry `{stale}`");
}
assert!(value["credentials"].get("session_token").is_none());
}
/// Decode-side mirror of madmin-go v3.0.109 `BucketTarget`/`Credentials`
/// (`bucket-targets.go`): the exact `json:"..."` tags mc's `encoding/json`
/// looks fields up under. Unknown keys are ignored like Go does, and a
/// missing key leaves the Go zero value, which is exactly how a misnamed
/// key turns into a blank column in `mc replicate ls`.
#[derive(Debug, Default, serde::Deserialize)]
#[serde(default)]
struct MadminBucketTarget {
sourcebucket: String,
endpoint: String,
credentials: Option<MadminCredentials>,
targetbucket: String,
arn: String,
bandwidthlimit: i64,
#[serde(rename = "replicationSync")]
replication_sync: bool,
storageclass: String,
#[serde(rename = "healthCheckDuration")]
health_check_duration: i64,
#[serde(rename = "resetID")]
reset_id: String,
#[serde(rename = "totalDowntime")]
total_downtime: i64,
#[serde(rename = "deploymentID")]
deployment_id: String,
}
#[derive(Debug, Default, serde::Deserialize)]
#[serde(default)]
struct MadminCredentials {
#[serde(rename = "accessKey")]
access_key: String,
#[serde(rename = "secretKey")]
secret_key: String,
#[serde(rename = "sessionToken")]
session_token: String,
}
#[test]
fn list_remote_targets_response_decodes_through_madmin_tags() {
// Regression for the review on backlog#1951: the response must decode
// a nonzero bandwidth limit through madmin's `bandwidthlimit` tag (not
// `bandwidth`, which Go would silently drop as an unknown key).
let target = BucketTarget {
source_bucket: "src".to_string(),
endpoint: "192.168.1.10:9000".to_string(),
target_bucket: "target".to_string(),
arn: "arn:rustfs:replication:us-east-1:dep:target".to_string(),
credentials: Some(TargetCredentials {
access_key: "access".to_string(),
secret_key: String::new(),
session_token: Some("session-token".to_string()),
expiration: None,
}),
bandwidth_limit: 1_073_741_824,
replication_sync: true,
storage_class: "STANDARD".to_string(),
health_check_duration: std::time::Duration::from_secs(60),
reset_id: "reset-123".to_string(),
total_downtime: std::time::Duration::from_secs(90),
deployment_id: "deploy-456".to_string(),
..Default::default()
};
let wire = serde_json::to_string(&super::remote_target_admin_json(&target).expect("admin response should serialize"))
.expect("admin response should encode");
let decoded: MadminBucketTarget = serde_json::from_str(&wire).expect("madmin-shaped decode must succeed");
assert_eq!(decoded.bandwidthlimit, 1_073_741_824, "mc must see the nonzero bandwidth limit");
assert_eq!(decoded.sourcebucket, "src");
assert_eq!(decoded.endpoint, "192.168.1.10:9000");
assert_eq!(decoded.targetbucket, "target");
assert_eq!(decoded.arn, "arn:rustfs:replication:us-east-1:dep:target");
assert!(decoded.replication_sync);
assert_eq!(decoded.storageclass, "STANDARD");
assert_eq!(decoded.health_check_duration, 60_000_000_000);
assert_eq!(decoded.reset_id, "reset-123");
assert_eq!(decoded.total_downtime, 90_000_000_000);
assert_eq!(decoded.deployment_id, "deploy-456");
let credentials = decoded.credentials.expect("credentials must decode");
assert_eq!(credentials.access_key, "access");
assert_eq!(credentials.secret_key, "");
assert_eq!(credentials.session_token, "session-token");
}
#[test] #[test]
fn remote_target_admin_json_latency_round_trips_through_go_duration() { fn remote_target_admin_json_latency_round_trips_through_go_duration() {
// Round trip: a madmin reader decodes the latency values as Go // Round trip: a madmin reader decodes the latency values as Go