db-mode config changes apply live in-process
Gates / frontend (push) Successful in 1m43s
Gates / test (push) Successful in 2m14s
Gates / test-aarch64 (push) Successful in 8m3s
Gates / package (push) Successful in 5m42s
Gates / container (push) Successful in 54s
CI / gates (push) Successful in 50m24s
Gates / frontend (push) Successful in 1m43s
Gates / test (push) Successful in 2m14s
Gates / test-aarch64 (push) Successful in 8m3s
Gates / package (push) Successful in 5m42s
Gates / container (push) Successful in 54s
CI / gates (push) Successful in 50m24s
settings and upstream writes now follow a prepare, commit, publish, retire contract: candidates are built and validated before the database transaction, published as infallible pointer swaps, and old generations retire after their readers drain. per-query policy values snapshot once per query; upstream pool, cache, rate limiter, sessions, api limiter, log sink, blocklist scheduler and the query-log queue each gained one named live operation. restart_required shrinks from every scalar key to the bind keys and web.enabled; the admin ui drops its restart notices for everything else. file mode is unchanged.
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { fireEvent, screen, waitFor, within } from "@testing-library/react";
|
||||
import { DATABASE, renderPage, stubApi, type Call } from "./testFixtures";
|
||||
import { DATABASE, contentArea, renderPage, stubApi, type Call } from "./testFixtures";
|
||||
|
||||
/**
|
||||
* Resolution in database mode: the upstream pool, the local records and the
|
||||
@@ -37,7 +37,7 @@ test("the upstream pool is the default tab and lists every field", async () => {
|
||||
expect((screen.getByLabelText("udp://1.1.1.1:53 enabled") as HTMLInputElement).checked).toBe(true);
|
||||
expect((screen.getByLabelText("tls://9.9.9.9:853 enabled") as HTMLInputElement).checked).toBe(false);
|
||||
expect(screen.getByRole("heading", { name: "Add upstream" })).toBeTruthy();
|
||||
expect(screen.getByText(/takes effect at the next restart/)).toBeTruthy();
|
||||
expect(screen.getByText(/applies to the next query/)).toBeTruthy();
|
||||
});
|
||||
|
||||
test("adding an upstream posts every field", async () => {
|
||||
@@ -56,24 +56,18 @@ test("adding an upstream posts every field", async () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("an upstream write re-reads the config status, and the shell states the pending restart", async () => {
|
||||
// The client never decides a restart is owed: the server sets the flag, and
|
||||
// the mutation's invalidation is only what makes the page ask again.
|
||||
let restartPending = false;
|
||||
await openResolution(undefined, {
|
||||
responses: { "GET /api/config/status": () => ({ ...DATABASE, restart_pending: restartPending }) },
|
||||
onWrite: () => {
|
||||
restartPending = true;
|
||||
return null;
|
||||
},
|
||||
});
|
||||
test("an upstream write applies live, so the tab says nothing about a restart", async () => {
|
||||
// The server rebuilds the pool on the write and echoes `restart_required:
|
||||
// false`, so `restart_pending` stays down and silence is the whole report.
|
||||
await openResolution(undefined, { responses: { "GET /api/config/status": () => DATABASE } });
|
||||
await screen.findByRole("heading", { name: "Add upstream" });
|
||||
expect(screen.queryByText(/Restart nxdns to apply them/)).toBeNull();
|
||||
|
||||
fireEvent.change(screen.getByLabelText("URL"), { target: { value: "udp://8.8.8.8:53" } });
|
||||
fireEvent.click(screen.getByRole("button", { name: "Add upstream" }));
|
||||
|
||||
await screen.findByText(/Saved changes are not running yet\. Restart nxdns to apply them\./);
|
||||
await waitFor(() => expect(writes()).toHaveLength(1));
|
||||
expect(screen.queryByText(/Restart nxdns to apply them/)).toBeNull();
|
||||
expect(contentArea().textContent).not.toMatch(/restart/i);
|
||||
});
|
||||
|
||||
test("toggling enabled resends the whole row", async () => {
|
||||
|
||||
@@ -1,16 +1,16 @@
|
||||
import { act, fireEvent, screen, waitFor, within } from "@testing-library/react";
|
||||
import { queryKeys } from "@/lib/queries";
|
||||
import type { Settings, SettingsPatch } from "@/lib/types";
|
||||
import { DATABASE, MANAGED_FILE, baseSettings, renderPage, stubApi } from "./testFixtures";
|
||||
import { DATABASE, MANAGED_FILE, RESTART_REQUIRED_KEYS, baseSettings, renderPage, stubApi } from "./testFixtures";
|
||||
|
||||
/**
|
||||
* System in database mode: the settings form, its diff contract, and the
|
||||
* certificate reload that is a runtime action under both authorities.
|
||||
*/
|
||||
|
||||
// `logging.level` is enum-backed, so the list covers both field renderings: an
|
||||
// input whose label carries the mark, and a `Select` that cannot.
|
||||
const RESTART_KEYS = ["dns.port", "web.port", "logging.level"];
|
||||
// What the server actually reports: the listener binds and `web.enabled`.
|
||||
// Every other key applies live, so it carries no mark at all.
|
||||
const RESTART_KEYS = RESTART_REQUIRED_KEYS;
|
||||
|
||||
let stored: Settings;
|
||||
let putBodies: SettingsPatch[];
|
||||
@@ -32,10 +32,10 @@ function applyPatch(patch: SettingsPatch): void {
|
||||
}
|
||||
}
|
||||
|
||||
/** Mirrors settings.zig: a patch touching only `web.password` applies live. */
|
||||
function needsRestart(patch: SettingsPatch): boolean {
|
||||
/** Mirrors apply.zig's table: only a listed key leaves the server owing a restart. */
|
||||
function needsRestart(patch: SettingsPatch, keys: readonly string[]): boolean {
|
||||
return Object.entries(patch).some(([section, fields]) =>
|
||||
Object.keys(fields as Record<string, unknown>).some((key) => !(section === "web" && key === "password")),
|
||||
Object.keys(fields as Record<string, unknown>).some((key) => keys.includes(`${section}.${key}`)),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -50,11 +50,11 @@ afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
async function openSystem() {
|
||||
async function openSystem(restartKeys: readonly string[] = RESTART_KEYS) {
|
||||
stubApi(DATABASE, {
|
||||
responses: {
|
||||
"GET /api/config/status": () => ({ ...DATABASE, restart_pending: restartPending }),
|
||||
"GET /api/settings": () => ({ settings: stored, restart_required: RESTART_KEYS }),
|
||||
"GET /api/settings": () => ({ settings: stored, restart_required: restartKeys }),
|
||||
},
|
||||
onWrite: (call) => {
|
||||
if (call.url !== "/api/settings") return null;
|
||||
@@ -62,8 +62,8 @@ async function openSystem() {
|
||||
putBodies.push(patch);
|
||||
if (putResponse !== null) return putResponse();
|
||||
applyPatch(patch);
|
||||
if (needsRestart(patch)) restartPending = true;
|
||||
return json({ settings: stored, restart_required: RESTART_KEYS });
|
||||
if (needsRestart(patch, restartKeys)) restartPending = true;
|
||||
return json({ settings: stored, restart_required: restartKeys });
|
||||
},
|
||||
});
|
||||
const router = await renderPage("/configuration/system", "System");
|
||||
@@ -103,15 +103,50 @@ test("a changed field enables Save and the PUT body is exactly the diff", async
|
||||
test("a restart-required key is marked as one, from the envelope's list", async () => {
|
||||
await openSystem();
|
||||
|
||||
expect(within(screen.getByRole("group", { name: "DNS" })).getByText("needs restart")).toBeTruthy();
|
||||
// `rate_limit` is not on the list, so it carries no mark.
|
||||
// The DNS binds and the port are the section's whole share of the list;
|
||||
// `rate_limit` and `rate_window_seconds` apply live and carry no mark.
|
||||
const dns = screen.getByRole("group", { name: "DNS" });
|
||||
expect(within(dns).getAllByText("needs restart")).toHaveLength(3);
|
||||
const cache = screen.getByRole("group", { name: "Cache" });
|
||||
expect(within(cache).queryByText("needs restart")).toBeNull();
|
||||
});
|
||||
|
||||
test("an enum-backed key on the list is marked too, not only text and number fields", async () => {
|
||||
test("every key the server applies live is drawn without restart messaging", async () => {
|
||||
await openSystem();
|
||||
|
||||
// Silence is the report for a live key: no mark on the field, and editing
|
||||
// one owes nothing afterwards either.
|
||||
for (const title of ["Upstream", "Blocking", "Cache", "EDNS", "Logging", "Disk", "Blocklist Update"]) {
|
||||
const section = screen.getByRole("group", { name: title });
|
||||
expect(within(section).queryByText("needs restart")).toBeNull();
|
||||
}
|
||||
|
||||
const logging = screen.getByRole("group", { name: "Logging" });
|
||||
fireEvent.change(within(logging).getByLabelText("retention_days"), { target: { value: "14" } });
|
||||
fireEvent.click(saveButton());
|
||||
|
||||
await waitFor(() => expect(putBodies).toHaveLength(1));
|
||||
expect(putBodies[0]).toEqual({ logging: { retention_days: 14 } });
|
||||
await waitFor(() => expect(saveButton().disabled).toBe(true));
|
||||
expect(restartNotice()).toBeNull();
|
||||
});
|
||||
|
||||
test("a port edit still owes a restart, and the shell says so", async () => {
|
||||
await openSystem();
|
||||
|
||||
const dns = screen.getByRole("group", { name: "DNS" });
|
||||
fireEvent.change(within(dns).getByLabelText(/^port/), { target: { value: "5353" } });
|
||||
fireEvent.click(saveButton());
|
||||
|
||||
await waitFor(() => expect(putBodies).toHaveLength(1));
|
||||
await screen.findByText(/Saved changes are not running yet/);
|
||||
});
|
||||
|
||||
test("an enum-backed key on the list is marked too, not only text and number fields", async () => {
|
||||
// No shipped restart-required key is enum-backed, but the list is the
|
||||
// server's to change, so the `Select` rendering is pinned against one.
|
||||
await openSystem(["logging.level"]);
|
||||
|
||||
const logging = screen.getByRole("group", { name: "Logging" });
|
||||
// `logging.level` is a Select and `logging.output` is not on the list, so
|
||||
// exactly one mark belongs to this section.
|
||||
@@ -176,7 +211,7 @@ test("password flow: note shown, confirm required, PUT sends web.password, no re
|
||||
expect(restartNotice()).toBeNull();
|
||||
});
|
||||
|
||||
test("a mixed patch makes the server owe a restart, and the shell says so", async () => {
|
||||
test("a patch of live keys alone leaves the server owing nothing", async () => {
|
||||
await openSystem();
|
||||
|
||||
const web = screen.getByRole("group", { name: "Web" });
|
||||
@@ -187,7 +222,8 @@ test("a mixed patch makes the server owe a restart, and the shell says so", asyn
|
||||
|
||||
await waitFor(() => expect(putBodies).toHaveLength(1));
|
||||
expect(putBodies[0]).toEqual({ web: { session_ttl_hours: 48, password: "hunter2" } });
|
||||
await screen.findByText(/Saved changes are not running yet/);
|
||||
await waitFor(() => expect(saveButton().disabled).toBe(true));
|
||||
expect(restartNotice()).toBeNull();
|
||||
});
|
||||
|
||||
test("the form is disabled while the PUT is pending and re-enabled after success", async () => {
|
||||
|
||||
@@ -13,7 +13,7 @@ import QueryPanel from "./QueryPanel";
|
||||
import UpstreamForm from "./UpstreamForm";
|
||||
import { styles as config } from "./styles";
|
||||
|
||||
const INTRO = "The pool builds its clients at startup, so an edit here takes effect at the next restart.";
|
||||
const INTRO = "The pool is rebuilt as you save, so an edit here applies to the next query.";
|
||||
|
||||
const styles = stylex.create({
|
||||
url: {
|
||||
|
||||
@@ -14,6 +14,7 @@ import { AuthProvider } from "@/auth/store";
|
||||
import { health } from "@/lib/healthFixture";
|
||||
import { createQueryClient } from "@/lib/queryClient";
|
||||
import { createAppRouter } from "@/routes";
|
||||
import { sample_get_settings } from "@/lib/contractSamples.gen";
|
||||
import type { ConfigStatus, Settings } from "@/lib/types";
|
||||
|
||||
export const CONFIG_PATH = "/etc/nxdns/config.zon";
|
||||
@@ -33,6 +34,13 @@ export const MANAGED_FILE: ConfigStatus = {
|
||||
restart_pending: false,
|
||||
};
|
||||
|
||||
/**
|
||||
* The keys `/api/settings` still reports as restart-required, taken from the
|
||||
* committed contract sample so a server-side change to the set fails the tests
|
||||
* that pin it rather than passing against a stale copy.
|
||||
*/
|
||||
export const RESTART_REQUIRED_KEYS: readonly string[] = sample_get_settings.restart_required;
|
||||
|
||||
export function baseSettings(): Settings {
|
||||
return {
|
||||
upstream: { attempt_timeout_ms: 2500, read_timeout_ms: 3000, total_timeout_ms: 5000 },
|
||||
@@ -197,7 +205,7 @@ function defaultResponses(status: ConfigStatus): Record<string, unknown> {
|
||||
"GET /api/upstreams": { upstreams: UPSTREAMS },
|
||||
"GET /api/local-records": { local_records: LOCAL_RECORDS },
|
||||
"GET /api/forward-zones": { forward_zones: FORWARD_ZONES },
|
||||
"GET /api/settings": { settings: baseSettings(), restart_required: ["dns.port", "web.port"] },
|
||||
"GET /api/settings": { settings: baseSettings(), restart_required: RESTART_REQUIRED_KEYS },
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -441,7 +441,7 @@ export const sample_create_upstream: UpstreamEcho = {
|
||||
enabled: true,
|
||||
id: 0,
|
||||
priority: 0,
|
||||
restart_required: true,
|
||||
restart_required: false,
|
||||
tls_name: "",
|
||||
url: "https://dns2.example/dns-query",
|
||||
};
|
||||
@@ -458,7 +458,7 @@ export const sample_update_upstream: UpstreamEcho = {
|
||||
enabled: true,
|
||||
id: 0,
|
||||
priority: 0,
|
||||
restart_required: true,
|
||||
restart_required: false,
|
||||
tls_name: "",
|
||||
url: "https://dns.example/dns-query",
|
||||
};
|
||||
@@ -633,51 +633,18 @@ export const sample_post_pause: PauseState = {
|
||||
|
||||
export const sample_get_settings: SettingsEnvelope = {
|
||||
restart_required: [
|
||||
"upstream.attempt_timeout_ms",
|
||||
"upstream.read_timeout_ms",
|
||||
"upstream.total_timeout_ms",
|
||||
"dns.bind_ipv4",
|
||||
"dns.bind_ipv6",
|
||||
"dns.port",
|
||||
"dns.rate_limit",
|
||||
"dns.rate_window_seconds",
|
||||
"blocking.response",
|
||||
"blocking.ttl",
|
||||
"cache.size",
|
||||
"cache.negative_ttl_max",
|
||||
"web.enabled",
|
||||
"web.bind",
|
||||
"web.port",
|
||||
"web.session_ttl_hours",
|
||||
"web.api_rate_limit_per_min",
|
||||
"web.api_localhost_exempt",
|
||||
"web.sse_max_connections_per_ip",
|
||||
"web.trusted_proxies",
|
||||
"doh_server.enabled",
|
||||
"doh_server.bind",
|
||||
"doh_server.port",
|
||||
"doh_server.cert_path",
|
||||
"doh_server.key_path",
|
||||
"dot_server.enabled",
|
||||
"dot_server.bind",
|
||||
"dot_server.port",
|
||||
"dot_server.cert_path",
|
||||
"dot_server.key_path",
|
||||
"edns.ecs_mode",
|
||||
"logging.level",
|
||||
"logging.retention_days",
|
||||
"logging.query_log_buffer_max",
|
||||
"logging.query_log_flush_interval_s",
|
||||
"logging.hide_domains",
|
||||
"logging.hide_client_ips",
|
||||
"logging.output",
|
||||
"logging.file_path",
|
||||
"logging.max_size_mb",
|
||||
"logging.max_files",
|
||||
"disk.min_free_mb",
|
||||
"disk.warn_free_mb",
|
||||
"blocklist_update.enabled",
|
||||
"blocklist_update.interval_hours",
|
||||
],
|
||||
settings: {
|
||||
blocking: {
|
||||
@@ -753,51 +720,18 @@ export const sample_get_settings: SettingsEnvelope = {
|
||||
|
||||
export const sample_put_settings: SettingsEnvelope = {
|
||||
restart_required: [
|
||||
"upstream.attempt_timeout_ms",
|
||||
"upstream.read_timeout_ms",
|
||||
"upstream.total_timeout_ms",
|
||||
"dns.bind_ipv4",
|
||||
"dns.bind_ipv6",
|
||||
"dns.port",
|
||||
"dns.rate_limit",
|
||||
"dns.rate_window_seconds",
|
||||
"blocking.response",
|
||||
"blocking.ttl",
|
||||
"cache.size",
|
||||
"cache.negative_ttl_max",
|
||||
"web.enabled",
|
||||
"web.bind",
|
||||
"web.port",
|
||||
"web.session_ttl_hours",
|
||||
"web.api_rate_limit_per_min",
|
||||
"web.api_localhost_exempt",
|
||||
"web.sse_max_connections_per_ip",
|
||||
"web.trusted_proxies",
|
||||
"doh_server.enabled",
|
||||
"doh_server.bind",
|
||||
"doh_server.port",
|
||||
"doh_server.cert_path",
|
||||
"doh_server.key_path",
|
||||
"dot_server.enabled",
|
||||
"dot_server.bind",
|
||||
"dot_server.port",
|
||||
"dot_server.cert_path",
|
||||
"dot_server.key_path",
|
||||
"edns.ecs_mode",
|
||||
"logging.level",
|
||||
"logging.retention_days",
|
||||
"logging.query_log_buffer_max",
|
||||
"logging.query_log_flush_interval_s",
|
||||
"logging.hide_domains",
|
||||
"logging.hide_client_ips",
|
||||
"logging.output",
|
||||
"logging.file_path",
|
||||
"logging.max_size_mb",
|
||||
"logging.max_files",
|
||||
"disk.min_free_mb",
|
||||
"disk.warn_free_mb",
|
||||
"blocklist_update.enabled",
|
||||
"blocklist_update.interval_hours",
|
||||
],
|
||||
settings: {
|
||||
blocking: {
|
||||
|
||||
@@ -328,14 +328,11 @@ export const clientPrefixesPutMutation = (qc: QueryClient) => ({
|
||||
},
|
||||
});
|
||||
|
||||
// The pool builds its clients at startup, so every upstream write leaves the
|
||||
// server owing a restart. The flag it sets lives on /api/config/status, and the
|
||||
// shell notice reads it there — hence the second invalidation.
|
||||
// An upstream write rebuilds the pool in the running process, so the row list
|
||||
// is the only thing it changes: no restart is owed and /api/config/status says
|
||||
// the same thing after the write as before it.
|
||||
function invalidateUpstreams(qc: QueryClient): Promise<unknown> {
|
||||
return Promise.all([
|
||||
qc.invalidateQueries({ queryKey: queryKeys.upstreams }),
|
||||
qc.invalidateQueries({ queryKey: queryKeys.configStatus }),
|
||||
]);
|
||||
return qc.invalidateQueries({ queryKey: queryKeys.upstreams });
|
||||
}
|
||||
|
||||
export const upstreamCreateMutation = (qc: QueryClient) => ({
|
||||
|
||||
@@ -550,7 +550,7 @@ export interface UpstreamEcho {
|
||||
priority: number;
|
||||
enabled: boolean;
|
||||
tls_name: string;
|
||||
restart_required: true;
|
||||
restart_required: false;
|
||||
}
|
||||
|
||||
export interface PauseState {
|
||||
@@ -646,8 +646,9 @@ export interface SettingsEnvelope {
|
||||
* authenticated route, never one of the open ones.
|
||||
*
|
||||
* `restart_pending` is true once the server has committed a change only a
|
||||
* restart applies (an upstream write, a settings key). Nothing but process
|
||||
* exit clears it, so a browser reload cannot dismiss it.
|
||||
* restart applies. Every settings key applies live except the listener binds
|
||||
* and `web.enabled`, so those are the only writes that raise it. Nothing but
|
||||
* process exit clears it, so a browser reload cannot dismiss it.
|
||||
*/
|
||||
export interface ConfigStatus {
|
||||
authority: "database" | "managed_file";
|
||||
|
||||
Reference in New Issue
Block a user