From 66e45beb91ecc22e9ae662434ecce6ac3ea1ceb8 Mon Sep 17 00:00:00 2001
From: Indrajit Raychaudhuri <irc@indrajit.com>
Date: Thu, 9 Nov 2023 16:12:58 -0600
Subject: ddclient: T5708: Additional smoketests for web-options

Add additional smoketests for web-options validation.
Also, format error messages to optionally include protocol name.
---
 smoketest/scripts/cli/test_service_dns_dynamic.py | 53 ++++++++++++++++++++++-
 src/conf_mode/dns_dynamic.py                      |  6 +--
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/smoketest/scripts/cli/test_service_dns_dynamic.py b/smoketest/scripts/cli/test_service_dns_dynamic.py
index 69ea5c1b3..a3b220f69 100755
--- a/smoketest/scripts/cli/test_service_dns_dynamic.py
+++ b/smoketest/scripts/cli/test_service_dns_dynamic.py
@@ -245,7 +245,58 @@ class TestServiceDDNS(VyOSUnitTestSHIM.TestCase):
             self.assertIn(f'password=\'{password}\'', ddclient_conf)
             self.assertIn(f'{name}', ddclient_conf)
 
-    def test_06_dyndns_vrf(self):
+    def test_06_dyndns_web_options(self):
+        # Check if DDNS service can be configured and runs
+        base_path_iface = base_path + ['address', interface]
+        base_path_web = base_path + ['address', 'web']
+        svc_path_iface = base_path_iface + ['service', 'cloudflare']
+        svc_path_web = base_path_web + ['service', 'cloudflare']
+        proto = 'cloudflare'
+        web_url_good = 'https://ifconfig.me/ip'
+        web_url_bad = 'http:/ifconfig.me/ip'
+
+        self.cli_set(svc_path_iface + ['protocol', proto])
+        self.cli_set(svc_path_iface + ['zone', zone])
+        self.cli_set(svc_path_iface + ['password', password])
+        self.cli_set(svc_path_iface + ['host-name', hostname])
+        self.cli_set(base_path_iface + ['web-options', 'url', web_url_good])
+
+        # web-options is supported only with web service based address lookup
+        # exception is raised for interface based address lookup
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_delete(base_path_iface + ['web-options'])
+
+        # commit changes
+        self.cli_commit()
+
+        # web-options is supported with web service based address lookup
+        # this should work, but clear interface based config first
+        self.cli_delete(base_path_iface)
+        self.cli_set(svc_path_web + ['protocol', proto])
+        self.cli_set(svc_path_web + ['zone', zone])
+        self.cli_set(svc_path_web + ['password', password])
+        self.cli_set(svc_path_web + ['host-name', hostname])
+
+        # web-options must be a valid URL
+        with self.assertRaises(ConfigSessionError) as cm:
+            self.cli_set(base_path_web + ['web-options', 'url', web_url_bad])
+        self.assertIn(f'"{web_url_bad.removeprefix("http:")}" is not a valid URI', str(cm.exception))
+        self.cli_set(base_path_web + ['web-options', 'url', web_url_good])
+
+        # commit changes
+        self.cli_commit()
+
+        # Check the generating config parameters
+        ddclient_conf = cmd(f'sudo cat {DDCLIENT_CONF}')
+        self.assertIn(f'usev4=webv4', ddclient_conf)
+        self.assertIn(f'webv4={web_url_good}', ddclient_conf)
+        self.assertIn(f'protocol={proto}', ddclient_conf)
+        self.assertIn(f'zone={zone}', ddclient_conf)
+        self.assertIn(f'password=\'{password}\'', ddclient_conf)
+        self.assertIn(f'{hostname}', ddclient_conf)
+
+    def test_07_dyndns_vrf(self):
         vrf_table = "".join(random.choices(string.digits, k=5))
         vrf_name = f'vyos-test-{vrf_table}'
         svc_path = ['address', interface, 'service', 'cloudflare']
diff --git a/src/conf_mode/dns_dynamic.py b/src/conf_mode/dns_dynamic.py
index d6ef620fe..2bccaee0f 100755
--- a/src/conf_mode/dns_dynamic.py
+++ b/src/conf_mode/dns_dynamic.py
@@ -93,7 +93,7 @@ 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_req = f'is required for Dynamic DNS service "{service}" on "{address}" with protocol "{config["protocol"]}"'
+                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"]}"'
 
                 for field in ['host_name', 'password', 'protocol']:
@@ -101,13 +101,13 @@ def verify(dyndns):
                         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_req}')
+                    raise ConfigError(f'"zone" {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'] not in username_unnecessary and 'username' not in config:
-                    raise ConfigError(f'"username" {error_msg_req}')
+                    raise ConfigError(f'"username" {error_msg_req} with protocol "{config["protocol"]}"')
 
                 if config['protocol'] not in ttl_supported and 'ttl' in config:
                     raise ConfigError(f'"ttl" {error_msg_uns}')
-- 
cgit v1.2.3