fix: config path advisory ignores XDG file on interactive root (#81) #83

Merged
razvandimescu merged 1 commits from fix/config-path-advisory into main 2026-04-12 07:17:33 +08:00
6 changed files with 288 additions and 9 deletions
Showing only changes of commit 08eb625ef4 - Show all commits

View File

@@ -612,6 +612,13 @@ pub fn load_config(path: &str) -> Result<ConfigLoad> {
let filename = p.file_name().unwrap_or(p.as_os_str());
v.push(crate::config_dir().join(filename));
v.push(crate::data_dir().join(filename));
// Interactive root and sudo'd users: always consult the XDG path
// so `touch ~/.config/numa/numa.toml` works regardless of whether
// config_dir() routed to FHS (issue #81).
let suggested = crate::suggested_config_path();
if !v.contains(&suggested) {
v.push(suggested);
}
}
v
};
@@ -632,11 +639,7 @@ pub fn load_config(path: &str) -> Result<ConfigLoad> {
}
}
// Show config_dir candidate as the "expected" path — it's actionable
let display_path = candidates
.get(1)
.map(|p| p.to_string_lossy().to_string())
.unwrap_or_else(|| resolve_path(path));
let display_path = crate::suggested_config_path().to_string_lossy().to_string();
log::info!("config not found, using defaults (create {})", display_path);
Ok(ConfigLoad {
config: Config::default(),

View File

@@ -44,6 +44,42 @@ pub fn hostname() -> String {
.unwrap_or_else(|| "numa".to_string())
}
/// Path to suggest to an interactive user when asking them to create
/// `numa.toml`. Prefers `$HOME/.config/numa/numa.toml` when HOME is set
/// (actionable without sudo); falls back to `config_dir()` otherwise.
///
/// Note: `config_dir()` routes interactive root to FHS (`/var/lib/numa`)
/// so that runtime state like `services.json` stays continuous with the
/// installed daemon. This helper exists specifically to give advisories
/// and `load_config` an XDG-aware path for user-authored config, without
/// moving runtime state out of FHS — see issue #81.
pub(crate) fn suggested_config_path() -> std::path::PathBuf {
#[cfg(not(windows))]
{
resolve_suggested_config_path(std::env::var("HOME").ok().as_deref(), config_dir)
}
#[cfg(windows)]
{
config_dir().join("numa.toml")
}
}
#[cfg(not(windows))]
fn resolve_suggested_config_path<F>(home: Option<&str>, fallback_dir: F) -> std::path::PathBuf
where
F: FnOnce() -> std::path::PathBuf,
{
if let Some(home) = home {
if !home.is_empty() && home != "/" {
return std::path::PathBuf::from(home)
.join(".config")
.join("numa")
.join("numa.toml");
}
}
fallback_dir().join("numa.toml")
}
/// Shared config directory for persistent data (services.json, etc).
/// Unix users: ~/.config/numa/
/// Linux root daemon: /var/lib/numa (FHS) — falls back to /usr/local/var/numa
@@ -163,4 +199,73 @@ mod tests {
fn linux_data_dir_only_fhs_uses_fhs() {
assert_eq!(resolve_linux_data_dir(false, true), "/var/lib/numa");
}
#[cfg(not(windows))]
fn fhs() -> std::path::PathBuf {
std::path::PathBuf::from("/var/lib/numa")
}
#[cfg(not(windows))]
#[test]
fn suggested_config_path_prefers_home() {
assert_eq!(
resolve_suggested_config_path(Some("/home/alice"), fhs),
std::path::PathBuf::from("/home/alice/.config/numa/numa.toml"),
);
}
#[cfg(not(windows))]
#[test]
fn suggested_config_path_prefers_root_home_over_fhs() {
// Interactive root: HOME=/root is a real user context, not a daemon signal.
// Advisory must point where load_config will actually look — issue #81.
assert_eq!(
resolve_suggested_config_path(Some("/root"), fhs),
std::path::PathBuf::from("/root/.config/numa/numa.toml"),
);
}
#[cfg(not(windows))]
#[test]
fn suggested_config_path_falls_back_when_home_unset() {
assert_eq!(
resolve_suggested_config_path(None, fhs),
std::path::PathBuf::from("/var/lib/numa/numa.toml"),
);
}
#[cfg(not(windows))]
#[test]
fn suggested_config_path_falls_back_when_home_is_root() {
// systemd services sometimes have HOME=/ — don't treat that as a real home.
assert_eq!(
resolve_suggested_config_path(Some("/"), fhs),
std::path::PathBuf::from("/var/lib/numa/numa.toml"),
);
}
#[cfg(not(windows))]
#[test]
fn suggested_config_path_falls_back_when_home_is_empty() {
assert_eq!(
resolve_suggested_config_path(Some(""), fhs),
std::path::PathBuf::from("/var/lib/numa/numa.toml"),
);
}
#[cfg(not(windows))]
#[test]
fn suggested_config_path_skips_fallback_when_home_valid() {
// Happy path shouldn't probe the filesystem via config_dir().
let called = std::cell::Cell::new(false);
let fallback = || {
called.set(true);
std::path::PathBuf::from("/should/not/be/used")
};
let _ = resolve_suggested_config_path(Some("/home/alice"), fallback);
assert!(
!called.get(),
"fallback must not be invoked when HOME is valid"
);
}
}

View File

@@ -91,7 +91,7 @@ pub fn try_port53_advisory(bind_addr: &str, err: &std::io::Error) -> Option<Stri
sudo numa install (on Windows, run as Administrator)
2. Run on a non-privileged port for testing.
Create ~/.config/numa/numa.toml with:
Create {} with:
[server]
bind_addr = \"127.0.0.1:5354\"
@@ -100,7 +100,8 @@ pub fn try_port53_advisory(bind_addr: &str, err: &std::io::Error) -> Option<Stri
Then run: numa
Test with: dig @127.0.0.1 -p 5354 example.com
"
",
crate::suggested_config_path().display()
))
}

