Merge pull request #1915 from indrajitr/pdns-port-round2

dns: T5115: Support custom port for name servers for forwarding zones

7 files changed, 83 insertions, 30 deletions

diff --git a/data/templates/dns-forwarding/recursor.forward-zones.conf.j2 b/data/templates/dns-forwarding/recursor.forward-zones.conf.j2
index de3269e47..593a98c24 100644
--- a/data/templates/dns-forwarding/recursor.forward-zones.conf.j2
+++ b/data/templates/dns-forwarding/recursor.forward-zones.conf.j2
@@ -23,7 +23,6 @@
 {% if forward_zones is vyos_defined %}
 # zones added via 'service dns forwarding domain'
 {%     for zone, zonedata in forward_zones.items() %}
-{{ "+" if zonedata.recursion_desired is vyos_defined }}{{ zone | replace('_', '-') }}={{ zonedata.server | join(', ') }}
+{{ "+" if zonedata.recursion_desired is vyos_defined }}{{ zone | replace('_', '-') }}={{ zonedata.name_server | join(', ') }}
 {%     endfor %}
 {% endif %}
-
diff --git a/interface-definitions/dns-forwarding.xml.in b/interface-definitions/dns-forwarding.xml.in
index b23eaa351..14b38b24d 100644
--- a/interface-definitions/dns-forwarding.xml.in
+++ b/interface-definitions/dns-forwarding.xml.in
@@ -85,24 +85,7 @@
                   <help>Domain to forward to a custom DNS server</help>
                 </properties>
                 <children>
-                  <leafNode name="server">
-                    <properties>
-                      <help>Domain Name Server (DNS) to forward queries to</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>
-                      <multi/>
-                      <constraint>
-                        <validator name="ipv4-address"/>
-                        <validator name="ipv6-address"/>
-                      </constraint>
-                    </properties>
-                  </leafNode>
+                  #include <include/name-server-ipv4-ipv6-port.xml.i>
                   <leafNode name="addnta">
                     <properties>
                       <help>Add NTA (negative trust anchor) for this domain (must be set if the domain does not support DNSSEC)</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
index cf86e66a2..fb0a4f4ae 100644
--- a/interface-definitions/include/name-server-ipv4-ipv6-port.xml.i
+++ b/interface-definitions/include/name-server-ipv4-ipv6-port.xml.i
@@ -1,7 +1,7 @@
 <!-- include start from name-server-ipv4-ipv6-port.xml.i -->
 <tagNode name="name-server">
   <properties>
-    <help>Domain Name Servers (DNS) addresses</help>
+    <help>Domain Name Servers (DNS) addresses to forward queries to</help>
     <valueHelp>
       <format>ipv4</format>
       <description>Domain Name Server (DNS) IPv4 address</description>
diff --git a/smoketest/scripts/cli/test_service_dns_forwarding.py b/smoketest/scripts/cli/test_service_dns_forwarding.py
index 04dced292..88492e348 100755
--- a/smoketest/scripts/cli/test_service_dns_forwarding.py
+++ b/smoketest/scripts/cli/test_service_dns_forwarding.py
@@ -169,10 +169,13 @@ class TestServicePowerDNS(VyOSUnitTestSHIM.TestCase):
             self.cli_set(base_path + ['listen-address', address])
 
         domains = ['vyos.io', 'vyos.net', 'vyos.com']
-        nameservers = ['192.0.2.1', '192.0.2.2']
+        nameservers = {'192.0.2.1': {}, '192.0.2.2': {'port': '53'}, '2001:db8::1': {'port': '853'}}
         for domain in domains:
-            for nameserver in nameservers:
-                self.cli_set(base_path + ['domain', domain, 'server', nameserver])
+            for h,p in nameservers.items():
+                if 'port' in p:
+                    self.cli_set(base_path + ['domain', domain, 'name-server', h, 'port', p['port']])
+                else:
+                    self.cli_set(base_path + ['domain', domain, 'name-server', h])
 
             # Test 'recursion-desired' flag for only one domain
             if domain == domains[0]:
@@ -192,7 +195,9 @@ class TestServicePowerDNS(VyOSUnitTestSHIM.TestCase):
             if domain == domains[0]: key =f'\+{domain}'
             else: key =f'{domain}'
             tmp = get_config_value(key, 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))
 
             # Test 'negative trust anchor' flag for the second domain only
             if domain == domains[1]:
