T6490: Allow creation of wireguard interfaces without requiring peers

[sskaje]

Change Summary

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

https://vyos.dev/T6490

Related PR(s)

Component(s) name

wireguard

Proposed changes

How to test

vyos@vyos:~$ configure
[edit]
vyos@vyos# delete interfaces wireguard
[edit]
vyos@vyos# commit
[edit]
vyos@vyos# set interfaces wireguard wg0 private-key xxxxxxx
[edit]
vyos@vyos# commit
[edit]
vyos@vyos# sudo wg
interface: wg0
  public key: xxxxxx=
  private key: (hidden)
  listening port: 42924
[edit]

Smoketest result

root@vyos:/home/vyos# python3 /usr/libexec/vyos/tests/smoke/cli/test_interfaces_wireguard.py
test_01_wireguard_peer (__main__.WireGuardInterfaceTest.test_01_wireguard_peer) ... ok
test_02_wireguard_add_remove_peer (__main__.WireGuardInterfaceTest.test_02_wireguard_add_remove_peer) ... ok
test_03_wireguard_same_public_key (__main__.WireGuardInterfaceTest.test_03_wireguard_same_public_key) ... ok
test_04_wireguard_threaded (__main__.WireGuardInterfaceTest.test_04_wireguard_threaded) ... ok
test_05_wireguard_peer_pubkey_change (__main__.WireGuardInterfaceTest.test_05_wireguard_peer_pubkey_change) ... ok

----------------------------------------------------------------------
Ran 5 tests in 51.219s

OK
root@vyos:/home/vyos#

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[github-actions]

👍
No issues in PR Title / Commit Title

[dmbaturin]

We have quite a few places where we allow the user to create inactive objects... like those named firewall rulesets that don't do anything by themselves until they are referenced in the main ruleset.

So I can't make a case against relaxing this validation either, if it doesn't create any problems.

[c-po]

We do NOT extend/alter the configuration dict in verify()

[sskaje]

I understand, I add this because it's the minimum changes.

Another change looks like diff below, the public key code block need to be indented

diff --git a/src/conf_mode/interfaces_wireguard.py b/src/conf_mode/interfaces_wireguard.py
index 197a472d7..e5c11966b 100755
--- a/src/conf_mode/interfaces_wireguard.py
+++ b/src/conf_mode/interfaces_wireguard.py
@@ -70,10 +70,6 @@ def verify(wireguard):
     if 'private_key' not in wireguard:
         raise ConfigError('Wireguard private-key not defined')
 
-    if 'peer' not in wireguard:
-        # T6490: initialize an empty peer object
-        wireguard['peer'] = {}
-
     if 'port' in wireguard and 'port_changed' in wireguard:
         listen_port = int(wireguard['port'])
         if check_port_availability('0.0.0.0', listen_port, 'udp') is not True:
@@ -81,28 +77,29 @@ def verify(wireguard):
                                'cannot be used for the interface!')
 
     # run checks on individual configured WireGuard peer
-    public_keys = []
-    for tmp in wireguard['peer']:
-        peer = wireguard['peer'][tmp]
+    if 'peer' in wireguard:
+        public_keys = []
+        for tmp in wireguard['peer']:
+            peer = wireguard['peer'][tmp]
 
-        if 'allowed_ips' not in peer:
-            raise ConfigError(f'Wireguard allowed-ips required for peer "{tmp}"!')
+            if 'allowed_ips' not in peer:
+                raise ConfigError(f'Wireguard allowed-ips required for peer "{tmp}"!')
 
-        if 'public_key' not in peer:
-            raise ConfigError(f'Wireguard public-key required for peer "{tmp}"!')
+            if 'public_key' not in peer:
+                raise ConfigError(f'Wireguard public-key required for peer "{tmp}"!')
 
-        if ('address' in peer and 'port' not in peer) or ('port' in peer and 'address' not in peer):
-            raise ConfigError('Both Wireguard port and address must be defined '
-                              f'for peer "{tmp}" if either one of them is set!')
+            if ('address' in peer and 'port' not in peer) or ('port' in peer and 'address' not in peer):
+                raise ConfigError('Both Wireguard port and address must be defined '
+                                  f'for peer "{tmp}" if either one of them is set!')
 
-        if peer['public_key'] in public_keys:
-            raise ConfigError(f'Duplicate public-key defined on peer "{tmp}"')
+            if peer['public_key'] in public_keys:
+                raise ConfigError(f'Duplicate public-key defined on peer "{tmp}"')
 
-        if 'disable' not in peer:
-            if is_wireguard_key_pair(wireguard['private_key'], peer['public_key']):
-                raise ConfigError(f'Peer "{tmp}" has the same public key as the interface "{wireguard["ifname"]}"')
+            if 'disable' not in peer:
+                if is_wireguard_key_pair(wireguard['private_key'], peer['public_key']):
+                    raise ConfigError(f'Peer "{tmp}" has the same public key as the interface "{wireguard["ifname"]}"')
 
-        public_keys.append(peer['public_key'])
+            public_keys.append(peer['public_key'])
 
 def generate(wireguard):
     return None

Is this the correct way?

[c-po]

Yes - that would be much better as it simply makes the peer key globally optional.

[sskaje]

@c-po code updated.

[github-actions]

CI integration ❌ failed!

Details

CI logs

• CLI Smoketests (no interfaces) ❌ failed
• CLI Smoketests (interfaces only) 👍 passed
• Config tests 👍 passed
• RAID1 tests 👍 passed
• TPM tests 👍 passed