igmp-proxy: T3088: migrate to get_config_dict()

3 files changed, 73 insertions, 103 deletions

diff --git a/data/templates/igmp-proxy/igmpproxy.conf.tmpl b/data/templates/igmp-proxy/igmpproxy.conf.tmpl
index c7fc5cef5..e3966def3 100644
--- a/data/templates/igmp-proxy/igmpproxy.conf.tmpl
+++ b/data/templates/igmp-proxy/igmpproxy.conf.tmpl
@@ -2,36 +2,39 @@
 #
 # autogenerated by igmp_proxy.py
 #
-#   The configuration file must define one upstream
-#   interface, and one or more downstream interfaces.
+# The configuration file must define one upstream interface, and one or more
+# downstream interfaces.
 #
-#   If multicast traffic originates outside the
-#   upstream subnet, the "altnet" option can be
-#   used in order to define legal multicast sources.
-#   (Se example...)
+# If multicast traffic originates outside the upstream subnet, the "altnet"
+# option can be used in order to define legal multicast sources.
 #
-#   The "quickleave" should be used to avoid saturation
-#   of the upstream link. The option should only
-#   be used if it's absolutely nessecary to
-#   accurately imitate just one Client.
+# The "quickleave" should be used to avoid saturation of the upstream link. The
+# option should only be used if it's absolutely nessecary to accurately imitate
+# just one Client.
 #
 ########################################################
 
-{% if not disable_quickleave -%}
+{% if disable_quickleave is not defined %}
 quickleave
-{% endif -%}
+{% endif %}
+{% if interface is defined and interface is not none %}
+{%   for iface, config in interface.items() %}
 
-{% for interface in interfaces %}
-# Configuration for {{ interface.name }} ({{ interface.role }} interface)
-{% if interface.role == 'disabled' -%}
-phyint {{ interface.name }} disabled
-{%- else -%}
-phyint {{ interface.name }} {{ interface.role }} ratelimit 0 threshold {{ interface.threshold }}
-{%- endif -%}
-{%- for subnet in interface.alt_subnet %}
+# Configuration for {{ iface }} ({{ config.role }} interface)
+{%     if config.role == 'disabled' %}
+phyint {{ iface }} disabled
+{%     else %}
+phyint {{ iface }} {{ config.role }} ratelimit 0 threshold {{ config.threshold }}
+{%     endif %}
+{%     if config.alt_subnet is defined and config.alt_subnet is not none %}
+{%       for subnet in config.alt_subnet %}
         altnet {{ subnet }}
-{%- endfor %}
-{%- for subnet in interface.whitelist %}
+{%       endfor %}
+{%     endif %}
+{%     if config.whitelist is defined and config.whitelist is not none %}
+{%       for subnet in config.whitelist %}
         whitelist {{ subnet }}
-{%- endfor %}
-{% endfor %}
+{%       endfor %}
+{%     endif %}
+{%   endfor %}
+{% endif %}
diff --git a/interface-definitions/igmp-proxy.xml.in b/interface-definitions/igmp-proxy.xml.in
index 74fec6b48..b9c52794f 100644
--- a/interface-definitions/igmp-proxy.xml.in
+++ b/interface-definitions/igmp-proxy.xml.in
@@ -44,7 +44,7 @@
               </leafNode>
               <leafNode name="role">
                 <properties>
-                  <help>Role of this IGMP interface</help>
+                  <help>IGMP interface role (default: downstream)</help>
                   <completionHelp>
                     <list>upstream downstream disabled</list>
                   </completionHelp>
@@ -61,13 +61,14 @@
                     <description>Disabled interface</description>
                   </valueHelp>
                   <constraint>
-                    <regex>(upstream|downstream|disabled)</regex>
+                    <regex>^(upstream|downstream|disabled)$</regex>
                   </constraint>
                 </properties>
+                <defaultValue>downstream</defaultValue>
               </leafNode>
               <leafNode name="threshold">
                 <properties>
-                  <help>TTL threshold</help>
+                  <help>TTL threshold (default: 1)</help>
                   <valueHelp>
                     <format>1-255</format>
                     <description>TTL threshold for the interfaces (default: 1)</description>
@@ -75,8 +76,9 @@
                   <constraint>
                     <validator name="numeric" argument="--range 1-255"/>
                   </constraint>
-                  <constraintErrorMessage>threshold must be between 1 and 255</constraintErrorMessage>
+                  <constraintErrorMessage>Threshold must be between 1 and 255</constraintErrorMessage>
                 </properties>
+                <defaultValue>1</defaultValue>
               </leafNode>
               <leafNode name="whitelist">
                 <properties>
diff --git a/src/conf_mode/igmp_proxy.py b/src/conf_mode/igmp_proxy.py
index d7d8ffd5e..90f3f30a8 100755
--- a/src/conf_mode/igmp_proxy.py
+++ b/src/conf_mode/igmp_proxy.py
@@ -17,99 +17,65 @@
 import os
 
 from sys import exit
-from copy import deepcopy
-
 from netifaces import interfaces
+
 from vyos.config import Config
-from vyos import ConfigError
-from vyos.util import call
+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'/etc/igmpproxy.conf'
 
