ovs: quote external-ids and other-config values

For complex values, ovs-vsctl requires that they are quoted or it will
error out. LP: #2070318

While here, add some debugging information so we can see the ovs-vsctl
command executed by "netplan apply" with --debug.

Author: daniloegea

netplan_cli/cli/ovs.py

@@ -18,7 +18,6 @@
 import logging
 import os
 import subprocess
-import re
 
 from .utils import systemctl_is_active, systemctl_is_installed
 
@@ -52,21 +51,22 @@ def _del_col(type, iface, column, value):
     default = DEFAULTS.get(column)
     if default is None:
         # removes the exact value only if it was set by netplan
-        subprocess.check_call([OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, value])
+        cmd = [OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, value]
+        logging.debug('Running: %s' % ' '.join(cmd))
+        subprocess.check_call(cmd)
     elif default and default != value:
         # reset to default, if its not the default already
-        subprocess.check_call([OPENVSWITCH_OVS_VSCTL, 'set', type, iface, '%s=%s' % (column, default)])
+        cmd = [OPENVSWITCH_OVS_VSCTL, 'set', type, iface, '%s=%s' % (column, default)]
+        logging.debug('Running: %s' % ' '.join(cmd))
+        subprocess.check_call(cmd)
 
 
 def _del_dict(type, iface, column, key, value):
     """Cleanup values from a dictionary (i.e. "column:key=value")"""
     # removes the exact value only if it was set by netplan
-    subprocess.check_call([OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, key, _escape_colon(value)])
-
-
-# for ovsdb remove: column key's value can not contain bare ':', need to escape with '\'
-def _escape_colon(literal):
-    return re.sub(r'([^\\]):', r'\g<1>\:', literal)
+    cmd = [OPENVSWITCH_OVS_VSCTL, 'remove', type, iface, column, '%s=\"%s\"' % (key, value)]
+    logging.debug('Running: %s' % ' '.join(cmd))
+    subprocess.check_call(cmd)
 
 
 def _del_global(type, iface, key, value):

src/openvswitch.c

@@ -134,7 +134,7 @@ write_ovs_tag_setting(const gchar* id, const char* type, const char* col, const
     g_string_append_printf(s, "%s", col);
     if (key)
         g_string_append_printf(s, "/%s", key);
-    g_string_append_printf(s, "=%s", clean_value);
+    g_string_append_printf(s, "=\"%s\"", clean_value);
     append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s", type, id, s->str);
     g_string_free(s, TRUE);
 }
@@ -150,7 +150,7 @@ write_ovs_additional_data(GHashTable *data, const char* type, const gchar* id, G
     while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &value)) {
         /* XXX: we need to check what happens when an invalid key=value pair
             gets supplied here. We might want to handle this somehow. */
-        append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s:%s=%s",
+        append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s:%s=\"%s\"",
                            type, id, setting, key, value);
         write_ovs_tag_setting(id, type, setting, key, value, cmds);
     }
@@ -210,7 +210,7 @@ STATIC void
 write_ovs_tag_netplan(const gchar* id, const char* type, GString* cmds)
 {
     /* Mark this bridge/port/interface as created by netplan */
-    append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s external-ids:netplan=true",
+    append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s external-ids:netplan=\"true\"",
                        type, id);
 }
 

tests/generator/base.py

@@ -65,11 +65,11 @@
 OVS_PHYSICAL = _OVS_BASE + 'Requires=sys-subsystem-net-devices-%(iface)s.device\nAfter=sys-subsystem-net-devices-%(iface)s\
 .device\nAfter=netplan-ovs-cleanup.service\nBefore=network.target\nWants=network.target\n%(extra)s'
 OVS_VIRTUAL = _OVS_BASE + 'After=netplan-ovs-cleanup.service\nBefore=network.target\nWants=network.target\n%(extra)s'
-OVS_BR_DEFAULT = 'ExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan=true\nExecStart=/usr/bin/ovs-vsctl \
+OVS_BR_DEFAULT = 'ExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan=\"true\"\nExecStart=/usr/bin/ovs-vsctl \
 set-fail-mode %(iface)s standalone\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan/global/set-fail-mode=\
-standalone\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s mcast_snooping_enable=false\nExecStart=/usr/bin/ovs-vsctl set \
-Bridge %(iface)s external-ids:netplan/mcast_snooping_enable=false\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s \
-rstp_enable=false\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan/rstp_enable=false\n'
+\"standalone\"\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s mcast_snooping_enable=false\nExecStart=/usr/bin/ovs-vsctl set \
+Bridge %(iface)s external-ids:netplan/mcast_snooping_enable="false"\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s \
+rstp_enable=false\nExecStart=/usr/bin/ovs-vsctl set Bridge %(iface)s external-ids:netplan/rstp_enable=\"false\"\n'
 OVS_BR_EMPTY = _OVS_BASE + 'After=netplan-ovs-cleanup.service\nBefore=network.target\nWants=network.target\n\n[Service]\n\
 Type=oneshot\nTimeoutStartSec=10s\nExecStart=/usr/bin/ovs-vsctl --may-exist add-br %(iface)s\n' + OVS_BR_DEFAULT
 OVS_CLEANUP = _OVS_BASE + 'ConditionFileIsExecutable=/usr/bin/ovs-vsctl\nBefore=network.target\nWants=network.target\n\n\