diff --git a/README.md b/README.md index 2c96042..373bd64 100644 --- a/README.md +++ b/README.md @@ -199,7 +199,7 @@ belongs to, which `--help` shows as two sections: * **System** — what the agent itself does: how it reaches peers (`--reach`), the one overlay interface it owns (`--interface`, `--mtu`, `--no-tun`), the - address range (`--ipv4-range`), and the local resolver (`--dns`). + address range (`--ipv4-range`), and the local resolver (`--no-dns` to disable). * **Transport** — which protocols carry packets (`--protocol`, a list) and their own settings (`-o key=value`, or `-o protocol:key=value`). @@ -228,20 +228,19 @@ session with no agreed protocol. ## Names -`--dns` serves a local DNS zone for **every network this device is in**, +DNS is enabled by default and serves a local zone for **every network this device is in**, each named after the network, so members can be reached by name instead of by address: ```bash -tsunagi up --dns # or `tsunagi dns on` against a running agent +tsunagi up # DNS is already enabled dig @127.0.0.1 -p 5354 music.lab ``` -It is remembered with the device rather than with the command line: once on -it stays on across restarts, and `tsunagi dns off` is what turns it off. A -resolver that quietly disappears because a flag was not retyped is worse -than none, because the names simply stop working. `tsunagi dns on` also -turns it on for an agent that is already running, without restarting it. +An explicit choice is remembered across restarts. `--no-dns` or `tsunagi dns off` +disables it; `--dns` or `tsunagi dns on` enables it again. Existing saved opt-outs +remain respected. The `dns on` and `dns off` commands also take effect immediately +on a running agent. `--dns` and `--no-dns` cannot be combined. The zone of a network is its name — `--dns-zone` is gone, because with several networks there is no single zone to name. A network name may contain diff --git a/crates/tsunagi-cli/src/main.rs b/crates/tsunagi-cli/src/main.rs index 7775639..0e305ed 100644 --- a/crates/tsunagi-cli/src/main.rs +++ b/crates/tsunagi-cli/src/main.rs @@ -422,7 +422,7 @@ struct UpArgs { #[arg(long, value_name = "CIDR", help_heading = "System")] ipv4_range: Option, - /// Serve a local DNS zone for every network this device is in. + /// Enable local DNS for every network (on by default). /// /// Each network becomes a zone named after it, and its members resolve /// as `.` from signed state, so a member that is @@ -430,10 +430,16 @@ struct UpArgs { /// addresses; questions are taken on loopback of both families, over /// UDP and TCP. /// - /// Remembered: once on it stays on, and `tsunagi dns off` turns it off. - #[arg(long, help_heading = "System")] + /// Overrides a remembered opt-out. Use --no-dns or `tsunagi dns off` to disable. + #[arg(long, conflicts_with = "no_dns", help_heading = "System")] dns: bool, + /// Disable local DNS and remember this choice for future starts. + /// + /// Use --dns or `tsunagi dns on` to enable it again. + #[arg(long, conflicts_with = "dns", help_heading = "System")] + no_dns: bool, + /// Port for the local DNS server, on loopback of both families. /// /// Remembered with the setting, so it needs giving only when changing. @@ -505,7 +511,7 @@ struct DnsSetting { impl Default for DnsSetting { fn default() -> Self { Self { - enabled: false, + enabled: true, port: DEFAULT_DNS_PORT, } } @@ -517,9 +523,9 @@ fn dns_setting(paths: &StoragePaths) -> DnsSetting { return DnsSetting::default(); }; DnsSetting { - enabled: matches!( + enabled: !matches!( store.get_setting(DNS_ENABLED).ok().flatten().as_deref(), - Some("1") + Some("0") ), port: store .get_setting(DNS_PORT) @@ -3256,11 +3262,10 @@ async fn up(args: UpArgs) -> Result<(), Box> { let ipv4_range = resolve_ipv4_range(args.ipv4_range.as_ref())?; // The resolver is a property of the device, not of this command line. - // `--dns` turns it on and it stays on; `tsunagi dns off` is what turns - // it off. Anything else means names work today and are gone tomorrow - // because a flag was not retyped. + // Enabled unless deliberately disabled. Explicit flags override the + // remembered choice, which otherwise survives subsequent restarts. let stored_dns = dns_setting(&paths); - let serve_dns = args.dns || stored_dns.enabled; + let serve_dns = !args.no_dns && (args.dns || stored_dns.enabled); let dns_port = args.dns_port.unwrap_or(stored_dns.port); if serve_dns != stored_dns.enabled || dns_port != stored_dns.port { // Written before the agent takes the directory, which it is about diff --git a/crates/tsunagi-cli/tests/dns_service.rs b/crates/tsunagi-cli/tests/dns_service.rs index 139435d..82b1c2e 100644 --- a/crates/tsunagi-cli/tests/dns_service.rs +++ b/crates/tsunagi-cli/tests/dns_service.rs @@ -22,6 +22,7 @@ const PORT_REBIND: u16 = 15362; const PORT_REFUSE: u16 = 15363; const PORT_TWO_ZONES: u16 = 15364; const PORT_SWITCH: u16 = 15365; +const PORT_FLAGS: u16 = 15366; /// Asks, and returns the raw reply. Raw because a parsed packet borrows /// from the bytes it came out of. @@ -73,6 +74,33 @@ struct Running { } impl Running { + fn wait_ready(&mut self) { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let socket = tsunagi::ipc::control_socket_path(&self.dir.path().join("state")); + let deadline = Instant::now() + Duration::from_secs(20); + while !runtime.block_on(tsunagi::ipc::is_serving(&socket)) { + assert!( + self.child.try_wait().unwrap().is_none(), + "the agent exited before becoming ready" + ); + assert!( + Instant::now() < deadline, + "the control socket never became ready" + ); + std::thread::sleep(Duration::from_millis(25)); + } + } + + fn restart(&mut self, dns_flag: Option<&str>) { + self.child.kill().unwrap(); + self.child.wait().unwrap(); + self.child = spawn_child(&self.dir, None, dns_flag); + self.wait_ready(); + } + /// Runs another `tsunagi` command against this agent's directory. fn run(&self, args: &[&str]) -> std::process::Output { std::process::Command::new(env!("CARGO_BIN_EXE_tsunagi")) @@ -135,6 +163,17 @@ fn start_without_dns(network: &str) -> Running { /// The agent alone: `up` runs it, and what it belongs to is decided after. fn start_without_dns_at(_network: &str, dns_port: Option) -> Running { let dir = TempDir::new().unwrap(); + let child = spawn_child(&dir, dns_port, dns_port.is_none().then_some("--no-dns")); + let mut agent = Running { child, dir }; + agent.wait_ready(); + agent +} + +fn spawn_child( + dir: &TempDir, + dns_port: Option, + dns_flag: Option<&str>, +) -> std::process::Child { let mut command = std::process::Command::new(env!("CARGO_BIN_EXE_tsunagi")); command .arg("up") @@ -145,21 +184,21 @@ fn start_without_dns_at(_network: &str, dns_port: Option) -> Running { // No real interface and no internet: this is about the wiring. .args(["--reach", "local", "--no-tun"]); if let Some(port) = dns_port { - command.arg("--dns").args(["--dns-port", &port.to_string()]); + command.args(["--dns-port", &port.to_string()]); } - let child = command + if let Some(flag) = dns_flag { + command.arg(flag); + } + command .args(["--log", "error", "--status-interval", "0"]) .stdout(std::process::Stdio::null()) .stderr(std::process::Stdio::null()) .spawn() - .expect("the agent binary starts"); - // Long enough for the control socket to be there to talk to. - std::thread::sleep(Duration::from_secs(2)); - Running { child, dir } + .expect("the agent binary starts") } #[test] -fn the_resolver_comes_up_even_with_no_overlay_interface_to_put_it_on() { +fn the_resolver_is_on_by_default_even_without_an_overlay_interface() { // The promise is that the port is served whatever else fails: with // `--no-tun` there is no interface to attach a resolver setting to, // and the zone is answered on loopback all the same. Getting this @@ -247,15 +286,11 @@ fn every_network_gets_a_zone_of_its_own() { #[test] fn the_resolver_can_be_switched_on_and_off_while_the_agent_runs() { - // Forgetting `--dns` on a command line should not be a decision that - // lasts until the next restart, and it is not: the setting belongs to - // the device, and turning it on takes effect at once. + // An intentional --no-dns can be reversed at runtime without restarting. let agent = start_without_dns("switch.internal"); let server: SocketAddr = format!("127.0.0.1:{PORT_SWITCH}").parse().unwrap(); let host = hostname(); - // Give the agent time to be up before asking it anything. - std::thread::sleep(Duration::from_secs(1)); assert!( query(server, &format!("{host}.switch.internal"), TYPE::A).is_none(), "nothing should be answering yet" @@ -280,3 +315,33 @@ fn the_resolver_can_be_switched_on_and_off_while_the_agent_runs() { std::thread::sleep(Duration::from_millis(200)); } } + +#[test] +fn explicit_dns_choices_override_the_saved_setting_and_survive_restart() { + let mut agent = start("flags.internal", PORT_FLAGS); + let server = format!("127.0.0.1:{PORT_FLAGS}").parse().unwrap(); + let name = format!("{}.flags.internal", hostname()); + wait_for_answer(server, &name); + + for flag in [Some("--no-dns"), None] { + agent.restart(flag); + let state = agent.run(&["dns"]); + assert!(state.status.success()); + assert!(String::from_utf8_lossy(&state.stdout).contains("not serving")); + assert!(query(server, &name, TYPE::A).is_none()); + } + agent.restart(Some("--dns")); + wait_for_answer(server, &name); + agent.restart(None); + wait_for_answer(server, &name); +} + +#[test] +fn opposing_dns_flags_are_rejected() { + let result = std::process::Command::new(env!("CARGO_BIN_EXE_tsunagi")) + .args(["up", "--dns", "--no-dns"]) + .output() + .unwrap(); + assert!(!result.status.success()); + assert!(String::from_utf8_lossy(&result.stderr).contains("cannot be used with")); +} diff --git a/crates/tsunagi-cli/tests/network_cli.rs b/crates/tsunagi-cli/tests/network_cli.rs index 3e51cbb..130de82 100644 --- a/crates/tsunagi-cli/tests/network_cli.rs +++ b/crates/tsunagi-cli/tests/network_cli.rs @@ -46,7 +46,7 @@ fn start_bare(port: u16) -> Running { .arg(dir.path().join("state")) .arg("--cache-dir") .arg(dir.path().join("cache")) - .args(["--reach", "local", "--no-tun"]) + .args(["--reach", "local", "--no-tun", "--no-dns"]) .arg("--bind") .arg(format!("127.0.0.1:{port}")) .args(["--log", "error", "--status-interval", "0"])