Merge pull request #2554 from indrajitr/ddclient-update-20231128

ddclient: T5791: Update dynamic dns configuration path for consistency

4 files changed, 135 insertions, 46 deletions

diff --git a/src/completion/list_ddclient_protocols.sh b/src/completion/list_ddclient_protocols.sh
index c8855b5d1..634981660 100755
--- a/src/completion/list_ddclient_protocols.sh
+++ b/src/completion/list_ddclient_protocols.sh
@@ -14,4 +14,4 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-echo -n $(ddclient -list-protocols | grep  -vE 'nsupdate|cloudns|porkbun')
+echo -n $(ddclient -list-protocols | grep  -vE 'cloudns|porkbun')
diff --git a/src/conf_mode/dns_dynamic.py b/src/conf_mode/dns_dynamic.py
index 2bccaee0f..3ddc8e7fd 100755
--- a/src/conf_mode/dns_dynamic.py
+++ b/src/conf_mode/dns_dynamic.py
@@ -30,16 +30,18 @@ config_file = r'/run/ddclient/ddclient.conf'
 systemd_override = r'/run/systemd/system/ddclient.service.d/override.conf'
 
 # Protocols that require zone
-zone_necessary = ['cloudflare', 'digitalocean', 'godaddy', 'hetzner', 'gandi', 'nfsn']
+zone_necessary = ['cloudflare', 'digitalocean', 'godaddy', 'hetzner', 'gandi',
+                  'nfsn', 'nsupdate']
 zone_supported = zone_necessary + ['dnsexit2', 'zoneedit1']
 
 # Protocols that do not require username
 username_unnecessary = ['1984', 'cloudflare', 'cloudns', 'digitalocean', 'dnsexit2',
                         'duckdns', 'freemyip', 'hetzner', 'keysystems', 'njalla',
-                        'regfishde']
+                        'nsupdate', 'regfishde']
 
 # Protocols that support TTL
