diff --git a/.github/workflows/container_dev.yml b/.github/workflows/container_dev.yml index 7a922a685..48069c606 100644 --- a/.github/workflows/container_dev.yml +++ b/.github/workflows/container_dev.yml @@ -6,6 +6,7 @@ on: paths: - 'weed/**' - 'seaweed-volume/**' + - 'seaweed-common/**' - 'seaweed-worker/**' - 'docker/**' - 'go.mod' diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml index 2be3782bd..95750f689 100644 --- a/.github/workflows/performance.yml +++ b/.github/workflows/performance.yml @@ -8,6 +8,7 @@ on: - 'go.mod' - 'go.sum' - 'seaweed-volume/**' + - 'seaweed-common/**' - 'test/perf/**' - '.github/workflows/performance.yml' workflow_dispatch: diff --git a/.github/workflows/rust-volume-server-tests.yml b/.github/workflows/rust-volume-server-tests.yml index ebbe6ce20..6db97fd55 100644 --- a/.github/workflows/rust-volume-server-tests.yml +++ b/.github/workflows/rust-volume-server-tests.yml @@ -5,6 +5,7 @@ on: branches: [ master ] paths: - 'seaweed-volume/**' + - 'seaweed-common/**' - 'test/volume_server/**' - 'weed/pb/volume_server.proto' - 'weed/pb/volume_server_pb/**' @@ -13,6 +14,7 @@ on: branches: [ master, main ] paths: - 'seaweed-volume/**' + - 'seaweed-common/**' - 'test/volume_server/**' - 'weed/pb/volume_server.proto' - 'weed/pb/volume_server_pb/**' @@ -75,7 +77,7 @@ jobs: ~/.cargo/registry ~/.cargo/git seaweed-volume/target - key: rust-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-volume/Cargo.lock') }} + key: rust-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-volume/Cargo.lock', 'seaweed-common/Cargo.lock') }} restore-keys: | rust-${{ steps.toolchain.outputs.fingerprint }}- @@ -93,6 +95,16 @@ jobs: # - name: Check formatting # run: cd seaweed-volume && cargo fmt --check + # seaweed-common is a path dependency of this crate, not a member of its + # workspace, so the run below does not reach its own tests. It builds into + # this job's cached target directory, and the cache key above covers the + # shared crate's lock, so the aws-lc-sys that rustls pulls in is restored + # with the cache instead of compiled from scratch on every run. + - name: Run shared-crate unit tests + env: + CARGO_TARGET_DIR: ${{ github.workspace }}/seaweed-volume/target + run: cd seaweed-common && cargo test + - name: Run Rust unit tests run: cd seaweed-volume && cargo test @@ -168,7 +180,7 @@ jobs: ~/.cargo/registry ~/.cargo/git seaweed-volume/target - key: rust-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-volume/Cargo.lock') }} + key: rust-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-volume/Cargo.lock', 'seaweed-common/Cargo.lock') }} restore-keys: | rust-${{ steps.toolchain.outputs.fingerprint }}- @@ -250,7 +262,7 @@ jobs: ~/.cargo/registry ~/.cargo/git seaweed-volume/target - key: rust-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-volume/Cargo.lock') }} + key: rust-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-volume/Cargo.lock', 'seaweed-common/Cargo.lock') }} restore-keys: | rust-${{ steps.toolchain.outputs.fingerprint }}- diff --git a/.github/workflows/rust-worker-tests.yml b/.github/workflows/rust-worker-tests.yml index ae08c06fb..d2fde3ed0 100644 --- a/.github/workflows/rust-worker-tests.yml +++ b/.github/workflows/rust-worker-tests.yml @@ -5,12 +5,14 @@ on: branches: [ master ] paths: - 'seaweed-worker/**' + - 'seaweed-common/**' - 'weed/pb/plugin.proto' - '.github/workflows/rust-worker-tests.yml' push: branches: [ master, main ] paths: - 'seaweed-worker/**' + - 'seaweed-common/**' - 'weed/pb/plugin.proto' - '.github/workflows/rust-worker-tests.yml' @@ -49,7 +51,7 @@ jobs: ~/.cargo/registry ~/.cargo/git seaweed-worker/target/release - key: rust-worker-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-worker/Cargo.lock') }} + key: rust-worker-${{ steps.toolchain.outputs.fingerprint }}-${{ hashFiles('seaweed-worker/Cargo.lock', 'seaweed-common/Cargo.lock') }} restore-keys: | rust-worker-${{ steps.toolchain.outputs.fingerprint }}- @@ -84,6 +86,17 @@ jobs: # - name: Check formatting # run: cd seaweed-worker && cargo fmt --all --check + # seaweed-common is a path dependency of core and lance, not a member of + # this workspace, so `--workspace` below does not reach its own tests. + # Release and this job's cached target directory, and the cache key above + # covers the shared crate's lock. That lock pins the same rustls and + # aws-lc-sys this workspace resolves, so the release build above has + # already paid for them. + - name: Run shared-crate unit tests + env: + CARGO_TARGET_DIR: ${{ github.workspace }}/seaweed-worker/target + run: cd seaweed-common && cargo test --release + # The tests that need a live gateway skip themselves without one, the way # the Go integration tests skip without Docker; the lifecycle suite in # test/s3tables/lifecycle is what runs them against a real cluster. diff --git a/.github/workflows/rust_binaries_dev.yml b/.github/workflows/rust_binaries_dev.yml index c8cb4f0c6..563022d02 100644 --- a/.github/workflows/rust_binaries_dev.yml +++ b/.github/workflows/rust_binaries_dev.yml @@ -5,6 +5,7 @@ on: branches: [ master ] paths: - 'seaweed-volume/**' + - 'seaweed-common/**' - '.github/workflows/rust_binaries_dev.yml' permissions: diff --git a/docker/Dockerfile.go_build b/docker/Dockerfile.go_build index 227f58c34..a2eb20568 100644 --- a/docker/Dockerfile.go_build +++ b/docker/Dockerfile.go_build @@ -14,6 +14,9 @@ RUN cd /go/src/github.com/seaweedfs/seaweedfs && \ git checkout $BRANCH) || \ (echo "ERROR: Branch/commit $BRANCH not found in repository" && \ echo "Available branches:" && git branch -a && exit 1)) +# seaweed-common only exists on revisions that have it; a BRANCH predating it +# still needs the directory so the COPY into rust_builder below never fails. +RUN mkdir -p /go/src/github.com/seaweedfs/seaweedfs/seaweed-common ARG TARGETOS TARGETARCH TARGETVARIANT RUN cd /go/src/github.com/seaweedfs/seaweedfs/weed \ && export LDFLAGS="-X github.com/seaweedfs/seaweedfs/weed/util/version.COMMIT=$(git rev-parse --short HEAD)" \ @@ -31,6 +34,9 @@ ARG TAGS COPY weed-volume-prebuilt/ /prebuilt/ COPY weed-worker-prebuilt/ /prebuilt-worker/ COPY --from=builder /go/src/github.com/seaweedfs/seaweedfs/seaweed-volume /build/seaweed-volume +# seaweed-common is a path dependency of seaweed-volume that lives beside it, +# so the source build below needs it in the same relative position. +COPY --from=builder /go/src/github.com/seaweedfs/seaweedfs/seaweed-common /build/seaweed-common COPY --from=builder /go/src/github.com/seaweedfs/seaweedfs/weed /build/weed WORKDIR /build/seaweed-volume RUN if [ -f "/prebuilt/weed-volume-${TARGETARCH}" ]; then \ diff --git a/seaweed-common/Cargo.lock b/seaweed-common/Cargo.lock new file mode 100644 index 000000000..8a6e53bb2 --- /dev/null +++ b/seaweed-common/Cargo.lock @@ -0,0 +1,293 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "aws-lc-rs" +version = "1.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce2b2dcc879c3bae0d371e77c99f2238400ef24ec001394befa67b6e543add9e" +dependencies = [ + "aws-lc-sys", + "zeroize", +] + +[[package]] +name = "aws-lc-sys" +version = "0.44.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f09fae7be8bb3174e05c6afdb34199e6dc0c7c04ba9fa237b1967adfbde27483" +dependencies = [ + "cc", + "cmake", + "dunce", + "fs_extra", + "pkg-config", +] + +[[package]] +name = "cc" +version = "1.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "509591b7bcd67f4ef775afad7662703b4935daaa6ec0e5605cfb1090b32a2b6d" +dependencies = [ + "find-msvc-tools", + "jobserver", + "libc", + "shlex", +] + +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + +[[package]] +name = "cmake" +version = "0.1.58" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0f78a02292a74a88ac736019ab962ece0bc380e3f977bf72e376c5d78ff0678" +dependencies = [ + "cc", +] + +[[package]] +name = "dunce" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" + +[[package]] +name = "find-msvc-tools" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" + +[[package]] +name = "fs_extra" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42703706b716c37f96a77aea830392ad231f44c9e9a67872fa5548707e11b11c" + +[[package]] +name = "getrandom" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ff2abc00be7fca6ebc474524697ae276ad847ad0a6b3faa4bcb027e9a4614ad0" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + +[[package]] +name = "getrandom" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" +dependencies = [ + "cfg-if", + "libc", + "r-efi", +] + +[[package]] +name = "jobserver" +version = "0.1.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c00acbd29eabad4a2392fa0e921c874934dbbf4194312ad20f04a0ed67a3cb3" +dependencies = [ + "getrandom 0.4.3", + "libc", +] + +[[package]] +name = "libc" +version = "0.2.189" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" + +[[package]] +name = "log" +version = "0.4.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" + +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + +[[package]] +name = "pkg-config" +version = "0.3.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6b464fbc74e149a392436b17d523f769e057cb6877f6a5c4618bc6f11800548" + +[[package]] +name = "r-efi" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" + +[[package]] +name = "ring" +version = "0.17.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" +dependencies = [ + "cc", + "cfg-if", + "getrandom 0.2.17", + "libc", + "untrusted", + "windows-sys", +] + +[[package]] +name = "rustls" +version = "0.23.43" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0283386ce02abc0151e1761d08802dfe86c173b0b494af5cbc086574e453da06" +dependencies = [ + "aws-lc-rs", + "log", + "once_cell", + "rustls-pki-types", + "rustls-webpki", + "subtle", + "zeroize", +] + +[[package]] +name = "rustls-pki-types" +version = "1.15.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f4925028c7eb5d1fcdaf196971378ed9d2c1c4efc7dc5d011256f76c99c0a96" +dependencies = [ + "zeroize", +] + +[[package]] +name = "rustls-webpki" +version = "0.103.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0527518605e68109d875e248ea259b6758801cf165e4b2c2733ae3b51f12535a" +dependencies = [ + "aws-lc-rs", + "ring", + "rustls-pki-types", + "untrusted", +] + +[[package]] +name = "seaweed-common" +version = "0.1.0" +dependencies = [ + "rustls", +] + +[[package]] +name = "shlex" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" + +[[package]] +name = "subtle" +version = "2.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" + +[[package]] +name = "untrusted" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" + +[[package]] +name = "wasi" +version = "0.11.1+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" + +[[package]] +name = "windows-sys" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" +dependencies = [ + "windows-targets", +] + +[[package]] +name = "windows-targets" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" +dependencies = [ + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_gnullvm", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", +] + +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" + +[[package]] +name = "windows_aarch64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" + +[[package]] +name = "windows_i686_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" + +[[package]] +name = "windows_i686_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" + +[[package]] +name = "windows_i686_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" + +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.52.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" + +[[package]] +name = "zeroize" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e13c156562582aa81c60cb29407084cdb54c4164760106ab78e6c5b0858cf64e" diff --git a/seaweed-common/Cargo.toml b/seaweed-common/Cargo.toml new file mode 100644 index 000000000..b521108ce --- /dev/null +++ b/seaweed-common/Cargo.toml @@ -0,0 +1,27 @@ +[package] +name = "seaweed-common" +version = "0.1.0" +edition = "2024" +# The lower of the two consumers' floors (seaweed-volume 1.91.1, +# seaweed-worker 1.94.1), so depending on this crate cannot raise either +# tree's MSRV. Verified with `cargo +1.91.1 check --all-targets`. +rust-version = "1.91.1" +description = "Helpers shared by the SeaweedFS Rust volume server and the Rust plugin workers" + +# There is no root manifest: seaweed-volume and seaweed-worker are separate +# cargo trees with their own lockfiles, and this crate is a path dependency of +# both rather than a member of either. Keeping the lint policy identical in all +# three manifests is what stops them drifting. +[lints.clippy] +# Protobuf message literals keep `..Default::default()` on purpose: it is +# what lets a proto gain a field without touching every constructor. +needless_update = "allow" + +[dependencies] +# The same requirement both consumers already write. Cargo unifies all +# semver-compatible `rustls = "0.23"` requirements into one crate per binary, +# which is what makes `install_default_crypto_provider` write the same +# process-wide static the consuming crate reads. rustls is already in both +# trees (the volume server directly, seaweed-worker-core through tonic's +# `tls-aws-lc`), so this adds no crate to either graph. +rustls = "0.23" diff --git a/seaweed-common/src/address.rs b/seaweed-common/src/address.rs new file mode 100644 index 000000000..9f972940c --- /dev/null +++ b/seaweed-common/src/address.rs @@ -0,0 +1,307 @@ +//! SeaweedFS server addresses, the way the Go tree does them. +//! +//! An operator gives a SeaweedFS process an HTTP address and the gRPC port is +//! derived from it rather than asked for separately: `host:port` means gRPC on +//! `port + 10000`, and the explicit `host:port.grpcPort` form names it outright. +//! Dialling the HTTP port by mistake fails as "frame with invalid size", which +//! reads like a protocol bug rather than a wrong port, so the rule is worth its +//! own module. Mirrors `pb.ServerToGrpcAddress` in +//! `weed/pb/grpc_client_server.go`. +//! +//! The volume server and the workers each had their own copy of this and the +//! copies had drifted: the worker's bracketed IPv6 literals and the volume +//! server's did not, so `::1:19333` produced `::1:29333`, which the HTTP +//! authority parser rejects. One implementation, two thin wrappers. + +use std::fmt; +use std::num::ParseIntError; + +/// SeaweedFS's HTTP↔gRPC port-offset convention. +pub const GRPC_PORT_OFFSET: u16 = 10000; + +/// Why an address could not be turned into a gRPC address. +/// +/// The `Display` text is the volume server's original wording, because its +/// `parse_grpc_address` wrapper hands it straight to callers that put it in a +/// `Status` or an `io::Error`. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum AddressError { + /// No `:` at all, so there is no port to translate. + MissingPort(String), + /// The HTTP port of the `host:port.grpcPort` form is not a `u16`. It is + /// validated even though it is then discarded, so that a malformed address + /// is rejected here instead of failing later as an opaque connect error. + InvalidHttpPort { port: String, source: ParseIntError }, + /// The gRPC port of the `host:port.grpcPort` form is not a `u16`. + InvalidGrpcPort { port: String, source: ParseIntError }, + /// The port of the `host:port` form is not a `u16`. + InvalidPort { port: String, source: ParseIntError }, + /// `port + GRPC_PORT_OFFSET` leaves the TCP port range, e.g. `host:60000`. + /// Without the check the cast would wrap silently. + ImplicitGrpcPortOutOfRange(u16), +} + +impl fmt::Display for AddressError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::MissingPort(address) => write!(f, "cannot parse address: {address}"), + Self::InvalidHttpPort { port, source } => { + write!(f, "invalid http port {port:?}: {source}") + } + Self::InvalidGrpcPort { port, source } => { + write!(f, "invalid grpc port {port:?}: {source}") + } + Self::InvalidPort { port, source } => write!(f, "invalid port {port:?}: {source}"), + Self::ImplicitGrpcPortOutOfRange(port) => write!( + f, + "implicit grpc port out of range: {port} + {GRPC_PORT_OFFSET} = {}", + u32::from(*port) + u32::from(GRPC_PORT_OFFSET) + ), + } + } +} + +impl std::error::Error for AddressError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::InvalidHttpPort { source, .. } + | Self::InvalidGrpcPort { source, .. } + | Self::InvalidPort { source, .. } => Some(source), + Self::MissingPort(_) | Self::ImplicitGrpcPortOutOfRange(_) => None, + } + } +} + +/// Turn a SeaweedFS server address (`"host:port.grpcPort"` or `"host:port"`) +/// into the `host:grpcPort` form the endpoint builders expect. +/// +/// With the trailing `.grpcPort` segment that segment *is* the gRPC port; +/// without it the gRPC port is `port + GRPC_PORT_OFFSET`. An unbracketed IPv6 +/// literal comes back bracketed, because otherwise the port reads as part of +/// the address. +pub fn to_grpc_address(server: &str) -> Result { + // rfind, not find: an IPv6 literal is full of colons and the port is after + // the last one. + let colon_idx = server + .rfind(':') + .ok_or_else(|| AddressError::MissingPort(server.to_string()))?; + let host = &server[..colon_idx]; + let port_part = &server[colon_idx + 1..]; + + // rfind again rather than split_once: the host may be an IPv4 address, and + // only the part after the last colon is being split here anyway. + if let Some(dot_idx) = port_part.rfind('.') { + let http_port = &port_part[..dot_idx]; + let grpc_port = &port_part[dot_idx + 1..]; + http_port + .parse::() + .map_err(|source| AddressError::InvalidHttpPort { + port: http_port.to_string(), + source, + })?; + let grpc_port = + grpc_port + .parse::() + .map_err(|source| AddressError::InvalidGrpcPort { + port: grpc_port.to_string(), + source, + })?; + return Ok(join_host_port(host, grpc_port)); + } + + let port: u16 = port_part + .parse() + .map_err(|source| AddressError::InvalidPort { + port: port_part.to_string(), + source, + })?; + let grpc_port = port + .checked_add(GRPC_PORT_OFFSET) + .ok_or(AddressError::ImplicitGrpcPortOutOfRange(port))?; + Ok(join_host_port(host, grpc_port)) +} + +/// Join a host and a port, bracketing an IPv6 literal that is not bracketed +/// already. Public because the address rule is not the only place that has to +/// put a host and a port back together. +pub fn join_host_port(host: &str, port: u16) -> String { + // An IPv6 literal has to keep its brackets or the port reads as part of it. + if host.contains(':') && !host.starts_with('[') { + format!("[{host}]:{port}") + } else { + format!("{host}:{port}") + } +} + +#[cfg(test)] +mod tests { + use super::{AddressError, GRPC_PORT_OFFSET, join_host_port, to_grpc_address}; + + // ---- the volume server's cases ------------------------------------- + + #[test] + fn dotted_form_states_the_grpc_port() { + assert_eq!( + to_grpc_address("127.0.0.1:8080.18080").unwrap(), + "127.0.0.1:18080" + ); + assert_eq!( + to_grpc_address("192.168.1.66:8080.18080").unwrap(), + "192.168.1.66:18080" + ); + } + + #[test] + fn implicit_form_adds_the_offset() { + assert_eq!( + to_grpc_address("127.0.0.1:8080").unwrap(), + "127.0.0.1:18080" + ); + assert_eq!( + to_grpc_address("192.168.1.66:8080").unwrap(), + "192.168.1.66:18080" + ); + assert_eq!( + to_grpc_address("localhost:9333").unwrap(), + "localhost:19333" + ); + } + + #[test] + fn the_dotted_grpc_port_comes_back_normalised() { + // The volume server's copy validated this segment as a u16 and then + // emitted the original text, so a padded or signed port produced an + // authority the URI parser rejects. The parsed value is emitted now. + assert_eq!(to_grpc_address("host:8080.018080").unwrap(), "host:18080"); + assert_eq!(to_grpc_address("host:8080.+18080").unwrap(), "host:18080"); + } + + #[test] + fn an_ipv4_host_is_not_confused_with_the_dotted_port() { + // Regression: a naive split on '.' breaks on IP addresses. + assert_eq!( + to_grpc_address("10.0.0.1:8080.18080").unwrap(), + "10.0.0.1:18080" + ); + assert_eq!(to_grpc_address("10.0.0.1:8080").unwrap(), "10.0.0.1:18080"); + } + + #[test] + fn rejects_a_non_numeric_http_port_in_the_dotted_form() { + let err = to_grpc_address("host:abc.18080").unwrap_err(); + assert!( + matches!(err, AddressError::InvalidHttpPort { .. }), + "{err:?}" + ); + assert!(err.to_string().contains("invalid http port"), "{err}"); + } + + #[test] + fn rejects_a_non_numeric_grpc_port_in_the_dotted_form() { + let err = to_grpc_address("host:8080.xyz").unwrap_err(); + assert!( + matches!(err, AddressError::InvalidGrpcPort { .. }), + "{err:?}" + ); + assert!(err.to_string().contains("invalid grpc port"), "{err}"); + } + + #[test] + fn rejects_an_implicit_port_that_leaves_the_tcp_range() { + let err = to_grpc_address("127.0.0.1:60000").unwrap_err(); + assert!( + matches!(err, AddressError::ImplicitGrpcPortOutOfRange(60000)), + "{err:?}" + ); + assert!(err.to_string().contains("out of range"), "{err}"); + } + + #[test] + fn the_messages_are_the_volume_servers_wording_verbatim() { + // parse_grpc_address hands these straight to callers that put them in a + // Status or an io::Error, so the whole string is the contract, not just + // the substring the older tests match on. Only the two variants whose + // text is entirely ours are pinned exactly; the other three end in a + // std ParseIntError message, which is std's to reword. + assert_eq!( + to_grpc_address("127.0.0.1:60000").unwrap_err().to_string(), + "implicit grpc port out of range: 60000 + 10000 = 70000" + ); + assert_eq!( + to_grpc_address("hostname").unwrap_err().to_string(), + "cannot parse address: hostname" + ); + } + + #[test] + fn rejects_an_address_without_a_port() { + for source in ["hostname", "no-colon", "localhost"] { + let err = to_grpc_address(source).unwrap_err(); + assert!(matches!(err, AddressError::MissingPort(_)), "{err:?}"); + assert!(err.to_string().contains("cannot parse"), "{err}"); + } + } + + // ---- the worker's cases -------------------------------------------- + + #[test] + fn derives_the_grpc_port() { + assert_eq!( + to_grpc_address("localhost:23646").unwrap(), + "localhost:33646" + ); + assert_eq!( + to_grpc_address("127.0.0.1:9333").unwrap(), + "127.0.0.1:19333" + ); + } + + #[test] + fn honours_an_explicit_grpc_port() { + assert_eq!( + to_grpc_address("localhost:23646.33999").unwrap(), + "localhost:33999" + ); + } + + #[test] + fn rejects_what_it_cannot_parse() { + let err = to_grpc_address("localhost:notaport").unwrap_err(); + assert!(matches!(err, AddressError::InvalidPort { .. }), "{err:?}"); + assert!(err.to_string().contains("invalid port"), "{err}"); + } + + // ---- IPv6, which only the worker's copy handled -------------------- + + #[test] + fn brackets_ipv6_literals() { + assert_eq!(to_grpc_address("::1:23646").unwrap(), "[::1]:33646"); + assert_eq!(to_grpc_address("::1:9333").unwrap(), "[::1]:19333"); + assert_eq!( + to_grpc_address("fe80::1:9333.19333").unwrap(), + "[fe80::1]:19333" + ); + } + + #[test] + fn leaves_an_already_bracketed_literal_alone() { + assert_eq!(to_grpc_address("[::1]:9333").unwrap(), "[::1]:19333"); + assert_eq!(to_grpc_address("[::1]:9333.19333").unwrap(), "[::1]:19333"); + } + + #[test] + fn join_host_port_brackets_only_unbracketed_literals() { + assert_eq!(join_host_port("127.0.0.1", 19333), "127.0.0.1:19333"); + assert_eq!(join_host_port("localhost", 19333), "localhost:19333"); + assert_eq!(join_host_port("::1", 19333), "[::1]:19333"); + assert_eq!(join_host_port("[::1]", 19333), "[::1]:19333"); + } + + // ---- the offset itself --------------------------------------------- + + #[test] + fn the_offset_is_the_seaweedfs_convention() { + assert_eq!(GRPC_PORT_OFFSET, 10000); + } +} diff --git a/seaweed-common/src/lib.rs b/seaweed-common/src/lib.rs new file mode 100644 index 000000000..817f46216 --- /dev/null +++ b/seaweed-common/src/lib.rs @@ -0,0 +1,11 @@ +//! Helpers the SeaweedFS Rust volume server and the Rust plugin workers both need. +//! +//! `seaweed-volume` and `seaweed-worker` are separate cargo trees with separate +//! lockfiles and no root manifest, so anything both of them need was, until this +//! crate existed, written twice. The two things in here are the ones where a +//! second copy is a correctness risk rather than a typing cost: the HTTP↔gRPC +//! address rule, which two copies had already drifted on, and the process-wide +//! rustls provider, which only works if every binary installs the same one. + +pub mod address; +pub mod tls; diff --git a/seaweed-common/src/tls.rs b/seaweed-common/src/tls.rs new file mode 100644 index 000000000..63ca7ce60 --- /dev/null +++ b/seaweed-common/src/tls.rs @@ -0,0 +1,28 @@ +//! The process-wide rustls crypto provider. +//! +//! Both binaries link aws-lc-rs and ring transitively — in the volume server +//! through the AWS SDK and reqwest, in the lance worker through lance's `aws` +//! backend and reqwest — so rustls cannot auto-select a provider and tonic's +//! client TLS panics on first use. Each binary has to pin one, and it has to be +//! the same one, which is why the choice lives here rather than in either tree. + +use rustls::crypto::aws_lc_rs; + +/// Pin rustls's process-wide default provider to aws-lc-rs, matching the +/// volume server's TLS config. Idempotent: the first call wins and every +/// later one is a no-op, so callers do not have to coordinate. +pub fn install_default_crypto_provider() { + let _ = aws_lc_rs::default_provider().install_default(); +} + +#[cfg(test)] +mod tests { + use super::install_default_crypto_provider; + + #[test] + fn installing_is_idempotent_and_leaves_a_default_behind() { + install_default_crypto_provider(); + install_default_crypto_provider(); + assert!(rustls::crypto::CryptoProvider::get_default().is_some()); + } +} diff --git a/seaweed-volume/Cargo.lock b/seaweed-volume/Cargo.lock index 93ad58ac3..7c459a2db 100644 --- a/seaweed-volume/Cargo.lock +++ b/seaweed-volume/Cargo.lock @@ -3487,6 +3487,13 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "seaweed-common" +version = "0.1.0" +dependencies = [ + "rustls", +] + [[package]] name = "sec1" version = "0.3.0" @@ -4584,6 +4591,7 @@ dependencies = [ "rustls", "rustls-pemfile", "rusty-leveldb", + "seaweed-common", "serde", "serde_json", "serde_urlencoded", diff --git a/seaweed-volume/Cargo.toml b/seaweed-volume/Cargo.toml index 708f77556..e47de98ee 100644 --- a/seaweed-volume/Cargo.toml +++ b/seaweed-volume/Cargo.toml @@ -31,6 +31,11 @@ needless_update = "allow" undocumented_unsafe_blocks = "warn" [dependencies] +# Helpers the Rust plugin workers (seaweed-worker) need as well. A path +# dependency because the two trees are separate cargo workspaces with no +# common root manifest. +seaweed-common = { path = "../seaweed-common" } + # Async runtime tokio = { version = "1", features = ["full"] } tokio-stream = { version = "0.1", features = ["net"] } diff --git a/seaweed-volume/src/security/tls.rs b/seaweed-volume/src/security/tls.rs index 42a0fba32..7dd2094b8 100644 --- a/seaweed-volume/src/security/tls.rs +++ b/seaweed-volume/src/security/tls.rs @@ -120,10 +120,11 @@ impl ClientCertVerifier for CommonNameVerifier { // aws-lc-rs and ring both get linked transitively, so rustls can't auto-select // a provider and tonic's client TLS panics on first use. Pin the default to -// aws-lc-rs, matching the server config. Idempotent. -pub fn install_default_crypto_provider() { - let _ = aws_lc_rs::default_provider().install_default(); -} +// aws-lc-rs, matching the server config. Idempotent. The body lives in +// seaweed-common so this binary and the Rust plugin workers cannot end up +// installing different providers; re-exported here so callers keep their +// import path. +pub use seaweed_common::tls::install_default_crypto_provider; pub fn build_rustls_server_config( cert_path: &str, diff --git a/seaweed-volume/src/server/grpc_client.rs b/seaweed-volume/src/server/grpc_client.rs index 2605f8428..8a0404f4d 100644 --- a/seaweed-volume/src/server/grpc_client.rs +++ b/seaweed-volume/src/server/grpc_client.rs @@ -158,46 +158,13 @@ pub async fn connect_guarded( /// Shared between `grpc_server.rs` and the distributed-EC-read path /// in `store_ec.rs` — keep this as the single source of truth so the /// HTTP↔gRPC port translation can't drift between callers. +/// +/// The rule itself lives in `seaweed_common::address`, which the Rust +/// plugin workers share; this wrapper only flattens the typed error +/// back to the `String` its callers already handle. Unbracketed IPv6 +/// literals come back bracketed, which this copy used to get wrong. pub fn parse_grpc_address(source: &str) -> Result { - let colon_idx = source - .rfind(':') - .ok_or_else(|| format!("cannot parse address: {}", source))?; - let host = &source[..colon_idx]; - let port_part = &source[colon_idx + 1..]; - - if let Some(dot_idx) = port_part.rfind('.') { - // Format: "ip:port.grpcPort". Validate BOTH ports as u16 - // so a malformed HTTP port (e.g. `host:abc.18080`) is - // rejected here rather than tripping a downstream - // `build_grpc_endpoint` URI parse failure with a less - // useful error. - let http_port = &port_part[..dot_idx]; - let grpc_port = &port_part[dot_idx + 1..]; - http_port - .parse::() - .map_err(|e| format!("invalid http port {:?}: {}", http_port, e))?; - grpc_port - .parse::() - .map_err(|e| format!("invalid grpc port {:?}: {}", grpc_port, e))?; - return Ok(format!("{}:{}", host, grpc_port)); - } - - // Format: "ip:port" → grpc = port + 10000. Reject inputs whose - // implicit grpc port would overflow the TCP port range (e.g. - // `host:60000` produces 70000 — invalid). Without this check - // the cast silently wraps and the endpoint call later fails - // with an opaque connection error. - let port: u16 = port_part - .parse() - .map_err(|e| format!("invalid port {:?}: {}", port_part, e))?; - let grpc_port = port as u32 + 10000; - if grpc_port > u16::MAX as u32 { - return Err(format!( - "implicit grpc port out of range: {} + 10000 = {}", - port, grpc_port - )); - } - Ok(format!("{}:{}", host, grpc_port)) + seaweed_common::address::to_grpc_address(source).map_err(|e| e.to_string()) } #[cfg(test)] @@ -399,4 +366,23 @@ mod tests { let err = parse_grpc_address("hostname").unwrap_err(); assert!(err.contains("cannot parse"), "{}", err); } + + #[test] + fn test_parse_grpc_address_brackets_ipv6_literals() { + use super::parse_grpc_address; + // This used to come back as `::1:29333`, which is not a valid + // authority: `build_grpc_endpoint` reads the last colon as the port + // separator and rejects the rest. + assert_eq!(parse_grpc_address("::1:19333").unwrap(), "[::1]:29333"); + assert_eq!(parse_grpc_address("::1:9333.19333").unwrap(), "[::1]:19333"); + // Already bracketed, so it is left alone. + assert_eq!(parse_grpc_address("[::1]:9333").unwrap(), "[::1]:19333"); + } + + #[test] + fn test_build_grpc_endpoint_accepts_an_ipv6_master_address() { + use super::parse_grpc_address; + let endpoint = build_grpc_endpoint(&parse_grpc_address("::1:9333").unwrap(), None).unwrap(); + assert_eq!(endpoint.uri().port_u16(), Some(19333)); + } } diff --git a/seaweed-worker/Cargo.lock b/seaweed-worker/Cargo.lock index 2de4fc91a..31b8cad14 100644 --- a/seaweed-worker/Cargo.lock +++ b/seaweed-worker/Cargo.lock @@ -5329,6 +5329,13 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "seaweed-common" +version = "0.1.0" +dependencies = [ + "rustls", +] + [[package]] name = "seaweed-worker-core" version = "0.1.0" @@ -5340,6 +5347,7 @@ dependencies = [ "prost", "prost-types", "protoc-bin-vendored", + "seaweed-common", "tokio", "tokio-stream", "tonic", @@ -6449,7 +6457,7 @@ dependencies = [ "lance-table", "prometheus", "reqwest 0.12.28", - "rustls", + "seaweed-common", "seaweed-worker-core", "seaweed-worker-sort", "serde", diff --git a/seaweed-worker/README.md b/seaweed-worker/README.md index f4de6ece3..4548348cb 100644 --- a/seaweed-worker/README.md +++ b/seaweed-worker/README.md @@ -9,6 +9,10 @@ one. crates/core the contract: stream, handshake, heartbeat, registry, config forms crates/lance maintenance jobs for Lance tables, and a binary +It also depends on `../seaweed-common`, a small crate outside this workspace +holding the few helpers the Rust volume server needs identically — the +HTTP<->gRPC address rule and the rustls provider choice. + `core` knows nothing about any job. A second worker is a new crate beside `lance` that depends on it, not a fork of the protocol. diff --git a/seaweed-worker/crates/core/Cargo.toml b/seaweed-worker/crates/core/Cargo.toml index 227f090f6..63619e8f9 100644 --- a/seaweed-worker/crates/core/Cargo.toml +++ b/seaweed-worker/crates/core/Cargo.toml @@ -9,6 +9,9 @@ description = "SeaweedFS plugin.proto worker contract" name = "seaweed_worker_core" [dependencies] +# Helpers the Rust volume server needs as well. A path dependency because +# the two trees are separate cargo workspaces with no common root manifest. +seaweed-common = { path = "../../../seaweed-common" } anyhow.workspace = true async-trait.workspace = true prost.workspace = true diff --git a/seaweed-worker/crates/core/src/address.rs b/seaweed-worker/crates/core/src/address.rs index dcc73a5af..f819fb70d 100644 --- a/seaweed-worker/crates/core/src/address.rs +++ b/seaweed-worker/crates/core/src/address.rs @@ -5,32 +5,15 @@ //! instead fails as "frame with invalid size", which reads like a protocol bug //! rather than a wrong port, so getting this right is worth its own module. //! Mirrors pb.ServerToGrpcAddress in weed/pb/grpc_client_server.go. - -const GRPC_PORT_OFFSET: u16 = 10000; +//! +//! The rule itself now lives in `seaweed_common::address`, shared with the Rust +//! volume server, which had its own copy of it. What stays here is the `Option` +//! shape this crate's callers expect, and the tests that pin it. /// Converts `host:port` to the gRPC address, and accepts the explicit /// `host:port.grpcPort` form the Go side also understands. pub fn server_to_grpc_address(server: &str) -> Option { - let (host, port_part) = server.rsplit_once(':')?; - - // "port.grpcPort" states the gRPC port outright. - if let Some((_, grpc_port)) = port_part.split_once('.') - && let Ok(port) = grpc_port.parse::() - { - return Some(join_host_port(host, port)); - } - - let port: u16 = port_part.parse().ok()?; - Some(join_host_port(host, port.checked_add(GRPC_PORT_OFFSET)?)) -} - -fn join_host_port(host: &str, port: u16) -> String { - // An IPv6 literal has to keep its brackets or the port reads as part of it. - if host.contains(':') && !host.starts_with('[') { - format!("[{host}]:{port}") - } else { - format!("{host}:{port}") - } + seaweed_common::address::to_grpc_address(server).ok() } #[cfg(test)] @@ -70,4 +53,11 @@ mod tests { assert!(server_to_grpc_address("localhost").is_none()); assert!(server_to_grpc_address("localhost:notaport").is_none()); } + + #[test] + fn rejects_a_dotted_form_whose_http_port_is_not_a_port() { + // Tightened by the move to seaweed-common: this copy used to ignore the + // HTTP port of the dotted form and answer Some("host:18080"). + assert!(server_to_grpc_address("host:abc.18080").is_none()); + } } diff --git a/seaweed-worker/crates/lance/Cargo.toml b/seaweed-worker/crates/lance/Cargo.toml index a5aca0e61..1637ab804 100644 --- a/seaweed-worker/crates/lance/Cargo.toml +++ b/seaweed-worker/crates/lance/Cargo.toml @@ -16,6 +16,10 @@ name = "weed-worker" path = "src/main.rs" [dependencies] +# Helpers the Rust volume server needs as well. A path dependency because +# the two trees are separate cargo workspaces with no common root manifest. +# It also carries the rustls requirement this crate used to state itself. +seaweed-common = { path = "../../../seaweed-common" } seaweed-worker-core = { path = "../core" } seaweed-worker-sort = { path = "../sort" } prometheus.workspace = true @@ -36,7 +40,6 @@ anyhow.workspace = true async-trait.workspace = true clap = { version = "4", features = ["derive", "env"] } reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } -rustls = "0.23" serde = { version = "1", features = ["derive"] } serde_json = "1" tokio.workspace = true diff --git a/seaweed-worker/crates/lance/src/tls.rs b/seaweed-worker/crates/lance/src/tls.rs index d2d37770c..c689c7c49 100644 --- a/seaweed-worker/crates/lance/src/tls.rs +++ b/seaweed-worker/crates/lance/src/tls.rs @@ -1,12 +1,10 @@ -use rustls::crypto::aws_lc_rs; - // aws-lc-rs and ring both get linked transitively (lance's aws backend pulls // aws-lc-rs, reqwest's rustls-tls pulls ring), so rustls can't auto-select a // provider and tonic's client TLS panics on first use. Pin the default to -// aws-lc-rs, matching the Rust volume server. Idempotent. -pub fn install_default_crypto_provider() { - let _ = aws_lc_rs::default_provider().install_default(); -} +// aws-lc-rs. The body lives in seaweed-common so this binary and the Rust +// volume server cannot end up installing different providers; re-exported here +// so callers keep their import path. +pub use seaweed_common::tls::install_default_crypto_provider; #[cfg(test)] mod tests {