Merge pull request #4412 from sarthurdev/kea_2.6

kea: T7281: Add ping-check, use built-in option for classless static routes

10 files changed, 72 insertions, 107 deletions

diff --git a/data/templates/dhcp-server/kea-ctrl-agent.conf.j2 b/data/templates/dhcp-server/kea-ctrl-agent.conf.j2
deleted file mode 100644
index b37cf4798..000000000
--- a/data/templates/dhcp-server/kea-ctrl-agent.conf.j2
+++ /dev/null
@@ -1,14 +0,0 @@
-{
-    "Control-agent": {
-{% if high_availability is vyos_defined %}
-        "http-host": "{{ high_availability.source_address }}",
-        "http-port": 647,
-        "control-sockets": {
-            "dhcp4": {
-                "socket-type": "unix",
-                "socket-name": "/run/kea/dhcp4-ctrl-socket"
-            }
-        }
-{% endif %}
-    }
-}
diff --git a/data/templates/dhcp-server/kea-dhcp4.conf.j2 b/data/templates/dhcp-server/kea-dhcp4.conf.j2
index 2e10d58e0..8d9ffb194 100644
--- a/data/templates/dhcp-server/kea-dhcp4.conf.j2
+++ b/data/templates/dhcp-server/kea-dhcp4.conf.j2
@@ -25,20 +25,6 @@
         },
         "option-def": [
             {
-                "name": "rfc3442-static-route",
-                "code": 121,
-                "type": "record",
-                "array": true,
-                "record-types": "uint8,uint8,uint8,uint8,uint8,uint8,uint8"
-            },
-            {
-                "name": "windows-static-route",
-                "code": 249,
-                "type": "record",
-                "array": true,
-                "record-types": "uint8,uint8,uint8,uint8,uint8,uint8,uint8"
-            },
-            {
                 "name": "wpad-url",
                 "code": 252,
                 "type": "string"
@@ -69,6 +55,16 @@
             },
 {% endif %}
             {
+                "library": "/usr/lib/{{ machine }}-linux-gnu/kea/hooks/libdhcp_ping_check.so",
+                "parameters": {
+                    "enable-ping-check" : false,
+                    "min-ping-requests" : 1,
+                    "reply-timeout" : 100,
+                    "ping-cltt-secs" : 60,
+                    "ping-channel-threads" : 0
+                }
+            },
+            {
                 "library": "/usr/lib/{{ machine }}-linux-gnu/kea/hooks/libdhcp_lease_cmds.so",
                 "parameters": {}
             }
diff --git a/interface-definitions/include/dhcp/ping-check.xml.i b/interface-definitions/include/dhcp/ping-check.xml.i
new file mode 100644
index 000000000..a506f68e4
--- /dev/null
+++ b/interface-definitions/include/dhcp/ping-check.xml.i
@@ -0,0 +1,8 @@
+<!-- include start from dhcp/ping-check.xml.i -->
+<leafNode name="ping-check">
+  <properties>
+    <help>Sends ICMP Echo request to the address being assigned</help>
+    <valueless/>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/service_dhcp-server.xml.in b/interface-definitions/service_dhcp-server.xml.in
index 9a194de4f..c0ab7c048 100644
--- a/interface-definitions/service_dhcp-server.xml.in
+++ b/interface-definitions/service_dhcp-server.xml.in
@@ -112,6 +112,7 @@
                 </properties>
               </leafNode>
               #include <include/dhcp/option-v4.xml.i>
+              #include <include/dhcp/ping-check.xml.i>
               #include <include/generic-description.xml.i>
               #include <include/generic-disable-node.xml.i>
               <tagNode name="subnet">
@@ -128,6 +129,7 @@
                 </properties>
                 <children>
                   #include <include/dhcp/option-v4.xml.i>
+                  #include <include/dhcp/ping-check.xml.i>
                   #include <include/generic-description.xml.i>
                   #include <include/generic-disable-node.xml.i>
                   <leafNode name="exclude">
diff --git a/python/vyos/kea.py b/python/vyos/kea.py
index de397d8f9..2b0cac7e6 100644
--- a/python/vyos/kea.py
+++ b/python/vyos/kea.py
@@ -21,7 +21,6 @@ from datetime import datetime
 from datetime import timezone
 
 from vyos.template import is_ipv6
-from vyos.template import isc_static_route
 from vyos.template import netmask_from_cidr
 from vyos.utils.dict import dict_search_args
 from vyos.utils.file import file_permissions
@@ -113,22 +112,21 @@ def kea_parse_options(config):
         default_route = ''
 
         if 'default_router' in config:
-            default_route = isc_static_route('0.0.0.0/0', config['default_router'])
+            default_route = f'0.0.0.0/0 - {config["default_router"]}'
 
         routes = [
-            isc_static_route(route, route_options['next_hop'])
+            f'{route} - {route_options["next_hop"]}'
             for route, route_options in config['static_route'].items()
         ]
 
         options.append(
             {
-                'name': 'rfc3442-static-route',
+                'name': 'classless-static-route',
                 'data': ', '.join(
                     routes if not default_route else routes + [default_route]
                 ),
             }
         )
-        options.append({'name': 'windows-static-route', 'data': ', '.join(routes)})
 
     if 'time_zone' in config:
         with open('/usr/share/zoneinfo/' + config['time_zone'], 'rb') as f:
@@ -149,7 +147,7 @@ def kea_parse_options(config):
 
 
 def kea_parse_subnet(subnet, config):
-    out = {'subnet': subnet, 'id': int(config['subnet_id'])}
+    out = {'subnet': subnet, 'id': int(config['subnet_id']), 'user-context': {}}
 
     if 'option' in config:
         out['option-data'] = kea_parse_options(config['option'])
@@ -167,6 +165,9 @@ def kea_parse_subnet(subnet, config):
         out['valid-lifetime'] = int(config['lease'])
         out['max-valid-lifetime'] = int(config['lease'])
 
+    if 'ping_check' in config:
+        out['user-context']['enable-ping-check'] = True
+
     if 'range' in config:
         pools = []
         for num, range_config in config['range'].items():
diff --git a/python/vyos/template.py b/python/vyos/template.py
index e75db1a8d..7ba85a046 100755
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -390,28 +390,6 @@ def compare_netmask(netmask1, netmask2):
     except:
         return False
 
-@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
-
 @register_filter('is_file')
 def is_file(filename):
     if os.path.exists(filename):
@@ -895,7 +873,8 @@ def kea_shared_network_json(shared_networks):
         network = {
             'name': name,
             'authoritative': ('authoritative' in config),
-            'subnet4': []
+            'subnet4': [],
+            'user-context': {}
         }
 
         if 'option' in config:
@@ -907,6 +886,9 @@ def kea_shared_network_json(shared_networks):
             if 'bootfile_server' in config['option']:
                 network['next-server'] = config['option']['bootfile_server']
 
+        if 'ping_check' in config:
+            network['user-context']['enable-ping-check'] = True
+
         if 'subnet' in config:
             for subnet, subnet_config in config['subnet'].items():
                 if 'disable' in subnet_config:
diff --git a/smoketest/scripts/cli/test_service_dhcp-server.py b/smoketest/scripts/cli/test_service_dhcp-server.py
index 7bb850b22..935be6227 100755
--- a/smoketest/scripts/cli/test_service_dhcp-server.py
+++ b/smoketest/scripts/cli/test_service_dhcp-server.py
@@ -32,7 +32,6 @@ from vyos.template import inc_ip
 from vyos.template import dec_ip
 
 PROCESS_NAME = 'kea-dhcp4'
-CTRL_PROCESS_NAME = 'kea-ctrl-agent'
 KEA4_CONF = '/run/kea/kea-dhcp4.conf'
 KEA4_CTRL = '/run/kea/dhcp4-ctrl-socket'
 HOSTSD_CLIENT = '/usr/bin/vyos-hostsd-client'
@@ -96,6 +95,10 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
             self.assertTrue(key in base_obj)
             self.assertEqual(base_obj[key], value)
 
+    def verify_service_running(self):
+        tmp = cmd('tail -n 100 /var/log/messages | grep kea')
+        self.assertTrue(process_named_running(PROCESS_NAME), msg=f'Service not running, log: {tmp}')
+
     def test_dhcp_single_pool_range(self):
         shared_net_name = 'SMOKE-1'
 
@@ -106,9 +109,12 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
 
         self.cli_set(base_path + ['listen-interface', interface])
 
+        self.cli_set(base_path + ['shared-network-name', shared_net_name, 'ping-check'])
+
         pool = base_path + ['shared-network-name', shared_net_name, 'subnet', subnet]
         self.cli_set(pool + ['subnet-id', '1'])
         self.cli_set(pool + ['ignore-client-id'])
+        self.cli_set(pool + ['ping-check'])
         # we use the first subnet IP address as default gateway
         self.cli_set(pool + ['option', 'default-router', router])
         self.cli_set(pool + ['option', 'name-server', dns_1])
@@ -151,6 +157,21 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
             obj, ['Dhcp4', 'shared-networks', 0, 'subnet4'], 'max-valid-lifetime', 86400
         )
 
