richm
commented
3 years ago Looks good, I had many comments, but they refer more to the way how is the design being described, rather than to the design itself. Some comments about the design itself:
* should one use nested dicts like `ipsec.` or `ike.`?
I would prefer nested dicts. In the global context you would use vpn_ipsec.parameter, and in the vpn_connections context you would use ipsec.parameter.
* should there be a `vpn_protocol` parameter in addition to `vpn_provider`? Some providers may use the same protocol, like IPSec. In this case, changing the provider and keeping the protocol for a subset of hosts should allow them to continue communicating with the rest, if the two implementations are properly interoperable. OTOH, hosts using different protocols would not be able to communicate. (Wiregard uses its own protocol, IIRC.) As currently each provider (Wireguard and libreswan) support only one protocol, you probably don't need to introduce the `vpn_protocol` variable now, as it can be inferred.
Right. I think for now we should omit a separate protocol field. I think the protocol is set via a combination of ipsec and ike parameters (@letoams please confirm)
jharuda
mprovenc
Looks good, I had many comments, but they refer more to the way how is the design being described, rather than to the design itself. Some comments about the design itself:
ipsec.orike.?vpn_protocolparameter in addition tovpn_provider? Some providers may use the same protocol, like IPSec. In this case, changing the provider and keeping the protocol for a subset of hosts should allow them to continue communicating with the rest, if the two implementations are properly interoperable. OTOH, hosts using different protocols would not be able to communicate. (Wiregard uses its own protocol, IIRC.) As currently each provider (Wireguard and libreswan) support only one protocol, you probably don't need to introduce thevpn_protocolvariable now, as it can be inferred.