Merge pull request #2768 from c-po/pki-ipsec-T5905

pki: T5905: do not use expand_nodes=Diff.ADD|Diff.DELETE) in node_changed()

smoketest/scripts/cli/test_vpn_ipsec.py

@@ -115,6 +115,7 @@ def setUpClass(cls):
         # ensure we can also run this test on a live system - so lets clean
         # out the current configuration :)
         cls.cli_delete(cls, base_path)
+        cls.cli_delete(cls, ['pki'])
 
         cls.cli_set(cls, base_path + ['interface', f'{interface}.{vif}'])
 

src/conf_mode/pki.py

@@ -104,10 +104,10 @@ def certbot_request(name: str, config: dict, dry_run: bool=True):
         return
 
     domains = '--domains ' + ' --domains '.join(config['domain_name'])
-    tmp = f'certbot certonly --config-dir {vyos_certbot_dir} --cert-name {name} '\
-            f'--non-interactive --standalone --agree-tos --no-eff-email --expand '\
-            f'--server {config["url"]} --email {config["email"]} '\
-            f'--key-type rsa --rsa-key-size {config["rsa_key_size"]} {domains}'
+    tmp = f'certbot certonly --non-interactive --config-dir {vyos_certbot_dir} --cert-name {name} '\
+          f'--standalone --agree-tos --no-eff-email --expand --server {config["url"]} '\
+          f'--email {config["email"]} --key-type rsa --rsa-key-size {config["rsa_key_size"]} '\
+          f'{domains}'
     if 'listen_address' in config:
         tmp += f' --http-01-address {config["listen_address"]}'
     # verify() does not need to actually request a cert but only test for plausability
@@ -135,8 +135,7 @@ def get_config(config=None):
         if 'changed' not in pki: pki.update({'changed':{}})
         pki['changed'].update({'ca' : tmp})
 
-    tmp = node_changed(conf, base + ['certificate'], key_mangling=('-', '_'),
-                       recursive=True, expand_nodes=Diff.ADD|Diff.DELETE)
+    tmp = node_changed(conf, base + ['certificate'], key_mangling=('-', '_'), recursive=True)
     if tmp:
         if 'changed' not in pki: pki.update({'changed':{}})
         pki['changed'].update({'certificate' : tmp})
@@ -211,7 +210,7 @@ def get_config(config=None):
                         if found_name == item_name:
                             path = search['path']
                             path_str = ' '.join(path + found_path)
-                            print(f'pki: Updating config: {path_str} {found_name}')
+                            print(f'PKI: Updating config: {path_str} {found_name}')
 
                             if path[0] == 'interfaces':
                                 ifname = found_path[0]
@@ -371,21 +370,29 @@ def generate(pki):
     if 'certbot_renew' in pki:
         return None
 
-    # list of certificates issued via certbot
     certbot_list = []
+    certbot_list_on_disk = []
+    if os.path.exists(f'{vyos_certbot_dir}/live'):
+        certbot_list_on_disk = [f.path.split('/')[-1] for f in os.scandir(f'{vyos_certbot_dir}/live') if f.is_dir()]
+
     if 'certificate' in pki:
+        changed_certificates = dict_search('changed.certificate', pki)
         for name, cert_conf in pki['certificate'].items():
             if 'acme' in cert_conf:
                 certbot_list.append(name)
-                # when something for the certificate changed, we should delete it
-                if name in dict_search('changed.certificate', pki):
-                    certbot_delete(name)
+                # generate certificate if not found on disk
+                if name not in certbot_list_on_disk:
+                    certbot_request(name, cert_conf['acme'], dry_run=False)
+                elif changed_certificates != None and name in changed_certificates:
+                    # when something for the certificate changed, we should delete it
+                    if name in certbot_list_on_disk:
+                        certbot_delete(name)
                     certbot_request(name, cert_conf['acme'], dry_run=False)
 
     # Cleanup certbot configuration and certificates if no longer in use by CLI
     # Get foldernames under vyos_certbot_dir which each represent a certbot cert
     if os.path.exists(f'{vyos_certbot_dir}/live'):
-        for cert in [f.path.split('/')[-1] for f in os.scandir(f'{vyos_certbot_dir}/live') if f.is_dir()]:
+        for cert in certbot_list_on_disk:
             if cert not in certbot_list:
                 # certificate is no longer active on the CLI - remove it
                 certbot_delete(cert)

src/conf_mode/vpn_ipsec.py

@@ -43,6 +43,7 @@
 from vyos.template import is_ipv6
 from vyos.template import render
 from vyos.utils.network import is_ipv6_link_local
+from vyos.utils.network import interface_exists
 from vyos.utils.dict import dict_search
 from vyos.utils.dict import dict_search_args
 from vyos.utils.process import call
@@ -65,11 +66,11 @@
 
 vici_socket = '/var/run/charon.vici'
 
-CERT_PATH = f'{swanctl_dir}/x509/'
+CERT_PATH   = f'{swanctl_dir}/x509/'
 PUBKEY_PATH = f'{swanctl_dir}/pubkey/'
-KEY_PATH  = f'{swanctl_dir}/private/'
-CA_PATH   = f'{swanctl_dir}/x509ca/'
-CRL_PATH  = f'{swanctl_dir}/x509crl/'
+KEY_PATH    = f'{swanctl_dir}/private/'
+CA_PATH     = f'{swanctl_dir}/x509ca/'
+CRL_PATH    = f'{swanctl_dir}/x509crl/'
 
 DHCP_HOOK_IFLIST = '/tmp/ipsec_dhcp_waiting'
 
@@ -394,7 +395,7 @@ def verify(ipsec):
 
                 if 'bind' in peer_conf['vti']:
                     vti_interface = peer_conf['vti']['bind']
-                    if not os.path.exists(f'/sys/class/net/{vti_interface}'):
+                    if not interface_exists(vti_interface):
                         raise ConfigError(f'VTI interface {vti_interface} for site-to-site peer {peer} does not exist!')
 
             if 'vti' not in peer_conf and 'tunnel' not in peer_conf: