From 5c260386d246fd76df70540d003ac808fe33b467 Mon Sep 17 00:00:00 2001 From: Indrajit Raychaudhuri Date: Thu, 2 Nov 2023 20:44:57 -0500 Subject: ddclient: T5708: Migrate `timeout` to `interval` Time interval in seconds to wait between DNS updates would be a bit more intuitive as `interval` than `timeout`. --- data/templates/dns-dynamic/ddclient.conf.j2 | 2 +- interface-definitions/dns-dynamic.xml.in | 6 +-- .../include/version/dns-dynamic-version.xml.i | 2 +- smoketest/scripts/cli/test_service_dns_dynamic.py | 6 +-- src/migration-scripts/dns-dynamic/1-to-2 | 52 ++++++++++++++++++++++ 5 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 src/migration-scripts/dns-dynamic/1-to-2 diff --git a/data/templates/dns-dynamic/ddclient.conf.j2 b/data/templates/dns-dynamic/ddclient.conf.j2 index 6e77abdb5..879887a1f 100644 --- a/data/templates/dns-dynamic/ddclient.conf.j2 +++ b/data/templates/dns-dynamic/ddclient.conf.j2 @@ -21,7 +21,7 @@ if{{ ipv }}={{ address }}, \ {{ host }} {% endmacro %} ### Autogenerated by dns_dynamic.py ### -daemon={{ timeout }} +daemon={{ interval }} syslog=yes ssl=yes pid={{ config_file | replace('.conf', '.pid') }} diff --git a/interface-definitions/dns-dynamic.xml.in b/interface-definitions/dns-dynamic.xml.in index 723223f1c..07b1bf1b8 100644 --- a/interface-definitions/dns-dynamic.xml.in +++ b/interface-definitions/dns-dynamic.xml.in @@ -134,9 +134,9 @@ - + - Time in seconds to wait between DNS updates + Interval in seconds to wait between Dynamic DNS updates u32:60-3600 Time in seconds @@ -144,7 +144,7 @@ - Timeout must be between 60 and 3600 seconds + Interval must be between 60 and 3600 seconds 300 diff --git a/interface-definitions/include/version/dns-dynamic-version.xml.i b/interface-definitions/include/version/dns-dynamic-version.xml.i index b25fc6e76..7bdb90a35 100644 --- a/interface-definitions/include/version/dns-dynamic-version.xml.i +++ b/interface-definitions/include/version/dns-dynamic-version.xml.i @@ -1,3 +1,3 @@ - + diff --git a/smoketest/scripts/cli/test_service_dns_dynamic.py b/smoketest/scripts/cli/test_service_dns_dynamic.py index acabc0070..6c2f584c9 100755 --- a/smoketest/scripts/cli/test_service_dns_dynamic.py +++ b/smoketest/scripts/cli/test_service_dns_dynamic.py @@ -112,7 +112,7 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): # IPv6 only DDNS service configuration def test_02_dyndns_service_ipv6(self): - timeout = '60' + interval = '60' svc_path = ['address', interface, 'service', 'dynv6'] proto = 'dyndns2' ip_version = 'ipv6' @@ -120,7 +120,7 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): expiry_time_good = '3600' expiry_time_bad = '360' - self.cli_set(base_path + ['timeout', timeout]) + self.cli_set(base_path + ['interval', interval]) self.cli_set(base_path + svc_path + ['ip-version', ip_version]) self.cli_set(base_path + svc_path + ['protocol', proto]) self.cli_set(base_path + svc_path + ['server', server]) @@ -140,7 +140,7 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): # Check the generating config parameters ddclient_conf = cmd(f'sudo cat {DDCLIENT_CONF}') - self.assertIn(f'daemon={timeout}', ddclient_conf) + self.assertIn(f'daemon={interval}', ddclient_conf) self.assertIn(f'usev6=ifv6', ddclient_conf) self.assertIn(f'ifv6={interface}', ddclient_conf) self.assertIn(f'protocol={proto}', ddclient_conf) diff --git a/src/migration-scripts/dns-dynamic/1-to-2 b/src/migration-scripts/dns-dynamic/1-to-2 new file mode 100644 index 000000000..b4679769c --- /dev/null +++ b/src/migration-scripts/dns-dynamic/1-to-2 @@ -0,0 +1,52 @@ +#!/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 . + +# T5708: +# - migrate "service dns dynamic timeout ..." +# to "service dns dynamic interval ..." + +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'] +timeout_path = base_path + ['timeout'] + +if not config.exists(base_path): + # Nothing to do + sys.exit(0) + +# Migrate "service dns dynamic timeout ..." +# to "service dns dynamic interval ..." +if config.exists(timeout_path): + config.rename(timeout_path, 'interval') + +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) -- cgit v1.2.3 From fd5cdaa7a6688eb8ec5c8df26be27253b8ca140f Mon Sep 17 00:00:00 2001 From: Indrajit Raychaudhuri Date: Sat, 4 Nov 2023 16:01:02 -0500 Subject: ddclient: T5708: Validate proper use of `web-options` `web-options` is only applicable when using HTTP(S) web request to obtain the IP address. Apply guard for that. --- src/conf_mode/dns_dynamic.py | 4 ++++ src/migration-scripts/dns-dynamic/1-to-2 | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/src/conf_mode/dns_dynamic.py b/src/conf_mode/dns_dynamic.py index 874c4b689..d71dc22fd 100755 --- a/src/conf_mode/dns_dynamic.py +++ b/src/conf_mode/dns_dynamic.py @@ -81,6 +81,10 @@ def verify(dyndns): raise ConfigError(f'"{field.replace("_", "-")}" is required for RFC2136 ' f'based Dynamic DNS service on "{address}"') + # 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') + # Dynamic DNS service provider - configuration validation if 'service' in dyndns['address'][address]: for service, config in dyndns['address'][address]['service'].items(): diff --git a/src/migration-scripts/dns-dynamic/1-to-2 b/src/migration-scripts/dns-dynamic/1-to-2 index b4679769c..8aaedf210 100644 --- a/src/migration-scripts/dns-dynamic/1-to-2 +++ b/src/migration-scripts/dns-dynamic/1-to-2 @@ -17,6 +17,7 @@ # T5708: # - migrate "service dns dynamic timeout ..." # to "service dns dynamic interval ..." +# - remove "service dns dynamic address web-options ..." when != "web" import sys from vyos.configtree import ConfigTree @@ -34,6 +35,7 @@ config = ConfigTree(config_file) base_path = ['service', 'dns', 'dynamic'] timeout_path = base_path + ['timeout'] +address_path = base_path + ['address'] if not config.exists(base_path): # Nothing to do @@ -44,6 +46,11 @@ if not config.exists(base_path): if config.exists(timeout_path): config.rename(timeout_path, 'interval') +# Remove "service dns dynamic address web-options ..." when != "web" +for address in config.list_nodes(address_path): + if config.exists(address_path + [address, 'web-options']) and address != 'web': + config.delete(address_path + [address, 'web-options']) + try: with open(file_name, 'w') as f: f.write(config.to_string()) -- cgit v1.2.3 From 50c506a9be56b548a8d461f3e019165c384f49d3 Mon Sep 17 00:00:00 2001 From: Indrajit Raychaudhuri Date: Thu, 2 Nov 2023 20:15:49 -0500 Subject: ddclient: T5708: Migration to 3.11.1 and related improvements - Migrate to ddclient 3.11.1 and enforce debian/control dependency - Add dual stack support for additional protocols - Restrict usage of `porkbun` protocol, VyOS configuration structure isn't compatible with porkbun yet - Improve and cleanup error messages --- debian/control | 2 +- smoketest/scripts/cli/test_service_dns_dynamic.py | 5 ++-- src/completion/list_ddclient_protocols.sh | 2 +- src/conf_mode/dns_dynamic.py | 34 +++++++++++++---------- src/migration-scripts/dns-dynamic/1-to-2 | 11 ++++++++ src/validators/ddclient-protocol | 2 +- 6 files changed, 36 insertions(+), 20 deletions(-) diff --git a/debian/control b/debian/control index 32de13f1b..d2ed3991a 100644 --- a/debian/control +++ b/debian/control @@ -155,7 +155,7 @@ Depends: # For "set service aws glb" aws-gwlbtun, # For "service dns dynamic" - ddclient (>= 3.9.1), + ddclient (>= 3.11.1), # End "service dns dynamic" # # For "service ids" fastnetmon [amd64], diff --git a/smoketest/scripts/cli/test_service_dns_dynamic.py b/smoketest/scripts/cli/test_service_dns_dynamic.py index 6c2f584c9..9624f823f 100755 --- a/smoketest/scripts/cli/test_service_dns_dynamic.py +++ b/smoketest/scripts/cli/test_service_dns_dynamic.py @@ -246,10 +246,11 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase): self.assertIn(f'{name}', ddclient_conf) def test_06_dyndns_vrf(self): - vrf_name = f'vyos-test-{"".join(random.choices(string.ascii_letters + string.digits, k=5))}' + vrf_table = "".join(random.choices(string.digits, k=5)) + vrf_name = f'vyos-test-{vrf_table}' svc_path = ['address', interface, 'service', 'cloudflare'] - self.cli_set(['vrf', 'name', vrf_name, 'table', '12345']) + self.cli_set(['vrf', 'name', vrf_name, 'table', vrf_table]) self.cli_set(base_path + ['vrf', vrf_name]) self.cli_set(base_path + svc_path + ['protocol', 'cloudflare']) diff --git a/src/completion/list_ddclient_protocols.sh b/src/completion/list_ddclient_protocols.sh index 3b4eff4d6..c8855b5d1 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 . -echo -n $(ddclient -list-protocols | grep -vE 'nsupdate|cloudns') +echo -n $(ddclient -list-protocols | grep -vE 'nsupdate|cloudns|porkbun') diff --git a/src/conf_mode/dns_dynamic.py b/src/conf_mode/dns_dynamic.py index d71dc22fd..d6ef620fe 100755 --- a/src/conf_mode/dns_dynamic.py +++ b/src/conf_mode/dns_dynamic.py @@ -30,16 +30,21 @@ 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', 'godaddy', 'hetzner', 'gandi', 'nfsn'] +zone_necessary = ['cloudflare', 'digitalocean', 'godaddy', 'hetzner', 'gandi', 'nfsn'] +zone_supported = zone_necessary + ['dnsexit2', 'zoneedit1'] # Protocols that do not require username -username_unnecessary = ['1984', 'cloudflare', 'cloudns', 'duckdns', 'freemyip', 'hetzner', 'keysystems', 'njalla'] +username_unnecessary = ['1984', 'cloudflare', 'cloudns', 'digitalocean', 'dnsexit2', + 'duckdns', 'freemyip', 'hetzner', 'keysystems', 'njalla', + 'regfishde'] # Protocols that support TTL -ttl_supported = ['cloudflare', 'gandi', 'hetzner', 'dnsexit', 'godaddy', 'nfsn'] +ttl_supported = ['cloudflare', 'dnsexit2', 'gandi', 'hetzner', 'godaddy', 'nfsn'] # Protocols that support both IPv4 and IPv6 -dualstack_supported = ['cloudflare', 'dyndns2', 'freedns', 'njalla'] +dualstack_supported = ['cloudflare', 'digitalocean', 'dnsexit2', 'duckdns', + 'dyndns2', 'easydns', 'freedns', 'hetzner', 'infomaniak', + 'njalla'] # dyndns2 protocol in ddclient honors dual stack for selective servers # because of the way it is implemented in ddclient @@ -88,32 +93,31 @@ def verify(dyndns): # Dynamic DNS service provider - configuration validation if 'service' in dyndns['address'][address]: for service, config in dyndns['address'][address]['service'].items(): - error_msg = f'is required for Dynamic DNS service "{service}" on "{address}"' + error_msg_req = f'is required for Dynamic DNS service "{service}" on "{address}" with protocol "{config["protocol"]}"' + error_msg_uns = f'is not supported for Dynamic DNS service "{service}" on "{address}" with protocol "{config["protocol"]}"' for field in ['host_name', 'password', 'protocol']: if field not in config: - raise ConfigError(f'"{field.replace("_", "-")}" {error_msg}') + raise ConfigError(f'"{field.replace("_", "-")}" {error_msg_req}') if config['protocol'] in zone_necessary and 'zone' not in config: - raise ConfigError(f'"zone" {error_msg}') + raise ConfigError(f'"zone" {error_msg_req}') - if config['protocol'] not in zone_necessary and 'zone' in config: - raise ConfigError(f'"{config["protocol"]}" does not support "zone"') + if config['protocol'] not in zone_supported and 'zone' in config: + raise ConfigError(f'"zone" {error_msg_uns}') if config['protocol'] not in username_unnecessary and 'username' not in config: - raise ConfigError(f'"username" {error_msg}') + raise ConfigError(f'"username" {error_msg_req}') if config['protocol'] not in ttl_supported and 'ttl' in config: - raise ConfigError(f'"{config["protocol"]}" does not support "ttl"') + raise ConfigError(f'"ttl" {error_msg_uns}') if config['ip_version'] == 'both': if config['protocol'] not in dualstack_supported: - raise ConfigError(f'"{config["protocol"]}" does not support ' - f'both IPv4 and IPv6 at the same time') + 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'"{config["protocol"]}" does not support ' - f'both IPv4 and IPv6 at the same time for "{config["server"]}"') + raise ConfigError(f'Both IPv4 and IPv6 at the same time {error_msg_uns} for "{config["server"]}"') 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"') diff --git a/src/migration-scripts/dns-dynamic/1-to-2 b/src/migration-scripts/dns-dynamic/1-to-2 index 8aaedf210..8b599b57a 100644 --- a/src/migration-scripts/dns-dynamic/1-to-2 +++ b/src/migration-scripts/dns-dynamic/1-to-2 @@ -18,6 +18,8 @@ # - migrate "service dns dynamic timeout ..." # to "service dns dynamic interval ..." # - remove "service dns dynamic address web-options ..." when != "web" +# - migrate "service dns dynamic address service protocol dnsexit" +# to "service dns dynamic address service protocol dnsexit2" import sys from vyos.configtree import ConfigTree @@ -51,6 +53,15 @@ for address in config.list_nodes(address_path): if config.exists(address_path + [address, 'web-options']) and address != 'web': config.delete(address_path + [address, 'web-options']) +# Migrate "service dns dynamic address service protocol dnsexit" +# to "service dns dynamic address service protocol dnsexit2" +for address in config.list_nodes(address_path): + for svc_cfg in config.list_nodes(address_path + [address, 'service']): + if config.exists(address_path + [address, 'service', svc_cfg, 'protocol']): + protocol = config.return_value(address_path + [address, 'service', svc_cfg, 'protocol']) + if protocol == 'dnsexit': + config.set(address_path + [address, 'service', svc_cfg, 'protocol'], 'dnsexit2') + try: with open(file_name, 'w') as f: f.write(config.to_string()) diff --git a/src/validators/ddclient-protocol b/src/validators/ddclient-protocol index bc6826120..8f455e12e 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 . -ddclient -list-protocols | grep -vE 'nsupdate|cloudns' | grep -qw $1 +ddclient -list-protocols | grep -vE 'nsupdate|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" -- cgit v1.2.3