refactor(protos): move compat manifest send-site assertions into owning crates (#6618)

refactor(protos): move internode compat manifest send-site assertions into owning crates Promotes the rolling-upgrade dual-write manifest from a test-only constant in rustfs-protos into the public rustfs_protos::compat_manifest module, moves the JSON-encoder send-site assertions into the crates that own the asserted sources (ecstore remote_disk.rs for requests, the rustfs binary node_service/disk.rs for responses), and splits the scanner Phase-0 overlap inventory so its heal- and ecstore-owned halves live in those crates. Adds a cross-crate include_str!/include! guard with fixture self-tests to scripts/check_layer_dependencies.sh so a library crate can never again read another crate's Rust source at compile time, and records the rule in docs/architecture/crate-boundaries.md. Part of rustfs/backlog#1884.
2026-08-29 16:37:07 +00:00 · 2026-08-26 11:11:16 +08:00
parent eaf0d4da81
commit 7cac528de3
9 changed files with 423 additions and 227 deletions
@@ -0,0 +1,222 @@
+// Copyright 2024 RustFS Team
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Rolling-upgrade compatibility manifest for the internode RPC dual-write
+//! payload fields in `node.proto`.
+//!
+//! Every `xxx` / `xxx_bin` field pair carries the same payload twice: legacy
+//! JSON for old readers and msgpack for new ones. If a new node stops
+//! producing the JSON side before the fleet-wide fallback count reaches zero,
+//! an old node silently decodes an empty payload mid-upgrade. This manifest
+//! pins, per message field, which JSON encoder call site must keep existing
+//! and under which policy.
+//!
+//! This is a guard contract surface, not a runtime API. Tests in this crate
+//! assert that the manifest exactly covers the `_bin` field pairs declared in
+//! `node.proto`; the crates that own the send sites assert their own source
+//! against the manifest (`crates/ecstore/src/cluster/rpc/remote_disk.rs` for
+//! request sites, `rustfs/src/storage/rpc/node_service/disk.rs` for response
+//! sites). Keeping those assertions in the owning crates keeps the dependency
+//! direction intact: a contract crate must never read implementation-crate or
+//! binary-crate sources (see `docs/architecture/crate-boundaries.md`, enforced
+//! by `scripts/check_layer_dependencies.sh`).
+
+/// One `json_field` / `bin_field` dual-write pair on an internode RPC message.
+#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
+pub struct CompatPayloadField {
+    pub message: &'static str,
+    pub json_field: &'static str,
+    pub bin_field: &'static str,
+}
+
+/// JSON-side production policy for a request payload field.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum RequestJsonPolicy {
+    /// May skip the JSON side once `internode_rpc_msgpack_only()` is on.
+    MsgpackOnlyEligible,
+    /// Must keep dual-writing JSON until the msgpack fallback count is zero.
+    AlwaysDualWriteUntilFallbackZero,
+}
+
+/// A request-side dual-write send site in `crates/ecstore/src/cluster/rpc/remote_disk.rs`.
+#[derive(Clone, Copy, Debug)]
+pub struct RequestCompatSendSite {
+    pub field: CompatPayloadField,
+    /// Exact JSON-encoder statement that must keep existing at the send site.
+    pub json_encoder: &'static str,
+    pub policy: RequestJsonPolicy,
+}
+
+/// A response-side dual-write send site in `rustfs/src/storage/rpc/node_service/disk.rs`.
+#[derive(Clone, Copy, Debug)]
+pub struct ResponseCompatSendSite {
+    pub field: CompatPayloadField,
+    /// Exact JSON-encoder statement that must keep existing at the send site.
+    pub json_encoder: &'static str,
+}
+
+pub const REQUEST_COMPAT_SEND_SITES: &[RequestCompatSendSite] = &[
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "BatchReadVersionRequest",
+            json_field: "batch_read_version_req",
+            bin_field: "batch_read_version_req_bin",
+        },
+        json_encoder: "let batch_read_version_req = compat_json(&req)?;",
+        policy: RequestJsonPolicy::MsgpackOnlyEligible,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "DeleteVersionRequest",
+            json_field: "file_info",
+            bin_field: "file_info_bin",
+        },
+        json_encoder: "let file_info = serde_json::to_string(&fi)?;",
+        policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "DeleteVersionRequest",
+            json_field: "opts",
+            bin_field: "opts_bin",
+        },
+        json_encoder: "let opts = serde_json::to_string(&opts)?;",
+        policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "DeleteVersionsRequest",
+            json_field: "opts",
+            bin_field: "opts_bin",
+        },
+        json_encoder: "let opts = match serde_json::to_string(&opts) {",
+        policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "DeleteVersionsRequest",
+            json_field: "versions",
+            bin_field: "versions_bin",
+        },
+        json_encoder: "versions_str.push(match serde_json::to_string(file_info_versions) {",
+        policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "ReadMultipleRequest",
+            json_field: "read_multiple_req",
+            bin_field: "read_multiple_req_bin",
+        },
+        json_encoder: "let read_multiple_req = compat_json(&req)?;",
+        policy: RequestJsonPolicy::MsgpackOnlyEligible,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "ReadVersionRequest",
+            json_field: "opts",
+            bin_field: "opts_bin",
+        },
+        json_encoder: "let encoded_opts = compat_json(opts).and_then(|opts_str| encode_msgpack(opts).map(|opts_bin| (opts_str, opts_bin)));",
+        policy: RequestJsonPolicy::MsgpackOnlyEligible,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "RenameDataRequest",
+            json_field: "file_info",
+            bin_field: "file_info_bin",
+        },
+        json_encoder: "let file_info = compat_json(&fi)?;",
+        policy: RequestJsonPolicy::MsgpackOnlyEligible,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "UpdateMetadataRequest",
+            json_field: "file_info",
+            bin_field: "file_info_bin",
+        },
+        json_encoder: "let file_info = compat_json(&fi)?;",
+        policy: RequestJsonPolicy::MsgpackOnlyEligible,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "UpdateMetadataRequest",
+            json_field: "opts",
+            bin_field: "opts_bin",
+        },
+        json_encoder: "let opts_str = compat_json(&opts)?;",
+        policy: RequestJsonPolicy::MsgpackOnlyEligible,
+    },
+    RequestCompatSendSite {
+        field: CompatPayloadField {
+            message: "WriteMetadataRequest",
+            json_field: "file_info",
+            bin_field: "file_info_bin",
+        },
+        json_encoder: "let file_info = compat_json(&fi)?;",
+        policy: RequestJsonPolicy::MsgpackOnlyEligible,
+    },
+];
+
+pub const RESPONSE_COMPAT_SEND_SITES: &[ResponseCompatSendSite] = &[
+    ResponseCompatSendSite {
+        field: CompatPayloadField {
+            message: "BatchReadVersionResponse",
+            json_field: "batch_read_version_resps",
+            bin_field: "batch_read_version_resps_bin",
+        },
+        json_encoder: "compat_response_json(batch_read_version_resp, request_decoded_from_msgpack)",
+    },
+    ResponseCompatSendSite {
+        field: CompatPayloadField {
+            message: "ReadMultipleResponse",
+            json_field: "read_multiple_resps",
+            bin_field: "read_multiple_resps_bin",
+        },
+        json_encoder: "compat_response_json(read_multiple_resp, false)",
+    },
+    ResponseCompatSendSite {
+        field: CompatPayloadField {
+            message: "ReadVersionResponse",
+            json_field: "file_info",
+            bin_field: "file_info_bin",
+        },
+        json_encoder: "let file_info_json = compat_response_json(&file_info, request_had_msgpack_payload);",
+    },
+    ResponseCompatSendSite {
+        field: CompatPayloadField {
+            message: "ReadXLResponse",
+            json_field: "raw_file_info",
+            bin_field: "raw_file_info_bin",
+        },
+        json_encoder: "let raw_file_info_json = compat_response_json(&raw_file_info, false);",
+    },
+    ResponseCompatSendSite {
+        field: CompatPayloadField {
+            message: "RenameDataResponse",
+            json_field: "rename_data_resp",
+            bin_field: "rename_data_resp_bin",
+        },
+        json_encoder: "let rename_data_resp_json = compat_response_json(rename_data_resp, request_decoded_from_msgpack)",
+    },
+];
+
+/// Cuts a source file at its trailing `#[cfg(test)] mod tests` module so that
+/// send-site assertions only match production code, never the asserting test
+/// itself.
+pub fn production_source(source: &'static str, file_name: &str) -> &'static str {
+    source
+        .split("\n#[cfg(test)]\nmod tests")
+        .next()
+        .unwrap_or_else(|| panic!("{file_name} should contain production source before tests"))
+}
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.

+pub mod compat_manifest;
 // SAFETY: `generated` is prost/tonic-generated protocol code. The allowance is
 // scoped to that module so generated internals do not relax lints elsewhere.
 #[allow(unsafe_code)]
@@ -2628,176 +2629,7 @@ mod tests {
        assert_eq!(decoded.protocol_version, 0);
    }

-    #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
-    struct CompatPayloadField {
-        message: &'static str,
-        json_field: &'static str,
-        bin_field: &'static str,
-    }
-
-    #[derive(Clone, Copy, Debug, Eq, PartialEq)]
-    enum RequestJsonPolicy {
-        MsgpackOnlyEligible,
-        AlwaysDualWriteUntilFallbackZero,
-    }
-
-    #[derive(Clone, Copy, Debug)]
-    struct RequestCompatSendSite {
-        field: CompatPayloadField,
-        json_encoder: &'static str,
-        policy: RequestJsonPolicy,
-    }
-
-    #[derive(Clone, Copy, Debug)]
-    struct ResponseCompatSendSite {
-        field: CompatPayloadField,
-        json_encoder: &'static str,
-    }
-
-    const REQUEST_COMPAT_SEND_SITES: &[RequestCompatSendSite] = &[
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "BatchReadVersionRequest",
-                json_field: "batch_read_version_req",
-                bin_field: "batch_read_version_req_bin",
-            },
-            json_encoder: "let batch_read_version_req = compat_json(&req)?;",
-            policy: RequestJsonPolicy::MsgpackOnlyEligible,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "DeleteVersionRequest",
-                json_field: "file_info",
-                bin_field: "file_info_bin",
-            },
-            json_encoder: "let file_info = serde_json::to_string(&fi)?;",
-            policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "DeleteVersionRequest",
-                json_field: "opts",
-                bin_field: "opts_bin",
-            },
-            json_encoder: "let opts = serde_json::to_string(&opts)?;",
-            policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "DeleteVersionsRequest",
-                json_field: "opts",
-                bin_field: "opts_bin",
-            },
-            json_encoder: "let opts = match serde_json::to_string(&opts) {",
-            policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "DeleteVersionsRequest",
-                json_field: "versions",
-                bin_field: "versions_bin",
-            },
-            json_encoder: "versions_str.push(match serde_json::to_string(file_info_versions) {",
-            policy: RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "ReadMultipleRequest",
-                json_field: "read_multiple_req",
-                bin_field: "read_multiple_req_bin",
-            },
-            json_encoder: "let read_multiple_req = compat_json(&req)?;",
-            policy: RequestJsonPolicy::MsgpackOnlyEligible,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "ReadVersionRequest",
-                json_field: "opts",
-                bin_field: "opts_bin",
-            },
-            json_encoder: "let encoded_opts = compat_json(opts).and_then(|opts_str| encode_msgpack(opts).map(|opts_bin| (opts_str, opts_bin)));",
-            policy: RequestJsonPolicy::MsgpackOnlyEligible,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "RenameDataRequest",
-                json_field: "file_info",
-                bin_field: "file_info_bin",
-            },
-            json_encoder: "let file_info = compat_json(&fi)?;",
-            policy: RequestJsonPolicy::MsgpackOnlyEligible,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "UpdateMetadataRequest",
-                json_field: "file_info",
-                bin_field: "file_info_bin",
-            },
-            json_encoder: "let file_info = compat_json(&fi)?;",
-            policy: RequestJsonPolicy::MsgpackOnlyEligible,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "UpdateMetadataRequest",
-                json_field: "opts",
-                bin_field: "opts_bin",
-            },
-            json_encoder: "let opts_str = compat_json(&opts)?;",
-            policy: RequestJsonPolicy::MsgpackOnlyEligible,
-        },
-        RequestCompatSendSite {
-            field: CompatPayloadField {
-                message: "WriteMetadataRequest",
-                json_field: "file_info",
-                bin_field: "file_info_bin",
-            },
-            json_encoder: "let file_info = compat_json(&fi)?;",
-            policy: RequestJsonPolicy::MsgpackOnlyEligible,
-        },
-    ];
-
-    const RESPONSE_COMPAT_SEND_SITES: &[ResponseCompatSendSite] = &[
-        ResponseCompatSendSite {
-            field: CompatPayloadField {
-                message: "BatchReadVersionResponse",
-                json_field: "batch_read_version_resps",
-                bin_field: "batch_read_version_resps_bin",
-            },
-            json_encoder: "compat_response_json(batch_read_version_resp, request_decoded_from_msgpack)",
-        },
-        ResponseCompatSendSite {
-            field: CompatPayloadField {
-                message: "ReadMultipleResponse",
-                json_field: "read_multiple_resps",
-                bin_field: "read_multiple_resps_bin",
-            },
-            json_encoder: "compat_response_json(read_multiple_resp, false)",
-        },
-        ResponseCompatSendSite {
-            field: CompatPayloadField {
-                message: "ReadVersionResponse",
-                json_field: "file_info",
-                bin_field: "file_info_bin",
-            },
-            json_encoder: "let file_info_json = compat_response_json(&file_info, request_had_msgpack_payload);",
-        },
-        ResponseCompatSendSite {
-            field: CompatPayloadField {
-                message: "ReadXLResponse",
-                json_field: "raw_file_info",
-                bin_field: "raw_file_info_bin",
-            },
-            json_encoder: "let raw_file_info_json = compat_response_json(&raw_file_info, false);",
-        },
-        ResponseCompatSendSite {
-            field: CompatPayloadField {
-                message: "RenameDataResponse",
-                json_field: "rename_data_resp",
-                bin_field: "rename_data_resp_bin",
-            },
-            json_encoder: "let rename_data_resp_json = compat_response_json(rename_data_resp, request_decoded_from_msgpack)",
-        },
-    ];
+    use crate::compat_manifest::{CompatPayloadField, REQUEST_COMPAT_SEND_SITES, RESPONSE_COMPAT_SEND_SITES, RequestJsonPolicy};

    fn proto_bin_json_fields(message_suffix: &str) -> Vec<CompatPayloadField> {
        let proto = include_str!("node.proto");
@@ -2837,13 +2669,6 @@ mod tests {
        fields
    }

-    fn production_source(source: &'static str, file_name: &str) -> &'static str {
-        source
-            .split("\n#[cfg(test)]\nmod tests")
-            .next()
-            .unwrap_or_else(|| panic!("{file_name} should contain production source before tests"))
-    }
-
    #[test]
    fn request_compat_send_site_manifest_covers_node_proto_bin_fields() {
        let mut manifest_fields = REQUEST_COMPAT_SEND_SITES
@@ -2861,31 +2686,6 @@ mod tests {
        assert_eq!(manifest_fields, proto_bin_json_fields("Request"));
    }

-    #[test]
-    fn request_compat_send_site_manifest_pins_json_policy_and_encoder() {
-        let source = production_source(include_str!("../../ecstore/src/cluster/rpc/remote_disk.rs"), "remote_disk.rs");
-        let msgpack_only_eligible = REQUEST_COMPAT_SEND_SITES
-            .iter()
-            .filter(|send_site| send_site.policy == RequestJsonPolicy::MsgpackOnlyEligible)
-            .count();
-        let always_dual_write = REQUEST_COMPAT_SEND_SITES
-            .iter()
-            .filter(|send_site| send_site.policy == RequestJsonPolicy::AlwaysDualWriteUntilFallbackZero)
-            .count();
-
-        assert_eq!(msgpack_only_eligible, 7);
-        assert_eq!(always_dual_write, 4);
-        for send_site in REQUEST_COMPAT_SEND_SITES {
-            assert!(
-                source.contains(send_site.json_encoder),
-                "{}.{} must keep its manifest encoder: {}",
-                send_site.field.message,
-                send_site.field.json_field,
-                send_site.json_encoder
-            );
-        }
-    }
-
    #[test]
    fn request_compat_send_site_manifest_pins_exact_json_policies() {
        let mut policies = REQUEST_COMPAT_SEND_SITES
@@ -2933,21 +2733,6 @@ mod tests {
        assert_eq!(manifest_fields, proto_bin_json_fields("Response"));
    }

-    #[test]
-    fn response_compat_send_site_manifest_pins_json_encoder() {
-        let source = production_source(include_str!("../../../rustfs/src/storage/rpc/node_service/disk.rs"), "disk.rs");
-
-        for send_site in RESPONSE_COMPAT_SEND_SITES {
-            assert!(
-                source.contains(send_site.json_encoder),
-                "{}.{} must keep its manifest encoder: {}",
-                send_site.field.message,
-                send_site.field.json_field,
-                send_site.json_encoder
-            );
-        }
-    }
-
    #[test]
    fn enforce_tls_generation_cache_bound_evicts_when_retained_entries_still_full() {
        let mut cache = HashMap::new();