Compare commits

...

1 Commits

Author SHA1 Message Date
唐小鸭 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 139 additions and 6 deletions
@@ -32,6 +32,10 @@ pub struct Credentials {
pub access_key: String,
#[serde(rename = "secretKey")]
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 expiration: Option<Timestamp>,
}
@@ -207,7 +211,7 @@ pub struct BucketTarget {
#[serde(rename = "replicationSync", default)]
pub replication_sync: bool,
#[serde(default)]
#[serde(alias = "storageclass", default)]
pub storage_class: String,
#[serde(rename = "skipTlsVerify", default)]
pub skip_tls_verify: bool,
@@ -220,7 +224,7 @@ pub struct BucketTarget {
#[serde(rename = "resetBeforeDate", with = "time::serde::rfc3339::option", default)]
pub reset_before_date: Option<OffsetDateTime>,
#[serde(default)]
#[serde(alias = "resetID", default)]
pub reset_id: String,
#[serde(rename = "totalDowntime", with = "duration_seconds", default)]
pub total_downtime: Duration,
@@ -233,7 +237,7 @@ pub struct BucketTarget {
#[serde(default)]
pub latency: LatencyStat,
#[serde(default)]
#[serde(alias = "deploymentID", default)]
pub deployment_id: String,
#[serde(default)]
@@ -531,6 +535,84 @@ mod tests {
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
// (`bandwidth`, `storageclass`, `resetID`, `deploymentID`,
// `credentials.sessionToken`). On migration these must land in the
// matching fields instead of silently defaulting (backlog#1946).
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,
"bandwidth": 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]
fn test_bucket_target_debug_redacts_credentials() {
let target = BucketTarget {
+54 -3
View File
@@ -374,8 +374,10 @@ impl RemoteTargetRequest {
/// Admin-response encoding of a remote target: the persisted bucket-targets
/// format keeps `healthCheckDuration`/`totalDowntime` in seconds and the
/// `latency` stats in milliseconds, but madmin decodes all of them as Go
/// `time.Duration` (nanoseconds) — re-encode just those fields without
/// touching the persistence wire format.
/// `time.Duration` (nanoseconds) — and it looks the fields up under its own
/// JSON tags (`bandwidth`, `storageclass`, `resetID`, `deploymentID`,
/// `credentials.sessionToken`), not the persisted snake_case keys. Re-encode
/// just those fields here without touching the persistence wire format.
fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value, serde_json::Error> {
fn go_duration_nanos(duration: Duration) -> serde_json::Value {
// Saturate instead of truncating: >u64::MAX nanoseconds (~584 years)
@@ -383,6 +385,12 @@ fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value,
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)?;
value["healthCheckDuration"] = go_duration_nanos(target.health_check_duration);
value["totalDowntime"] = go_duration_nanos(target.total_downtime);
@@ -391,6 +399,11 @@ fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value,
"avg": go_duration_nanos(target.latency.avg),
"max": go_duration_nanos(target.latency.max),
});
rename_key(&mut value, "bandwidth_limit", "bandwidth");
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)
}
@@ -1473,7 +1486,7 @@ mod tests {
parse_remote_target_update_ops, render_mrf_backlog, render_replication_diff, unique_replication_peers,
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 http::Uri;
@@ -2268,6 +2281,44 @@ mod tests {
assert_eq!(persisted["latency"]["max"], 250);
}
#[test]
fn list_remote_targets_response_uses_madmin_key_names() {
// madmin's BucketTarget JSON tags are `bandwidth`, `storageclass`,
// `resetID`, `deploymentID`, and `credentials.sessionToken`
// (backlog#1946); 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["bandwidth"], 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", "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());
}
#[test]
fn remote_target_admin_json_latency_round_trips_through_go_duration() {
// Round trip: a madmin reader decodes the latency values as Go