fix(admin): use madmin key names in list-remote-targets response (#6377)

rustfs/src/admin/handlers/replication.rs

@@ -376,7 +376,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
+/// `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.
 fn remote_target_admin_json(target: &BucketTarget) -> Result<serde_json::Value, serde_json::Error> {
     fn go_duration_nanos(duration: Duration) -> serde_json::Value {
@@ -385,6 +388,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);
@@ -393,6 +402,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", "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)
 }
 
@@ -1476,7 +1490,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;
 
@@ -2272,6 +2286,129 @@ mod tests {
         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]
     fn remote_target_admin_json_latency_round_trips_through_go_duration() {
         // Round trip: a madmin reader decodes the latency values as Go