T810: bugfix broadcast-relay address validator, add 'disable' functionality

Whole broadcast relay service can be temporary disabled via
  set service broadcast-relay disable

Individual instances of the broadcast relay service can be disabled
  set service broadcast-relay id <n> disable

2 files changed, 129 insertions, 57 deletions

diff --git a/interface-definitions/bcast-relay.xml b/interface-definitions/bcast-relay.xml
index 0437192fa..fdba554db 100644
--- a/interface-definitions/bcast-relay.xml
+++ b/interface-definitions/bcast-relay.xml
@@ -3,12 +3,18 @@
 <interfaceDefinition>
   <node name="service">
     <children>
-      <node name="broadcast-relay">
+      <node name="broadcast-relay" owner="${vyos_conf_scripts_dir}/bcast_relay.py">
         <properties>
-          <help>UDP Broadcast Relay parameters</help>
+          <help>UDP broadcast relay service</help>
         </properties>
         <children>
-          <tagNode name="id" owner="${vyos_conf_scripts_dir}/bcast_relay.py">
+          <leafNode name="disable">
+            <properties>
+              <help>Globally disable broadcast relay service</help>
+              <valueless/>
+            </properties>
+          </leafNode>
+          <tagNode name="id">
             <properties>
               <help>Unique ID for each UDP port to forward</help>
               <valueHelp>
@@ -21,6 +27,12 @@
               </constraint>
             </properties>
             <children>
+              <leafNode name="disable">
+                <properties>
+                  <help>Disable broadcast relay service instance</help>
+                  <valueless/>
+                </properties>
+              </leafNode>
               <leafNode name="address">
                 <properties>
                   <help>Set source IP of forwarded packets, otherwise original senders address is used</help>
@@ -29,7 +41,7 @@
                     <description>Optional source address for forwarded packets</description>
                   </valueHelp>
                   <constraint>
-                    <validator name="ipv4"/>
+                    <validator name="ipv4-address"/>
                   </constraint>
                 </properties>
               </leafNode>
diff --git a/src/conf_mode/bcast_relay.py b/src/conf_mode/bcast_relay.py
index 95f6215b5..8cc948610 100755
--- a/src/conf_mode/bcast_relay.py
+++ b/src/conf_mode/bcast_relay.py
@@ -20,55 +20,104 @@ import sys
 import os
 import fnmatch
 import subprocess
+import jinja2
 
 from vyos.config import Config
 from vyos import ConfigError
 
 config_file = r'/etc/default/udp-broadcast-relay'
 
+config_tmpl = """
+### Autogenerated by bcast_relay.py ###
+
+# UDP broadcast relay configuration for instance {{ id }}
+{%- if description %}
+# Comment: {{ description }}
+{% endif -%}
+DAEMON_ARGS="{% if address %}-s {{ address }} {% endif %}{{ id }} {{ port }} {{ interfaces | join(' ') }}"
+"""
+
+default_config_data = {
+    'disabled': False,
+    'instances': []
+}
+
 def get_config():
+    relay = default_config_data
     conf = Config()
