Re: [PATCH cluster/manager/network/proxmox{,-ve-rs,-perl-rs} v4 00/31] Add WireGuard as protocol to SDN fabrics

[Stefan Hanreich <s.hanreich@proxmox.com>]

thanks for the review!

On 5/11/26 3:38 PM, Arthur Bied-Charreton wrote:

[snip]

> I did find some issues:
> 
> 1.  I deleted the external node from the WG fabric described above.
>     Applying the configuration update failed with:
> 
>     ```
>     TASK ERROR: could not find node referenced in peer definition: qdevice
>     ```
> 
>     The deletion does remove the `wireguard_node` entry from `fabrics.cfg`,
>     however the reference to it in the other nodes is not removed
> 
>     You mentioned this limitation in the cover letter, however it does
>     break the UI with an uncaught TypeError that causes the node edit
>     component to not render properly. This makes it impossible to remove
>     the dangling reference from the UI, I had to edit the config
>     manually (might have missed something though)
> 
>     Apart from that, deleting internal nodes works well, the private
>     keys are cleaned up as expected.

added the validation for external nodes as well - good catch!

> 2.  I was able to set the interface IP of a WG node to an IP that is
>     already used by another node in the same WG fabric, which makes
>     them unable to talk to each other

Hmmm, arguably this is only a problem if the interfaces are peers as
well - which would make validation a lot more convoluted. But having the
same IP twice on any interface is probably asking for potential trouble
down the road...

I'll check how exactly / restrictive I'll implement this.

> 3.  It looks like the WG config directory is never created. When setting
>     up WG for the first time, I got the following error:
> 
>     ```
>     TASK ERROR: unable to open file '/etc/wireguard/proxmox/wg0.conf.tmp.22477' - No such file or directory
>     ```
> 
>     Creating `/etc/wireguard/proxmox` fixes it.

seems like the auto-creation code went missing when I refactored the key
generation :/ - re-added it!

> 4.  Applying the config without `wireguard-tools` installed on one or
>     more node(s) still succeeds, it only logs that the `wg syncconf`
>     post-up command failed. Judging by the explicit warn statement
>     it looks like it's intentional but it took me a little bit to 
>     figure out why my config was not working, might make sense to
>     promote this to an error log?

made this a proper warning!