dhcp: T3100: migrate server configuration to get_config_dict()

4 files changed, 284 insertions, 593 deletions

diff --git a/data/templates/dhcp-server/dhcpd.conf.tmpl b/data/templates/dhcp-server/dhcpd.conf.tmpl
index d172018bf..e8425aa6c 100644
--- a/data/templates/dhcp-server/dhcpd.conf.tmpl
+++ b/data/templates/dhcp-server/dhcpd.conf.tmpl
@@ -5,7 +5,7 @@
 #
 # log-facility local7;
 
-{% if hostfile_update %}
+{% if hostfile_update is defined %}
 on release {
     set ClientName = pick-first-value(host-decl-name, option fqdn.hostname, option host-name);
     set ClientIp = binary-to-ascii(10, 8, ".",leased-address);
@@ -23,170 +23,187 @@ on expiry {
 }
 {% endif %}
 
-{% if host_decl_name %}
-use-host-decl-names on;
-{% endif %}
+{{ 'use-host-decl-names on;' if host_decl_name is defined }}
+ddns-update-style {{ 'interim' if dynamic_dns_update is defined else 'none' }};
 
-ddns-update-style {{ 'interim' if ddns_enable else 'none' }};
-{% if static_route %}
 option rfc3442-static-route code 121 = array of integer 8;
 option windows-static-route code 249 = array of integer 8;
-{% endif %}
-{% if wpad %}
 option wpad-url code 252 = text;
-{% endif %}
 
-{% if global_parameters %}
-# The following {{ global_parameters | length }} line(s) were added as global-parameters in the CLI and have not been validated
-{%   for param in global_parameters %}
-{{ param }}
+{% if global_parameters is defined and global_parameters is not none %}
+# The following {{ global_parameters | length }} line(s) have been added as
+# global-parameters in the CLI and have not been validated !!!
+{%   for parameter in global_parameters %}
+{{ parameter }}
 {%   endfor %}
-{% endif %}
 
+{% endif %}
 # Failover configuration
-{% for network in shared_network %}
-{%   if not network.disabled %}
-{%     for subnet in network.subnet %}
-{%       if subnet.failover_name %}
-failover peer "{{ subnet.failover_name }}" {
-{%         if subnet.failover_status == 'primary' %}
+{% if shared_network_name is defined and shared_network_name is not none %}
+{%   for network, network_config in shared_network_name.items() if network_config.disable is not defined %}
+{%     if network_config.subnet is defined and network_config.subnet is not none %}
+{%       for subnet, subnet_config in network_config.subnet.items() %}
+{%         if subnet_config.failover is defined and subnet_config.failover is defined and subnet_config.failover.name is defined and subnet_config.failover.name is not none %}
+failover peer "{{ subnet_config.failover.name }}" {
+{%           if subnet_config.failover.status == 'primary' %}
     primary;
     mclt 1800;
     split 128;
-{%         elif subnet.failover_status == 'secondary' %}
+{%           elif subnet_config.failover.status == 'secondary' %}
     secondary;
-{%         endif %}
-    address {{ subnet.failover_local_addr }};
+{%           endif %}
+    address {{ subnet_config.failover.local_address }};
     port 520;
-    peer address {{ subnet.failover_peer_addr }};
+    peer address {{ subnet_config.failover.peer_address }};
     peer port 520;
     max-response-delay 30;
     max-unacked-updates 10;
     load balance max seconds 3;
 }
-{%       endif %}
-{%     endfor %}
-{%   endif %}
-{% endfor %}
+{%         endif %}
+{%       endfor %}
+{%     endif %}
+{%   endfor %}
+{% endif %}
 
 # Shared network configration(s)
-{% for network in shared_network if not network.disabled %}
-shared-network {{ network.name }} {
-{%   if network.authoritative %}
+{% if shared_network_name is defined and shared_network_name is not none %}
+{%   for network, network_config in shared_network_name.items() if network_config.disable is not defined %}
+shared-network {{ network | replace('_','-') }} {
+{%     if network_config.authoritative is defined %}
     authoritative;
-{%   endif %}
-{%   if network.network_parameters %}
-    # The following {{ network.network_parameters | length }} line(s) were added as shared-network-parameters in the CLI and have not been validated
-{%     for param in network.network_parameters %}
-    {{ param }}
-{%     endfor %}
-{%   endif %}
-{%   for subnet in network.subnet %}
-    subnet {{ subnet.address }} netmask {{ subnet.netmask }} {
-{%     if subnet.dns_server %}
-        option domain-name-servers {{ subnet.dns_server | join(', ') }};
-{%     endif %}
-{%     if subnet.domain_search %}
-        option domain-search {{ subnet.domain_search | join(', ') }};
-{%     endif %}
-{%     if subnet.ntp_server %}
-        option ntp-servers {{ subnet.ntp_server | join(', ') }};
-{%     endif %}
-{%     if subnet.pop_server %}
-        option pop-server {{ subnet.pop_server | join(', ') }};
-{%     endif %}
-{%     if subnet.smtp_server %}
-        option smtp-server {{ subnet.smtp_server | join(', ') }};
 {%     endif %}
-{%     if subnet.time_server %}
-        option time-servers {{ subnet.time_server | join(', ') }};
-{%     endif %}
-{%     if subnet.wins_server %}
-        option netbios-name-servers {{ subnet.wins_server | join(', ') }};
-{%     endif %}
-{%     if subnet.static_route %}
-        option rfc3442-static-route {{ subnet.static_route }}{% if subnet.rfc3442_default_router %}, {{ subnet.rfc3442_default_router }}{% endif %};
-        option windows-static-route {{ subnet.static_route }};
-{%     endif %}
-{%     if subnet.ip_forwarding %}
-        option ip-forwarding true;
-{%     endif %}
-{%     if subnet.default_router %}
-        option routers {{ subnet.default_router }};
-{%     endif %}
-{%     if subnet.server_identifier %}
-        option dhcp-server-identifier {{ subnet.server_identifier }};
-{%     endif %}
-{%     if subnet.domain_name %}
-        option domain-name "{{ subnet.domain_name }}";
-{%     endif %}
-{%     if subnet.subnet_parameters %}
-        # The following {{ subnet.subnet_parameters | length }} line(s) were added as subnet-parameters in the CLI and have not been validated
-{%       for param in subnet.subnet_parameters %}
-        {{ param }}
+{%     if network_config.shared_network_parameters is defined and network_config.shared_network_parameters is not none %}
+    # The following {{ network_config.shared_network_parameters | length }} line(s)
+    # were added as shared-network-parameters in the CLI and have not been validated
+{%       for parameter in network_config.shared_network_parameters %}
+    {{ parameter }}
 {%       endfor %}
 {%     endif %}
-{%     if subnet.tftp_server %}
-        option tftp-server-name "{{ subnet.tftp_server }}";
-{%     endif %}
-{%     if subnet.bootfile_name %}
-        option bootfile-name "{{ subnet.bootfile_name }}";
-        filename "{{ subnet.bootfile_name }}";
-{%     endif %}
-{%     if subnet.bootfile_server %}
-        next-server {{ subnet.bootfile_server }};
-{%     endif %}
-{%     if subnet.time_offset %}
-        option time-offset {{ subnet.time_offset }};
-{%     endif %}
-{%     if subnet.wpad_url %}
-        option wpad-url "{{ subnet.wpad_url }}";
-{%     endif %}
-{%     if subnet.client_prefix_length %}
-        option subnet-mask {{ subnet.client_prefix_length }};
-{%     endif %}
-{%     if subnet.lease %}
-        default-lease-time {{ subnet.lease }};
-        max-lease-time {{ subnet.lease }};
-{%     endif %}
-{%     for host in subnet.static_mapping if not host.disabled %}
-        host {{ host.name if host_decl_name else network.name + '_' + host.name }} {
-{%       if host.ip_address %}
-            fixed-address {{ host.ip_address }};
-{%       endif %}
-            hardware ethernet {{ host.mac_address }};
-{%       if host.static_parameters %}
-            # The following {{ host.static_parameters | length }} line(s) were added as static-mapping-parameters in the CLI and have not been validated
-{%         for param in host.static_parameters %}
-            {{ param }}
-{%         endfor %}
-{%       endif %}
+{%     if network_config.subnet is defined and network_config.subnet is not none %}
+{%       for subnet, subnet_config in network_config.subnet.items() %}
+    subnet {{ subnet | address_from_cidr }} netmask {{ subnet | netmask_from_cidr }} {
+{%         if subnet_config.dns_server is defined and subnet_config.dns_server is not none %}
+        option domain-name-servers {{ subnet_config.dns_server | join(', ') }};
+{%         endif %}
+{%         if subnet_config.domain_search is defined and subnet_config.domain_search is not none %}
+        option domain-search "{{ subnet_config.domain_search | join(', ') }}";
+{%         endif %}
+{%         if subnet_config.ntp_server is defined and subnet_config.ntp_server is not none %}
+        option ntp-servers {{ subnet_config.ntp_server | join(', ') }};
+{%         endif %}
+{%         if subnet_config.pop_server is defined and subnet_config.pop_server is not none %}
+        option pop-server {{ subnet_config.pop_server | join(', ') }};
+{%         endif %}
+{%         if subnet_config.smtp_server is defined and subnet_config.smtp_server is not none %}
+        option smtp-server {{ subnet_config.smtp_server | join(', ') }};
+{%         endif %}
+{%         if subnet_config.time_server is defined and subnet_config.time_server is not none %}
+        option time-servers {{ subnet_config.time_server | join(', ') }};
+{%         endif %}
+{%         if subnet_config.wins_server is defined and subnet_config.wins_server is not none %}
+        option netbios-name-servers {{ subnet_config.wins_server | join(', ') }};
+{%         endif %}
+{%         if subnet_config.static_route is defined and subnet_config.static_route is not none %}
+{%           set static_default_route = '' %}
+{%           if subnet_config.default_router and subnet_config.default_router is not none %}
+{%             set static_default_route = ', ' + '0.0.0.0/0' | isc_static_route(subnet_config.default_router) %}
+{%           endif %}
+{%           if subnet_config.static_route.router is defined and subnet_config.static_route.router is not none and subnet_config.static_route.destination_subnet is defined and subnet_config.static_route.destination_subnet is not none %}
+        option rfc3442-static-route {{ subnet_config.static_route.destination_subnet | isc_static_route(subnet_config.static_route.router) }}{{ static_default_route }};
+        option windows-static-route {{ subnet_config.static_route.destination_subnet | isc_static_route(subnet_config.static_route.router) }};
+{%           endif %}
+{%         endif %}
+{%         if subnet_config.ip_forwarding is defined %}
+        option ip-forwarding true;
+{%         endif %}
+{%         if subnet_config.default_router and subnet_config.default_router is not none %}
+        option routers {{ subnet_config.default_router }};
+{%         endif %}
+{%         if subnet_config.server_identifier is defined and subnet_config.server_identifier is not none %}
+        option dhcp-server-identifier {{ subnet_config.server_identifier }};
+{%         endif %}
+{%         if subnet_config.domain_name is defined and subnet_config.domain_name is not none %}
+        option domain-name "{{ subnet_config.domain_name }}";
+{%         endif %}
+{%         if subnet_config.subnet_parameters is defined and subnet_config.subnet_parameters is not none %}
+        # The following {{ subnet_config.subnet_parameters | length }} line(s) were added as
+        # subnet-parameters in the CLI and have not been validated!!!
+{%           for parameter in subnet_config.subnet_parameters %}
+        {{ parameter }}
+{%           endfor %}
+{%         endif %}
+{%         if subnet_config.tftp_server_name is defined and subnet_config.tftp_server_name is not none %}
+        option tftp-server-name "{{ subnet_config.tftp_server_name }}";
+{%         endif %}
+{%         if subnet_config.bootfile_name is defined and subnet_config.bootfile_name is not none %}
+        option bootfile-name "{{ subnet_config.bootfile_name }}";
+        filename "{{ subnet_config.bootfile_name }}";
+{%         endif %}
+{%         if subnet_config.bootfile_server is defined and subnet_config.bootfile_server is not none %}
+        next-server {{ subnet_config.bootfile_server }};
+{%         endif %}
+{%         if subnet_config.time_offset is defined and subnet_config.time_offset is not none %}
+        option time-offset {{ subnet_config.time_offset }};
+{%         endif %}
+{%         if subnet_config.wpad_url is defined and subnet_config.wpad_url is not none %}
+        option wpad-url "{{ subnet_config.wpad_url }}";
+{%         endif %}
+{%         if subnet_config.client_prefix_length is defined and subnet_config.client_prefix_length is not none %}
+        option subnet-mask {{ subnet_config.client_prefix_length }};
+{%         endif %}
+{%         if subnet_config.lease is defined and subnet_config.lease is not none %}
+        default-lease-time {{ subnet_config.lease }};
+        max-lease-time {{ subnet_config.lease }};
+{%         endif %}
+{%         if subnet_config.static_mapping is defined and subnet_config.static_mapping is not none %}
+{%           for host, host_config in subnet_config.static_mapping.items() if host_config.disable is not defined %}
+        host {{ host | replace('_','-') if host_decl_name is defined else network | replace('_','-') + '_' + host | replace('_','-') }} {
+{%             if host_config.ip_address is defined and host_config.ip_address is not none %}
+            fixed-address {{ host_config.ip_address }};
+{%             endif %}
+            hardware ethernet {{ host_config.mac_address }};
+{%             if host_config.static_mapping_parameters is defined and host_config.static_mapping_parameters is not none %}
+            # The following {{ host_config.static_mapping_parameters | length }} line(s) were added
+            # as static-mapping-parameters in the CLI and have not been validated
+{%               for parameter in host_config.static_mapping_parameters %}
+            {{ parameter }}
+{%               endfor %}
+{%             endif %}
         }
-{%     endfor %}
-{%     if subnet.failover_name %}
+{%           endfor %}
+{%         endif %}
+{%         if subnet_config.failover is defined and subnet_config.failover.name is defined and subnet_config.failover.name is not none %}
         pool {
-            failover peer "{{ subnet.failover_name }}";
+            failover peer "{{ subnet_config.failover.name }}";
             deny dynamic bootp clients;
-            {% for range in subnet.range %}
-            range {{ range.start }} {{ range.stop }};
-            {% endfor %}
+{%           if subnet_config.range is defined and subnet_config.range is not none %}
+{%             for range, range_options in subnet_config.range.items() %}
+            range {{ range_options.start }} {{ range_options.stop }};
+{%             endfor %}
+{%           endif %}
         }
-{%     else %}
-{%       for range in subnet.range %}
-        range {{ range.start }} {{ range.stop }};
+{%         else %}
+{%           if subnet_config.range is defined and subnet_config.range is not none %}
+{%             for range, range_options in subnet_config.range.items() %}
+        range {{ range_options.start }} {{ range_options.stop }};
+{%             endfor %}
+{%           endif %}
+{%         endif %}
+    }
 {%       endfor %}
 {%     endif %}
-    }
-{%   endfor %}
     on commit {
-        set shared-networkname = "{{ network.name }}";
-{%   if hostfile_update %}
+        set shared-networkname = "{{ network | replace('_','-') }}";
+{%     if hostfile_update is defined %}
         set ClientName = pick-first-value(host-decl-name, option fqdn.hostname, option host-name);
         set ClientIp = binary-to-ascii(10, 8, ".", leased-address);
         set ClientMac = binary-to-ascii(16, 8, ":", substring(hardware, 1, 6));
         set ClientDomain = pick-first-value(config-option domain-name, "..YYZ!");
         execute("/usr/libexec/vyos/system/on-dhcp-event.sh", "commit", ClientName, ClientIp, ClientMac, ClientDomain);
-{%   endif %}
+{%     endif %}
     }
 }
-{% endfor %}
+
+{%   endfor %}
+{% endif %}
diff --git a/interface-definitions/dhcp-server.xml.in b/interface-definitions/dhcp-server.xml.in
index 978118b31..2f78f11ea 100644
--- a/interface-definitions/dhcp-server.xml.in
+++ b/interface-definitions/dhcp-server.xml.in
@@ -232,6 +232,7 @@
                       </constraint>
                       <constraintErrorMessage>DHCP lease time must be between 0 and 4294967295 (49 days)</constraintErrorMessage>
                     </properties>
+                    <defaultValue>86400</defaultValue>
                   </leafNode>
                   <leafNode name="ntp-server">
                     <properties>
diff --git a/python/vyos/template.py b/python/vyos/template.py
index b31f5bea2..5993ffd95 100644
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -214,3 +214,26 @@ def dec_ip(address, decrement):
     """
     from ipaddress import ip_interface
     return str(ip_interface(address).ip - int(decrement))
+
+
+@register_filter('isc_static_route')
+def isc_static_route(subnet, router):
+    # https://ercpe.de/blog/pushing-static-routes-with-isc-dhcp-server
+    # Option format is:
+    # <netmask>, <network-byte1>, <network-byte2>, <network-byte3>, <router-byte1>, <router-byte2>, <router-byte3>
+    # where bytes with the value 0 are omitted.
+    from ipaddress import ip_network
+    net = ip_network(subnet)
+    # add netmask
+    string = str(net.prefixlen) + ','
+    # add network bytes
+    if net.prefixlen:
+        width = net.prefixlen // 8
+        if net.prefixlen % 8:
+            width += 1
+        string += ','.join(map(str,tuple(net.network_address.packed)[:width])) + ','
+
+    # add router bytes
+    string += ','.join(router.split('.'))
+
+    return string
diff --git a/src/conf_mode/dhcp_server.py b/src/conf_mode/dhcp_server.py
index fd4e2ec61..6df9d4a25 100755
--- a/src/conf_mode/dhcp_server.py
+++ b/src/conf_mode/dhcp_server.py
@@ -16,32 +16,22 @@
 
 import os
 
-from ipaddress import ip_address, ip_network
-from socket import inet_ntoa
-from struct import pack
+from ipaddress import ip_address
+from ipaddress import ip_network
 from sys import exit
 
 from vyos.config import Config
+from vyos.configdict import dict_merge
+from vyos.template import render
+from vyos.util import call
+from vyos.util import dict_search
 from vyos.validate import is_subnet_connected
+from vyos.xml import defaults
 from vyos import ConfigError
-from vyos.template import render
-from vyos.util import call, chown
-
 from vyos import airbag
 airbag.enable()
 
-config_file = r'/run/dhcp-server/dhcpd.conf'
-
-default_config_data = {
-    'disabled': False,
-    'ddns_enable': False,
-    'global_parameters': [],
-    'hostfile_update': False,
-    'host_decl_name': False,
-    'static_route': False,
-    'wpad': False,
-    'shared_network': [],
-}
+config_file = '/run/dhcp-server/dhcpd.conf'
 
 def dhcp_slice_range(exclude_list, range_list):
     """
@@ -106,356 +96,37 @@ def dhcp_slice_range(exclude_list, range_list):
 
     return output
 
-def dhcp_static_route(static_subnet, static_router):
-    # https://ercpe.de/blog/pushing-static-routes-with-isc-dhcp-server
-    # Option format is:
-    # <netmask>, <network-byte1>, <network-byte2>, <network-byte3>, <router-byte1>, <router-byte2>, <router-byte3>
-    # where bytes with the value 0 are omitted.
-    net = ip_network(static_subnet)
-    # add netmask
-    string = str(net.prefixlen) + ','
-    # add network bytes
-    if net.prefixlen:
-        width = net.prefixlen // 8
-        if net.prefixlen % 8:
-            width += 1
-        string += ','.join(map(str,tuple(net.network_address.packed)[:width])) + ','
-
-    # add router bytes
-    string += ','.join(static_router.split('.'))
-
-    return string
-
 def get_config(config=None):
-    dhcp = default_config_data
     if config:
         conf = config
     else:
         conf = Config()
-    if not conf.exists('service dhcp-server'):
+
+    base = ['service', 'dhcp-server']
+    if not conf.exists(base):
         return None
-    else:
-        conf.set_level('service dhcp-server')
-
-    # check for global disable of DHCP service
-    if conf.exists('disable'):
-        dhcp['disabled'] = True
-
-    # check for global dynamic DNS upste
-    if conf.exists('dynamic-dns-update'):
-        dhcp['ddns_enable'] = True
-
-    # HACKS AND TRICKS
-    #
-    # check for global 'raw' ISC DHCP parameters configured by users
-    # actually this is a bad idea in general to pass raw parameters from any user
-    if conf.exists('global-parameters'):
-        dhcp['global_parameters'] = conf.return_values('global-parameters')
-
-    # check for global DHCP server updating /etc/host per lease
-    if conf.exists('hostfile-update'):
-        dhcp['hostfile_update'] = True
-
-    # If enabled every host declaration within that scope, the name provided
-    # for the host declaration will be supplied to the client as its hostname.
-    if conf.exists('host-decl-name'):
-        dhcp['host_decl_name'] = True
-
-    # check for multiple, shared networks served with DHCP addresses
-    if conf.exists('shared-network-name'):
-        for network in conf.list_nodes('shared-network-name'):
-            conf.set_level('service dhcp-server shared-network-name {0}'.format(network))
-            config = {
-                'name': network,
-                'authoritative': False,
-                'description': '',
-                'disabled': False,
-                'network_parameters': [],
-                'subnet': []
-            }
-            # check if DHCP server should be authoritative on this network
-            if conf.exists('authoritative'):
-                config['authoritative'] = True
-
-            # A description for this given network
-            if conf.exists('description'):
-                config['description'] = conf.return_value('description')
-
-            # If disabled, the shared-network configuration becomes inactive in
-            # the running DHCP server instance
-            if conf.exists('disable'):
-                config['disabled'] = True
-
-            # HACKS AND TRICKS
-            #
-            # check for 'raw' ISC DHCP parameters configured by users
-            # actually this is a bad idea in general to pass raw parameters
-            # from any user
-            #
-            # deprecate this and issue a warning like we do for DNS forwarding?
-            if conf.exists('shared-network-parameters'):
-                config['network_parameters'] = conf.return_values('shared-network-parameters')
-
-            # check for multiple subnet configurations in a shared network
-            # config segment
-            if conf.exists('subnet'):
-                for net in conf.list_nodes('subnet'):
-                    conf.set_level('service dhcp-server shared-network-name {0} subnet {1}'.format(network, net))
-                    subnet = {
-                        'network': net,
-                        'address': str(ip_network(net).network_address),
-                        'netmask': str(ip_network(net).netmask),
-                        'bootfile_name': '',
-                        'bootfile_server': '',
-                        'client_prefix_length': '',
-                        'default_router': '',
-                        'rfc3442_default_router': '',
-                        'dns_server': [],
-                        'domain_name': '',
-                        'domain_search': [],
-                        'exclude': [],
-                        'failover_local_addr': '',
-                        'failover_name': '',
-                        'failover_peer_addr': '',
-                        'failover_status': '',
-                        'ip_forwarding': False,
-                        'lease': '86400',
-                        'ntp_server': [],
-                        'pop_server': [],
-                        'server_identifier': '',
-                        'smtp_server': [],
-                        'range': [],
-                        'static_mapping': [],
-                        'static_subnet': '',
-                        'static_router': '',
-                        'static_route': '',
-                        'subnet_parameters': [],
-                        'tftp_server': '',
-                        'time_offset': '',
-                        'time_server': [],
-                        'wins_server': [],
-                        'wpad_url': ''
-                    }
 
-                    # Used to identify a bootstrap file
-                    if conf.exists('bootfile-name'):
-                        subnet['bootfile_name'] = conf.return_value('bootfile-name')
-
-                    # Specify host address of the server from which the initial boot file
-                    # (specified above) is to be loaded. Should be a numeric IP address or
-                    # domain name.
-                    if conf.exists('bootfile-server'):
-                        subnet['bootfile_server'] = conf.return_value('bootfile-server')
-
-                    # The subnet mask option specifies the client's subnet mask as per RFC 950. If no subnet
-                    # mask option is provided anywhere in scope, as a last resort dhcpd will use the subnet
-                    # mask from the subnet declaration for the network on which an address is being assigned.
-                    if conf.exists('client-prefix-length'):
-                        # snippet borrowed from https://stackoverflow.com/questions/33750233/convert-cidr-to-subnet-mask-in-python
-                        host_bits = 32 - int(conf.return_value('client-prefix-length'))
-                        subnet['client_prefix_length'] = inet_ntoa(pack('!I', (1 << 32) - (1 << host_bits)))
-
-                    # Default router IP address on the client's subnet
-                    if conf.exists('default-router'):
-                        subnet['default_router'] = conf.return_value('default-router')
-                        subnet['rfc3442_default_router'] = dhcp_static_route("0.0.0.0/0", subnet['default_router'])
-
-                    # Specifies a list of Domain Name System (STD 13, RFC 1035) name servers available to
-                    # the client. Servers should be listed in order of preference.
-                    if conf.exists('dns-server'):
-                        subnet['dns_server'] = conf.return_values('dns-server')
-
-                    # Option specifies the domain name that client should use when resolving hostnames
-                    # via the Domain Name System.
-                    if conf.exists('domain-name'):
-                        subnet['domain_name'] = conf.return_value('domain-name')
-
-                    # The domain-search option specifies a 'search list' of Domain Names to be used
-                    # by the client to locate not-fully-qualified domain names.
-                    if conf.exists('domain-search'):
-                        for domain in conf.return_values('domain-search'):
-                            subnet['domain_search'].append('"' + domain + '"')
-
-                    # IP address (local) for failover peer to connect
-                    if conf.exists('failover local-address'):
-                        subnet['failover_local_addr'] = conf.return_value('failover local-address')
-
-                    # DHCP failover peer name
-                    if conf.exists('failover name'):
-                        subnet['failover_name'] = conf.return_value('failover name')
-
-                    # IP address (remote) of failover peer
-                    if conf.exists('failover peer-address'):
-                        subnet['failover_peer_addr'] = conf.return_value('failover peer-address')
-
-                    # DHCP failover peer status (primary|secondary)
-                    if conf.exists('failover status'):
-                        subnet['failover_status'] = conf.return_value('failover status')
-
-                    # Option specifies whether the client should configure its IP layer for packet
-                    # forwarding
-                    if conf.exists('ip-forwarding'):
-                        subnet['ip_forwarding'] = True
-
-                    # Time should be the length in seconds that will be assigned to a lease if the
-                    # client requesting the lease does not ask for a specific expiration time
-                    if conf.exists('lease'):
-                        subnet['lease'] = conf.return_value('lease')
-
-                    # Specifies a list of IP addresses indicating NTP (RFC 5905) servers available
-                    # to the client.
-                    if conf.exists('ntp-server'):
-                        subnet['ntp_server'] = conf.return_values('ntp-server')
-
-                    # POP3 server option specifies a list of POP3 servers available to the client.
-                    # Servers should be listed in order of preference.
-                    if conf.exists('pop-server'):
-                        subnet['pop_server'] = conf.return_values('pop-server')
-
-                    # DHCP servers include this option in the DHCPOFFER in order to allow the client
-                    # to distinguish between lease offers. DHCP clients use the contents of the
-                    # 'server identifier' field as the destination address for any DHCP messages
-                    # unicast to the DHCP server
-                    if conf.exists('server-identifier'):
-                        subnet['server_identifier'] = conf.return_value('server-identifier')
-
-                    # SMTP server option specifies a list of SMTP servers available to the client.
-                    # Servers should be listed in order of preference.
-                    if conf.exists('smtp-server'):
-                        subnet['smtp_server'] = conf.return_values('smtp-server')
-
-                    # For any subnet on which addresses will be assigned dynamically, there must be at
-                    # least one range statement. The range statement gives the lowest and highest IP
-                    # addresses in a range. All IP addresses in the range should be in the subnet in
-                    # which the range statement is declared.
-                    if conf.exists('range'):
-                        for range in conf.list_nodes('range'):
-                            range = {
-                                'start': conf.return_value('range {0} start'.format(range)),
-                                'stop':  conf.return_value('range {0} stop'.format(range))
-                            }
-                            subnet['range'].append(range)
-
-                    # IP address that needs to be excluded from DHCP lease range
-                    if conf.exists('exclude'):
-                        subnet['exclude'] = conf.return_values('exclude')
-                        subnet['range'] = dhcp_slice_range(subnet['exclude'], subnet['range'])
-
-                    # Static DHCP leases
-                    if conf.exists('static-mapping'):
-                        addresses_for_exclude = []
-                        for mapping in conf.list_nodes('static-mapping'):
-                            conf.set_level('service dhcp-server shared-network-name {0} subnet {1} static-mapping {2}'.format(network, net, mapping))
-                            mapping = {
-                                'name': mapping,
-                                'disabled': False,
-                                'ip_address': '',
-                                'mac_address': '',
-                                'static_parameters': []
-                            }
-
-                            # This static lease is disabled
-                            if conf.exists('disable'):
-                                mapping['disabled'] = True
-
-                            # IP address used for this DHCP client
-                            if conf.exists('ip-address'):
-                                mapping['ip_address'] = conf.return_value('ip-address')
-                                addresses_for_exclude.append(mapping['ip_address'])
-
-                            # MAC address of requesting DHCP client
-                            if conf.exists('mac-address'):
-                                mapping['mac_address'] = conf.return_value('mac-address')
-
-                            # HACKS AND TRICKS
-                            #
-                            # check for 'raw' ISC DHCP parameters configured by users
-                            # actually this is a bad idea in general to pass raw parameters
-                            # from any user
-                            #
-                            # deprecate this and issue a warning like we do for DNS forwarding?
-                            if conf.exists('static-mapping-parameters'):
-                                mapping['static_parameters'] = conf.return_values('static-mapping-parameters')
-
-                            # append static-mapping configuration to subnet list
-                            subnet['static_mapping'].append(mapping)
-
-                        # Now we have all static DHCP leases - we also need to slice them
-                        # out of our DHCP ranges to avoid ISC DHCPd warnings as:
-                        #   dhcpd: Dynamic and static leases present for 192.0.2.51.
-                        #   dhcpd: Remove host declaration DMZ_PC1 or remove 192.0.2.51
-                        #   dhcpd: from the dynamic address pool for DMZ
-                        subnet['range'] = dhcp_slice_range(addresses_for_exclude, subnet['range'])
-
-                    # Reset config level to matching hirachy
-                    conf.set_level('service dhcp-server shared-network-name {0} subnet {1}'.format(network, net))
-
-                    # This option specifies a list of static routes that the client should install in its routing
-                    # cache. If multiple routes to the same destination are specified, they are listed in descending
-                    # order of priority.
-                    if conf.exists('static-route destination-subnet'):
-                        subnet['static_subnet'] = conf.return_value('static-route destination-subnet')
-                        # Required for global config section
-                        dhcp['static_route'] = True
-
-                    if conf.exists('static-route router'):
-                        subnet['static_router'] = conf.return_value('static-route router')
-
-                    if subnet['static_router'] and subnet['static_subnet']:
-                        subnet['static_route'] = dhcp_static_route(subnet['static_subnet'], subnet['static_router'])
-
-                    # HACKS AND TRICKS
-                    #
-                    # check for 'raw' ISC DHCP parameters configured by users
-                    # actually this is a bad idea in general to pass raw parameters
-                    # from any user
-                    #
-                    # deprecate this and issue a warning like we do for DNS forwarding?
-                    if conf.exists('subnet-parameters'):
-                        subnet['subnet_parameters'] = conf.return_values('subnet-parameters')
-
-                    # This option is used to identify a TFTP server and, if supported by the client, should have
-                    # the same effect as the server-name declaration. BOOTP clients are unlikely to support this
-                    # option. Some DHCP clients will support it, and others actually require it.
-                    if conf.exists('tftp-server-name'):
-                        subnet['tftp_server'] = conf.return_value('tftp-server-name')
-
-                    # The time-offset option specifies the offset of the client’s subnet in seconds from
-                    # Coordinated Universal Time (UTC).
-                    if conf.exists('time-offset'):
-                        subnet['time_offset'] = conf.return_value('time-offset')
-
-                    # The time-server option specifies a list of RFC 868 time servers available to the client.
-                    # Servers should be listed in order of preference.
-                    if conf.exists('time-server'):
-                        subnet['time_server'] = conf.return_values('time-server')
-
-                    # The NetBIOS name server (NBNS) option specifies a list of RFC 1001/1002 NBNS name servers
-                    # listed in order of preference. NetBIOS Name Service is currently more commonly referred to
-                    # as WINS. WINS servers can be specified using the netbios-name-servers option.
-                    if conf.exists('wins-server'):
-                        subnet['wins_server'] = conf.return_values('wins-server')
-
-                    # URL for Web Proxy Autodiscovery Protocol
-                    if conf.exists('wpad-url'):
-                        subnet['wpad_url'] = conf.return_value('wpad-url')
-                        # Required for global config section
-                        dhcp['wpad'] = True
-
-                    # append subnet configuration to shared network subnet list
-                    config['subnet'].append(subnet)
-
-            # append shared network configuration to config dictionary
-            dhcp['shared_network'].append(config)
+    dhcp = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
+
+    # T2665: defaults include lease time per TAG node which need to be added to
+    # individual subnet definitions
+    default_values = defaults(base + ['shared-network-name', 'subnet'])
+    for network, network_config in (dict_search('shared_network_name', dhcp).items() or {}):
+        print(network)
+        for subnet, subnet_config in (dict_search('subnet', network_config).items() or {}):
+            if 'lease' not in subnet_config:
+                dhcp['shared_network_name'][network]['subnet'][subnet] = dict_merge(
+                    default_values, dhcp['shared_network_name'][network]['subnet'][subnet])
 
     return dhcp
 
 def verify(dhcp):
-    if not dhcp or dhcp['disabled']:
+    # bail out early - looks like removal from running config
+    if not dhcp or 'disable' in dhcp:
         return None
 
     # If DHCP is enabled we need one share-network
-    if len(dhcp['shared_network']) == 0:
+    if 'shared_network_name' not in dhcp:
         raise ConfigError('No DHCP shared networks configured.\n' \
                           'At least one DHCP shared network must be configured.')
 
@@ -465,139 +136,117 @@ def verify(dhcp):
     subnets = []
 
     # A shared-network requires a subnet definition
-    for network in dhcp['shared_network']:
-        if len(network['subnet']) == 0:
-            raise ConfigError('No DHCP lease subnets configured for {0}. At least one\n' \
-                              'lease subnet must be configured for each shared network.'.format(network['name']))
-
-        for subnet in network['subnet']:
-            # Subnet static route declaration requires destination and router
-            if subnet['static_subnet'] or subnet['static_router']:
-                if not (subnet['static_subnet'] and subnet['static_router']):
-                    raise ConfigError('Please specify missing DHCP static-route parameter(s):\n' \
-                                      'destination-subnet | router')
-
-            # Failover requires all 4 parameters set
-            if subnet['failover_local_addr'] or subnet['failover_peer_addr'] or subnet['failover_name'] or subnet['failover_status']:
-                if not (subnet['failover_local_addr'] and subnet['failover_peer_addr'] and subnet['failover_name'] and subnet['failover_status']):
-                    raise ConfigError('Please specify missing DHCP failover parameter(s):\n' \
-                                      'local-address | peer-address | name | status')
+    for network, network_config in dhcp['shared_network_name'].items():
+        if 'subnet' not in network_config:
+            raise ConfigError(f'No subnets defined for {network}. At least one\n' \
+                              'lease subnet must be configured.')
+
+        for subnet, subnet_config in network_config['subnet'].items():
+            if 'static_route' in subnet_config and len(subnet_config['static_route']) != 2:
+                raise ConfigError('Missing DHCP static-route parameter(s):\n' \
+                                  'destination-subnet | router must be defined!')
+
+            # Check if DHCP address range is inside configured subnet declaration
+            if 'range' in subnet_config:
+                range_start = []
+                range_stop = []
+                for range, range_config in subnet_config['range'].items():
+                    if not {'start', 'stop'} <= set(range_config):
+                        raise ConfigError(f'DHCP range "{range}" start and stop address must be defined!')
+
+                    # Start/Stop address must be inside network
+                    for key in ['start', 'stop']:
+                        if ip_address(range_config[key]) not in ip_network(subnet):
+                            raise ConfigError(f'DHCP range "{range}" {key} address not within shared-network "{network}, {subnet}"!')
+
+                    # Stop address must be greater or equal to start address
+                    if ip_address(range_config['stop']) < ip_address(range_config['start']):
+                        raise ConfigError(f'DHCP range "{range}" stop address must be greater or equal\n' \
+                                          'to the ranges start address!')
+
+                    # Range start address must be unique
+                    if range_config['start'] in range_start:
+                        raise ConfigError('Conflicting DHCP lease range: Pool start\n' \
+                                          'address "{start}" defined multipe times!'.format(range_config))
+
+                    # Range stop address must be unique
+                    if range_config['stop'] in range_start:
+                        raise ConfigError('Conflicting DHCP lease range: Pool stop\n' \
+                                          'address "{stop}" defined multipe times!'.format(range_config))
+
+                    range_start.append(range_config['start'])
+                    range_stop.append(range_config['stop'])
+
+            if 'failover' in subnet_config:
+                for key in ['local_address', 'peer_address', 'name', 'status']:
+                    if key not in subnet_config['failover']:
+                        raise ConfigError(f'Missing DHCP failover parameter "{key}"!')
 
                 # Failover names must be uniquie
-                if subnet['failover_name'] in failover_names:
-                    raise ConfigError('Failover names must be unique:\n' \
-                                      '{0} has already been configured!'.format(subnet['failover_name']))
-                else:
-                    failover_names.append(subnet['failover_name'])
+                if subnet_config['failover']['name'] in failover_names:
+                    name = subnet_config['failover']['name']
+                    raise ConfigError(f'DHCP failover names must be unique:\n' \
+                                      f'{name} has already been configured!')
+                failover_names.append(subnet_config['failover']['name'])
 
                 # Failover requires start/stop ranges for pool
-                if (len(subnet['range']) == 0):
-                    raise ConfigError('At least one start-stop range must be configured for {0}\n' \
-                                      'to set up DHCP failover!'.format(subnet['network']))
-
-            # Check if DHCP address range is inside configured subnet declaration
-            range_start = []
-            range_stop = []
-            for range in subnet['range']:
-                start = range['start']
-                stop = range['stop']
-                # DHCP stop IP required after start IP
-                if start and not stop:
-                    raise ConfigError('DHCP range stop address for start {0} is not defined!'.format(start))
-
-                # Start address must be inside network
-                if not ip_address(start) in ip_network(subnet['network']):
-                    raise ConfigError('DHCP range start address {0} is not in subnet {1}\n' \
-                                      'specified for shared network {2}!'.format(start, subnet['network'], network['name']))
-
-                # Stop address must be inside network
-                if not ip_address(stop) in ip_network(subnet['network']):
-                    raise ConfigError('DHCP range stop address {0} is not in subnet {1}\n' \
-                                      'specified for shared network {2}!'.format(stop, subnet['network'], network['name']))
-
-                # Stop address must be greater or equal to start address
-                if not ip_address(stop) >= ip_address(start):
-                    raise ConfigError('DHCP range stop address {0} must be greater or equal\n' \
-                                      'to the range start address {1}!'.format(stop, start))
-
-                # Range start address must be unique
-                if start in range_start:
-                    raise ConfigError('Conflicting DHCP lease range:\n' \
-                                      'Pool start address {0} defined multipe times!'.format(start))
-                else:
-                    range_start.append(start)
-
-                # Range stop address must be unique
-                if stop in range_stop:
-                    raise ConfigError('Conflicting DHCP lease range:\n' \
-                                      'Pool stop address {0} defined multipe times!'.format(stop))
-                else:
-                    range_stop.append(stop)
+                if 'range' not in subnet_config:
+                    raise ConfigError(f'DHCP failover requires at least one start-stop range to be configured\n'\
+                                      f'within shared-network "{network}, {subnet}" for using failover!')
 
             # Exclude addresses must be in bound
-            for exclude in subnet['exclude']:
-                if not ip_address(exclude) in ip_network(subnet['network']):
-                    raise ConfigError('Exclude IP address {0} is outside of the DHCP lease network {1}\n' \
-                                      'under shared network {2}!'.format(exclude, subnet['network'], network['name']))
+            if 'exclude' in subnet_config:
+                for exclude in subnet_config['exclude']:
+                    if ip_address(exclude) not in ip_network(subnet):
+                        raise ConfigError(f'Excluded IP address "{exclude}" not within shared-network "{network}, {subnet}"!')
 
             # At least one DHCP address range or static-mapping required
-            active_mapping = False
-            if (len(subnet['range']) == 0):
-                for mapping in subnet['static_mapping']:
-                    # we need at least one active mapping
-                    if (not active_mapping) and (not mapping['disabled']):
-                        active_mapping = True
-            else:
-                active_mapping = True
-
-            if not active_mapping:
-                raise ConfigError('No DHCP address range or active static-mapping set\n' \
-                                  'for subnet {0}!'.format(subnet['network']))
-
-            # Static mappings require just a MAC address (will use an IP from the dynamic pool if IP is not set)
-            for mapping in subnet['static_mapping']:
-
-                if mapping['ip_address']:
-                    # Static IP address must be in bound
-                    if not ip_address(mapping['ip_address']) in ip_network(subnet['network']):
-                        raise ConfigError('DHCP static lease IP address {0} for static mapping {1}\n' \
-                                          'in shared network {2} is outside DHCP lease subnet {3}!' \
-                                          .format(mapping['ip_address'], mapping['name'], network['name'], subnet['network']))
-
-                # Static mapping requires MAC address
-                if not mapping['mac_address']:
-                    raise ConfigError('DHCP static lease MAC address not specified for static mapping\n' \
-                                       '{0} under shared network name {1}!'.format(mapping['name'], network['name']))
+            if 'range' not in subnet_config and 'static_mapping' not in subnet_config:
+                raise ConfigError(f'No DHCP address range or active static-mapping configured\n' \
+                                  f'within shared-network "{network}, {subnet}"!')
+
+            if 'static_mapping' in subnet_config:
+                # Static mappings require just a MAC address (will use an IP from the dynamic pool if IP is not set)
+                for mapping, mapping_config in subnet_config['static_mapping'].items():
+                    if 'ip_address' in mapping_config:
+                        if ip_address(mapping_config['ip_address']) not in ip_network(subnet):
+                            raise ConfigError(f'Configured static lease address for mapping "{mapping}" is\n' \
+                                              f'not within shared-network "{network}, {subnet}"!')
+
+                        if 'mac_address' not in mapping_config:
+                            raise ConfigError(f'MAC address required for static mapping "{mapping}"\n' \
+                                              f'within shared-network "{network}, {subnet}"!')
 
             # There must be one subnet connected to a listen interface.
             # This only counts if the network itself is not disabled!
-            if not network['disabled']:
-                if is_subnet_connected(subnet['network'], primary=True):
+            if 'disable' not in network_config:
+                if is_subnet_connected(subnet, primary=True):
                     listen_ok = True
 
             # Subnets must be non overlapping
-            if subnet['network'] in subnets:
-                raise ConfigError('DHCP subnets must be unique! Subnet {0} defined multiple times!'.format(subnet['network']))
-            else:
-                subnets.append(subnet['network'])
+            if subnet in subnets:
+                raise ConfigError(f'Configured subnets must be unique! Subnet "{subnet}"\n'
+                                   'defined multiple times!')
+            subnets.append(subnet)
 
             # Check for overlapping subnets
-            net = ip_network(subnet['network'])
+            net = ip_network(subnet)
             for n in subnets:
                 net2 = ip_network(n)
                 if (net != net2):
                     if net.overlaps(net2):
-                        raise ConfigError('DHCP conflicting subnet ranges: {0} overlaps {1}'.format(net, net2))
+                        raise ConfigError('Conflicting subnet ranges: "{net}" overlaps "{net2}"!')
 
     if not listen_ok:
-        raise ConfigError('DHCP server configuration error!\n' \
-                          'None of configured DHCP subnets does not have appropriate\n' \
-                          'primary IP address on any broadcast interface.')
+        raise ConfigError('DHCP server configuration error! None of the configured\n' \
+                          'subnets have an appropriate primary IP address on any\n'
+                          'broadcast interface.')
 
     return None
 
 def generate(dhcp):
-    if not dhcp or dhcp['disabled']:
+    # bail out early - looks like removal from running config
+    if not dhcp or 'disable' in dhcp:
         return None
 
     # Please see: https://phabricator.vyos.net/T1129 for quoting of the raw parameters
@@ -607,11 +256,12 @@ def generate(dhcp):
     return None
 
 def apply(dhcp):
-    if not dhcp or dhcp['disabled']:
-        # DHCP server is removed in the commit
+    # bail out early - looks like removal from running config
+    if not dhcp or 'disable' in dhcp:
         call('systemctl stop isc-dhcp-server.service')
         if os.path.exists(config_file):
             os.unlink(config_file)
+
         return None
 
     call('systemctl restart isc-dhcp-server.service')