-ttl_supported = ['cloudflare', 'dnsexit2', 'gandi', 'hetzner', 'godaddy', 'nfsn']
+ttl_supported = ['cloudflare', 'dnsexit2', 'gandi', 'hetzner', 'godaddy', 'nfsn',
+                 'nsupdate']
 
 # Protocols that support both IPv4 and IPv6
 dualstack_supported = ['cloudflare', 'digitalocean', 'dnsexit2', 'duckdns',
@@ -70,63 +72,65 @@ def get_config(config=None):
 
 def verify(dyndns):
     # bail out early - looks like removal from running config
-    if not dyndns or 'address' not in dyndns:
+    if not dyndns or 'name' not in dyndns:
         return None
 
-    for address in dyndns['address']:
-        # If dyndns address is an interface, ensure it exists
-        if address != 'web':
-            verify_interface_exists(address)
+    # Dynamic DNS service provider - configuration validation
+    for service, config in dyndns['name'].items():
 
-        # RFC2136 - configuration validation
-        if 'rfc2136' in dyndns['address'][address]:
-            for config in dyndns['address'][address]['rfc2136'].values():
-                for field in ['host_name', 'zone', 'server', 'key']:
-                    if field not in config:
-                        raise ConfigError(f'"{field.replace("_", "-")}" is required for RFC2136 '
-                                          f'based Dynamic DNS service on "{address}"')
+        error_msg_req = f'is required for Dynamic DNS service "{service}"'
+        error_msg_uns = f'is not supported for Dynamic DNS service "{service}"'
 
-        # Dynamic DNS service provider - configuration validation
-        if 'web_options' in dyndns['address'][address] and address != 'web':
-            raise ConfigError(f'"web-options" is applicable only when using HTTP(S) web request to obtain the IP address')
+        for field in ['protocol', 'address', 'host_name']:
+            if field not in config:
+                raise ConfigError(f'"{field.replace("_", "-")}" {error_msg_req}')
 
-        # Dynamic DNS service provider - configuration validation
-        if 'service' in dyndns['address'][address]:
-            for service, config in dyndns['address'][address]['service'].items():
-                error_msg_req = f'is required for Dynamic DNS service "{service}" on "{address}"'
-                error_msg_uns = f'is not supported for Dynamic DNS service "{service}" on "{address}" with protocol "{config["protocol"]}"'
+        # If dyndns address is an interface, ensure that it exists
+        # and that web-options are not set
+        if config['address'] != 'web':
+            verify_interface_exists(config['address'])
+            if 'web_options' in config:
+                raise ConfigError(f'"web-options" is applicable only when using HTTP(S) web request to obtain the IP address')
 
-                for field in ['host_name', 'password', 'protocol']:
-                    if field not in config:
-                        raise ConfigError(f'"{field.replace("_", "-")}" {error_msg_req}')
+        # RFC2136 uses 'key' instead of 'password'
+        if config['protocol'] != 'nsupdate' and 'password' not in config:
+            raise ConfigError(f'"password" {error_msg_req}')
 
-                if config['protocol'] in zone_necessary and 'zone' not in config:
-                    raise ConfigError(f'"zone" {error_msg_req} with protocol "{config["protocol"]}"')
+        # Other RFC2136 specific configuration validation
+        if config['protocol'] == 'nsupdate':
+            if 'password' in config:
+                raise ConfigError(f'"password" {error_msg_uns} with protocol "{config["protocol"]}"')
+            for field in ['server', 'key']:
+                if field not in config:
+                    raise ConfigError(f'"{field}" {error_msg_req} with protocol "{config["protocol"]}"')
 
-                if config['protocol'] not in zone_supported and 'zone' in config:
-                    raise ConfigError(f'"zone" {error_msg_uns}')
+        if config['protocol'] in zone_necessary and 'zone' not in config:
+            raise ConfigError(f'"zone" {error_msg_req} with protocol "{config["protocol"]}"')
 
-                if config['protocol'] not in username_unnecessary and 'username' not in config:
-                    raise ConfigError(f'"username" {error_msg_req} with protocol "{config["protocol"]}"')
+        if config['protocol'] not in zone_supported and 'zone' in config:
+            raise ConfigError(f'"zone" {error_msg_uns} with protocol "{config["protocol"]}"')
 
-                if config['protocol'] not in ttl_supported and 'ttl' in config:
-                    raise ConfigError(f'"ttl" {error_msg_uns}')
+        if config['protocol'] not in username_unnecessary and 'username' not in config:
+            raise ConfigError(f'"username" {error_msg_req} with protocol "{config["protocol"]}"')
 
-                if config['ip_version'] == 'both':
-                    if config['protocol'] not in dualstack_supported:
-                        raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns}')
-                    # dyndns2 protocol in ddclient honors dual stack only for dyn.com (dyndns.org)
-                    if config['protocol'] == 'dyndns2' and 'server' in config and config['server'] not in dyndns_dualstack_servers:
-                        raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} for "{config["server"]}"')
+        if config['protocol'] not in ttl_supported and 'ttl' in config:
+            raise ConfigError(f'"ttl" {error_msg_uns} with protocol "{config["protocol"]}"')
 
-                if {'wait_time', 'expiry_time'} <= config.keys() and int(config['expiry_time']) < int(config['wait_time']):
-                        raise ConfigError(f'"expiry-time" must be greater than "wait-time"')
+        if config['ip_version'] == 'both':
+            if config['protocol'] not in dualstack_supported:
+                raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} with protocol "{config["protocol"]}"')
+            # dyndns2 protocol in ddclient honors dual stack only for dyn.com (dyndns.org)
+            if config['protocol'] == 'dyndns2' and 'server' in config and config['server'] not in dyndns_dualstack_servers:
+                raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} for "{config["server"]}" with protocol "{config["protocol"]}"')
+
+        if {'wait_time', 'expiry_time'} <= config.keys() and int(config['expiry_time']) < int(config['wait_time']):
+                raise ConfigError(f'"expiry-time" must be greater than "wait-time" for Dynamic DNS service "{service}"')
 
     return None
 
 def generate(dyndns):
     # bail out early - looks like removal from running config
-    if not dyndns or 'address' not in dyndns:
+    if not dyndns or 'name' not in dyndns:
         return None
 
     render(config_file, 'dns-dynamic/ddclient.conf.j2', dyndns, permission=0o600)
@@ -139,7 +143,7 @@ def apply(dyndns):
     call('systemctl daemon-reload')
 
     # bail out early - looks like removal from running config
