Merge pull request #326 from thomas-mangin/T2265

dhcp: T2265: refactor DHCP class

6 files changed, 191 insertions, 153 deletions

diff --git a/python/vyos/defaults.py b/python/vyos/defaults.py
index a2ad142bc..88894674f 100644
--- a/python/vyos/defaults.py
+++ b/python/vyos/defaults.py
@@ -21,6 +21,7 @@ directories = {
   "current": "/opt/vyatta/etc/config-migrate/current",
   "migrate": "/opt/vyatta/etc/config-migrate/migrate",
   "log": "/var/log/vyatta",
+  "templates": "/usr/share/vyos/templates/"
 }
 
 cfg_group = 'vyattacfg'
diff --git a/python/vyos/dicts.py b/python/vyos/dicts.py
new file mode 100644
index 000000000..79cab4a08
--- /dev/null
+++ b/python/vyos/dicts.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2019 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/>.
+
+
+class FixedDict(dict):
+    """
+    FixedDict: A dictionnary not allowing new keys to be created after initialisation.
+
+    >>> f = FixedDict(**{'count':1})
+    >>> f['count'] = 2
+    >>> f['king'] = 3
+      File "...", line ..., in __setitem__
+    raise ConfigError(f'Option "{k}" has no defined default')
+    """
+
+    def __init__(self, **options):
+        self._allowed = options.keys()
+        super().__init__(**options)
+
+    def __setitem__(self, k, v):
+        """
+        __setitem__ is a builtin which is called by python when setting dict values:
+        >>> d = dict()
+        >>> d['key'] = 'value'
+        >>> d
+        {'key': 'value'}
+
+        is syntaxic sugar for
+
+        >>> d = dict()
+        >>> d.__setitem__('key','value')
+        >>> d
+        {'key': 'value'}
+        """
+        if k not in self._allowed:
+            raise ConfigError(f'Option "{k}" has no defined default')
+        super().__setitem__(k, v)
diff --git a/python/vyos/ifconfig/dhcp.py b/python/vyos/ifconfig/dhcp.py
index 8ec8263b5..d4ff9c2cd 100644
--- a/python/vyos/ifconfig/dhcp.py
+++ b/python/vyos/ifconfig/dhcp.py
@@ -14,105 +14,37 @@
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
-import jinja2
 
+from vyos.dicts import FixedDict
 from vyos.ifconfig.control import Control
+from vyos.template import render
 
-template_v4 = """
-# generated by ifconfig.py
-option rfc3442-classless-static-routes code 121 = array of unsigned integer 8;
-timeout 60;
-retry 300;
-
-interface "{{ intf }}" {
-    send host-name "{{ hostname }}";
-    {% if client_id -%}
-    send dhcp-client-identifier "{{ client_id }}";
-    {% endif -%}
-    {% if vendor_class_id -%}
-    send vendor-class-identifier "{{ vendor_class_id }}";
-    {% endif -%}
-    request subnet-mask, broadcast-address, routers, domain-name-servers,
-        rfc3442-classless-static-routes, domain-name, interface-mtu;
-    require subnet-mask;
-}
-
-"""
-
-template_v6 = """
-# generated by ifconfig.py
-interface "{{ intf }}" {
-    request routers, domain-name-servers, domain-name;
-}
-
-"""
-
-class DHCP (Control):
+
+class _DHCP (Control):
     client_base = r'/var/lib/dhcp/dhclient_'
 
-    def __init__ (self, ifname, **kargs):
+    def __init__(self, ifname, version, **kargs):
         super().__init__(**kargs)
