milestone 32: task-shaped configuration, file mode as a rendering, config status api

src/web/handlers/config.zig

@@ -0,0 +1,88 @@
+//! `GET /api/config/status` — which source governs this process's
+//! configuration, and whether a change already written needs a restart.
+//!
+//! One home for both facts. `authority`, `path` and `reconciled_at` are the
+//! invocation's answer (milestone-20 ruling 1): the database carries no record
+//! of who wrote it, so only the running process can say. `restart_pending` is
+//! the running process's own memory of a mutation whose effect waits for the
+//! next start — the upstream pool and the scalar settings are both built at
+//! startup. Nothing clears it but process exit, so a client that sees it false
+//! after a restart is seeing the truth rather than a cleared flag.
+//!
+//! The route needs a session: `path` is a filesystem path and must never reach
+//! the open `/api/version` or `/api/health`.
+
+const std = @import("std");
+
+const http_util = @import("../http_util.zig");
+const server = @import("../server.zig");
+
+const Request = http_util.Request;
+const HandlerError = http_util.HandlerError;
+
+pub const View = struct {
+    authority: []const u8,
+    /// The managed file, or null in database mode.
+    path: ?[]const u8,
+    /// When this process loaded the managed file, in epoch seconds; null in
+    /// database mode, which never reconciles. It means exactly that: a file
+    /// whose mtime is newer has not been loaded by the running process.
+    reconciled_at: ?i64,
+    restart_pending: bool,
+};
+
+pub fn view(state: *const server.WebState) View {
+    return switch (state.authority) {
+        .database => .{
+            .authority = "database",
+            .path = null,
+            .reconciled_at = state.reconciled_at,
+            .restart_pending = state.restart_pending.load(.monotonic),
+        },
+        .managed_file => |path| .{
+            .authority = "managed_file",
+            .path = path,
+            .reconciled_at = state.reconciled_at,
+            .restart_pending = state.restart_pending.load(.monotonic),
+        },
+    };
+}
+
+pub fn get(state: *server.WebState, io: std.Io, request: *Request) HandlerError!void {
+    _ = io;
+    return http_util.respondJson(request, .ok, view(state), &.{});
+}
+
+// ---------------------------------------------------------------------------
+// tests
+// ---------------------------------------------------------------------------
+
+const testing = std.testing;
+
+test "database authority reports no file and no reconcile time" {
+    var state: server.WebState = .{ .gpa = testing.allocator };
+    const current = view(&state);
+    try testing.expectEqualStrings("database", current.authority);
+    try testing.expectEqual(@as(?[]const u8, null), current.path);
+    try testing.expectEqual(@as(?i64, null), current.reconciled_at);
+    try testing.expect(!current.restart_pending);
+}
+
+test "file authority reports the path and the load time" {
+    var state: server.WebState = .{
+        .gpa = testing.allocator,
+        .authority = .{ .managed_file = "/etc/nxdns/config.zon" },
+        .reconciled_at = 1_700_000_042,
+    };
+    const current = view(&state);
+    try testing.expectEqualStrings("managed_file", current.authority);
+    try testing.expectEqualStrings("/etc/nxdns/config.zon", current.path.?);
+    try testing.expectEqual(@as(?i64, 1_700_000_042), current.reconciled_at);
+}
+
+test "the flag the mutations raise is what the view reports" {
+    var state: server.WebState = .{ .gpa = testing.allocator };
+    try testing.expect(!view(&state).restart_pending);
+    state.restart_pending.store(true, .monotonic);
+    try testing.expect(view(&state).restart_pending);
+}

src/web/handlers/settings.zig

@@ -184,6 +184,32 @@ fn merge(cfg: *model.Config, patch: Patch, bad_key: *[]const u8) bool {
     return true;
 }
 
