From 9ed8d38a81c70448b92ff55373beab8e7426a1f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=94=90=E5=B0=8F=E9=B8=AD?= Date: Fri, 21 Aug 2026 19:18:48 +0800 Subject: [PATCH] fix(admin): expose per-target disableProxy through remote target admin API The read-proxy selector already honors a target's disable_proxy flag (PR #6172), but the admin API still rejected the field, so the only way to set it was importing a MinIO-written bucket-targets.json. - move disableProxy from REMOTE_TARGET_UNSUPPORTED_FIELDS to REMOTE_TARGET_WRITABLE_FIELDS (set-remote-target create accepts it) - add TargetUpdateOp::Proxy so set-remote-target?update=true&proxy=true overlays only the proxy group (MinIO TargetUpdateType parity) - bump REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION 1 -> 2 and update the runtime capability pin tests - keep edge/edgeSyncBeforeExpiry rejected (no implementation behind them) - pin that a published TargetClient carries disable_proxy, the field the proxy-target selector consults Refs rustfs/backlog#1950 --- .../ecstore/src/bucket/bucket_target_sys.rs | 38 +++++++++++++ crates/replication/src/config.rs | 9 ++- rustfs/src/admin/handlers/replication.rs | 56 ++++++++++++++++--- rustfs/src/admin/handlers/system.rs | 25 +++++++-- 4 files changed, 115 insertions(+), 13 deletions(-) diff --git a/crates/ecstore/src/bucket/bucket_target_sys.rs b/crates/ecstore/src/bucket/bucket_target_sys.rs index f5ac0c013..c30a21221 100644 --- a/crates/ecstore/src/bucket/bucket_target_sys.rs +++ b/crates/ecstore/src/bucket/bucket_target_sys.rs @@ -3425,6 +3425,44 @@ mod tests { assert!(mutexes.contains_key("second")); } + #[tokio::test] + async fn update_all_targets_publishes_disable_proxy_on_target_client() { + // The read-proxy selector (replication_proxy::get_proxy_targets) skips + // targets whose TargetClient carries disable_proxy — the persisted + // per-target opt-out must survive client publication. + let sys = BucketTargetSys::default(); + let target = |arn: &str, disable_proxy: bool| BucketTarget { + arn: arn.to_string(), + endpoint: "192.168.1.10:9000".to_string(), + target_bucket: "target-bucket".to_string(), + region: "us-east-1".to_string(), + disable_proxy, + credentials: Some(Credentials { + access_key: "access".to_string(), + secret_key: "secret".to_string(), + session_token: None, + expiration: None, + }), + ..Default::default() + }; + let targets = BucketTargets { + targets: vec![target("arn:proxied", false), target("arn:opted-out", true)], + }; + + sys.update_all_targets("bucket", Some(&targets)).await; + + let proxied = sys + .get_remote_target_client("bucket", "arn:proxied") + .await + .expect("client should be published"); + assert!(!proxied.disable_proxy); + let opted_out = sys + .get_remote_target_client("bucket", "arn:opted-out") + .await + .expect("client should be published"); + assert!(opted_out.disable_proxy, "disable_proxy must reach the published TargetClient"); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn target_updates_serialize_client_build_through_publication_per_bucket() { let sys = Arc::new(BucketTargetSys::default()); diff --git a/crates/replication/src/config.rs b/crates/replication/src/config.rs index c8ca328a5..405799f94 100644 --- a/crates/replication/src/config.rs +++ b/crates/replication/src/config.rs @@ -60,7 +60,9 @@ pub const REPLICATION_READ_ONLY_HISTORICAL_FIELDS: &[&str] = &[ "Destination.ReplicationTime", ]; -pub const REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION: u32 = 1; +// v2: disableProxy moved from unsupported to writable (per-target read-proxy +// opt-out is accepted by set-remote-target and the `proxy` update op). +pub const REMOTE_TARGET_CAPABILITY_CONTRACT_VERSION: u32 = 2; pub const REMOTE_TARGET_WRITABLE_FIELDS: &[&str] = &[ "sourcebucket", @@ -83,9 +85,12 @@ pub const REMOTE_TARGET_WRITABLE_FIELDS: &[&str] = &[ // madmin default of 60s); the per-target health-check interval is not // yet applied — the heartbeat keeps its global env-configured interval. "healthCheckDuration", + // Per-target read-proxy opt-out, consumed by the proxy-target selector + // (contract v2; previously only importable via MinIO bucket-targets.json). + "disableProxy", ]; -pub const REMOTE_TARGET_UNSUPPORTED_FIELDS: &[&str] = &["disableProxy", "edge", "edgeSyncBeforeExpiry"]; +pub const REMOTE_TARGET_UNSUPPORTED_FIELDS: &[&str] = &["edge", "edgeSyncBeforeExpiry"]; #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct ObjectOpts { diff --git a/rustfs/src/admin/handlers/replication.rs b/rustfs/src/admin/handlers/replication.rs index 106c25790..229c3c9fd 100644 --- a/rustfs/src/admin/handlers/replication.rs +++ b/rustfs/src/admin/handlers/replication.rs @@ -73,6 +73,8 @@ enum TargetUpdateOp { /// Connection group: credentials plus endpoint, target bucket, and TLS settings. Credentials, Sync, + /// Per-target read-proxy opt-out (`disableProxy`). + Proxy, Bandwidth, Path, } @@ -81,12 +83,13 @@ fn parse_remote_target_update_ops(queries: &HashMap) -> S3Result const SUPPORTED_OPS: &[(&str, TargetUpdateOp)] = &[ ("creds", TargetUpdateOp::Credentials), ("sync", TargetUpdateOp::Sync), + ("proxy", TargetUpdateOp::Proxy), ("bandwidth", TargetUpdateOp::Bandwidth), ("path", TargetUpdateOp::Path), ]; // Present in the MinIO wire contract, but they drive target fields this // version rejects as unsupported — fail loudly instead of silently ignoring. - const UNSUPPORTED_OPS: &[&str] = &["proxy", "healthcheck", "edge", "edgeSyncBeforeExpiry"]; + const UNSUPPORTED_OPS: &[&str] = &["healthcheck", "edge", "edgeSyncBeforeExpiry"]; for key in UNSUPPORTED_OPS { if queries.get(*key).is_some_and(|value| value == "true") { @@ -312,11 +315,10 @@ impl RemoteTargetRequest { )); } - for (unsupported, configured) in - REMOTE_TARGET_UNSUPPORTED_FIELDS - .iter() - .copied() - .zip([self.disable_proxy, self.edge, self.edge_sync_before_expiry]) + for (unsupported, configured) in REMOTE_TARGET_UNSUPPORTED_FIELDS + .iter() + .copied() + .zip([self.edge, self.edge_sync_before_expiry]) { if configured { return Err(s3_error!( @@ -702,6 +704,7 @@ impl Operation for SetRemoteTargetHandler { target.deployment_id = remote_target.deployment_id.clone(); } TargetUpdateOp::Sync => target.replication_sync = remote_target.replication_sync, + TargetUpdateOp::Proxy => target.disable_proxy = remote_target.disable_proxy, TargetUpdateOp::Bandwidth => target.bandwidth_limit = remote_target.bandwidth_limit, TargetUpdateOp::Path => target.path = remote_target.path.clone(), } @@ -1520,6 +1523,7 @@ mod tests { ("update", "true"), ("creds", "true"), ("sync", "true"), + ("proxy", "true"), ("bandwidth", "true"), ("path", "true"), ])) @@ -1529,6 +1533,7 @@ mod tests { vec![ TargetUpdateOp::Credentials, TargetUpdateOp::Sync, + TargetUpdateOp::Proxy, TargetUpdateOp::Bandwidth, TargetUpdateOp::Path ] @@ -2070,7 +2075,6 @@ mod tests { ("credentials.session_token", serde_json::json!("session-token")), ("credentials.expiration", serde_json::json!("2026-01-01T00:00:00Z")), ("api", serde_json::json!("s3v2")), - ("disableProxy", serde_json::json!(true)), ("edge", serde_json::json!(true)), ("edgeSyncBeforeExpiry", serde_json::json!(true)), ] { @@ -2300,6 +2304,44 @@ mod tests { assert!(!REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"healthCheckDuration")); } + #[test] + fn remote_target_disable_proxy_is_declared_writable_edge_stays_unsupported() { + assert!(REMOTE_TARGET_WRITABLE_FIELDS.contains(&"disableProxy")); + assert!(!REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"disableProxy")); + // edge sync has no implementation behind it — it must stay rejected. + assert!(REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"edge")); + assert!(REMOTE_TARGET_UNSUPPORTED_FIELDS.contains(&"edgeSyncBeforeExpiry")); + } + + #[test] + fn remote_target_create_accepts_disable_proxy() { + let mut request = valid_remote_target_request(); + request["disableProxy"] = serde_json::json!(true); + + let target = serde_json::from_value::(request) + .expect("request should deserialize") + .into_bucket_target() + .expect("disableProxy is a supported per-target read-proxy opt-out"); + + assert!(target.disable_proxy); + } + + #[test] + fn update_body_with_proxy_op_toggles_disable_proxy_without_credentials() { + // Mirrors the other partial-update groups: a proxy-only update body may + // omit the connection fields entirely. + let body = serde_json::json!({ + "arn": "arn:rustfs:replication:us-east-1:dep:target", + "type": "replication", + "disableProxy": true + }); + let request: RemoteTargetRequest = serde_json::from_value(body).expect("partial update body should deserialize"); + let target = request + .into_update_bucket_target(&[TargetUpdateOp::Proxy]) + .expect("proxy-only update must not require credentials"); + assert!(target.disable_proxy); + } + #[test] fn remote_target_capability_fields_do_not_overlap() { for field in REMOTE_TARGET_UNSUPPORTED_FIELDS { diff --git a/rustfs/src/admin/handlers/system.rs b/rustfs/src/admin/handlers/system.rs index 9df481c11..981baaefb 100644 --- a/rustfs/src/admin/handlers/system.rs +++ b/rustfs/src/admin/handlers/system.rs @@ -1262,7 +1262,9 @@ mod tests { assert_eq!(response.summary.manual_transition_jobs.state, CapabilityState::Supported); assert_eq!(response.replication.contract_version, 1); assert_eq!(response.replication.bucket_replication.contract_version, 1); - assert_eq!(response.replication.remote_targets.contract_version, 1); + // v2: disableProxy moved from unsupported to writable (per-target + // read-proxy opt-out reached the admin API). + assert_eq!(response.replication.remote_targets.contract_version, 2); assert_eq!(response.replication.bucket_replication.status.state, CapabilityState::Supported); assert_eq!(response.replication.remote_targets.status.state, CapabilityState::Supported); assert_eq!( @@ -1293,7 +1295,15 @@ mod tests { .remote_targets .fields .iter() - .any(|field| field.name == "disableProxy" && field.state == super::ReplicationFieldState::Unsupported) + .any(|field| field.name == "disableProxy" && field.state == super::ReplicationFieldState::Supported) + ); + assert!( + response + .replication + .remote_targets + .fields + .iter() + .any(|field| field.name == "edge" && field.state == super::ReplicationFieldState::Unsupported) ); assert!( response @@ -1364,7 +1374,7 @@ mod tests { assert_eq!(value["summary"]["manual_transition_jobs"]["state"], "supported"); assert_eq!(value["replication"]["contract_version"], 1); assert_eq!(value["replication"]["bucket_replication"]["contract_version"], 1); - assert_eq!(value["replication"]["remote_targets"]["contract_version"], 1); + assert_eq!(value["replication"]["remote_targets"]["contract_version"], 2); assert_eq!(value["replication"]["bucket_replication"]["status"]["state"], "supported"); assert_eq!(value["replication"]["remote_targets"]["status"]["state"], "supported"); assert_eq!( @@ -1383,7 +1393,14 @@ mod tests { .as_array() .expect("remote target fields should be an array") .iter() - .any(|field| field["name"] == "disableProxy" && field["state"] == "unsupported") + .any(|field| field["name"] == "disableProxy" && field["state"] == "supported") + ); + assert!( + value["replication"]["remote_targets"]["fields"] + .as_array() + .expect("remote target fields should be an array") + .iter() + .any(|field| field["name"] == "edge" && field["state"] == "unsupported") ); assert!( value["replication"]["remote_targets"]["fields"]