v0.6.2: Mythos Validation — full-codebase polish, hot-path optimizations, dhcproto 0.15
Codebase-wide review pass: finish or remove every loose end, take the safe performance wins on the serving hot paths, and refresh the dependency tree for reliability. No behavior changes for working clients; legacy clients get clearer protocol errors. Finalize / cleanup: - Remove mac_allowlist/subnet_allowlist config fields — parsed but never enforced since introduction; the operator wants line-of-sight serving, so the honest fix is deletion, not wiring. - Remove dead ClientRegistry API (get, set_selected_target, always-None selected_target field, never-emitted DhcpRequest/ HttpIsoAsset events). - TFTP: reject WRQ with ERR_ILLEGAL_OP and non-octet modes with a clear error instead of silent timeouts (legacy-client friendliness); fold plan_window into cfg(test); drop the unused-constant keep-alive hack. - rustfmt sweep over the six files with accumulated drift. Hot-path optimizations (all behavior-preserving): - Serve embedded iPXE binaries zero-copy (Cow over rodata) on both TFTP and HTTP — was a ~1 MiB heap copy per boot file request. - Cache the composited PXE boot-menu background PNG keyed on the branding logo revision — was ~50-200 ms of image work per booting client; now one compose per logo change. - Run bcrypt verify/hash on the blocking pool (boot password gate, login, setup, credential rotation) so CPU-heavy auth can't stall the workers streaming ISO ranges to imaging machines. - iso_raw: reuse the already-cloned IsoMeta for path resolution instead of a second registry lock + deep clone per range request. - DriverEscalation: amortize the TTL sweep (1-min interval + inline staleness check) instead of an O(map) retain per DHCP packet. - format_mac: one allocation instead of four per datagram. - Introspection haystack sized to min(scan cap, file size) — was guaranteed a 32 MiB realloc on every large-ISO probe. Robustness: - parse_range: malformed Range headers are now ignored per RFC 7233 (200 + full body) instead of answered with a bogus 206. Dependencies: - dhcproto 0.12 -> 0.15: drops the deprecated/unmaintained trust-dns-proto from the tree (hickory-proto), three releases of DHCP option coverage. Compiles + passes the full suite unchanged. - socket2 0.6 (dedupes tree), bcrypt 0.19, tower-http 0.6.11 (sheds iri-string), tokio 1.52.3 / hyper 1.10 lockfile refresh; dead nom workspace entry removed; requested versions synced to shipped reality. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
4f193cac05
commit
5da05a519d
@@ -103,7 +103,13 @@ pub fn introspect(path: &Path) -> IntrospectionReport {
|
||||
let scan_bytes = 64 * 1024 * 1024;
|
||||
let mut buf = vec![0u8; 1024 * 1024];
|
||||
let mut read_total = 0usize;
|
||||
let mut haystack = Vec::with_capacity(scan_bytes.min(32 * 1024 * 1024));
|
||||
// Size the haystack to what will actually be read — the scan cap or
|
||||
// the file itself, whichever is smaller — so the fill never reallocs
|
||||
// and a small ISO doesn't reserve the full 64 MiB.
|
||||
let file_len = f.metadata().map_or(usize::MAX, |m| {
|
||||
usize::try_from(m.len()).unwrap_or(usize::MAX)
|
||||
});
|
||||
let mut haystack = Vec::with_capacity(scan_bytes.min(file_len));
|
||||
while read_total < scan_bytes {
|
||||
let n = f.read(&mut buf).unwrap_or(0);
|
||||
if n == 0 {
|
||||
|
||||
@@ -63,8 +63,8 @@ use bytes::Bytes;
|
||||
use nfs3_client::tokio::TokioConnector;
|
||||
use nfs3_client::Nfs3ConnectionBuilder;
|
||||
use nfs3_types::nfs3::{
|
||||
self as nfs3, diropargs3, entry3, filename3, nfs_fh3, GETATTR3args, LOOKUP3args,
|
||||
Nfs3Result, READ3args, READDIR3args,
|
||||
self as nfs3, diropargs3, entry3, filename3, nfs_fh3, GETATTR3args, LOOKUP3args, Nfs3Result,
|
||||
READ3args, READDIR3args,
|
||||
};
|
||||
use nfs3_types::rpc::{auth_unix, opaque_auth};
|
||||
use nfs3_types::xdr_codec::Opaque;
|
||||
@@ -220,10 +220,7 @@ impl NfsShareManager {
|
||||
/// Register an NFS share. Validates, probes connectivity by
|
||||
/// performing a real MOUNT3 + READDIR3, and registers the
|
||||
/// resulting ISOs with the store.
|
||||
pub async fn add(
|
||||
&self,
|
||||
req: NfsAddRequest,
|
||||
) -> std::result::Result<NfsShare, NfsShareError> {
|
||||
pub async fn add(&self, req: NfsAddRequest) -> std::result::Result<NfsShare, NfsShareError> {
|
||||
let server = normalize_server(&req.server);
|
||||
let export = req.export.trim().to_string();
|
||||
if server.is_empty() {
|
||||
@@ -258,7 +255,9 @@ impl NfsShareManager {
|
||||
if let Err(e) = self.rescan_inner(&id).await {
|
||||
let m = self.get(&id);
|
||||
return Err(NfsShareError {
|
||||
error: m.as_ref().and_then(|m| m.last_error.clone())
|
||||
error: m
|
||||
.as_ref()
|
||||
.and_then(|m| m.last_error.clone())
|
||||
.unwrap_or_else(|| e.to_string()),
|
||||
stderr: String::new(),
|
||||
hint: m.and_then(|m| m.last_hint),
|
||||
@@ -333,8 +332,7 @@ impl NfsShareManager {
|
||||
return Err(Error::Invalid(format!("invalid filename '{filename}'")));
|
||||
}
|
||||
|
||||
let (tx, rx) =
|
||||
tokio::sync::mpsc::channel::<std::io::Result<Bytes>>(STREAM_BUFFER_DEPTH);
|
||||
let (tx, rx) = tokio::sync::mpsc::channel::<std::io::Result<Bytes>>(STREAM_BUFFER_DEPTH);
|
||||
let server = share.server.clone();
|
||||
let export = share.export.clone();
|
||||
let port = share.port;
|
||||
@@ -358,16 +356,11 @@ impl NfsShareManager {
|
||||
if let Err(e) = result {
|
||||
// Best-effort signal of the error to the consumer.
|
||||
// If the receiver has already dropped we just exit.
|
||||
let _ = tx
|
||||
.send(Err(std::io::Error::other(e.to_string())))
|
||||
.await;
|
||||
let _ = tx.send(Err(std::io::Error::other(e.to_string()))).await;
|
||||
}
|
||||
});
|
||||
|
||||
Ok(NfsStream {
|
||||
rx,
|
||||
_task: task,
|
||||
})
|
||||
Ok(NfsStream { rx, _task: task })
|
||||
}
|
||||
|
||||
// ── internals ─────────────────────────────────────────────────────
|
||||
@@ -982,8 +975,14 @@ mod tests {
|
||||
// the allow-list and the secure/insecure angle.
|
||||
let h = hint_for("connect failed: MNT3ERR_ACCES").unwrap();
|
||||
let lc = h.to_lowercase();
|
||||
assert!(lc.contains("insecure") || lc.contains("privileged"), "got: {h}");
|
||||
assert!(lc.contains("allow") || lc.contains("permission"), "got: {h}");
|
||||
assert!(
|
||||
lc.contains("insecure") || lc.contains("privileged"),
|
||||
"got: {h}"
|
||||
);
|
||||
assert!(
|
||||
lc.contains("allow") || lc.contains("permission"),
|
||||
"got: {h}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -231,10 +231,7 @@ impl SmbShareManager {
|
||||
|
||||
/// Add or refresh a share. Validates the input, writes a creds
|
||||
/// file, probes connectivity, and scans for ISOs.
|
||||
pub async fn add(
|
||||
&self,
|
||||
req: SmbAddRequest,
|
||||
) -> std::result::Result<SmbShare, SmbShareError> {
|
||||
pub async fn add(&self, req: SmbAddRequest) -> std::result::Result<SmbShare, SmbShareError> {
|
||||
let server = normalize_server(&req.server);
|
||||
let share = req.share.trim().trim_start_matches('/').to_string();
|
||||
if server.is_empty() {
|
||||
@@ -309,7 +306,9 @@ impl SmbShareManager {
|
||||
if let Err(e) = self.rescan_inner(&id).await {
|
||||
let m = self.get(&id);
|
||||
return Err(SmbShareError {
|
||||
error: m.as_ref().and_then(|m| m.last_error.clone())
|
||||
error: m
|
||||
.as_ref()
|
||||
.and_then(|m| m.last_error.clone())
|
||||
.unwrap_or_else(|| e.to_string()),
|
||||
stderr: String::new(),
|
||||
hint: m.and_then(|m| m.last_hint),
|
||||
@@ -365,11 +364,7 @@ impl SmbShareManager {
|
||||
/// throttling concurrent smbclients) would need to await without
|
||||
/// changing the call sites.
|
||||
#[allow(clippy::unused_async)]
|
||||
pub async fn stream_iso(
|
||||
&self,
|
||||
share_id: &str,
|
||||
filename: &str,
|
||||
) -> Result<SmbStream> {
|
||||
pub async fn stream_iso(&self, share_id: &str, filename: &str) -> Result<SmbStream> {
|
||||
let share = self
|
||||
.get(share_id)
|
||||
.ok_or_else(|| Error::Invalid(format!("no such SMB share '{share_id}'")))?;
|
||||
@@ -377,9 +372,7 @@ impl SmbShareManager {
|
||||
// share root. smbclient itself accepts only filenames at the
|
||||
// share root in our `get` form, but belt-and-suspenders.
|
||||
if filename.contains('/') || filename.contains('\\') || filename.contains("..") {
|
||||
return Err(Error::Invalid(format!(
|
||||
"invalid filename '{filename}'"
|
||||
)));
|
||||
return Err(Error::Invalid(format!("invalid filename '{filename}'")));
|
||||
}
|
||||
let creds = share
|
||||
.creds_path
|
||||
@@ -537,10 +530,7 @@ impl SmbShareManager {
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
return Err((
|
||||
format!("could not exec smbclient: {e}"),
|
||||
stderr,
|
||||
));
|
||||
return Err((format!("could not exec smbclient: {e}"), stderr));
|
||||
}
|
||||
};
|
||||
if !output.status.success() {
|
||||
@@ -751,10 +741,7 @@ fn parse_ls_iso(out: &str) -> Vec<SmbListEntry> {
|
||||
|
||||
/// Pre-flight TCP probe to `server:port`. Format matches v0.4.64 NFS
|
||||
/// probe so the UI banner reads consistently.
|
||||
async fn tcp_probe(
|
||||
server: &str,
|
||||
port: u16,
|
||||
) -> std::result::Result<(), (String, String)> {
|
||||
async fn tcp_probe(server: &str, port: u16) -> std::result::Result<(), (String, String)> {
|
||||
use tokio::net::TcpStream;
|
||||
let addr = format!("{server}:{port}");
|
||||
match tokio::time::timeout(PROBE_TIMEOUT, TcpStream::connect(&addr)).await {
|
||||
@@ -931,8 +918,8 @@ mod tests {
|
||||
// exec error in `error` plus an empty `stderr`. The
|
||||
// SmbShareError constructor's hint_for fallback checks error
|
||||
// too, so this pattern needs to translate as well.
|
||||
let h2 = hint_for("could not exec smbclient: No such file or directory (os error 2)")
|
||||
.unwrap();
|
||||
let h2 =
|
||||
hint_for("could not exec smbclient: No such file or directory (os error 2)").unwrap();
|
||||
assert!(h2.contains("smbclient"));
|
||||
}
|
||||
|
||||
|
||||
@@ -342,9 +342,18 @@ impl IsoStore {
|
||||
/// SMB sources or when the file is missing.
|
||||
pub fn iso_path_for(&self, id: &str) -> Option<PathBuf> {
|
||||
let meta = self.get(id)?;
|
||||
self.local_path(&meta)
|
||||
}
|
||||
|
||||
/// Same resolution as [`Self::iso_path_for`], but for a meta the
|
||||
/// caller already holds — skips the second registry lock + deep
|
||||
/// clone, which matters on the per-range-request ISO serving path
|
||||
/// (a sanboot install issues hundreds of those).
|
||||
#[must_use]
|
||||
pub fn local_path(&self, meta: &IsoMeta) -> Option<PathBuf> {
|
||||
match &meta.source {
|
||||
IsoSource::Local => {
|
||||
let path = self.iso_path(id);
|
||||
let path = self.iso_path(&meta.id);
|
||||
if path.exists() {
|
||||
Some(path)
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user