+/// Whether the patch names a key whose change waits for a restart. The key
+/// table above is the authority, so a key that stops being restart-required
+/// stops raising the flag without anyone remembering this function exists.
+/// A patch carrying `web.password` alone touches no such key: the new hash is
+/// installed live (ruling 17 of milestone 16).
+fn touchesRestartRequiredKey(patch: Patch) bool {
+    inline for (@typeInfo(Patch).@"struct".fields) |section_field| {
+        if (@field(patch, section_field.name)) |section| {
+            inline for (@typeInfo(@TypeOf(section)).@"struct".fields) |field| {
+                if (@field(section, field.name) != null) {
+                    if (comptime isRestartRequired(section_field.name ++ "." ++ field.name)) return true;
+                }
+            }
+        }
+    }
+    return false;
+}
+
+fn isRestartRequired(comptime key: []const u8) bool {
+    @setEvalBranchQuota(10_000);
+    for (keys) |listed| {
+        if (std.mem.eql(u8, listed, key)) return true;
+    }
+    return false;
+}
+
 /// Whether the patch carries a new password.
 fn newPassword(patch: Patch) ?[]const u8 {
     const web = patch.web orelse return null;
@@ -346,6 +372,10 @@ pub fn applyPut(
         return .{ .fail = .{ .internal = err } };
     };
 
+    // After the commit, never before: a validation failure or a write that
+    // rolled back changed nothing, so it owes nobody a restart.
+    if (touchesRestartRequiredKey(patch)) state.restart_pending.store(true, .monotonic);
+
     if (replacement) |hash| {
         // Ruling 17, both halves: the running server must verify against the
         // new hash at once — a restart-free credential change — and the
@@ -470,36 +500,6 @@ pub const hash_stall_control = if (builtin.is_test) struct {
 // routes
 // ---------------------------------------------------------------------------
 
-/// Which source governs this process's configuration, and when it last read
-/// it (milestone-20 ruling 7). This is how the UI learns that configuration is
-/// read-only — declaratively, rather than by probing a route for a 403.
-///
-/// It rides `GET /api/settings` because that route needs a session: the
-/// managed path is a filesystem path and must never reach the open
-/// `/api/version` or `/api/health`.
-///
-/// `reconciled_at` means exactly "this process loaded the file at T". A file
-/// whose mtime is newer has not been loaded by the running process. It cannot
-/// answer "is the file what the server uses" — a stepped clock or a preserved
-/// mtime defeats the comparison in either direction, and the database can move
-/// under `nxdns import` without either timestamp moving.
-const AuthorityView = struct {
-    mode: []const u8,
-    path: ?[]const u8,
-    reconciled_at: ?i64,
-};
-
-fn authorityView(state: *const server.WebState) AuthorityView {
-    return switch (state.authority) {
-        .database => .{ .mode = "database", .path = null, .reconciled_at = state.reconciled_at },
-        .managed_file => |path| .{
-            .mode = "managed_file",
-            .path = path,
-            .reconciled_at = state.reconciled_at,
-        },
-    };
-}
-
 pub fn get(state: *server.WebState, io: std.Io, request: *Request) HandlerError!void {
     const database = mutations.requireConfigDb(state) catch
         return mutations.respondFailure(request, mutations.no_config_db, "reading the settings");
@@ -515,7 +515,7 @@ pub fn get(state: *server.WebState, io: std.Io, request: *Request) HandlerError!
     const cfg = loaded catch |err|
         return mutations.respondFailure(request, .{ .internal = err }, "reading the settings");
 
-    return respondSettings(request, state, .ok, cfg);
+    return respondSettings(request, .ok, cfg);
 }
 
 pub fn put(state: *server.WebState, io: std.Io, request: *Request) HandlerError!void {
@@ -524,20 +524,18 @@ pub fn put(state: *server.WebState, io: std.Io, request: *Request) HandlerError!
 
     return switch (try applyPut(state, io, request.arena, parsed.value)) {
         .fail => |failure| mutations.respondFailure(request, failure, "writing the settings"),
-        .config => |cfg| respondSettings(request, state, .ok, cfg),
+        .config => |cfg| respondSettings(request, .ok, cfg),
     };
 }
 
-fn respondSettings(
-    request: *Request,
-    state: *const server.WebState,
-    status: std.http.Status,
-    cfg: model.Config,
-) HandlerError!void {
+/// `restart_required` is field-level metadata about this form — which keys need
+/// a restart to take effect. Whether one is *owed* right now is process state
+/// and lives on `GET /api/config/status`, which is also the one home of the
+/// configuration authority: two copies of the same fact drift.
+fn respondSettings(request: *Request, status: std.http.Status, cfg: model.Config) HandlerError!void {
     return http_util.respondJson(request, status, .{
         .settings = view(cfg),
         .restart_required = restart_required_keys,
-        .authority = authorityView(state),
     }, &.{});
 }
 
@@ -769,6 +767,76 @@ test "an empty password is not a password change" {
     try testing.expectEqualStrings("", outcome.config.web.password_hash orelse "");
 }
 
+test "a write that fails at the SQL layer owes no restart" {
+    var bench: mutations.Bench = undefined;
+    try bench.init(testing.allocator);
+    defer bench.deinit(testing.allocator);
+    try seeded(&bench);
+
+    var patch: Patch = .{};
+    patch.dns = .{ .port = 5353 };
+
+    // The seam fails `writeSettings` after its transaction has begun, which is
+    // the ordering this pins: the flag is stored only once the commit lands, so
+    // a rolled-back write leaves the operator nothing to restart for.
+    write_fault.armed = true;
+    const outcome = try applyPut(&bench.state, bench.io(), bench.arena(), patch);
+    try testing.expect(outcome.fail == .internal);
+    try testing.expect(!bench.state.restart_pending.load(.monotonic));
+
+    // The same patch, written for real, does raise it.
+    try testing.expect(try applyPut(&bench.state, bench.io(), bench.arena(), patch) == .config);
+    try testing.expect(bench.state.restart_pending.load(.monotonic));
+}
+
+test "a rejected patch raises no restart flag" {
+    var bench: mutations.Bench = undefined;
+    try bench.init(testing.allocator);
+    defer bench.deinit(testing.allocator);
+    try seeded(&bench);
+
+    var invalid: Patch = .{};
+    invalid.dns = .{ .port = 0 };
+    try testing.expect((try applyPut(&bench.state, bench.io(), bench.arena(), invalid)).fail == .invalid);
+    try testing.expect(!bench.state.restart_pending.load(.monotonic));
+
+    var unknown_enum: Patch = .{};
+    unknown_enum.logging = .{ .level = "verbose" };
+    try testing.expect((try applyPut(&bench.state, bench.io(), bench.arena(), unknown_enum)).fail == .invalid);
+    try testing.expect(!bench.state.restart_pending.load(.monotonic));
+}
+
+test "only a patch naming a restart-required key raises the flag" {
+    var password_only: Patch = .{};
+    password_only.web = .{ .password = "correct horse battery staple" };
+    try testing.expect(!touchesRestartRequiredKey(password_only));
+
+    var mixed: Patch = .{};
+    mixed.web = .{ .password = "correct horse battery staple", .port = 9090 };
+    try testing.expect(touchesRestartRequiredKey(mixed));
+
+    var elsewhere: Patch = .{};
+    elsewhere.dns = .{ .port = 5353 };
+    try testing.expect(touchesRestartRequiredKey(elsewhere));
+
+    try testing.expect(!touchesRestartRequiredKey(.{}));
+}
+
+test "a password-only put leaves the restart flag alone" {
+    var bench: mutations.Bench = undefined;
+    try bench.init(testing.allocator);
+    defer bench.deinit(testing.allocator);
+    try seeded(&bench);
+
+    var sessions: auth.Sessions = .init(24);
+    bench.state.sessions = &sessions;
+
+    var patch: Patch = .{};
+    patch.web = .{ .password = "correct horse battery staple" };
+    try testing.expect(try applyPut(&bench.state, bench.io(), bench.arena(), patch) == .config);
+    try testing.expect(!bench.state.restart_pending.load(.monotonic));
+}
+
 test "reading the settings with no database is unavailable" {
     var state: server.WebState = .{ .gpa = testing.allocator };
     var arena_state: std.heap.ArenaAllocator = .init(testing.allocator);

src/web/handlers/upstreams.zig

@@ -52,6 +52,10 @@ pub fn applyCreate(
     state.config_lock.unlock(io);
 
     const id = inserted catch |err| return .{ .fail = mutations.dbFailure(err, url_conflict) };
+    // Ruling 12: the pool is built at startup, so the row now stored governs
+    // nothing until the next one. Stored after the insert, never before — a
+    // rejected url or a conflict owes no restart.
+    state.restart_pending.store(true, .monotonic);
     return .{ .id = id };
 }
 
@@ -83,6 +87,7 @@ pub fn applyUpdate(
 
     upstreams_repo.updateUpstream(database, id, item) catch |err|
         return mutations.dbFailure(err, url_conflict);
+    state.restart_pending.store(true, .monotonic);
     return null;
 }
 
@@ -106,6 +111,7 @@ pub fn applyDelete(state: *server.WebState, io: std.Io, arena: Allocator, id: i6
 
     upstreams_repo.deleteUpstream(database, id) catch |err|
         return mutations.dbFailure(err, url_conflict);
+    state.restart_pending.store(true, .monotonic);
     return null;
 }
 
@@ -245,6 +251,26 @@ test "a url the validator refuses never reaches the database" {
     try testing.expectEqual(@as(i64, 0), try bench.queryInt("SELECT count(*) FROM upstreams"));
 }
 
+test "a refused upstream write owes no restart, and an accepted one does" {
+    var bench: mutations.Bench = undefined;
+    try bench.init(testing.allocator);
+    defer bench.deinit(testing.allocator);
+
+    try testing.expect((try applyCreate(&bench.state, bench.io(), bench.arena(), .{
+        .url = "udp://1.1.1.1:53",
+    })).fail == .invalid);
+    try testing.expect(!bench.state.restart_pending.load(.monotonic));
+
+    try testing.expect((try applyUpdate(&bench.state, bench.io(), bench.arena(), 999, doh)).? == .not_found);
+    try testing.expect(!bench.state.restart_pending.load(.monotonic));
+
+    try testing.expect(applyDelete(&bench.state, bench.io(), bench.arena(), 999).? == .not_found);
+    try testing.expect(!bench.state.restart_pending.load(.monotonic));
+
+    _ = try applyCreate(&bench.state, bench.io(), bench.arena(), doh);
+    try testing.expect(bench.state.restart_pending.load(.monotonic));
+}
+
 test "a duplicate url is a conflict" {
     var bench: mutations.Bench = undefined;
     try bench.init(testing.allocator);