-default_config_data = {
-    'disable': False,
-    'disable_quickleave': False,
-    'igmp_conf': False,
-    'pim_conf': False,
-    'interfaces': [],
-}
-
 def get_config(config=None):
-    igmp_proxy = deepcopy(default_config_data)
     if config:
         conf = config
     else:
         conf = Config()
 
-    if conf.exists('protocols igmp'):
-        igmp_proxy['igmp_conf'] = True
-
-    if conf.exists('protocols pim'):
-        igmp_proxy['pim_conf'] = True
-
     base = ['protocols', 'igmp-proxy']
-    if not conf.exists(base):
-        return None
-    else:
-        conf.set_level(base)
-
-    # Network interfaces to listen on
-    if conf.exists(['disable']):
-        igmp_proxy['disable'] = True
-
-    # Option to disable "quickleave"
-    if conf.exists(['disable-quickleave']):
-        igmp_proxy['disable_quickleave'] = True
-
-    for intf in conf.list_nodes(['interface']):
-        conf.set_level(base + ['interface', intf])
-        interface = {
-            'name': intf,
-            'alt_subnet': [],
-            'role': 'downstream',
-            'threshold': '1',
-            'whitelist': []
-        }
+    igmp_proxy = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
 
-        if conf.exists(['alt-subnet']):
-            interface['alt_subnet'] = conf.return_values(['alt-subnet'])
+    if 'interface' in igmp_proxy:
+        # T2665: we must add the tagNode defaults individually until this is
+        # moved to the base class
+        default_values = defaults(base + ['interface'])
+        for interface in igmp_proxy['interface']:
+            igmp_proxy['interface'][interface] = dict_merge(default_values,
+                igmp_proxy['interface'][interface])
 
-        if conf.exists(['role']):
-            interface['role'] = conf.return_value(['role'])
 
-        if conf.exists(['threshold']):
-            interface['threshold'] = conf.return_value(['threshold'])
+    if conf.exists(['protocols', 'igmp']):
+        igmp_proxy.update({'igmp_configured': ''})
 
-        if conf.exists(['whitelist']):
-            interface['whitelist'] = conf.return_values(['whitelist'])
-
-        # Append interface configuration to global configuration list
-        igmp_proxy['interfaces'].append(interface)
+    if conf.exists(['protocols', 'pim']):
+        igmp_proxy.update({'pim_configured': ''})
 
     return igmp_proxy
 
 def verify(igmp_proxy):
     # bail out early - looks like removal from running config
-    if igmp_proxy is None:
+    if not igmp_proxy or 'disable' in igmp_proxy:
         return None
 
-    # bail out early - service is disabled
-    if igmp_proxy['disable']:
-        return None
+    if 'igmp_configured' in igmp_proxy or 'pim_configured' in igmp_proxy:
+        raise ConfigError('Can not configure both IGMP proxy and PIM '\
+                          'at the same time')
 
     # at least two interfaces are required, one upstream and one downstream
-    if len(igmp_proxy['interfaces']) < 2:
-        raise ConfigError('Must define an upstream and at least 1 downstream interface!')
+    if 'interface' not in igmp_proxy or len(igmp_proxy['interface']) < 2:
+        raise ConfigError('Must define exactly one upstream and at least one ' \
+                          'downstream interface!')
 
     upstream = 0
-    for interface in igmp_proxy['interfaces']:
-        if interface['name'] not in interfaces():
-            raise ConfigError('Interface "{}" does not exist'.format(interface['name']))
-        if "upstream" == interface['role']:
+    for interface, config in igmp_proxy['interface'].items():
+        if interface not in interfaces():
+            raise ConfigError(f'Interface "{interface}" does not exist')
+        if dict_search('role', config) == 'upstream':
             upstream += 1
 
     if upstream == 0:
@@ -121,27 +87,26 @@ def verify(igmp_proxy):
 
 def generate(igmp_proxy):
     # bail out early - looks like removal from running config
-    if igmp_proxy is None:
+    if not igmp_proxy:
         return None
 
     # bail out early - service is disabled, but inform user
-    if igmp_proxy['disable']:
-        print('Warning: IGMP Proxy will be deactivated because it is disabled')
+    if 'disable' in igmp_proxy:
+        print('WARNING: IGMP Proxy will be deactivated because it is disabled')
         return None
 
-    render(config_file, 'igmp-proxy/igmpproxy.conf.tmpl', igmp_proxy)
+    render(config_file, 'igmp-proxy/igmpproxy.conf.tmpl', igmp_proxy,
+           trim_blocks=True)
+
     return None
 
 def apply(igmp_proxy):
-    if igmp_proxy is None or igmp_proxy['disable']:
-        # IGMP Proxy support is removed in the commit
-        call('systemctl stop igmpproxy.service')
-        if os.path.exists(config_file):
-            os.unlink(config_file)
+    if not igmp_proxy or 'disable' in igmp_proxy:
+         # IGMP Proxy support is removed in the commit
+         call('systemctl stop igmpproxy.service')
+         if os.path.exists(config_file):
+             os.unlink(config_file)
     else:
-        if igmp_proxy['igmp_conf'] or igmp_proxy['pim_conf']:
-            raise ConfigError('IGMP proxy and PIM cannot be both configured at the same time')
-
         call('systemctl restart igmpproxy.service')
 
     return None