Replies: 2 comments 1 reply
How Position B came about — and a lean toward revertingFor transparency on the history, since it bears directly on the decision: Position B (preserving hand-added peers) was not a deliberate choice to move away from managed-only. It came out of our automated GUI-testing tooling. An adversarial save-fuzzing check saw that saving the WireGuard page issued a wholesale delete of the The catch: that deletion isn't a bug — it's the intended managed-only behaviour (Position A). The plugin owns the config and regenerates the whole peer set on save. Our tooling can't distinguish "intended managed-only deletion" from "accidental data loss" — it only sees a wholesale delete and flags it. We took the flag at face value instead of asking whether the deletion was by design. Reading it back with that context, Position B looks like a fix to a false positive. It's real reconciliation code and ongoing maintenance for a fairly narrow benefit, and it introduces the two-sources-of-truth / invisible-state problems listed above — which is precisely the anti-pattern raised in review. So we're leaning toward reverting to Position A (managed-only), and only pursuing Position C (expanding what the UI can natively express) if a concrete use case turns up that the GUI genuinely can't represent. The revert is cheap — PR #57 was purely additive — so this isn't costly to undo. Still genuinely interested in the Position C question, which is the one that actually matters going forward: which WireGuard configurations are people hand-editing today because the current UI can't represent them? |
I don't think this is correct.
If you want |
Uh oh!
There was an error while loading. Please reload this page.
Design question: how should the WireGuard plugin treat manually-added peers?
This is an open design question, not a decision that's been made. It came out of review feedback from Lantis, and both positions have real merit. Laying out the trade-offs neutrally so it can be discussed on the substance — input welcome, especially real-world use cases.
Background
The WireGuard plugin manages peers through its own UI (the
allowed_clientmodel — inbound/"server mode" clients that connect to the router). A user can also add peers by hand, directly in/etc/config/network(config wireguard_wg0sections) — for example a site-to-site tunnel or an outbound peer set up by following a VPN provider's guide.The question is what a GUI Save should do with those hand-added peers.
Two behaviours are on the table (a third option may reconcile them — see below).
Position A — Managed-only: the plugin owns the config; unmanaged peers are deleted on save
This is the maintainer's position, and how Gargoyle's plugins generally work.
In favour
Against
Position B — Preserve: a save keeps hand-added peers it didn't create
This is the behaviour currently shipped in the fork (peers the plugin didn't create are snapshotted and restored across a save).
In favour
Against
Position C — Close the gap: expand what the plugin can natively express
Lantis's actual suggestion: "Is Gargoyle not able to import/read/work with some configurations you are trying to do? That should be looked at and expanded on."
The plugin's current model is inbound/server-mode only — it has no field for
endpoint,persistent_keepalive, orpreshared_key, and no site-to-site (subnetAllowedIPs) peer type. Those are exactly the things a hand-added peer usually needs. If the UI could express them, there would be little reason to hand-edit — and managed-only (Position A) becomes safe, because everything is representable in the GUI.In favour
Against / open questions
What would help decide
The most useful input is concrete: which WireGuard configurations are people setting up by hand today that the GUI can't currently express? (Outbound peer with an endpoint? Site-to-site with subnet routes? A pre-shared key? Multiple peers on one interface?) Knowing the real use cases pins down whether Position C is worth building and what it needs to cover.
Thoughts welcome — particularly from anyone running WireGuard configurations the current UI doesn't handle.
All reactions