Merge pull request #1914 from indrajitr/pdns-port

dns: T5113: Support custom port for name-server forwarders

4 files changed, 51 insertions, 7 deletions

diff --git a/interface-definitions/dns-forwarding.xml.in b/interface-definitions/dns-forwarding.xml.in
index 371f198c6..b23eaa351 100644
--- a/interface-definitions/dns-forwarding.xml.in
+++ b/interface-definitions/dns-forwarding.xml.in
@@ -635,7 +635,7 @@
                 </properties>
                 <defaultValue>1500</defaultValue>
               </leafNode>
-              #include <include/name-server-ipv4-ipv6.xml.i>
+              #include <include/name-server-ipv4-ipv6-port.xml.i>
               <leafNode name="source-address">
                 <properties>
                   <help>Local addresses from which to send DNS queries</help>
diff --git a/interface-definitions/include/name-server-ipv4-ipv6-port.xml.i b/interface-definitions/include/name-server-ipv4-ipv6-port.xml.i
new file mode 100644
index 000000000..cf86e66a2
--- /dev/null
+++ b/interface-definitions/include/name-server-ipv4-ipv6-port.xml.i
@@ -0,0 +1,25 @@
+<!-- include start from name-server-ipv4-ipv6-port.xml.i -->
+<tagNode name="name-server">
+  <properties>
+    <help>Domain Name Servers (DNS) addresses</help>
+    <valueHelp>
+      <format>ipv4</format>
+      <description>Domain Name Server (DNS) IPv4 address</description>
+    </valueHelp>
+    <valueHelp>
+      <format>ipv6</format>
+      <description>Domain Name Server (DNS) IPv6 address</description>
+    </valueHelp>
+    <constraint>
+      <validator name="ipv4-address"/>
+      <validator name="ipv6-address"/>
+    </constraint>
+  </properties>
+  <children>
+    #include <include/port-number.xml.i>
+    <leafNode name="port">
+      <defaultValue>53</defaultValue>
+    </leafNode>
+  </children>
+</tagNode>
+<!-- include end -->
diff --git a/smoketest/scripts/cli/test_service_dns_forwarding.py b/smoketest/scripts/cli/test_service_dns_forwarding.py
index 94e0597ad..04dced292 100755
--- a/smoketest/scripts/cli/test_service_dns_forwarding.py
+++ b/smoketest/scripts/cli/test_service_dns_forwarding.py
@@ -20,6 +20,7 @@ import unittest
 from base_vyostest_shim import VyOSUnitTestSHIM
 
 from vyos.configsession import ConfigSessionError
+from vyos.template import bracketize_ipv6
 from vyos.util import read_file
 from vyos.util import process_named_running
 
@@ -141,15 +142,20 @@ class TestServicePowerDNS(VyOSUnitTestSHIM.TestCase):
         for address in listen_adress:
             self.cli_set(base_path + ['listen-address', address])
 
-        nameservers = ['192.0.2.1', '192.0.2.2']
-        for nameserver in nameservers:
-            self.cli_set(base_path + ['name-server', nameserver])
+        nameservers = {'192.0.2.1': {}, '192.0.2.2': {'port': '53'}, '2001:db8::1': {'port': '853'}}
+        for h,p in nameservers.items():
+            if 'port' in p:
+                self.cli_set(base_path + ['name-server', h, 'port', p['port']])
+            else:
+                self.cli_set(base_path + ['name-server', h])
 
         # commit changes
         self.cli_commit()
 
         tmp = get_config_value(r'\+.', file=FORWARD_FILE)
-        self.assertEqual(tmp, ', '.join(nameservers))
+        canonical_entries = [(lambda h, p: f"{bracketize_ipv6(h)}:{p['port'] if 'port' in p else 53}")(h, p)
+                             for (h, p) in nameservers.items()]
+        self.assertEqual(tmp, ', '.join(canonical_entries))
 
         # Do not use local /etc/hosts file in name resolution
         # default: yes
diff --git a/src/conf_mode/dns_forwarding.py b/src/conf_mode/dns_forwarding.py
index d0d87d73e..4d6b85d92 100755
--- a/src/conf_mode/dns_forwarding.py
+++ b/src/conf_mode/dns_forwarding.py
@@ -24,7 +24,7 @@ from vyos.config import Config
 from vyos.configdict import dict_merge
 from vyos.hostsd_client import Client as hostsd_client
 from vyos.template import render
-from vyos.template import is_ipv6
+from vyos.template import bracketize_ipv6
 from vyos.util import call
 from vyos.util import chown
 from vyos.util import dict_search
@@ -58,8 +58,16 @@ def get_config(config=None):
     default_values = defaults(base)
     # T2665 due to how defaults under tag nodes work, we must clear these out before we merge
     del default_values['authoritative_domain']
+    del default_values['name_server']
     dns = dict_merge(default_values, dns)
 
+    # T2665: we cleared default values for tag node 'name_server' above.
+    # We now need to add them back back in a granular way.
+    if 'name_server' in dns:
+        default_values = defaults(base + ['name-server'])
+        for server in dns['name_server']:
+            dns['name_server'][server] = dict_merge(default_values, dns['name_server'][server])
+
     # some additions to the default dictionary
     if 'system' in dns:
         base_nameservers = ['system', 'name-server']
@@ -329,7 +337,12 @@ def apply(dns):
         # sources
         hc.delete_name_servers([hostsd_tag])
         if 'name_server' in dns:
-            hc.add_name_servers({hostsd_tag: dns['name_server']})
+            # 'name_server' is a dict of the form
+            # {'192.0.2.1': {'port': 53}, '2001:db8::1': {'port': 853}, ...}
+            # canonicalize them as ['192.0.2.1:53', '[2001:db8::1]:853', ...] with IPv6 hosts bracketized
+            nslist = [(lambda h, p: f"{bracketize_ipv6(h)}:{p['port']}")(h, p)
+                      for (h, p) in dns['name_server'].items()]
+            hc.add_name_servers({hostsd_tag: nslist})
 
         # delete all nameserver tags
         hc.delete_name_server_tags_recursor(hc.get_name_server_tags_recursor())