diff --git a/.github/workflows/windows-filesystem.yml b/.github/workflows/windows-filesystem.yml new file mode 100644 index 000000000..2d3aa3b89 --- /dev/null +++ b/.github/workflows/windows-filesystem.yml @@ -0,0 +1,86 @@ +# 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. + +name: Windows Filesystem Tests + +on: + push: + branches: [ main ] + paths: + - "crates/ecstore/src/disk/**" + - "crates/ecstore/src/store/init_format.rs" + - "crates/ecstore/Cargo.toml" + - "Cargo.toml" + - "Cargo.lock" + - ".github/actions/setup/**" + - ".github/workflows/windows-filesystem.yml" + pull_request: + branches: [ main ] + paths: + - "crates/ecstore/src/disk/**" + - "crates/ecstore/src/store/init_format.rs" + - "crates/ecstore/Cargo.toml" + - "Cargo.toml" + - "Cargo.lock" + - ".github/actions/setup/**" + - ".github/workflows/windows-filesystem.yml" + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +env: + CARGO_TERM_COLOR: always + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" + RUST_BACKTRACE: 1 + +jobs: + rename-safety: + name: Rename Safety + runs-on: windows-latest + timeout-minutes: 60 + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + with: + persist-credentials: false + + - name: Setup Rust environment + uses: ./.github/actions/setup + with: + rust-version: stable + cache-shared-key: build-x86_64-pc-windows-msvc + cache-save-if: 'false' + install-build-packaging-tools: 'false' + install-test-tools: 'false' + + - name: Test guarded rename publication + shell: pwsh + run: cargo test -p rustfs-ecstore --lib rename_all_ -- --nocapture + + - name: Test Windows handle guards + shell: pwsh + run: cargo test -p rustfs-ecstore --lib windows_ -- --nocapture + + - name: Test startup temporary-directory cleanup + shell: pwsh + run: cargo test -p rustfs-ecstore --lib cleanup_tmp_on_startup_ -- --nocapture + + - name: Test fresh format publication + shell: pwsh + run: cargo test -p rustfs-ecstore --lib fresh_format_load_initializes_all_disks -- --nocapture diff --git a/crates/ecstore/src/disk/os.rs b/crates/ecstore/src/disk/os.rs index 75da8d0e7..2280ec340 100644 --- a/crates/ecstore/src/disk/os.rs +++ b/crates/ecstore/src/disk/os.rs @@ -677,21 +677,22 @@ pub(crate) type ExistingBaseDirectoryGuard = (); #[cfg(windows)] fn lock_windows_directory(path: &Path) -> io::Result { use std::os::windows::fs::OpenOptionsExt; + use windows_sys::Win32::Storage::FileSystem::{ + FILE_ATTRIBUTE_DIRECTORY, FILE_ATTRIBUTE_REPARSE_POINT, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT, + FILE_SHARE_READ, FILE_SHARE_WRITE, + }; - const FILE_ATTRIBUTE_DIRECTORY: u64 = 0x10; - const FILE_ATTRIBUTE_REPARSE_POINT: u32 = 0x400; - const FILE_FLAG_BACKUP_SEMANTICS: u32 = 0x0200_0000; - const FILE_FLAG_OPEN_REPARSE_POINT: u32 = 0x0020_0000; - const FILE_SHARE_READ: u32 = 0x1; - + // Relative child publication requires write sharing on every guarded + // ancestor. Omitting delete sharing still prevents any directory in the + // resolved path from being renamed or removed before the commit finishes. let file = std::fs::OpenOptions::new() .read(true) - .share_mode(FILE_SHARE_READ) + .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE) .custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT) .open(path)?; let handle = winapi_util::Handle::from_file(file); let info = winapi_util::file::information(&handle)?; - if info.file_attributes() & FILE_ATTRIBUTE_DIRECTORY == 0 + if info.file_attributes() & u64::from(FILE_ATTRIBUTE_DIRECTORY) == 0 || info.file_attributes() & u64::from(FILE_ATTRIBUTE_REPARSE_POINT) != 0 { return Err(io::Error::from(io::ErrorKind::NotADirectory)); @@ -1223,23 +1224,53 @@ mod tests { #[cfg(windows)] #[test] - fn windows_parent_guard_blocks_base_and_intermediate_replacement() { + fn windows_parent_guard_blocks_parent_replacement() { let temp_dir = tempdir().expect("create temp dir"); let base = temp_dir.path().join("bucket"); std::fs::create_dir(&base).expect("create destination base"); let parent = base.join("object").join("nested"); let guard = mkdir_all_below_existing_base_std(&parent, &base).expect("create and lock destination parents"); + std::fs::read_dir(&parent).expect("the locked parent must remain readable"); std::fs::rename(&base, temp_dir.path().join("replacement-base")) .expect_err("the locked base must not be replaceable before commit"); std::fs::rename(base.join("object"), base.join("replacement-object")) .expect_err("a locked intermediate directory must not be replaceable before commit"); + std::fs::rename(&parent, base.join("replacement-parent")) + .expect_err("the locked destination parent must not be replaceable before commit"); + assert!(parent.is_dir(), "failed replacement must leave the guarded parent in place"); drop(guard); std::fs::rename(base.join("object"), base.join("replacement-object")) .expect("replacement should succeed after the commit guard is released"); } + #[cfg(windows)] + #[tokio::test] + async fn windows_guarded_parent_allows_same_and_descendant_publication() { + let temp_dir = tempdir().expect("create temp dir"); + let base = temp_dir.path().join("bucket"); + let parent = base.join("object"); + std::fs::create_dir_all(&parent).expect("create destination parent"); + let _guard = mkdir_all_below_existing_base_std(&parent, &base).expect("guard destination parent"); + let first_src = temp_dir.path().join("first-stage"); + let second_src = temp_dir.path().join("second-stage"); + std::fs::write(&first_src, b"first").expect("write first source"); + std::fs::write(&second_src, b"second").expect("write second source"); + + rename_all(&first_src, parent.join("first"), &base) + .await + .expect("same-parent rename must succeed while a guard is held"); + rename_all(&second_src, parent.join("nested").join("second"), &base) + .await + .expect("descendant-parent rename must succeed while an ancestor guard is held"); + assert_eq!(std::fs::read(parent.join("first")).expect("read first destination"), b"first"); + assert_eq!( + std::fs::read(parent.join("nested").join("second")).expect("read second destination"), + b"second" + ); + } + #[cfg(unix)] #[tokio::test] async fn rename_parent_creation_rejects_symlinked_base() { diff --git a/crates/ecstore/src/store/init_format.rs b/crates/ecstore/src/store/init_format.rs index 95cf17c66..5528ff71b 100644 --- a/crates/ecstore/src/store/init_format.rs +++ b/crates/ecstore/src/store/init_format.rs @@ -1279,6 +1279,22 @@ mod tests { assert!(disks[2].is_none(), "the malformed outlier must be isolated"); } + #[tokio::test] + async fn fresh_format_load_initializes_all_disks() { + let (_temp_dir, mut disks) = local_disks(3).await; + + let format = connect_load_init_formats(true, &mut disks, 1, 3, None) + .await + .expect("fresh disks should receive a storage format"); + + let (formats, errors) = load_format_erasure_all(&disks, false).await; + assert!(errors.iter().all(Option::is_none), "every disk should load its fresh format: {errors:?}"); + assert!( + formats_match_reference_slots(&formats, &format, 0), + "fresh format publication must preserve every disk slot" + ); + } + #[tokio::test] async fn fresh_format_load_does_not_initialize_with_a_missing_disk() { let (_temp_dir, mut disks) = two_local_disks_with_missing_third().await;