-    if not dyndns or 'address' not in dyndns:
+    if not dyndns or 'name' not in dyndns:
         call(f'systemctl stop {systemd_service}')
         if os.path.exists(config_file):
             os.unlink(config_file)
diff --git a/src/migration-scripts/dns-dynamic/2-to-3 b/src/migration-scripts/dns-dynamic/2-to-3
new file mode 100644
index 000000000..02bc9324a
--- /dev/null
+++ b/src/migration-scripts/dns-dynamic/2-to-3
@@ -0,0 +1,85 @@
+#!/usr/bin/env python3
+
+# Copyright (C) 2023 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# T5791:
+# - migrate "service dns dynamic address web web-options ..."
+#        to "service dns dynamic name <service> address web ..." (per service)
+# - migrate "service dns dynamic address <address> rfc2136 <service> ..."
+#        to "service dns dynamic name <service> address <interface> protocol 'nsupdate'"
+# - migrate "service dns dynamic address <interface> service <service> ..."
+#        to "service dns dynamic name <service> address <interface> ..."
+
+import sys
+from vyos.configtree import ConfigTree
+
+if len(sys.argv) < 2:
+    print("Must specify file name!")
+    sys.exit(1)
+
+file_name = sys.argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+config = ConfigTree(config_file)
+
+base_path = ['service', 'dns', 'dynamic']
+address_path = base_path + ['address']
+name_path = base_path + ['name']
+
+if not config.exists(address_path):
+    # Nothing to do
+    sys.exit(0)
+
+# config.copy does not recursively create a path, so initialize the name path
+if not config.exists(name_path):
+    config.set(name_path)
+
+for address in config.list_nodes(address_path):
+
+    # Move web-option as a configuration in each service instead of top level web-option
+    if config.exists(address_path + [address, 'web-options']) and address == 'web':
+        for svc_type in ['service', 'rfc2136']:
+            if config.exists(address_path + [address, svc_type]):
+                for svc_cfg in config.list_nodes(address_path + [address, svc_type]):
+                    config.copy(address_path + [address, 'web-options'],
+                                address_path + [address, svc_type, svc_cfg, 'web-options'])
+        config.delete(address_path + [address, 'web-options'])
+
+    for svc_type in ['service', 'rfc2136']:
+        if config.exists(address_path + [address, svc_type]):
+            # Move RFC2136 as service configuration, rename to avoid name conflict and set protocol to 'nsupdate'
+            if svc_type == 'rfc2136':
+                for rfc_cfg_old in config.list_nodes(address_path + [address, 'rfc2136']):
+                    rfc_cfg_new = f'{rfc_cfg_old}-rfc2136'
+                    config.rename(address_path + [address, 'rfc2136', rfc_cfg_old], rfc_cfg_new)
+                    config.set(address_path + [address, 'rfc2136', rfc_cfg_new, 'protocol'], 'nsupdate')
+
+            # Add address as config value in each service before moving the service path
+            # And then copy the services from 'address <interface> service <service>' to 'name <service>'
+            for svc_cfg in config.list_nodes(address_path + [address, svc_type]):
+                config.set(address_path + [address, svc_type, svc_cfg, 'address'], address)
+                config.copy(address_path + [address, svc_type, svc_cfg], name_path + [svc_cfg])
+
+# Finally cleanup the old address path
+config.delete(address_path)
+
+try:
+    with open(file_name, 'w') as f:
+        f.write(config.to_string())
+except OSError as e:
+    print("Failed to save the modified config: {}".format(e))
+    sys.exit(1)
diff --git a/src/validators/ddclient-protocol b/src/validators/ddclient-protocol
index 8f455e12e..ce5efbd52 100755
--- a/src/validators/ddclient-protocol
+++ b/src/validators/ddclient-protocol
@@ -14,7 +14,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-ddclient -list-protocols | grep -vE 'nsupdate|cloudns|porkbun' | grep -qw $1
+ddclient -list-protocols | grep -vE 'cloudns|porkbun' | grep -qw $1
 
 if [ $? -gt 0 ]; then
     echo "Error: $1 is not a valid protocol, please choose from the supported list of protocols"