fix(site-replication): hash repair tasks and retry snapshots with sorted JSON keys (backlog#2289)

Service-account items carry their claims in a HashMap, and serde_json is
built with preserve_order, so two serializations of the same plan could
differ in key order. The repair preflight token then went stale between
dry-run and execute (412 on the real VMs once snapshots carried service
accounts) and a retry snapshot resend could never look stable. Serialize
through a key-sorted JSON value for the task id and the fingerprint.

rustfs/src/site_replication/mod.rs

@@ -110,6 +110,26 @@ use tracing::{info, warn};
 use url::{Url, form_urlencoded};
 use uuid::Uuid;
 
+/// Serialize `value` with every JSON object's keys sorted, for hashing and
+/// equality checks. `HashMap` fields (service-account claims) iterate in a
+/// per-instance random order and `serde_json` is built with `preserve_order`,
+/// so two identical plans would otherwise hash differently: the repair
+/// preflight token went stale between dry-run and execute, and a retry
+/// snapshot resend never looked "stable" (backlog#2289 follow-up).
+pub(crate) fn canonical_json_vec<T: Serialize>(value: &T) -> serde_json::Result<Vec<u8>> {
+    fn sort_keys(value: Value) -> Value {
+        match value {
+            Value::Object(map) => {
+                let sorted: BTreeMap<String, Value> = map.into_iter().map(|(key, value)| (key, sort_keys(value))).collect();
+                Value::Object(sorted.into_iter().collect())
+            }
+            Value::Array(items) => Value::Array(items.into_iter().map(sort_keys).collect()),
+            other => other,
+        }
+    }
+    serde_json::to_vec(&sort_keys(serde_json::to_value(value)?))
+}
+
 pub(crate) const LOG_COMPONENT_ADMIN: &str = "admin";
 
 pub(crate) const LOG_SUBSYSTEM_SITE_REPLICATION: &str = "site_replication";

rustfs/src/site_replication/repair.rs

@@ -234,9 +234,9 @@ impl SiteReplicationRepairTask<'_> {
 
     pub(crate) fn id(&self) -> S3Result<String> {
         let payload = match self {
-            Self::Iam(item) => serde_json::to_vec(item),
+            Self::Iam(item) => canonical_json_vec(item),
             Self::BucketMake(_) | Self::Replication(_) => serde_json::to_vec(&serde_json::json!({})),
-            Self::BucketMetadata(item) => serde_json::to_vec(item),
+            Self::BucketMetadata(item) => canonical_json_vec(item),
         }
         .map_err(|err| S3Error::with_message(S3ErrorCode::InternalError, format!("serialize repair task failed: {err}")))?;
         let mut digest = Sha256::new();

rustfs/src/site_replication/retry.rs

@@ -867,8 +867,8 @@ impl RetrySnapshot {
 
     pub(crate) fn fingerprint(&self) -> S3Result<Vec<Vec<u8>>> {
         let mut payloads = match self {
-            Self::Iam(items) => items.iter().map(serde_json::to_vec).collect::<Result<Vec<_>, _>>(),
-            Self::BucketMetadata(items) => items.iter().map(serde_json::to_vec).collect::<Result<Vec<_>, _>>(),
+            Self::Iam(items) => items.iter().map(canonical_json_vec).collect::<Result<Vec<_>, _>>(),
+            Self::BucketMetadata(items) => items.iter().map(canonical_json_vec).collect::<Result<Vec<_>, _>>(),
         }
         .map_err(|err| S3Error::with_message(S3ErrorCode::InternalError, format!("serialize retry snapshot failed: {err}")))?;
         payloads.sort_unstable();

rustfs/src/site_replication/tests.rs

@@ -3609,3 +3609,73 @@ async fn test_broadcast_json_reaches_healthy_peers_after_a_peer_without_transpor
     );
     server.abort();
 }
+
+fn service_account_item_with_claims(order: &[&str]) -> SRIAMItem {
+    let mut claims = HashMap::new();
+    for key in order {
+        claims.insert((*key).to_string(), serde_json::json!(format!("value-of-{key}")));
+    }
+    SRIAMItem {
+        r#type: "service-account".to_string(),
+        svc_acc_change: Some(SRSvcAccChange {
+            create: Some(rustfs_madmin::SRSvcAccCreate {
+                parent: "alice".to_string(),
+                access_key: "alice-svc".to_string(),
+                secret_key: "alice-svc-secret".to_string(),
+                groups: Vec::new(),
+                claims,
+                session_policy: SRSessionPolicy::default(),
+                status: "on".to_string(),
+                name: String::new(),
+                description: String::new(),
+                expiration: None,
+                api_version: Some(SITE_REPL_API_VERSION.to_string()),
+            }),
+            api_version: Some(SITE_REPL_API_VERSION.to_string()),
+            ..Default::default()
+        }),
+        updated_at: Some(OffsetDateTime::from_unix_timestamp(1_700_000_000).expect("timestamp")),
+        api_version: Some(SITE_REPL_API_VERSION.to_string()),
+        ..Default::default()
+    }
+}
+
+/// The repair preflight token and the retry-snapshot fingerprint hash the
+/// serialized items. Service-account claims live in a `HashMap`, whose
+/// iteration order differs between instances, so the hash must not depend on
+/// it (the real-VM repair returned 412 "preflight is stale" between dry-run
+/// and execute once snapshots carried service accounts).
+#[test]
+fn test_repair_task_id_and_retry_fingerprint_ignore_claim_map_order() {
+    let forward = service_account_item_with_claims(&["accessKey", "exp", "parent", "sa-policy", "sub", "tenant"]);
+    let backward = service_account_item_with_claims(&["tenant", "sub", "sa-policy", "parent", "exp", "accessKey"]);
+
+    let canonical = canonical_json_vec(&forward).expect("canonical json");
+    let text = String::from_utf8(canonical).expect("utf8");
+    let positions: Vec<usize> = [
+        "\"accessKey\"",
+        "\"exp\"",
+        "\"parent\"",
+        "\"sa-policy\"",
+        "\"sub\"",
+        "\"tenant\"",
+    ]
+    .iter()
+    .map(|key| text.find(key).expect("claim key present"))
+    .collect();
+    assert!(
+        positions.windows(2).all(|pair| pair[0] < pair[1]),
+        "claim keys must serialize sorted: {text}"
+    );
+
+    assert_eq!(
+        SiteReplicationRepairTask::Iam(&forward).id().expect("id"),
+        SiteReplicationRepairTask::Iam(&backward).id().expect("id"),
+        "identical items must yield the same repair task id regardless of claim map order"
+    );
+    assert_eq!(
+        RetrySnapshot::Iam(vec![forward]).fingerprint().expect("fingerprint"),
+        RetrySnapshot::Iam(vec![backward]).fingerprint().expect("fingerprint"),
+        "identical snapshots must fingerprint equal regardless of claim map order"
+    );
+}