fix: config path advisory ignores XDG file on interactive root (#81) #83
@@ -612,6 +612,13 @@ pub fn load_config(path: &str) -> Result<ConfigLoad> {
|
|||||||
let filename = p.file_name().unwrap_or(p.as_os_str());
|
let filename = p.file_name().unwrap_or(p.as_os_str());
|
||||||
v.push(crate::config_dir().join(filename));
|
v.push(crate::config_dir().join(filename));
|
||||||
v.push(crate::data_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
|
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 = crate::suggested_config_path().to_string_lossy().to_string();
|
||||||
let display_path = candidates
|
|
||||||
.get(1)
|
|
||||||
.map(|p| p.to_string_lossy().to_string())
|
|
||||||
.unwrap_or_else(|| resolve_path(path));
|
|
||||||
log::info!("config not found, using defaults (create {})", display_path);
|
log::info!("config not found, using defaults (create {})", display_path);
|
||||||
Ok(ConfigLoad {
|
Ok(ConfigLoad {
|
||||||
config: Config::default(),
|
config: Config::default(),
|
||||||
|
|||||||
105
src/lib.rs
105
src/lib.rs
@@ -44,6 +44,42 @@ pub fn hostname() -> String {
|
|||||||
.unwrap_or_else(|| "numa".to_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).
|
/// Shared config directory for persistent data (services.json, etc).
|
||||||
/// Unix users: ~/.config/numa/
|
/// Unix users: ~/.config/numa/
|
||||||
/// Linux root daemon: /var/lib/numa (FHS) — falls back to /usr/local/var/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() {
|
fn linux_data_dir_only_fhs_uses_fhs() {
|
||||||
assert_eq!(resolve_linux_data_dir(false, true), "/var/lib/numa");
|
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"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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)
|
sudo numa install (on Windows, run as Administrator)
|
||||||
|
|
||||||
2. Run on a non-privileged port for testing.
|
2. Run on a non-privileged port for testing.
|
||||||
Create ~/.config/numa/numa.toml with:
|
Create {} with:
|
||||||
|
|
||||||
[server]
|
[server]
|
||||||
bind_addr = \"127.0.0.1:5354\"
|
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
|
Then run: numa
|
||||||
Test with: dig @127.0.0.1 -p 5354 example.com
|
Test with: dig @127.0.0.1 -p 5354 example.com
|
||||||
|
|
||||||
"
|
",
|
||||||
|
crate::suggested_config_path().display()
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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)
|
sudo numa install (on Windows, run as Administrator)
|
||||||
|
|
||||||
2. Point data_dir at a path you can write.
|
2. Point data_dir at a path you can write.
|
||||||
Create ~/.config/numa/numa.toml with:
|
Create {} with:
|
||||||
|
|
||||||
[server]
|
[server]
|
||||||
data_dir = \"/path/you/can/write\"
|
data_dir = \"/path/you/can/write\"
|
||||||
|
|
||||||
",
|
",
|
||||||
data_dir.display()
|
data_dir.display(),
|
||||||
|
crate::suggested_config_path().display()
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
5
tests/docker/hold53.py
Normal file
5
tests/docker/hold53.py
Normal 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
164
tests/docker/issue-81.sh
Executable 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
|
||||||
|
'
|
||||||
Reference in New Issue
Block a user