Edit configuration and restart service from the web UI

danny commented

2026-07-06 09:28:27 +01:00

(Migrated from gitlab.keyop.co.uk)

Split out from #22 (view configuration from the web UI), per the "ideally... but that may need
to be split out to a separate ticket" note on that ticket's description.

Scope

Once #22 (view-only) lands, extend the web UI to also let an authenticated user edit the
running configuration and restart the service to apply it.

Why this is a separate, bigger piece of work than "view"

No live-reload path exists today. nfq_forwarder.service's ExecReload already does a
full process restart, not an in-process config reload - there's no mechanism today for the
running process to pick up a changed config file without actually restarting.
"Restart" means a real process restart, most likely by shelling out to systemctl --user restart nfq_forwarder (matching the project's --user-service design - see CLAUDE.md) from
an authenticated HTTP request. That's a meaningfully different, more sensitive capability than
rendering a page: the web UI process holds CAP_DAC_READ_SEARCH/CAP_NET_ADMIN/CAP_NET_RAW,
and "let an authenticated web session trigger a service restart" deserves its own design pass
(how the restart is invoked, what happens to in-flight tunnels/connections during the restart,
error handling if the new config is invalid, etc.) rather than being bolted onto the view-only
ticket.
Editing needs validation before writing. Whatever's written back needs to go through the
same validation config.LoadFromFile already does (duplicate queue numbers, bad CIDRs, missing
jump_user/jump_host, etc.) before it's saved and before a restart is triggered - a bad edit
should never be allowed to leave the service unable to start back up.

Suggested approach (for whoever picks this up)

A form-based edit view, submitting to a new CSRF-protected POST /api/config endpoint (the
existing requireSessionAPI + requireCSRF pattern already used by /api/tunnels/close).
Validate the submitted config the same way config.LoadFromFile does before writing anything
to disk.
A separate, explicit "Restart service" action (not bundled into every edit save) that shells
out to systemctl --user restart nfq_forwarder, with clear success/failure feedback in the UI.

Split out from #22 (view configuration from the web UI), per the "ideally... but that may need to be split out to a separate ticket" note on that ticket's description. ## Scope Once #22 (view-only) lands, extend the web UI to also let an authenticated user **edit** the running configuration and **restart the service** to apply it. ## Why this is a separate, bigger piece of work than "view" - **No live-reload path exists today.** `nfq_forwarder.service`'s `ExecReload` already does a full process restart, not an in-process config reload - there's no mechanism today for the running process to pick up a changed config file without actually restarting. - **"Restart" means a real process restart**, most likely by shelling out to `systemctl --user restart nfq_forwarder` (matching the project's `--user`-service design - see `CLAUDE.md`) from an authenticated HTTP request. That's a meaningfully different, more sensitive capability than rendering a page: the web UI process holds `CAP_DAC_READ_SEARCH`/`CAP_NET_ADMIN`/`CAP_NET_RAW`, and "let an authenticated web session trigger a service restart" deserves its own design pass (how the restart is invoked, what happens to in-flight tunnels/connections during the restart, error handling if the new config is invalid, etc.) rather than being bolted onto the view-only ticket. - **Editing needs validation before writing.** Whatever's written back needs to go through the same validation `config.LoadFromFile` already does (duplicate queue numbers, bad CIDRs, missing jump_user/jump_host, etc.) *before* it's saved and *before* a restart is triggered - a bad edit should never be allowed to leave the service unable to start back up. ## Suggested approach (for whoever picks this up) - A form-based edit view, submitting to a new CSRF-protected `POST /api/config` endpoint (the existing `requireSessionAPI` + `requireCSRF` pattern already used by `/api/tunnels/close`). - Validate the submitted config the same way `config.LoadFromFile` does before writing anything to disk. - A separate, explicit "Restart service" action (not bundled into every edit save) that shells out to `systemctl --user restart nfq_forwarder`, with clear success/failure feedback in the UI.

danny commented

2026-07-06 09:28:49 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in issue #22

danny commented

2026-07-06 09:43:46 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit 5604aef9e6

mentioned in commit 5604aef9e613be258c6ab05e447ff00c4e18f398

danny commented

2026-07-06 09:44:11 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in merge request !68

danny commented

2026-07-07 11:24:16 +01:00

(Migrated from gitlab.keyop.co.uk)

Plan

Danny confirmed both editing modes are wanted (raw YAML textarea + structured per-field form,
switchable via a toggle), not one or the other.

Key findings shaping the design

The config file's path is currently discarded after loading - LoadFromFile never stores
it anywhere. Adding a ConfigPath field (not YAML-tagged - set by Go code, not read from the
file) to Configuration, populated by LoadFromFile itself, is the natural fix - threaded
through the same appConfig that already flows into the web UI (see #22).
The "no --config file" case doesn't need handling: web_ui.enabled can only be set via
YAML, so if the web UI is running at all, a config file is guaranteed to exist.
"Restart" already has an exact precedent: docs/nfq_forwarder.service's ExecReload is
literally systemctl --user restart nfq_forwarder.service - "reload = full restart" is already
this project's deliberate design (every connection already tears down its own iptables rules on
the way out via the existing SIGTERM path, so a restart converges cleanly regardless of what
changed). No new shutdown/signal-handling logic is needed - SIGTERM already goes through the
same clean teardown Ctrl-C uses.
Self-restart subtlety: triggering systemctl --user restart on the service that's doing
the triggering needs --no-block, so the call queues the job and returns almost instantly
(systemd's manager - a separate long-running daemon - has already accepted the job before this
process gets torn down moments later), letting the handler return a clean HTTP response first.

Design

internal/config: extract LoadFromFile's pipeline into LoadFromBytes(data []byte) (*Configuration, error) (parse → apply defaults → assign queue numbers → validate - the same
path startup already uses, now reusable for validating an edit before it's saved), plus a
ParseRawBytes that just unmarshals YAML with no resolution - used to show the edit form the
true, unresolved on-disk content (not the already-defaults-merged view issue #22's read-only
page shows, which would be misleading to edit directly).

New endpoints (internal/nfq_forwarder):

GET /api/config/raw - the current file's raw YAML text and its structured (unresolved)
JSON form, for populating either edit mode.
POST /api/config/validate - accepts either {"yaml": "..."} or {"config": {...}}
(whichever mode is active), validates via LoadFromBytes, and always echoes back both
representations (converting one into the other server-side) - this single endpoint is what
makes the mode toggle work without needing a YAML parser in the browser (this project has no
JS dependencies/build step, so that's not on the table).
POST /api/config/save - same validation, then an atomic write (temp file + rename) to
ConfigPath of exactly the submitted (validated) YAML - never the resolved/expanded form,
since writing back auto-assigned queue numbers or merged-in defaults would defeat the point of
those features. A bad edit is never written to disk.
POST /api/config/restart - systemctl --user restart --no-block nfq_forwarder.service.

All state-changing routes get the existing requireSessionAPI + requireCSRF treatment already
used by /api/tunnels/close. Saving does not hot-swap the running process's in-memory
config - matches this project's explicit no-live-reload design; the UI will clearly say a restart
is needed to apply a save.

Testing: extend internal/config/load_test.go for LoadFromBytes/ParseRawBytes/
ConfigPath. New handler tests following the established pattern (authenticated/CSRF/validation-
rejects-without-writing). The restart handler will use a package-level function-variable
indirection (matching sshDialFunc/addDNATRulesFunc elsewhere in this codebase) so tests can
confirm it invokes the right command without a real systemd session in the sandbox.

Work happening on branch issue25_webui_config_edit; an MR will follow referencing this issue.

## Plan Danny confirmed both editing modes are wanted (raw YAML textarea + structured per-field form, switchable via a toggle), not one or the other. ### Key findings shaping the design - **The config file's path is currently discarded after loading** - `LoadFromFile` never stores it anywhere. Adding a `ConfigPath` field (not YAML-tagged - set by Go code, not read from the file) to `Configuration`, populated by `LoadFromFile` itself, is the natural fix - threaded through the same `appConfig` that already flows into the web UI (see #22). - **The "no `--config` file" case doesn't need handling**: `web_ui.enabled` can only be set via YAML, so if the web UI is running at all, a config file is guaranteed to exist. - **"Restart" already has an exact precedent**: `docs/nfq_forwarder.service`'s `ExecReload` is literally `systemctl --user restart nfq_forwarder.service` - "reload = full restart" is already this project's deliberate design (every connection already tears down its own iptables rules on the way out via the existing SIGTERM path, so a restart converges cleanly regardless of what changed). No new shutdown/signal-handling logic is needed - SIGTERM already goes through the same clean teardown Ctrl-C uses. - **Self-restart subtlety**: triggering `systemctl --user restart` on the service that's *doing the triggering* needs `--no-block`, so the call queues the job and returns almost instantly (systemd's manager - a separate long-running daemon - has already accepted the job before this process gets torn down moments later), letting the handler return a clean HTTP response first. ### Design **`internal/config`**: extract `LoadFromFile`'s pipeline into `LoadFromBytes(data []byte) (*Configuration, error)` (parse → apply defaults → assign queue numbers → validate - the same path startup already uses, now reusable for validating an *edit* before it's saved), plus a `ParseRawBytes` that just unmarshals YAML with no resolution - used to show the edit form the *true, unresolved* on-disk content (not the already-defaults-merged view issue #22's read-only page shows, which would be misleading to edit directly). **New endpoints** (`internal/nfq_forwarder`): - `GET /api/config/raw` - the current file's raw YAML text *and* its structured (unresolved) JSON form, for populating either edit mode. - `POST /api/config/validate` - accepts either `{"yaml": "..."}` or `{"config": {...}}` (whichever mode is active), validates via `LoadFromBytes`, and always echoes back *both* representations (converting one into the other server-side) - this single endpoint is what makes the mode toggle work without needing a YAML parser in the browser (this project has no JS dependencies/build step, so that's not on the table). - `POST /api/config/save` - same validation, then an atomic write (temp file + rename) to `ConfigPath` of exactly the submitted (validated) YAML - never the resolved/expanded form, since writing back auto-assigned queue numbers or merged-in defaults would defeat the point of those features. A bad edit is never written to disk. - `POST /api/config/restart` - `systemctl --user restart --no-block nfq_forwarder.service`. All state-changing routes get the existing `requireSessionAPI` + `requireCSRF` treatment already used by `/api/tunnels/close`. Saving does **not** hot-swap the running process's in-memory config - matches this project's explicit no-live-reload design; the UI will clearly say a restart is needed to apply a save. **Testing**: extend `internal/config/load_test.go` for `LoadFromBytes`/`ParseRawBytes`/ `ConfigPath`. New handler tests following the established pattern (authenticated/CSRF/validation- rejects-without-writing). The restart handler will use a package-level function-variable indirection (matching `sshDialFunc`/`addDNATRulesFunc` elsewhere in this codebase) so tests can confirm it invokes the right command without a real systemd session in the sandbox. Work happening on branch `issue25_webui_config_edit`; an MR will follow referencing this issue.

danny commented

2026-07-07 11:43:06 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit 6ecbdd9b1c

mentioned in commit 6ecbdd9b1ca8549c0a284bb73f29d22856929f07

danny commented

2026-07-07 11:43:42 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in merge request !69

danny commented

2026-07-07 11:43:57 +01:00

(Migrated from gitlab.keyop.co.uk)

Implementation is up for review: !69 (branch issue25_webui_config_edit). Both editing modes (raw YAML textarea + structured per-field form, toggle between them) are implemented, along with a separate explicit restart action. Waiting on review before merging - see the MR description for the full breakdown.

Implementation is up for review: !69 (branch `issue25_webui_config_edit`). Both editing modes (raw YAML textarea + structured per-field form, toggle between them) are implemented, along with a separate explicit restart action. Waiting on review before merging - see the MR description for the full breakdown.

danny commented

2026-07-07 13:13:04 +01:00

(Migrated from gitlab.keyop.co.uk)

UI refinements added after initial review feedback:

Structured form is now the default edit mode (raw YAML is the opt-in toggle, moved to the right); the connections table became a compact one-row-per-connection grid instead of full labeled fields per row, with an inline Save button on each row (disabled until that row's own fields are modified) so editing several connections doesn't require scrolling to the bottom action bar.
Fixed a header/column misalignment in the compact tables (header and data rows were computing max-content action-button column widths independently, since they were separate CSS grids - fixed by sharing one grid across header + rows via display: contents). Applied the same compact one-row-per-rule treatment to the defaults.by_network table. Made the Global settings / Web UI / Defaults field cards more compact too (tighter spacing, smaller inputs).
Replaced the connection row up/down buttons with drag-and-drop reordering (native HTML5 drag-and-drop, no library) - drag the handle to reorder; the saved YAML preserves whatever order the rows end up in.

All changes are on the same branch/MR (!69), lint and the full test suite stay clean throughout. Currently awaiting review.

**UI refinements added after initial review feedback:** 1. Structured form is now the default edit mode (raw YAML is the opt-in toggle, moved to the right); the connections table became a compact one-row-per-connection grid instead of full labeled fields per row, with an inline **Save** button on each row (disabled until that row's own fields are modified) so editing several connections doesn't require scrolling to the bottom action bar. 2. Fixed a header/column misalignment in the compact tables (header and data rows were computing `max-content` action-button column widths independently, since they were separate CSS grids - fixed by sharing one grid across header + rows via `display: contents`). Applied the same compact one-row-per-rule treatment to the `defaults.by_network` table. Made the Global settings / Web UI / Defaults field cards more compact too (tighter spacing, smaller inputs). 3. Replaced the connection row up/down buttons with drag-and-drop reordering (native HTML5 drag-and-drop, no library) - drag the handle to reorder; the saved YAML preserves whatever order the rows end up in. All changes are on the same branch/MR (!69), lint and the full test suite stay clean throughout. Currently awaiting review.

danny commented

2026-07-07 13:50:07 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit 551f561100

mentioned in commit 551f5611002c2bb2523ff601c1064d2bee389298

danny (Migrated from gitlab.keyop.co.uk) closed this issue

2026-07-07 13:50:08 +01:00

danny commented

2026-07-07 13:51:49 +01:00

(Migrated from gitlab.keyop.co.uk)

Merged via !69 (merge commit 551f561) and released as v1.1.0 (a minor version bump rather than a patch, per Danny's call on the MR, since this adds a whole new capability rather than fixing a bug).

Merged via !69 (merge commit `551f561`) and released as **v1.1.0** (a minor version bump rather than a patch, per Danny's call on the MR, since this adds a whole new capability rather than fixing a bug).

Rows
Columns

Edit configuration and restart service from the web UI #25

Scope

Why this is a separate, bigger piece of work than "view"

Suggested approach (for whoever picks this up)

Plan

Key findings shaping the design

Design