networking.wireguard: added allowedIpsAsRoutes boolean to control p…
#29753
Conversation
|
@andir, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ericsagnes, @aristidb and @evujumenuk to be potential reviewers. |
| @@ -148,6 +148,15 @@ let | |||
| for matching all IPv6 addresses.''; | |||
| }; | |||
|
|
|||
| allowedIPsAsRoutes = mkOption { | |||
There was a problem hiding this comment.
Seems like this would be better as a per-device option instead of a per-peer option, for simplicity. It's hard to imagine a situation in which you'd want this on for some and off for others, and in the event you want it off, that's probably for everyone, so it might be a bit of a pain to have to add the =false entry to each and every peer, especially if you have quite a few of them.
There was a problem hiding this comment.
Your comment makes sens. In my scenario I have a bunch of machines which are allowed to originate ::/0 and some that only have a /28 assigned to them. So I would prefer a per peer configuration. Peers with ::/0 are announcing routes to my machine, the ones with static networks wont. So the per-peer option makes sense for me.
I am not in favor of a "global" (per-device) option which could be flipped per-peer. That seems like a bit too much complexity for this kind of thing.
What do you think?
There was a problem hiding this comment.
You can't give an allowed ips of 0.0.0.0/0 to multiple peers on a single interface, so I'm not totally certain what you mean.
There was a problem hiding this comment.
I think he talks about routing policies, where some peers are allowed to announce all subnet sizes in ::/0 using a dynamic routing protocol and some peers only have static/precalculated routes that can be set by the module itself.
There was a problem hiding this comment.
I think @zx2c4 is right, it might be more straight forward to only enable/disable that on a per-device level. If you plan to manage some very custom setup you can probably just add some additional services or pre/post exec statements.
I adjusted my PR to do exactly that.
| @@ -148,6 +148,15 @@ let | |||
| for matching all IPv6 addresses.''; | |||
| }; | |||
|
|
|||
| allowedIPsAsRoutes = mkOption { | |||
| example = false; | |||
There was a problem hiding this comment.
Why is example=false? (I'm not sure precisely what this controls.)
There was a problem hiding this comment.
It will be displayed as part of our generated documentation: https://nixos.org/nixos/options.html#zsh.enable
In case of boolean values this is kind of obvious, which is why it is often left out.
andir
force-pushed
the
wireguard-allowed-ips-as-route-optional
branch
from
87927cf
to
223374b
Compare
…eer routes Sometimes (especially in the default route case) it is required to NOT add routes for all allowed IP ranges. One might run it's own custom routing on-top of wireguard and only use the wireguard addresses to exchange prefixes with the remote host.
andir
force-pushed
the
wireguard-allowed-ips-as-route-optional
branch
from
223374b
to
846070e
Compare
Motivation for this change
Sometimes (especially in the default route case) it is required to NOT
add routes for all allowed IP ranges. One might run it's own custom
routing on-top of wireguard and only use the wireguard addresses to
exchange prefixes with the remote host.
Open for opinions on this. Especially from @zx2c4 and @Mic92.
Things done
nixos-rebuild build-vmbuild-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)