Merge branch 'dhcp-relay' of github.com:c-po/vyos-1x into current

* 'dhcp-relay' of github.com:c-po/vyos-1x:
  dhcp-relay: T3095: migrate service to get_config_dict()
  smoketest: dhcp-relay: T3095: initial tests

6 files changed, 133 insertions, 65 deletions

diff --git a/data/templates/dhcp-relay/config.tmpl b/data/templates/dhcp-relay/config.tmpl
deleted file mode 100644
index b223807cf..000000000
--- a/data/templates/dhcp-relay/config.tmpl
+++ /dev/null
@@ -1,4 +0,0 @@
-### Autogenerated by dhcp_relay.py ###
-
-# Defaults for isc-dhcp-relay6.service
-OPTIONS="{{ options | join(' ') }} -i {{ interface | join(' -i ') }} {{ server | join(' ') }}"
diff --git a/data/templates/dhcp-relay/dhcrelay.conf.tmpl b/data/templates/dhcp-relay/dhcrelay.conf.tmpl
new file mode 100644
index 000000000..df990207b
--- /dev/null
+++ b/data/templates/dhcp-relay/dhcrelay.conf.tmpl
@@ -0,0 +1,6 @@
+### Autogenerated by dhcp_relay.py ###
+
+{% set max_size = '-A ' + relay_options.max_size if relay_options.max_size is defined and relay_options.max_size is not none %}
+{# hop_count and relay_agents_packets is a default option, thus it is always present #}
+OPTIONS="-4 -c {{ relay_options.hop_count }} -a -m {{ relay_options.relay_agents_packets }} {{ max_size }} -i {{ interface | join(' -i ') }} {{ server | join(' ') }}"
+
diff --git a/interface-definitions/dhcp-relay.xml.in b/interface-definitions/dhcp-relay.xml.in
index b83402aa1..8c95239d9 100644
--- a/interface-definitions/dhcp-relay.xml.in
+++ b/interface-definitions/dhcp-relay.xml.in
@@ -35,6 +35,7 @@
                   </constraint>
                   <constraintErrorMessage>hop-count must be a value between 1 and 255</constraintErrorMessage>
                 </properties>
+                <defaultValue>10</defaultValue>
               </leafNode>
               <leafNode name="max-size">
                 <properties>
@@ -72,9 +73,10 @@
                     <description>discard packet (default action if giaddr not set in packet)</description>
                   </valueHelp>
                   <constraint>
-                    <regex>(append|replace|forward|discard)</regex>
+                    <regex>^(append|replace|forward|discard)$</regex>
                   </constraint>
                 </properties>
+                <defaultValue>forward</defaultValue>
               </leafNode>
             </children>
           </node>
diff --git a/smoketest/scripts/cli/test_service_dhcp-relay.py b/smoketest/scripts/cli/test_service_dhcp-relay.py
new file mode 100755
index 000000000..d56f6aa16
--- /dev/null
+++ b/smoketest/scripts/cli/test_service_dhcp-relay.py
@@ -0,0 +1,101 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2020 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/>.
+
+import re
+import os
+import unittest
+
+from vyos.configsession import ConfigSession
+from vyos.configsession import ConfigSessionError
+from vyos.ifconfig import Section
+from vyos.util import cmd
+from vyos.util import process_named_running
+from vyos.util import read_file
+
+PROCESS_NAME = 'dhcrelay'
+RELAY_CONF = '/run/dhcp-relay/dhcrelay.conf'
+base_path = ['service', 'dhcp-relay']
+
+def get_config_value(key):
+    tmp = read_file(SSHD_CONF)
+    tmp = re.findall(f'\n?{key}\s+(.*)', tmp)
+    return tmp
+
+class TestServiceDHCPRelay(unittest.TestCase):
+    def setUp(self):
+        self.session = ConfigSession(os.getpid())
+        # ensure we can also run this test on a live system - so lets clean
+        # out the current configuration :)
+        self.session.delete(base_path)
+
+    def tearDown(self):
+        self.session.delete(base_path)
+        self.session.commit()
+        del self.session
+
+    def test_relay_default(self):
+        max_size = '800'
+        hop_count = '20'
+        agents_packets = 'append'
+        servers = ['192.0.2.1', '192.0.2.2']
+
+        self.session.set(base_path + ['interface', 'lo'])
+        # check validate() - DHCP relay does not support the loopback interface
+        with self.assertRaises(ConfigSessionError):
+            self.session.commit()
+        self.session.delete(base_path + ['interface', 'lo'])
+
+        # activate DHCP relay on all ethernet interfaces
+        for tmp in Section.interfaces("ethernet"):
+            self.session.set(base_path + ['interface', tmp])
+
+        # check validate() - No DHCP relay server(s) configured
+        with self.assertRaises(ConfigSessionError):
+            self.session.commit()
+        for server in servers:
+            self.session.set(base_path + ['server', server])
+
+        self.session.set(base_path + ['relay-options', 'max-size', max_size])
+        self.session.set(base_path + ['relay-options', 'hop-count', hop_count])
+        self.session.set(base_path + ['relay-options', 'relay-agents-packets', agents_packets])
+
+        # commit changes
+        self.session.commit()
+
+        # Check configured port
+        config = read_file(RELAY_CONF)
+
+        # Test configured relay interfaces
+        for tmp in Section.interfaces("ethernet"):
+            self.assertIn(f'-i {tmp}', config)
+
+        # Test relay servers
+        for server in servers:
+            self.assertIn(f' {server}', config)
+
+        # Test max-size
+        self.assertIn(f'-A {max_size}', config)
+        # Hop count
+        self.assertIn(f'-c {hop_count}', config)
+        # relay-agents-packets
+        self.assertIn(f'-a -m {agents_packets}', config)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+if __name__ == '__main__':
+    unittest.main()
+
diff --git a/src/conf_mode/dhcp_relay.py b/src/conf_mode/dhcp_relay.py
index 352865b9d..6352e0b4a 100755
--- a/src/conf_mode/dhcp_relay.py
+++ b/src/conf_mode/dhcp_relay.py
@@ -19,81 +19,43 @@ import os
 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.xml import defaults
 from vyos import ConfigError
-
 from vyos import airbag
 airbag.enable()
 
-config_file = r'/run/dhcp-relay/dhcp.conf'
-
-default_config_data = {
-    'interface': [],
-    'server': [],
-    'options': [],
-    'hop_count': '10',
-    'relay_agent_packets': 'forward'
-}
+config_file = r'/run/dhcp-relay/dhcrelay.conf'
 
 def get_config(config=None):
-    relay = default_config_data
     if config:
         conf = config
     else:
         conf = Config()
-    if not conf.exists(['service', 'dhcp-relay']):
+    base = ['service', 'dhcp-relay']
+    if not conf.exists(base):
         return None
-    else:
-        conf.set_level(['service', 'dhcp-relay'])
-
-    # Network interfaces to listen on
-    if conf.exists(['interface']):
-        relay['interface'] = conf.return_values(['interface'])
-
-    # Servers equal to the address of the DHCP server(s)
-    if conf.exists(['server']):
-        relay['server'] = conf.return_values(['server'])
-
-    conf.set_level(['service', 'dhcp-relay', 'relay-options'])
-
-    if conf.exists(['hop-count']):
-        count = '-c ' + conf.return_value(['hop-count'])
-        relay['options'].append(count)
-
-    # Specify the maximum packet size to send to a DHCPv4/BOOTP server.
-    # This might be done to allow sufficient space for addition of relay agent
-    # options while still fitting into the Ethernet MTU size.
-    #
-    # Available in DHCPv4 mode only:
-    if conf.exists(['max-size']):
-        size = '-A ' + conf.return_value(['max-size'])
-        relay['options'].append(size)
-
-    # Control the handling of incoming DHCPv4 packets which already contain
-    # relay agent options. If such a packet does not have giaddr set in its
-    # header, the DHCP standard requires that the packet be discarded. However,
-    # if giaddr is set, the relay agent may handle the situation in four ways:
-    # It may append its own set of relay options to the packet, leaving the
-    # supplied option field intact; it may replace the existing agent option
-    # field; it may forward the packet unchanged; or, it may discard it.
-    #
-    # Available in DHCPv4 mode only:
-    if conf.exists(['relay-agents-packets']):
-        pkt = '-a -m ' + conf.return_value(['relay-agents-packets'])
-        relay['options'].append(pkt)
+
+    relay = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
+    # We have gathered the dict representation of the CLI, but there are default
+    # options which we need to update into the dictionary retrived.
+    default_values = defaults(base)
+    relay = dict_merge(default_values, relay)
 
     return relay
 
 def verify(relay):
     # bail out early - looks like removal from running config
-    if relay is None:
+    if not relay:
         return None
 
-    if 'lo' in relay['interface']:
+    if 'lo' in (dict_search('interface', relay) or []):
         raise ConfigError('DHCP relay does not support the loopback interface.')
 
-    if len(relay['server']) == 0:
+    if 'server' not in relay :
         raise ConfigError('No DHCP relay server(s) configured.\n' \
                           'At least one DHCP relay server required.')
 
@@ -104,17 +66,18 @@ def generate(relay):
     if not relay:
         return None
 
-    render(config_file, 'dhcp-relay/config.tmpl', relay)
+    render(config_file, 'dhcp-relay/dhcrelay.conf.tmpl', relay)
     return None
 
 def apply(relay):
-    if relay:
-        call('systemctl restart isc-dhcp-relay.service')
-    else:
-        # DHCP relay support is removed in the commit
+    # bail out early - looks like removal from running config
+    if not relay:
         call('systemctl stop isc-dhcp-relay.service')
         if os.path.exists(config_file):
             os.unlink(config_file)
+        return None
+
+    call('systemctl restart isc-dhcp-relay.service')
 
     return None
 
diff --git a/src/systemd/isc-dhcp-relay.service b/src/systemd/isc-dhcp-relay.service
index 56bcec840..de2e51a87 100644
--- a/src/systemd/isc-dhcp-relay.service
+++ b/src/systemd/isc-dhcp-relay.service
@@ -3,7 +3,7 @@ Description=ISC DHCP IPv4 relay
 Documentation=man:dhcrelay(8)
 Wants=network-online.target
 RequiresMountsFor=/run
-ConditionPathExists=/run/dhcp-relay/dhcp.conf
+ConditionPathExists=/run/dhcp-relay/dhcrelay.conf
 After=vyos-router.service
 
 [Service]
@@ -11,7 +11,7 @@ Type=forking
 WorkingDirectory=/run/dhcp-relay
 RuntimeDirectory=dhcp-relay
 RuntimeDirectoryPreserve=yes
-EnvironmentFile=/run/dhcp-relay/dhcp.conf
+EnvironmentFile=/run/dhcp-relay/dhcrelay.conf
 PIDFile=/run/dhcp-relay/dhcrelay.pid
 ExecStart=/usr/sbin/dhcrelay -4 -pf /run/dhcp-relay/dhcrelay.pid $OPTIONS
 Restart=always