Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions crates/fractal/src/fleet_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,11 @@ fn read_code_without_echo() -> Result<String> {
const FABRIC_PAIRING_UNAVAILABLE: &str = "Fabric invitations need st-fabric-adapter installed and \
Fabric running with --runtime-peers; use --via tailscale for a Tailscale-only invitation";

/// The pairing helper beside st, named by ST_FABRIC_ADAPTER, or on PATH. A configured
/// path that is not a file counts as no helper, so `auto` invitations route around it.
fn fabric_adapter() -> Option<PathBuf> {
if let Some(path) = std::env::var_os("ST_FABRIC_ADAPTER") {
let path = PathBuf::from(path);
return path.is_file().then_some(path);
}
std::env::current_exe()
.ok()
.and_then(|path| path.parent().map(|parent| parent.join("st-fabric-adapter")))
.filter(|path| path.is_file())
.or_else(|| fractal_daemon::fleet::transport::resolve_tool(None, "st-fabric-adapter"))
/// A configured path that is not a file counts as no helper, so `auto` invitations
/// route around it. Conflicting environment overrides are errors, not absence.
fn fabric_adapter() -> Result<Option<PathBuf>> {
Ok(fractal_daemon::fleet::transport::resolve_fabric_adapter()?
.filter(|path| path.is_file()))
}

/// The `--via` an invitation is created with. Without the pairing helper, a Fabric endpoint
Expand Down Expand Up @@ -611,7 +604,7 @@ pub(super) async fn run_fleet(endpoint: &Endpoint, command: FleetCommand, json_o
} else {
None
};
let adapter = fabric_adapter();
let adapter = fabric_adapter()?;
let mut via = args.via.clone();
if adapter.is_none() && !args.migrate {
via = invite_via(&via, &member_endpoints(&client, &config.node).await?, false)?;
Expand Down
18 changes: 7 additions & 11 deletions crates/smallclaims/src/fleet/join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use super::file::{FleetFile, FleetMode};
use super::handshake::{JoinRequest, JoinResponse, Joiner, SealedJoin, WriterHead, verify_response};
use super::keys::MemberKey;
use super::transport::{
Fabric, Route, bindable_tailnet_addresses, local_addresses, parse_route, resolve_tool,
tailscale_addresses,
Fabric, Route, bindable_tailnet_addresses, local_addresses, parse_route, resolve_fabric_adapter,
resolve_tool, tailscale_addresses,
};
use crate::error::Error;
use crate::store::{AdmittedStore, Runtime, Store, valid_fleet_node_name};
Expand Down Expand Up @@ -528,15 +528,11 @@ impl PairingHelper {
fn launch(command: &str, executable: Option<&Path>) -> Result<Self> {
use std::process::Stdio;
use tokio::io::AsyncBufReadExt as _;
let executable = executable.map(Path::to_path_buf)
.or_else(|| std::env::var_os("ST_FABRIC_ADAPTER").map(PathBuf::from))
.or_else(|| {
std::env::current_exe().ok()
.and_then(|exe| exe.parent().map(|parent| parent.join("st-fabric-adapter")))
.filter(|path| path.is_file())
})
.or_else(|| resolve_tool(None, "st-fabric-adapter"))
.context("st-fabric-adapter is unavailable; install it alongside st or set ST_FABRIC_ADAPTER")?;
let executable = match executable {
Some(path) => Some(path.to_path_buf()),
None => resolve_fabric_adapter()?,
}
.context("st-fabric-adapter is unavailable; install it alongside st or set ST_FABRIC_ADAPTER")?;
let mut child = tokio::process::Command::new(executable)
.arg(command)
.stdin(Stdio::piped())
Expand Down
102 changes: 102 additions & 0 deletions crates/smallclaims/src/fleet/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,49 @@ pub fn resolve_tool(override_path: Option<&Path>, name: &str) -> Option<PathBuf>
})
}

/// Find the pairing helper: new then legacy environment, beside this executable, then
/// each PATH directory. Within each directory the new executable name wins.
/// Configured paths are returned even if absent, so callers retain their absence policy.
pub fn resolve_fabric_adapter() -> Result<Option<PathBuf>> {
let executable = std::env::current_exe().ok();
let paths = std::env::var_os("PATH");
discover_fabric_adapter(
std::env::var_os("FRACTAL_FABRIC_ADAPTER").as_deref(),
std::env::var_os("ST_FABRIC_ADAPTER").as_deref(),
executable.as_deref().and_then(Path::parent),
paths.as_deref(),
)
}

fn discover_fabric_adapter(
new_env: Option<&std::ffi::OsStr>,
old_env: Option<&std::ffi::OsStr>,
beside: Option<&Path>,
paths: Option<&std::ffi::OsStr>,
) -> Result<Option<PathBuf>> {
if let (Some(new), Some(old)) = (new_env, old_env) {
anyhow::ensure!(
new == old,
"FRACTAL_FABRIC_ADAPTER and ST_FABRIC_ADAPTER name different paths; \
unset one or set both to the same path"
);
}
if let Some(path) = new_env.or(old_env) {
return Ok(Some(PathBuf::from(path)));
}
let in_directory = |directory: &Path| {
["fractal-fabric-adapter", "st-fabric-adapter"]
.into_iter()
.map(|name| directory.join(name))
.find(|candidate| candidate.is_file())
};
Ok(beside.and_then(in_directory).or_else(|| {
paths.and_then(|paths| {
std::env::split_paths(paths).find_map(|directory| in_directory(&directory))
})
}))
}

#[derive(Debug)]
struct CommandFailure(String);

Expand Down Expand Up @@ -1268,6 +1311,65 @@ mod tests {
assert!(relative.dial_socket("node-b", "p").await.is_err());
}

#[test]
fn fabric_adapter_environment_precedence_and_conflict() {
let new = std::ffi::OsStr::new("new-adapter");
let old = std::ffi::OsStr::new("old-adapter");
for (new_env, old_env, expected) in [
(Some(new), None, new),
(None, Some(old), old),
(Some(new), Some(new), new),
] {
assert_eq!(
discover_fabric_adapter(new_env, old_env, Some(Path::new(".")), None).unwrap(),
Some(PathBuf::from(expected))
);
}
let error = discover_fabric_adapter(Some(new), Some(old), None, None).unwrap_err();
assert!(error.to_string().contains("FRACTAL_FABRIC_ADAPTER and ST_FABRIC_ADAPTER"));
assert!(error.to_string().contains("different paths"));
}

#[test]
fn fabric_adapter_location_and_name_precedence() {
let root = tempfile::tempdir().unwrap();
let directories = ["beside", "first", "second"].map(|name| root.path().join(name));
for directory in &directories {
std::fs::create_dir(directory).unwrap();
}
let paths = std::env::join_paths(&directories[1..]).unwrap();
let candidates: Vec<_> = directories.iter().flat_map(|directory| {
["fractal-fabric-adapter", "st-fabric-adapter"]
.map(|name| directory.join(name))
}).collect();
for candidate in &candidates {
std::fs::write(candidate, "").unwrap();
}
for (new, old) in [(Some("configured"), None), (None, Some("configured"))] {
assert_eq!(
discover_fabric_adapter(
new.map(std::ffi::OsStr::new),
old.map(std::ffi::OsStr::new),
Some(&directories[0]),
Some(&paths),
).unwrap(),
Some(PathBuf::from("configured"))
);
}
for expected in &candidates {
// Removing each winner proves both names at every location and that a
// legacy name in an earlier directory beats a new name later on.
assert_eq!(
discover_fabric_adapter(None, None, Some(&directories[0]), Some(&paths)).unwrap(),
Some(expected.clone())
);
std::fs::remove_file(expected).unwrap();
}
let absent = discover_fabric_adapter(None, None, Some(&directories[0]), Some(&paths))
.unwrap();
assert!(absent.is_none());
}

#[test]
fn an_override_is_used_without_searching_path() {
let override_path = Path::new("/nonexistent/shim/fabric");
Expand Down
11 changes: 11 additions & 0 deletions release-notes/fabric-adapter-discovery-readers.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"version": 1,
"summary": "Fabric pairing accepts new and legacy adapter executable and environment names, refusing conflicting environment overrides.",
"replay": { "status": "none", "detail": "No claims or projections change." },
"schema": { "status": "none", "detail": "No stored formats change." },
"rules": { "status": "none", "detail": "Pairing trust and authorization are unchanged." },
"compatibility": { "status": "changed", "detail": "Discovery accepts FRACTAL_FABRIC_ADAPTER then ST_FABRIC_ADAPTER. Different paths in both variables are an error. Otherwise it searches beside the executable then each PATH directory, preferring fractal-fabric-adapter over st-fabric-adapter within each directory. The produced binary remains st-fabric-adapter." },
"downtime": { "status": "none", "detail": "No runtime migration is required." },
"manual": { "status": "none", "detail": "Existing installations continue working. If both overrides are set, use the same path or unset one." },
"recovery": { "status": "none", "detail": "Rollback retains support for the legacy executable and environment variable." }
}
Loading