diff --git a/src/conf_mode/dns_forwarding.py b/src/conf_mode/dns_forwarding.py
index 4d6b85d92..36c1098fe 100755
--- a/src/conf_mode/dns_forwarding.py
+++ b/src/conf_mode/dns_forwarding.py
@@ -59,6 +59,7 @@ def get_config(config=None):
     # 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']
+    del default_values['domain']['name_server']
     dns = dict_merge(default_values, dns)
 
     # T2665: we cleared default values for tag node 'name_server' above.
@@ -68,6 +69,15 @@ def get_config(config=None):
         for server in dns['name_server']:
             dns['name_server'][server] = dict_merge(default_values, dns['name_server'][server])
 
+    # T2665: we cleared default values for tag node 'domain' above.
+    # We now need to add them back back in a granular way.
+    if 'domain' in dns:
+        default_values = defaults(base + ['domain', 'name-server'])
+        for domain in dns['domain'].keys():
+            for server in dns['domain'][domain]['name_server']:
+                dns['domain'][domain]['name_server'][server] = dict_merge(
+                    default_values, dns['domain'][domain]['name_server'][server])
+
     # some additions to the default dictionary
     if 'system' in dns:
         base_nameservers = ['system', 'name-server']
@@ -271,7 +281,7 @@ def verify(dns):
     # as a domain will contains dot's which is out dictionary delimiter.
     if 'domain' in dns:
         for domain in dns['domain']:
-            if 'server' not in dns['domain'][domain]:
+            if 'name_server' not in dns['domain'][domain]:
                 raise ConfigError(f'No server configured for domain {domain}!')
 
     if 'dns64_prefix' in dns:
@@ -337,9 +347,9 @@ def apply(dns):
         # sources
         hc.delete_name_servers([hostsd_tag])
         if 'name_server' in dns:
-            # 'name_server' is a dict of the form
+            # 'name_server' is 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
+            # canonicalize them as ['192.0.2.1:53', '[2001:db8::1]:853', ...]
             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})
@@ -371,7 +381,14 @@ def apply(dns):
         # the list and keys() are required as get returns a dict, not list
         hc.delete_forward_zones(list(hc.get_forward_zones().keys()))
         if 'domain' in dns:
-            hc.add_forward_zones(dns['domain'])
+            zones = dns['domain']
+            for domain in zones.keys():
+                # 'name_server' is 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', ...]
+                zones[domain]['name_server'] = [(lambda h, p: f"{bracketize_ipv6(h)}:{p['port']}")(h, p)
+                                                for (h, p) in zones[domain]['name_server'].items()]
+            hc.add_forward_zones(zones)
 
         # hostsd generates NTAs for the authoritative zones
         # the list and keys() are required as get returns a dict, not list
diff --git a/src/migration-scripts/dns-forwarding/3-to-4 b/src/migration-scripts/dns-forwarding/3-to-4
new file mode 100755
index 000000000..55165c2c5
--- /dev/null
+++ b/src/migration-scripts/dns-forwarding/3-to-4
@@ -0,0 +1,49 @@
+#!/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/>.
+
+# T5115: migrate "service dns forwarding domain example.com server" to
+#                "service dns forwarding domain example.com name-server"
+
+import sys
+from vyos.configtree import ConfigTree
+
+if (len(sys.argv) < 1):
+    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 = ['service', 'dns', 'forwarding', 'domain']
+if not config.exists(base):
+    # Nothing to do
+    sys.exit(0)
+
+for domain in config.list_nodes(base):
+    if config.exists(base + [domain, 'server']):
+        config.copy(base + [domain, 'server'], base + [domain, 'name-server'])
+        config.delete(base + [domain, 'server'])
+
+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/services/vyos-hostsd b/src/services/vyos-hostsd
index a380f2e66..894f9e24d 100755
--- a/src/services/vyos-hostsd
+++ b/src/services/vyos-hostsd
@@ -329,7 +329,7 @@ tag_regex_schema = op_type_schema.extend({
 forward_zone_add_schema = op_type_schema.extend({
     'data': {
         str: {
-            'server': [str],
+            'name_server': [str],
             'addnta': Any({}, None),
             'recursion_desired': Any({}, None),
             }