-
-        # per interface DHCP config files
-        self._dhcp = {
-            4: {
-                'ifname': ifname,
-                'conf': self.client_base + ifname + '.conf',
-                'pid':  self.client_base + ifname + '.pid',
-                'lease': self.client_base + ifname + '.leases',
-                'options': {
-                    'intf': ifname,
-                    'hostname': '',
-                    'client_id': '',
-                    'vendor_class_id': ''
-                },
-            },
-            6: {
-                'ifname': ifname,
-                'conf': self.client_base + ifname + '.v6conf',
-                'pid':  self.client_base + ifname + '.v6pid',
-                'lease': self.client_base + ifname + '.v6leases',
-                'accept_ra': f'/proc/sys/net/ipv6/conf/{ifname}/accept_ra',
-                'options': {
-                    'intf': ifname,
-                    'dhcpv6_prm_only': False,
-                    'dhcpv6_temporary': False
-                },
-            },
+        self.version = version
+        self.file = {
+            'ifname': ifname,
+            'conf': self.client_base + ifname + '.' + version + 'conf',
+            'pid': self.client_base + ifname + '.' + version + 'pid',
+            'lease': self.client_base + ifname + '.' + version + 'leases',
         }
 
-    def get_dhcp_options(self):
-        """
-        Return dictionary with supported DHCP options.
-
-        Dictionary should be altered and send back via set_dhcp_options()
-        so those options are applied when DHCP is run.
-        """
-        return self._dhcp[4]['options']
-
-    def set_dhcp_options(self, options):
-        """
-        Store new DHCP options used by next run of DHCP client.
-        """
-        self._dhcp[4]['options'] = options
-
-    def get_dhcpv6_options(self):
-        """
-        Return dictionary with supported DHCPv6 options.
-
-        Dictionary should be altered and send back via set_dhcp_options()
-        so those options are applied when DHCP is run.
-        """
-        return self._dhcp[6]['options']
-
-    def set_dhcpv6_options(self, options):
-        """
-        Store new DHCP options used by next run of DHCP client.
-        """
-        self._dhcp[6]['options'] = options
+class _DHCPv4 (_DHCP):
+    def __init__(self, ifname):
+        super().__init__(ifname, '')
+        self.options = FixedDict(**{
+            'ifname': ifname,
+            'hostname': '',
+            'client_id': '',
+            'vendor_class_id': ''
+        })
 
     # replace dhcpv4/v6 with systemd.networkd?
-    def _set_dhcp(self):
+    def set(self):
         """
         Configure interface as DHCP client. The dhclient binary is automatically
         started in background!
@@ -121,21 +53,16 @@ class DHCP (Control):
 
         >>> from vyos.ifconfig import Interface
         >>> j = Interface('eth0')
-        >>> j.set_dhcp()
+        >>> j.dhcp.v4.set()
         """
 
-        dhcp = self.get_dhcp_options()
-        if not dhcp['hostname']:
+        if not self.options['hostname']:
             # read configured system hostname.
             # maybe change to vyos hostd client ???
             with open('/etc/hostname', 'r') as f:
-                dhcp['hostname'] = f.read().rstrip('\n')
+                self.options['hostname'] = f.read().rstrip('\n')
 
-        # render DHCP configuration
-        tmpl = jinja2.Template(template_v4)
-        dhcp_text = tmpl.render(dhcp)
-        with open(self._dhcp[4]['conf'], 'w') as f:
-            f.write(dhcp_text)
+        render(self.file['conf'], 'dhcp-client/ipv4.tmpl' ,self.options)
 
         cmd = 'start-stop-daemon'
         cmd += ' --start'
@@ -146,9 +73,9 @@ class DHCP (Control):
         cmd += ' --'
         # now pass arguments to dhclient binary
         cmd += ' -4 -nw -cf {conf} -pf {pid} -lf {lease} {ifname}'
-        return self._cmd(cmd.format(**self._dhcp[4]))
+        return self._cmd(cmd.format(**self.file))
 
-    def _del_dhcp(self):
+    def delete(self):
         """
         De-configure interface as DHCP clinet. All auto generated files like
         pid, config and lease will be removed.
@@ -157,14 +84,14 @@ class DHCP (Control):
 
         >>> from vyos.ifconfig import Interface
         >>> j = Interface('eth0')
-        >>> j.del_dhcp()
+        >>> j.dhcp.v4.delete()
         """
-        if not os.path.isfile(self._dhcp[4]['pid']):
+        if not os.path.isfile(self.file['pid']):
             self._debug_msg('No DHCP client PID found')
             return None
 
-		# with open(self._dhcp[4]['pid'], 'r') as f:
-		# 	pid = int(f.read())
+	# with open(self.file['pid'], 'r') as f:
+	# 	pid = int(f.read())
 
         # stop dhclient, we need to call dhclient and tell it should release the
         # aquired IP address. tcpdump tells me:
@@ -178,14 +105,27 @@ class DHCP (Control):
         #    Hostname Option 12, length 10: "vyos"
         #
         cmd = '/sbin/dhclient -cf {conf} -pf {pid} -lf {lease} -r {ifname}'
-        self._cmd(cmd.format(**self._dhcp[4]))
+        self._cmd(cmd.format(**self.file))
 
         # cleanup old config files
         for name in ('conf', 'pid', 'lease'):
-            if os.path.isfile(self._dhcp[4][name]):
-                os.remove(self._dhcp[4][name])
+            if os.path.isfile(self.file[name]):
+                os.remove(self.file[name])
+
 
-    def _set_dhcpv6(self):
+class _DHCPv6 (_DHCP):
+    def __init__(self, ifname):
+        super().__init__(ifname, 'v6')
+        self.options = FixedDict(**{
+            'ifname': ifname,
+            'dhcpv6_prm_only': False,
+            'dhcpv6_temporary': False,
+        })
+        self.file.update({
+            'accept_ra': f'/proc/sys/net/ipv6/conf/{ifname}/accept_ra',
+        })
+
+    def set(self):
         """
         Configure interface as DHCPv6 client. The dhclient binary is automatically
         started in background!
@@ -196,22 +136,17 @@ class DHCP (Control):
         >>> j = Interface('eth0')
         >>> j.set_dhcpv6()
         """
-        dhcpv6 = self.get_dhcpv6_options()
 
         # better save then sorry .. should be checked in interface script
         # but if you missed it we are safe!
-        if dhcpv6['dhcpv6_prm_only'] and dhcpv6['dhcpv6_temporary']:
+        if self.options['dhcpv6_prm_only'] and self.options['dhcpv6_temporary']:
             raise Exception(
                 'DHCPv6 temporary and parameters-only options are mutually exclusive!')
 
-        # render DHCP configuration
-        tmpl = jinja2.Template(template_v6)
-        dhcpv6_text = tmpl.render(dhcpv6)
-        with open(self._dhcp[6]['conf'], 'w') as f:
-            f.write(dhcpv6_text)
+        render(self.file['conf'], 'dhcp-client/ipv6.tmpl', self.options)
 
         # no longer accept router announcements on this interface
-        self._write_sysfs(self._dhcp[6]['accept_ra'], 0)
+        self._write_sysfs(self.file['accept_ra'], 0)
 
         # assemble command-line to start DHCPv6 client (dhclient)
         cmd = 'start-stop-daemon'
@@ -224,15 +159,15 @@ class DHCP (Control):
         # now pass arguments to dhclient binary
         cmd += ' -6 -nw -cf {conf} -pf {pid} -lf {lease}'
         # add optional arguments
-        if dhcpv6['dhcpv6_prm_only']:
+        if self.options['dhcpv6_prm_only']:
             cmd += ' -S'
-        if dhcpv6['dhcpv6_temporary']:
+        if self.options['dhcpv6_temporary']:
             cmd += ' -T'
         cmd += ' {ifname}'
 
-        return self._cmd(cmd.format(**self._dhcp[6]))
+        return self._cmd(cmd.format(**self.file))
 
-    def _del_dhcpv6(self):
+    def delete(self):
         """
         De-configure interface as DHCPv6 clinet. All auto generated files like
         pid, config and lease will be removed.
@@ -243,12 +178,12 @@ class DHCP (Control):
         >>> j = Interface('eth0')
         >>> j.del_dhcpv6()
         """
-        if not os.path.isfile(self._dhcp[6]['pid']):
+        if not os.path.isfile(self.file['pid']):
             self._debug_msg('No DHCPv6 client PID found')
             return None
 
-		# with open(self._dhcp[6]['pid'], 'r') as f:
-		# 	pid = int(f.read())
+        # with open(self.file['pid'], 'r') as f:
+        # 	pid = int(f.read())
 
         # stop dhclient
         cmd = 'start-stop-daemon'
@@ -256,13 +191,18 @@ class DHCP (Control):
         cmd += ' --oknodo'
         cmd += ' --quiet'
         cmd += ' --pidfile {pid}'
-        self._cmd(cmd.format(**self._dhcp[6]))
+        self._cmd(cmd.format(**self.file))
 
         # accept router announcements on this interface
-        self._write_sysfs(self._dhcp[6]['accept_ra'], 1)
+        self._write_sysfs(self.options['accept_ra'], 1)
 
         # cleanup old config files
         for name in ('conf', 'pid', 'lease'):
-            if os.path.isfile(self._dhcp[6][name]):
-                os.remove(self._dhcp[6][name])
+            if os.path.isfile(self.file[name]):
+                os.remove(self.file[name])
+
 
+class DHCP (object):
+    def __init__(self, ifname):
+        self.v4 = _DHCPv4(ifname)
+        self.v6 = _DHCPv6(ifname)
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 22c71a464..43f823eca 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -41,8 +41,10 @@ from vyos.validate import assert_mtu
 from vyos.validate import assert_positive
 from vyos.validate import assert_range
 
+from vyos.ifconfig.control import Control
 
-class Interface(DHCP):
+
+class Interface(Control):
     options = []
     required = []
     default = {
@@ -181,7 +183,8 @@ class Interface(DHCP):
         self.config['ifname'] = ifname
 
         # we must have updated config before initialising the Interface
-        super().__init__(ifname, **kargs)
+        super().__init__(**kargs)
+        self.dhcp = DHCP(ifname)
 
         if not os.path.exists('/sys/class/net/{}'.format(self.config['ifname'])):
             # Any instance of Interface, such as Interface('eth0')
@@ -224,8 +227,8 @@ class Interface(DHCP):
         >>> i.remove()
         """
         # stop DHCP(v6) if running
-        self._del_dhcp()
-        self._del_dhcpv6()
+        self.dhcp.v4.delete()
+        self.dhcp.v6.delete()
 
         # remove all assigned IP addresses from interface - this is a bit redundant
         # as the kernel will remove all addresses on interface deletion, but we
@@ -668,12 +671,13 @@ class Interface(DHCP):
                 # do not change below 'if' ordering esle you will get an exception as:
                 #   ValueError: 'dhcp' does not appear to be an IPv4 or IPv6 address
                 if addr != 'dhcp' and is_ipv4(addr):
-                    raise ConfigError("Can't configure both static IPv4 and DHCP address on the same interface")
+                    raise ConfigError(
+                        "Can't configure both static IPv4 and DHCP address on the same interface")
 
         if addr == 'dhcp':
-            self._set_dhcp()
+            self.dhcp.v4.set()
         elif addr == 'dhcpv6':
-            self._set_dhcpv6()
+            self.dhcp.v6.set()
         else:
             if not is_intf_addr_assigned(self.config['ifname'], addr):
                 cmd = 'ip addr add "{}" dev "{}"'.format(addr, self.config['ifname'])
@@ -702,9 +706,9 @@ class Interface(DHCP):
         ['2001:db8::ffff/64']
         """
         if addr == 'dhcp':
-            self._del_dhcp()
+            self.dhcp.v4.delete()
         elif addr == 'dhcpv6':
-            self._del_dhcpv6()
+            self.dhcp.v6.delete()
         else:
             if is_intf_addr_assigned(self.config['ifname'], addr):
                 cmd = 'ip addr del "{}" dev "{}"'.format(addr, self.config['ifname'])
diff --git a/python/vyos/ifconfig_vlan.py b/python/vyos/ifconfig_vlan.py
index ed22646c1..899fd17da 100644
--- a/python/vyos/ifconfig_vlan.py
+++ b/python/vyos/ifconfig_vlan.py
@@ -25,32 +25,20 @@ def apply_vlan_config(vlan, config):
     if not vlan.definition['vlan']:
         raise TypeError()
 
-    # get DHCP config dictionary and update values
-    opt = vlan.get_dhcp_options()
-
     if config['dhcp_client_id']:
-        opt['client_id'] = config['dhcp_client_id']
+        vlan.dhcp.v4.options['client_id'] = config['dhcp_client_id']
 
     if config['dhcp_hostname']:
-        opt['hostname'] = config['dhcp_hostname']
+        vlan.dhcp.v4.options['hostname'] = config['dhcp_hostname']
 
     if config['dhcp_vendor_class_id']:
-        opt['vendor_class_id'] = config['dhcp_vendor_class_id']
-
-    # store DHCP config dictionary - used later on when addresses are aquired
-    vlan.set_dhcp_options(opt)
-
-    # get DHCPv6 config dictionary and update values
-    opt = vlan.get_dhcpv6_options()
+        vlan.dhcp.v4.options['vendor_class_id'] = config['dhcp_vendor_class_id']
 
     if config['dhcpv6_prm_only']:
-        opt['dhcpv6_prm_only'] = True
+        vlan.dhcp.v6.options['dhcpv6_prm_only'] = True
 
     if config['dhcpv6_temporary']:
-        opt['dhcpv6_temporary'] = True
-
-    # store DHCPv6 config dictionary - used later on when addresses are aquired
-    vlan.set_dhcpv6_options(opt)
+        vlan.dhcp.v6.options['dhcpv6_temporary'] = True
 
     # update interface description used e.g. within SNMP
     vlan.set_alias(config['description'])
diff --git a/python/vyos/template.py b/python/vyos/template.py
new file mode 100644
index 000000000..e559120c0
--- /dev/null
+++ b/python/vyos/template.py
@@ -0,0 +1,55 @@
+# Copyright 2019 VyOS maintainers and contributors <maintainers@vyos.io>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library 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
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+import os
+
+from jinja2 import Environment
+from jinja2 import FileSystemLoader
+
+from vyos.defaults import directories
+
+
+# reuse the same Environment to improve performance
+_templates_env = Environment(loader=FileSystemLoader(directories['templates']))
+_templates_mem = {}
+
+def render(destination, template, content):
+    """
+    render a template from the template directory, it will raise on any errors
+    destination: the file where the rendered template must be saved
+    template: the path to the template relative to the template folder
+    content: the dictionary to use to render the template
+
+    This classes cache the renderer, so rendering the same file multiple time
+    does not cause as too much overhead. If use everywhere, it could be changed
+    and load the template from python environement variables from an import 
+    python module generated when the debian package is build 
+    (recovering the load time and overhead caused by having the file out of the code)
+    """
+
+    # Setup a renderer for the given template
+    # This is cached and re-used for performance
+    if template not in _templates_mem:
+        _templates_mem[template] = _templates_env.get_template(template)
+    template = _templates_mem[template]
+
+    # As we are opening the file with 'w', we are performing the rendering
+    # before calling open() to not accidentally erase the file if the 
+    # templating fails
+    content = template.render(content)
+
+    # Write client config file
+    with open(destination, 'w') as f:
+        f.write(content)