View File

@@ -66,13 +66,14 @@ pub fn try_data_dir_advisory(err: &crate::Error, data_dir: &Path) -> Option<Stri
sudo numa install (on Windows, run as Administrator)
2. Point data_dir at a path you can write.
Create ~/.config/numa/numa.toml with:
Create {} with:
[server]
data_dir = \"/path/you/can/write\"
",
data_dir.display()
data_dir.display(),
crate::suggested_config_path().display()
))
}

5
tests/docker/hold53.py Normal file
View File

@@ -0,0 +1,5 @@
import socket, signal
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 0)
s.bind(("", 53))
signal.pause()

164
tests/docker/issue-81.sh Executable file
View File

@@ -0,0 +1,164 @@
#!/usr/bin/env bash
#
# End-to-end validation of the issue #81 fix (config path advisory).
#
# Builds numa from two source trees — the buggy baseline and the fix
# candidate — inside one debian:bookworm container, then runs four
# scenarios to prove:
#
# 1. replication/main — reporter's sequence, bug confirmed
# 2. replication/fix — reporter's sequence, bug is gone
# 3. existing/main — pre-installed config at FHS data dir still loads
# 4. existing/fix — same, unchanged by the fix (no regression)
#
# Scenarios 3 and 4 guard against the fear that the fix might change
# candidate order and break existing daemon installs (including the
# macOS Homebrew-prefix layout at /usr/local/var/numa/).
#
# Usage:
# MAIN_SRC=/path/to/main-checkout FIX_SRC=/path/to/fix-worktree \
# ./tests/docker/issue-81.sh
#
# Defaults: MAIN_SRC = $(git rev-parse --show-toplevel), FIX_SRC = same.
set -euo pipefail
MAIN_SRC="${MAIN_SRC:-$(git rev-parse --show-toplevel)}"
FIX_SRC="${FIX_SRC:-$MAIN_SRC}"
GREEN="\033[32m"; RED="\033[31m"; RESET="\033[0m"
echo "── issue #81 validation ──"
echo " main: $MAIN_SRC"
echo " fix: $FIX_SRC"
echo
docker run --rm \
--platform linux/amd64 \
-v "$MAIN_SRC:/main:ro" \
-v "$FIX_SRC:/fix:ro" \
-v "$(dirname "$0")/hold53.py:/tmp/hold53.py:ro" \
-v numa-port53-cargo:/root/.cargo \
-v numa-port53-target:/work/target \
debian:bookworm bash -c '
set -euo pipefail
# Paths and ports used by all scenarios — keep in one place so the
# heredocs and the verdict greps cannot drift.
XDG_CONFIG="/root/.config/numa/numa.toml"
FHS_CONFIG="/var/lib/numa/numa.toml"
TEST_PORT="5354"
TEST_API_PORT="5380"
apt-get update -qq && apt-get install -y -qq curl build-essential python3 2>&1 | tail -1
if ! command -v cargo &>/dev/null; then
curl -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal --quiet
fi
. "$HOME/.cargo/env"
build_from() {
local label="$1"; local src="$2"
mkdir -p "/work/$label"
tar -C "$src" --exclude=./target --exclude=./.git -cf - . | tar -C "/work/$label" -xf -
(cd "/work/$label" && cargo build --release --locked 2>&1 | tail -1)
cp "/work/$label/target/release/numa" "/work/numa-$label"
}
build_from main /main
build_from fix /fix
holder=0
stop_holder() {
if [ "$holder" -ne 0 ]; then
kill "$holder" 2>/dev/null || true
wait "$holder" 2>/dev/null || true
holder=0
fi
}
trap stop_holder EXIT
start_holder() {
python3 /tmp/hold53.py &
holder=$!
sleep 0.3
}
write_test_config() {
local path="$1"
mkdir -p "$(dirname "$path")"
cat > "$path" <<EOF
[server]
bind_addr = "127.0.0.1:$TEST_PORT"
api_port = $TEST_API_PORT
EOF
}
verdict() {
local label="$1"; local expected="$2"; local file="$3"
# "cannot bind to" is printed by the advisory when numa fails to start.
# Its absence is a reliable proxy for "numa bound successfully" because
# the banner-only log we capture contains no other failure surface.
if grep -q "cannot bind to" "$file"; then
echo " [$label] did not bind $TEST_PORT — numa ignored the XDG config"
[ "$expected" = "ignored" ] && return 0 || return 1
else
echo " [$label] bound $TEST_PORT — config loaded"
[ "$expected" = "bound" ] && return 0 || return 1
fi
}
scenario_replication() {
local label="$1"; local bin="/work/numa-$label"; local expected="$2"
echo
echo "════════ REPLICATION / $label ════════"
rm -rf /root/.config/numa /var/lib/numa
mkdir -p "$(dirname "$XDG_CONFIG")"
start_holder
set +e
timeout 5 "$bin" > /tmp/run1.txt 2>&1
set -e
echo "── step 1: advisory printed by $label ──"
grep -E "Create .* with:" /tmp/run1.txt | sed "s/^/ /" || echo " <no advisory line>"
write_test_config "$XDG_CONFIG"
echo "── step 2: wrote config at $XDG_CONFIG ──"
set +e
timeout 3 "$bin" > /tmp/run2.txt 2>&1
set -e
stop_holder
verdict "$label" "$expected" /tmp/run2.txt
}
scenario_existing_install() {
local label="$1"; local bin="/work/numa-$label"
echo
echo "════════ EXISTING INSTALL / $label ════════"
rm -rf /root/.config/numa /var/lib/numa
write_test_config "$FHS_CONFIG"
start_holder
set +e
timeout 3 "$bin" > /tmp/run.txt 2>&1
set -e
stop_holder
verdict "$label" "bound" /tmp/run.txt
}
RC=0
scenario_replication main ignored || RC=1
scenario_replication fix bound || RC=1
scenario_existing_install main || RC=1
scenario_existing_install fix || RC=1
echo
if [ "$RC" -eq 0 ]; then
echo "── all scenarios matched expectations ──"
else
echo "── FAILURE: one or more scenarios diverged ──"
fi
exit $RC
'