fix(replication): tolerate Go zero-value expiration and ignore latency in remote target requests (#5789)

* test(replication): accept real madmin marshal payload with zero-value expiration

* fix(replication): tolerate Go zero-value expiration and ignore latency in remote target requests

rustfs/src/admin/handlers/replication.rs

@@ -58,6 +58,13 @@ use url::Host;
 
 const SUPPORTED_REMOTE_TARGET_API: &str = "s3v4";
 
+/// Go encodes the zero `time.Time` as the year-1 instant
+/// (`0001-01-01T00:00:00Z`, possibly re-encoded with an offset); no real
+/// credential expiry lives in year 1, so any such timestamp means "unset".
+fn is_go_zero_time(timestamp: Timestamp) -> bool {
+    timestamp.to_zoned(jiff::tz::TimeZone::UTC).year() == 1
+}
+
 /// Field groups a `set-remote-target?update=true` request may modify, mirroring
 /// MinIO's `TargetUpdateType` / `GetTargetUpdateOps` query contract: the update
 /// overlays only the requested groups onto the stored target, so a client can
@@ -214,8 +221,10 @@ struct RemoteTargetRequest {
     last_online: Option<OffsetDateTime>,
     #[serde(rename = "isOnline", default)]
     online: bool,
-    #[serde(default)]
-    latency: LatencyStat,
+    // Accepted so mc's round-tripped runtime latency still deserializes under
+    // deny_unknown_fields, but deliberately unread: it is never persisted.
+    #[serde(rename = "latency", default)]
+    _latency: LatencyStat,
     #[serde(alias = "deploymentID", default)]
     deployment_id: String,
     #[serde(default)]
@@ -283,7 +292,14 @@ impl RemoteTargetRequest {
             ));
         }
 