+        # Verify ping-check
+        self.verify_config_value(
+            obj,
+            ['Dhcp4', 'shared-networks', 0, 'user-context'],
+            'enable-ping-check',
+            True
+        )
+
+        self.verify_config_value(
+            obj,
+            ['Dhcp4', 'shared-networks', 0, 'subnet4', 0, 'user-context'],
+            'enable-ping-check',
+            True
+        )
+
         # Verify options
         self.verify_config_object(
             obj,
@@ -181,7 +202,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_single_pool_options(self):
         shared_net_name = 'SMOKE-0815'
@@ -221,8 +242,11 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
             pool + ['option', 'capwap-controller', capwap_access_controller]
         )
 
+        static_route = '10.0.0.0/24'
+        static_route_nexthop = '192.0.2.1'
+
         self.cli_set(
-            pool + ['option', 'static-route', '10.0.0.0/24', 'next-hop', '192.0.2.1']
+            pool + ['option', 'static-route', static_route, 'next-hop', static_route_nexthop]
         )
         self.cli_set(pool + ['option', 'ipv6-only-preferred', ipv6_only_preferred])
         self.cli_set(pool + ['option', 'time-zone', 'Europe/London'])
@@ -321,18 +345,13 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
             obj,
             ['Dhcp4', 'shared-networks', 0, 'subnet4', 0, 'option-data'],
             {
-                'name': 'rfc3442-static-route',
-                'data': '24,10,0,0,192,0,2,1, 0,192,0,2,1',
+                'name': 'classless-static-route',
+                'data': f'{static_route} - {static_route_nexthop}, 0.0.0.0/0 - {router}',
             },
         )
         self.verify_config_object(
             obj,
             ['Dhcp4', 'shared-networks', 0, 'subnet4', 0, 'option-data'],
-            {'name': 'windows-static-route', 'data': '24,10,0,0,192,0,2,1'},
-        )
-        self.verify_config_object(
-            obj,
-            ['Dhcp4', 'shared-networks', 0, 'subnet4', 0, 'option-data'],
             {'name': 'v6-only-preferred', 'data': ipv6_only_preferred},
         )
         self.verify_config_object(
@@ -361,7 +380,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_single_pool_options_scoped(self):
         shared_net_name = 'SMOKE-2'
@@ -447,7 +466,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_single_pool_static_mapping(self):
         shared_net_name = 'SMOKE-2'
@@ -593,7 +612,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
             client_base += 1
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_multiple_pools(self):
         lease_time = '14400'
@@ -735,7 +754,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
                 client_base += 1
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_exclude_not_in_range(self):
         # T3180: verify else path when slicing DHCP ranges and exclude address
@@ -782,7 +801,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_exclude_in_range(self):
         # T3180: verify else path when slicing DHCP ranges and exclude address
@@ -845,7 +864,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_relay_server(self):
         # Listen on specific address and return DHCP leases from a non
@@ -893,7 +912,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_high_availability(self):
         shared_net_name = 'FAILOVER'
@@ -996,8 +1015,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
-        self.assertTrue(process_named_running(CTRL_PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_high_availability_standby(self):
         shared_net_name = 'FAILOVER'
@@ -1096,8 +1114,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
-        self.assertTrue(process_named_running(CTRL_PROCESS_NAME))
+        self.verify_service_running()
 
     def test_dhcp_on_interface_with_vrf(self):
         self.cli_set(['interfaces', 'ethernet', 'eth1', 'address', '10.1.1.1/30'])
@@ -1259,7 +1276,7 @@ class TestServiceDHCPServer(VyOSUnitTestSHIM.TestCase):
         )
 
         # Check for running process
-        self.assertTrue(process_named_running(PROCESS_NAME))
+        self.verify_service_running()
 
         # All up and running, now test vyos-hostsd store
 
diff --git a/src/conf_mode/service_dhcp-server.py b/src/conf_mode/service_dhcp-server.py
index 5a729af74..e46d916fd 100755
--- a/src/conf_mode/service_dhcp-server.py
+++ b/src/conf_mode/service_dhcp-server.py
@@ -41,7 +41,6 @@ from vyos import airbag
 
 airbag.enable()
 
-ctrl_config_file = '/run/kea/kea-ctrl-agent.conf'
 ctrl_socket = '/run/kea/dhcp4-ctrl-socket'
 config_file = '/run/kea/kea-dhcp4.conf'
 lease_file = '/config/dhcp/dhcp4-leases.csv'
@@ -480,13 +479,6 @@ def generate(dhcp):
             dhcp['high_availability']['ca_cert_file'] = ca_cert_file
 
     render(
-        ctrl_config_file,
-        'dhcp-server/kea-ctrl-agent.conf.j2',
-        dhcp,
-        user=user_group,
-        group=user_group,
-    )
-    render(
         config_file,
         'dhcp-server/kea-dhcp4.conf.j2',
         dhcp,
@@ -498,7 +490,7 @@ def generate(dhcp):
 
 
 def apply(dhcp):
-    services = ['kea-ctrl-agent', 'kea-dhcp4-server', 'kea-dhcp-ddns-server']
+    services = ['kea-dhcp4-server', 'kea-dhcp-ddns-server']
 
     if not dhcp or 'disable' in dhcp:
         for service in services:
@@ -515,9 +507,6 @@ def apply(dhcp):
         if service == 'kea-dhcp-ddns-server' and 'dynamic_dns_update' not in dhcp:
             action = 'stop'
 
-        if service == 'kea-ctrl-agent' and 'high_availability' not in dhcp:
-            action = 'stop'
-
         call(f'systemctl {action} {service}.service')
 
     return None
diff --git a/src/etc/systemd/system/kea-ctrl-agent.service.d/override.conf b/src/etc/systemd/system/kea-ctrl-agent.service.d/override.conf
deleted file mode 100644
index c74fafb42..000000000
--- a/src/etc/systemd/system/kea-ctrl-agent.service.d/override.conf
+++ /dev/null
@@ -1,10 +0,0 @@
-[Unit]
-After=
-After=vyos-router.service
-ConditionFileNotEmpty=
-
-[Service]
-ExecStart=
-ExecStart=/usr/sbin/kea-ctrl-agent -c /run/kea/kea-ctrl-agent.conf
-AmbientCapabilities=CAP_NET_BIND_SERVICE
-CapabilityBoundingSet=CAP_NET_BIND_SERVICE
diff --git a/src/migration-scripts/dhcp-server/7-to-8 b/src/migration-scripts/dhcp-server/7-to-8
index 7fcb62e86..d0f9455bb 100644
--- a/src/migration-scripts/dhcp-server/7-to-8
+++ b/src/migration-scripts/dhcp-server/7-to-8
@@ -41,9 +41,6 @@ def migrate(config: ConfigTree) -> None:
         for network in config.list_nodes(base + ['shared-network-name']):
             base_network = base + ['shared-network-name', network]
 
-            if config.exists(base_network + ['ping-check']):
-                config.delete(base_network + ['ping-check'])
-
             if config.exists(base_network + ['shared-network-parameters']):
                 config.delete(base_network +['shared-network-parameters'])
 
@@ -57,9 +54,6 @@ def migrate(config: ConfigTree) -> None:
                 if config.exists(base_subnet + ['enable-failover']):
                     config.delete(base_subnet + ['enable-failover'])
 
-                if config.exists(base_subnet + ['ping-check']):
-                    config.delete(base_subnet + ['ping-check'])
-
                 if config.exists(base_subnet + ['subnet-parameters']):
                     config.delete(base_subnet + ['subnet-parameters'])