diff --git a/crates/ecstore/src/cluster/rpc/remote_disk.rs b/crates/ecstore/src/cluster/rpc/remote_disk.rs index 07d121da3..5e2b8ef8a 100644 --- a/crates/ecstore/src/cluster/rpc/remote_disk.rs +++ b/crates/ecstore/src/cluster/rpc/remote_disk.rs @@ -3885,6 +3885,27 @@ mod tests { static INIT: Once = Once::new(); + #[test] + fn request_compat_send_sites_keep_manifest_json_encoders() { + // Rolling-upgrade contract (rustfs-protos compat manifest): every + // dual-write request field must keep producing its JSON side with the + // exact encoder the manifest pins, until the msgpack-only switch (and + // fallback-zero confirmation) retires it. The manifest itself is + // pinned against node.proto by tests in rustfs-protos; this test keeps + // the send-site assertion in the crate that owns the source file. + let source = rustfs_protos::compat_manifest::production_source(include_str!("remote_disk.rs"), "remote_disk.rs"); + + for send_site in rustfs_protos::compat_manifest::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 delete_versions_response_preserves_typed_item_errors() { let errors = decode_delete_versions_errors( diff --git a/crates/ecstore/tests/scanner_heal_admission_contract_test.rs b/crates/ecstore/tests/scanner_heal_admission_contract_test.rs new file mode 100644 index 000000000..f00b300f9 --- /dev/null +++ b/crates/ecstore/tests/scanner_heal_admission_contract_test.rs @@ -0,0 +1,29 @@ +// 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. + +//! ECStore-owned half of the scanner/heal overlap Phase-0 inventory (formerly +//! a cross-crate source include in crates/scanner). The assertions +//! deliberately check that the documented write-exclusion guards still exist; +//! they do not claim that a shared admission primitive already exists. + +const HEAL_OBJECT_SOURCE: &str = include_str!("../src/set_disk/ops/heal.rs"); +const SET_LOCKING_SOURCE: &str = include_str!("../src/set_disk/ops/locking.rs"); + +#[test] +fn set_disk_overlap_inventory_keeps_write_exclusion_guards() { + assert!(HEAL_OBJECT_SOURCE.contains("heal_object")); + assert!(HEAL_OBJECT_SOURCE.contains("get_write_lock")); + assert!(SET_LOCKING_SOURCE.contains("scanning_disks")); + assert!(SET_LOCKING_SOURCE.contains("new_disks.extend(scanning_disks)")); +} diff --git a/crates/heal/tests/auto_scan_admission_contract_test.rs b/crates/heal/tests/auto_scan_admission_contract_test.rs new file mode 100644 index 000000000..126556fc5 --- /dev/null +++ b/crates/heal/tests/auto_scan_admission_contract_test.rs @@ -0,0 +1,26 @@ +// 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. + +//! Heal-owned half of the scanner/heal overlap Phase-0 inventory (formerly a +//! cross-crate source include in crates/scanner). The assertions deliberately +//! check that the documented admission guards still exist; they do not claim +//! that a shared admission primitive already exists. + +const HEAL_AUTO_SCAN_SOURCE: &str = include_str!("../src/heal/manager/auto_scan.rs"); + +#[test] +fn auto_scan_overlap_inventory_keeps_admission_guards() { + assert!(HEAL_AUTO_SCAN_SOURCE.contains("active_heals")); + assert!(HEAL_AUTO_SCAN_SOURCE.contains("contains_erasure_set")); +} diff --git a/crates/protos/src/compat_manifest.rs b/crates/protos/src/compat_manifest.rs new file mode 100644 index 000000000..0bf47574f --- /dev/null +++ b/crates/protos/src/compat_manifest.rs @@ -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")) +} diff --git a/crates/protos/src/lib.rs b/crates/protos/src/lib.rs index a0b99781d..e50fdf031 100644 --- a/crates/protos/src/lib.rs +++ b/crates/protos/src/lib.rs @@ -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 { 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(); diff --git a/crates/scanner/src/scanner_heal_admission_baseline.rs b/crates/scanner/src/scanner_heal_admission_baseline.rs index 5fdd76291..97ee9c16f 100644 --- a/crates/scanner/src/scanner_heal_admission_baseline.rs +++ b/crates/scanner/src/scanner_heal_admission_baseline.rs @@ -7,12 +7,12 @@ #[cfg(test)] mod tests { + // The heal- and ecstore-owned halves of the Phase-0 overlap inventory live + // with their owning crates (crates/heal/tests and crates/ecstore/tests): + // cross-crate source includes are rejected by + // scripts/check_layer_dependencies.sh. const SCANNER_IO_SOURCE: &str = include_str!("scanner_io/io_disk.rs"); const SCANNER_FOLDER_SOURCE: &str = include_str!("scanner_folder.rs"); - const HEAL_AUTO_SCAN_SOURCE: &str = - include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/../heal/src/heal/manager/auto_scan.rs")); - const HEAL_OBJECT_SOURCE: &str = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/../ecstore/src/set_disk/ops/heal.rs")); - const SET_LOCKING_SOURCE: &str = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/../ecstore/src/set_disk/ops/locking.rs")); #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum Operation { @@ -149,12 +149,6 @@ mod tests { assert!(SCANNER_IO_SOURCE.contains("scan_data_folder")); assert!(SCANNER_FOLDER_SOURCE.contains("send_required_scanner_heal_request")); assert!(SCANNER_FOLDER_SOURCE.contains("update_pending_scanner_heal_after_admission")); - assert!(HEAL_AUTO_SCAN_SOURCE.contains("active_heals")); - assert!(HEAL_AUTO_SCAN_SOURCE.contains("contains_erasure_set")); - assert!(HEAL_OBJECT_SOURCE.contains("heal_object")); - assert!(HEAL_OBJECT_SOURCE.contains("get_write_lock")); - assert!(SET_LOCKING_SOURCE.contains("scanning_disks")); - assert!(SET_LOCKING_SOURCE.contains("new_disks.extend(scanning_disks)")); } #[test] diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index 36f4019a9..accf68b4e 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -35,6 +35,13 @@ wire types still live in `rustfs-filemeta`. This keeps the temporary dependency centralized until those wire contracts can move without introducing a `rustfs-replication` / `rustfs-storage-api` cycle. +Dependency direction also applies to compile-time source reads: +`include_str!`/`include!` of a `.rs` file must not resolve outside the +including crate's own directory (`scripts/check_layer_dependencies.sh` +enforces this). A source-text tripwire belongs in the crate that owns the +asserted file; shared expectations move into a contract surface such as +`rustfs_protos::compat_manifest` and are asserted by each owning crate. + Existing migration checks live in: - `scripts/check_layer_dependencies.sh` diff --git a/rustfs/src/storage/rpc/node_service/disk.rs b/rustfs/src/storage/rpc/node_service/disk.rs index 810a499f6..e2457c996 100644 --- a/rustfs/src/storage/rpc/node_service/disk.rs +++ b/rustfs/src/storage/rpc/node_service/disk.rs @@ -1789,6 +1789,26 @@ mod tests { count: u32, } + #[test] + fn response_compat_send_sites_keep_manifest_json_encoders() { + // Rolling-upgrade contract (rustfs-protos compat manifest): every + // dual-write response field must keep producing its JSON side with the + // exact encoder the manifest pins. The manifest itself is pinned + // against node.proto by tests in rustfs-protos; this test keeps the + // send-site assertion in the crate that owns the source file. + let source = rustfs_protos::compat_manifest::production_source(include_str!("disk.rs"), "disk.rs"); + + for send_site in rustfs_protos::compat_manifest::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 delete_versions_response_dual_writes_typed_item_errors() { let raw_not_found = super::DiskError::Io(std::io::Error::from(std::io::ErrorKind::NotFound)); diff --git a/scripts/check_layer_dependencies.sh b/scripts/check_layer_dependencies.sh index 52dbfc072..605cc2933 100755 --- a/scripts/check_layer_dependencies.sh +++ b/scripts/check_layer_dependencies.sh @@ -326,6 +326,98 @@ normalize_baseline_file() { TMP_DIR="$(mktemp -d)" trap 'rm -rf "$TMP_DIR"' EXIT +# --- Cross-crate Rust source include guard (rustfs/backlog#1884) --- +# +# `use`-based layer checks cannot see `include_str!`/`include!` edges, and a +# crate that includes another crate's `.rs` source couples itself to that +# crate's file layout — a contract crate reverse-including an implementation +# or binary crate was exactly the unguarded edge found in backlog#1884. Rule: +# an `include_str!`/`include!` whose target is a `.rs` file must not resolve +# outside the including crate's own directory. Same-crate source includes +# (self-asserting tripwires) stay allowed; non-`.rs` targets (protos, fixtures) +# and `OUT_DIR` includes of generated code are out of scope. +INCLUDE_GUARD="${TMP_DIR}/include_guard.pl" +cat >"$INCLUDE_GUARD" <<'PERL' +use strict; +use warnings; +use File::Basename qw(dirname); +use File::Find qw(find); + +my ($root, @dirs) = @ARGV; +my @files; +find( + sub { push @files, $File::Find::name if -f && /\.rs$/ && $File::Find::name !~ m{/target/} }, + map { "$root/$_" } @dirs +); + +sub normalize { + my @parts; + for my $part (split m{/+}, $_[0]) { + next if $part eq '' || $part eq '.'; + if ($part eq '..') { pop @parts } else { push @parts, $part } + } + return '/' . join('/', @parts); +} + +my @violations; +for my $file (sort @files) { + open my $fh, '<', $file or next; + my $src = do { local $/; <$fh> }; + close $fh; + + my $dir = dirname($file); + my $crate = $dir; + $crate = dirname($crate) while $crate ne $root && $crate ne '/' && !-f "$crate/Cargo.toml"; + next if $crate eq $root || $crate eq '/'; + + while ( + $src =~ /include(?:_str)?!\s*\(\s*(?:concat!\s*\(\s*env!\s*\(\s*"CARGO_MANIFEST_DIR"\s*\)\s*,\s*"([^"]+)"|"([^"]+)")/gs + ) { + my ($manifest_rel, $file_rel) = ($1, $2); + my $target = defined $manifest_rel ? "$crate$manifest_rel" : "$dir/$file_rel"; + next unless $target =~ /\.rs$/; + my $resolved = normalize($target); + my $crate_prefix = normalize($crate) . '/'; + if (index($resolved, $crate_prefix) != 0) { + push @violations, "$file includes $resolved outside its crate " . normalize($crate); + } + } +} +print "$_\n" for @violations; +exit(@violations ? 1 : 0); +PERL + +# Self-test: a fixture workspace with one escaping file-relative include, one +# escaping CARGO_MANIFEST_DIR include, and several allowed forms. +FIXTURE_ROOT="${TMP_DIR}/include_fixture" +mkdir -p "$FIXTURE_ROOT/crates/a/src" "$FIXTURE_ROOT/crates/a/tests" "$FIXTURE_ROOT/crates/b/src" +touch "$FIXTURE_ROOT/crates/a/Cargo.toml" "$FIXTURE_ROOT/crates/b/Cargo.toml" +cat >"$FIXTURE_ROOT/crates/a/src/lib.rs" <<'EOF' +const ESCAPE_RELATIVE: &str = include_str!("../../b/src/lib.rs"); +const ESCAPE_MANIFEST: &str = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/../b/src/other.rs")); +const SAME_CRATE: &str = include_str!("sibling.rs"); +const SAME_CRATE_MANIFEST: &str = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/sibling.rs")); +const NON_RUST: &str = include_str!("../../b/src/schema.proto"); +EOF +cat >"$FIXTURE_ROOT/crates/a/tests/contract.rs" <<'EOF' +const OWN_SRC: &str = include_str!("../src/lib.rs"); +EOF +INCLUDE_SELF_TEST_OUTPUT="$(perl "$INCLUDE_GUARD" "$FIXTURE_ROOT" crates 2>&1 || true)" +if [[ "$(printf '%s\n' "$INCLUDE_SELF_TEST_OUTPUT" | grep -c 'outside its crate')" != "2" ]] || + ! grep -q 'crates/b/src/lib.rs' <<<"$INCLUDE_SELF_TEST_OUTPUT" || + ! grep -q 'crates/b/src/other.rs' <<<"$INCLUDE_SELF_TEST_OUTPUT"; then + echo "Cross-crate include guard self-test failed; expected exactly the two escaping fixtures to be flagged:" >&2 + printf '%s\n' "$INCLUDE_SELF_TEST_OUTPUT" >&2 + exit 1 +fi + +if ! INCLUDE_VIOLATIONS="$(perl "$INCLUDE_GUARD" "$ROOT_DIR" crates rustfs 2>&1)"; then + echo "Cross-crate include guard failed: a Rust source include escapes its crate directory." + echo "Move the assertion into the crate that owns the included file (or assert the compiled artifact) instead of reading another crate's source." + printf '%s\n' "$INCLUDE_VIOLATIONS" + exit 1 +fi + VIOLATIONS_RAW="${TMP_DIR}/violations_raw.txt" EDGES_RAW="${TMP_DIR}/edges_raw.txt" CURRENT_BASELINE="${TMP_DIR}/current_baseline.txt"