-        if self.credentials.expiration.is_some() {
+        // Go's `omitempty` never elides a zero `time.Time`, so every madmin
+        // marshal carries `"expiration":"0001-01-01T00:00:00Z"`; only a real
+        // (non-year-1) expiry means the client wants expiring credentials.
+        if self
+            .credentials
+            .expiration
+            .is_some_and(|expiration| !is_go_zero_time(expiration))
+        {
             return Err(s3_error!(
                 InvalidRequest,
                 "remote target field credentials.expiration is not supported by this RustFS version"
@@ -311,10 +327,15 @@ impl RemoteTargetRequest {
             }
         }
 
+        let mut credentials = TargetCredentials::from(self.credentials);
+        // Past the check above the expiration can only be the zero-value
+        // sentinel, i.e. "no expiration" — never persist it.
+        credentials.expiration = None;
+
         Ok(BucketTarget {
             source_bucket: self.source_bucket,
             endpoint: self.endpoint,
-            credentials: Some(self.credentials.into()),
+            credentials: Some(credentials),
             target_bucket: self.target_bucket,
             secure: self.secure,
             path: self.path,
@@ -338,7 +359,11 @@ impl RemoteTargetRequest {
             total_downtime: duration_from_secs_or_nanos(self.total_downtime),
             last_online: self.last_online,
             online: self.online,
-            latency: self.latency,
+            // Latency is a server-measured runtime stat that mc echoes back
+            // from list-remote-targets (Go time.Duration nanoseconds), while
+            // the persisted format is milliseconds — never store the client
+            // value.
+            latency: LatencyStat::default(),
             deployment_id: self.deployment_id,
             edge: self.edge,
             edge_sync_before_expiry: self.edge_sync_before_expiry,
@@ -1242,6 +1267,26 @@ mod tests {
         })
     }
 
+    /// Verbatim `encoding/json` marshal of the madmin-go v3.0.109 `BucketTarget`
+    /// that `mc replicate add` builds (path=auto, api=s3v4, 60s healthcheck).
+    /// Field presence follows Go `omitempty` semantics against the v3.0.109 tags:
+    /// zero strings (`sessionToken`, `arn`, `region`, `storageclass`, `resetID`,
+    /// `deploymentID`) and the zero `bandwidthlimit` int are elided; untagged
+    /// fields (`sourcebucket`, `secure`, `type`, `replicationSync`,
+    /// `disableProxy`, `totalDowntime`, `lastOnline`, `isOnline`, `latency`,
+    /// `edge`, `edgeSyncBeforeExpiry`, `offlineCount`) always appear; and
+    /// `omitempty` never elides a struct, so the zero `time.Time` fields
+    /// (`credentials.expiration`, `resetBeforeDate`) still appear as
+    /// `0001-01-01T00:00:00Z`.
+    const MADMIN_REPLICATE_ADD_BODY: &str = r#"{"sourcebucket":"","endpoint":"192.168.1.10:9000","credentials":{"accessKey":"access","secretKey":"secret","expiration":"0001-01-01T00:00:00Z"},"targetbucket":"target","secure":false,"path":"auto","api":"s3v4","type":"replication","replicationSync":false,"healthCheckDuration":60000000000,"disableProxy":false,"resetBeforeDate":"0001-01-01T00:00:00Z","totalDowntime":0,"lastOnline":"0001-01-01T00:00:00Z","isOnline":false,"latency":{"curr":0,"avg":0,"max":0},"edge":false,"edgeSyncBeforeExpiry":false,"offlineCount":0}"#;
+
+    /// Verbatim marshal of the same target after `mc replicate update --sync`:
+    /// mc lists remote targets (latency reported in Go `time.Duration`
+    /// nanoseconds), `BucketTarget::Clone()` strips only the secret key, and the
+    /// mutated clone round-trips every other stored field — including the
+    /// nanosecond latency echo — back to set-remote-target.
+    const MADMIN_REPLICATE_UPDATE_BODY: &str = r#"{"sourcebucket":"src","endpoint":"192.168.1.10:9000","credentials":{"accessKey":"access","expiration":"0001-01-01T00:00:00Z"},"targetbucket":"target","secure":false,"path":"auto","api":"s3v4","arn":"arn:rustfs:replication:us-east-1:dep:target","type":"replication","replicationSync":true,"healthCheckDuration":60000000000,"disableProxy":false,"resetBeforeDate":"0001-01-01T00:00:00Z","totalDowntime":0,"lastOnline":"0001-01-01T00:00:00Z","isOnline":true,"latency":{"curr":60000000000,"avg":45000000000,"max":90000000000},"edge":false,"edgeSyncBeforeExpiry":false,"offlineCount":0}"#;
+
     fn query_map(pairs: &[(&str, &str)]) -> std::collections::HashMap<String, String> {
         pairs.iter().map(|(k, v)| (k.to_string(), v.to_string())).collect()
     }
@@ -1674,6 +1719,78 @@ mod tests {
         }
     }
 
+    #[test]
+    fn remote_target_request_accepts_real_madmin_add_marshal() {
+        let request: RemoteTargetRequest =
+            serde_json::from_str(MADMIN_REPLICATE_ADD_BODY).expect("real mc replicate add body should deserialize");
+        let target = request
+            .into_bucket_target()
+            .expect("real mc replicate add body must be accepted");
+
+        assert_eq!(target.endpoint, "192.168.1.10:9000");
+        assert_eq!(target.target_bucket, "target");
+        assert_eq!(target.health_check_duration, std::time::Duration::from_secs(60));
+        let credentials = target.credentials.expect("credentials should be present");
+        assert_eq!(credentials.access_key, "access");
+        assert_eq!(credentials.secret_key, "secret");
+        assert!(
+            credentials.expiration.is_none(),
+            "Go zero-value expiration is an unset sentinel and must not be persisted"
+        );
+    }
+
+    #[test]
+    fn remote_target_request_accepts_go_zero_time_expiration_offset_forms() {
+        // Go marshals the zero time.Time as 0001-01-01T00:00:00Z, but a body
+        // that re-encoded the same instant with an offset is equally "unset".
+        for expiration in ["0001-01-01T00:00:00Z", "0001-01-01T08:00:00+08:00"] {
+            let mut request = valid_remote_target_request();
+            request["credentials"]["expiration"] = serde_json::json!(expiration);
+            let request: RemoteTargetRequest = serde_json::from_value(request).expect("request should deserialize");
+            request
+                .into_bucket_target()
+                .unwrap_or_else(|err| panic!("zero-value expiration {expiration} must be accepted: {err}"));
+        }
+    }
+
+    #[test]
+    fn remote_target_update_accepts_madmin_clone_round_trip() {
+        let request: RemoteTargetRequest =
+            serde_json::from_str(MADMIN_REPLICATE_UPDATE_BODY).expect("mc replicate update body should deserialize");
+        let target = request
+            .into_update_bucket_target(&[TargetUpdateOp::Sync])
+            .expect("mc replicate update round-trip body must be accepted");
+
+        assert!(target.replication_sync);
+        assert_eq!(
+            target.latency.curr,
+            std::time::Duration::ZERO,
+            "nanosecond latency echoed back by mc must not be persisted as milliseconds"
+        );
+    }
+
+    #[test]
+    fn remote_target_request_ignores_client_supplied_latency() {
+        // Latency is a server-measured runtime stat; mc echoes the
+        // list-remote-targets nanosecond values back on update, while the
+        // request parser historically read them as milliseconds (1e6 blow-up).
+        let mut request = valid_remote_target_request();
+        request["latency"] = serde_json::json!({
+            "curr": 60_000_000_000u64,
+            "avg": 45_000_000_000u64,
+            "max": 90_000_000_000u64
+        });
+
+        let target = serde_json::from_value::<RemoteTargetRequest>(request)
+            .expect("request should deserialize")
+            .into_bucket_target()
+            .expect("request should pass semantic validation");
+
+        assert_eq!(target.latency.curr, std::time::Duration::ZERO);
+        assert_eq!(target.latency.avg, std::time::Duration::ZERO);
+        assert_eq!(target.latency.max, std::time::Duration::ZERO);
+    }
+
     #[test]
     fn remote_target_request_accepts_static_credentials_and_supported_api() {
         let mut request = valid_remote_target_request();