-    conf.set_level("service broadcast-relay id")
-    relay_id = conf.list_nodes("")
-    relays = []
-
-    for id in relay_id:
-        interface_list = []
-        address = conf.return_value("{0} address".format(id))
-        description = conf.return_value("{0} description".format(id))
-        port = conf.return_value("{0} port".format(id))
-
-        # split the interface name listing and form a list
-        if conf.exists("{0} interface".format(id)):
-            intfs_names = []
-            intfs_names = conf.return_values("{0} interface".format(id))
-
-            for name in intfs_names:
-                interface_list.append(name)
-
-        relay = {
-            "id": id,
-            "address": address,
-            "description": description,
-            "interfaces" : interface_list,
-            "port": port
+    if not conf.exists('service broadcast-relay'):
+        return None
+    else:
+        conf.set_level('service broadcast-relay')
+
+    # Service can be disabled by user
+    if conf.exists('disable'):
+        relay['disabled'] = True
+        return relay
+
+    # Parse configuration of each individual instance
+    if conf.exists('id'):
+        for id in conf.list_nodes('id'):
+            conf.set_level('service broadcast-relay id {0}'.format(id))
+            config = {
+                'id': id,
+                'disabled': False,
+                'address': '',
+                'description': '',
+                'interfaces': [],
+                'port': ''
             }
-        relays.append(relay)
 
-    return relays
+            # Check if individual broadcast relay service is disabled
+            if conf.exists('disable'):
+                config['disabled'] = True
+
+            # Source IP of forwarded packets, if empty original senders address is used
+            if conf.exists('address'):
+                config['address'] = conf.return_value('address')
+
+            # A description for each individual broadcast relay service
+            if conf.exists('description'):
+                config['description'] = conf.return_value('description')
+
+            # UDP port to listen on for broadcast frames
+            if conf.exists('port'):
+                config['port'] = conf.return_value('port')
+
+            # Network interfaces to listen on for broadcast frames to be relayed
+            if conf.exists('interface'):
+                config['interfaces'] = conf.return_values('interface')
+
+            relay['instances'].append(config)
 
-def verify(relays):
-    for relay in relays:
-        if not relay["port"]:
-            raise ConfigError("UDP broadcast relay 'id {0}' requires a port number".format(relay["id"]))
+    return relay
 
-        if len(relay["interfaces"]) < 2:
-            raise ConfigError("UDP broadcast relay 'id {0}' requires at least 2 interfaces".format(relay["id"]))
+def verify(relay):
+    if relay is None:
+        return None
+
+    if relay['disabled']:
+        return None
+
+    for r in relay['instances']:
+        # we don't have to check this instance when it's disabled
+        if r['disabled']:
+            continue
+
+        # we certainly require a UDP port to listen to
+        if not r['port']:
+            raise ConfigError('UDP broadcast relay "{0}" requires a port number'.format(r['id']))
+
+        # Relaying data without two interface is kinda senseless ...
+        if len(r['interfaces']) < 2:
+            raise ConfigError('UDP broadcast relay "id {0}" requires at least 2 interfaces'.format(r['id']))
 
     return None
 
-def generate(relays):
-    config_header = '### Autogenerated by bcast_relay.py ###\n'
+
+def generate(relay):
+    if relay is None:
+        return None
 
     config_dir = os.path.dirname(config_file)
     config_filename = os.path.basename(config_file)
@@ -82,32 +131,43 @@ def generate(relays):
     # sort our list
     active_configs.sort()
 
+    # delete old configuration files
     for id in active_configs[:]:
-        os.unlink(config_file + id)
-
-    for relay in relays:
-         file = config_file + str(relay["id"])
-         interfaces = ' '.join(str(intf) for intf in relay["interfaces"])
-         config_args = 'DAEMON_ARGS="{0} {1}"\n'.format(relay["port"], interfaces)
-
-         f = open(file, 'w')
-         f.write(config_header)
-         if relay["description"]:
-             f.write('# ' + relay["description"] + '\n')
-         f.write(config_args)
-         f.close()
+        if os.path.exists(config_file + id):
+            os.unlink(config_file + id)
+
+    # If the service is disabled, we can bail out here
+    if relay['disabled']:
+        print('Warning: UDP broadcast relay service will be deactivated because it is disabled')
+        return None
+
+    for r in relay['instances']:
+        # Skip writing instance config when it's disabled
+        if r['disabled']:
+            continue
+
+        # configuration filename contains instance id
+        file = config_file + str(r['id'])
+        tmpl = jinja2.Template(config_tmpl)
+        config_text = tmpl.render(r)
+        with open(file, 'w') as f:
+            f.write(config_text)
 
     return None
 
-def apply(relays):
+def apply(relay):
     # first stop all running services
-    cmd = "sudo systemctl stop udp-broadcast-relay@{1..99}"
-    os.system(cmd)
+    os.system('sudo systemctl stop udp-broadcast-relay@{1..99}')
+
+    if (relay is None) or relay['disabled']:
+        return None
 
     # start only required service instances
-    for relay in relays:
-        cmd = "sudo systemctl start udp-broadcast-relay@{0}".format(relay["id"])
-        os.system(cmd)
+    for r in relay['instances']:
+        # Don't start individual instance when it's disabled
+        if r['disabled']:
+            continue
+        os.system('sudo systemctl start udp-broadcast-relay@{0}'.format(r['id']))
 
     return None