Permit multiple patterns for the driver globs in match (LP: #1918421) (#202)

* Permit multiple driver globs, provided as list

Co-authored-by: Lukas Märdian <slyon@ubuntu.com>

* Tests for multiple driver globs, provided as list

Co-authored-by: Lukas Märdian <slyon@ubuntu.com>

* Update docs

* parse:netplan: test: enhance match.driver YAML parser & generator

* parse:netplan: match.driver tab separation

* TODO: update for pending ABI breaking change

* cli:utils: allow passing any iterable

Co-authored-by: Lukas Märdian <slyon@ubuntu.com>

9 files changed, 138 insertions, 7 deletions

diff --git a/TODO b/TODO
index 710b2df..f4fe082 100644
--- a/TODO
+++ b/TODO
@@ -47,3 +47,5 @@
 - change route->scope to ENUM
 
 - move tunnel_ttl into tunnel struct
+
+- store match.driver as a list rather than a string
\ No newline at end of file
diff --git a/doc/netplan.md b/doc/netplan.md
index 27bc6f0..c87102b 100644
--- a/doc/netplan.md
+++ b/doc/netplan.md
@@ -96,10 +96,10 @@ Virtual devices
      :   Device's MAC address in the form "XX:XX:XX:XX:XX:XX". Globs are not
          allowed.
 
-     ``driver`` (scalar)
+     ``driver`` (scalar or sequence of scalars) – sequence since **0.104**
      :   Kernel driver name, corresponding to the ``DRIVER`` udev property.
-         Globs are supported. Matching on driver is *only* supported with
-         networkd.
+         A sequence of globs is supported, any of which must match.
+         Matching on driver is *only* supported with networkd.
 
      Examples:
 
@@ -119,6 +119,12 @@ Virtual devices
               driver: ixgbe
               name: en*s0
 
+     - first card with a driver matching ``bcmgenet`` or ``smsc*``:
+
+            match:
+              driver: ["bcmgenet", "smsc*"]
+              name: en*
+
 ``set-name`` (scalar)
 
 :    When matching on unique properties such as path or MAC, or with additional
diff --git a/netplan/cli/utils.py b/netplan/cli/utils.py
index 94a48aa..a05b42b 100644
--- a/netplan/cli/utils.py
+++ b/netplan/cli/utils.py
@@ -157,9 +157,15 @@ def is_interface_matching_name(interface, match_name):
 
 
 def is_interface_matching_driver_name(interface, match_driver):
+    driver_globs = match_driver
+    if isinstance(driver_globs, str):
+        driver_globs = [match_driver]
     driver_name = get_interface_driver_name(interface)
     # globs are supported
-    return fnmatch.fnmatchcase(driver_name, match_driver)
+    return any(
+        fnmatch.fnmatchcase(driver_name, pattern)
+        for pattern in driver_globs
+    )
 
 
 def is_interface_matching_macaddress(interface, match_mac):
diff --git a/src/netplan.c b/src/netplan.c
index 6d411fb..bc3db26 100644
--- a/src/netplan.c
+++ b/src/netplan.c
@@ -108,7 +108,16 @@ write_match(yaml_event_t* event, yaml_emitter_t* emitter, const NetplanNetDefini
     YAML_MAPPING_OPEN(event, emitter);
     YAML_NONNULL_STRING(event, emitter, "name", def->match.original_name);
     YAML_NONNULL_STRING(event, emitter, "macaddress", def->match.mac)
-    YAML_NONNULL_STRING(event, emitter, "driver", def->match.driver)
+    if (def->match.driver && strchr(def->match.driver, '\t')) {
+        gchar **split = g_strsplit(def->match.driver, "\t", 0);
+        YAML_SCALAR_PLAIN(event, emitter, "driver");
+        YAML_SEQUENCE_OPEN(event, emitter);
+        for (unsigned i = 0; split[i]; ++i)
+            YAML_SCALAR_QUOTED(event, emitter, split[i]);
+        YAML_SEQUENCE_CLOSE(event, emitter);
+        g_strfreev(split);
+    } else
+        YAML_NONNULL_STRING(event, emitter, "driver", def->match.driver);
     YAML_MAPPING_CLOSE(event, emitter);
     return TRUE;
 err_path: return FALSE; // LCOV_EXCL_LINE
diff --git a/src/networkd.c b/src/networkd.c
index 55c3d36..6d26047 100644
--- a/src/networkd.c
+++ b/src/networkd.c
@@ -73,7 +73,14 @@ append_match_section(const NetplanNetDefinition* def, GString* s, gboolean match
      * (of the given type) */
 
     g_string_append(s, "[Match]\n");
-    if (def->match.driver)
+    if (def->match.driver && strchr(def->match.driver, '\t')) {
+        gchar **split = g_strsplit(def->match.driver, "\t", 0);
+        g_string_append_printf(s, "Driver=%s", split[0]);
+        for (unsigned i = 1; split[i]; ++i)
+            g_string_append_printf(s, " %s", split[i]);
+        g_string_append(s, "\n");
+        g_strfreev(split);
+    } else if (def->match.driver)
         g_string_append_printf(s, "Driver=%s\n", def->match.driver);
     if (def->match.mac)
         g_string_append_printf(s, "MACAddress=%s\n", def->match.mac);
diff --git a/src/parse.c b/src/parse.c
index e6e5f26..2dece97 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -620,8 +620,47 @@ handle_netdef_datalist(NetplanParser* npp, yaml_node_t* node, const void* data,
  * Grammar and handlers for network config "match" entry
  ****************************************************/
 
+static gboolean
+handle_match_driver(NetplanParser* npp, yaml_node_t* node, const void* _, GError** error)
+{
+    gboolean ret = FALSE;
+    yaml_node_t *elem = NULL;
+    GString *sequence = NULL;
+
+    /* We overload the 'driver' setting for matches; such that it can either be a
+     * single scalar specifying a single driver glob/match, or a sequence of many
+     * globs any of which must match. */
+    if (node->type == YAML_SCALAR_NODE) {
+        if (g_strrstr(scalar(node), " "))
+            return yaml_error(npp, node, error, "A 'driver' glob cannot contain whitespace");
+        ret = handle_netdef_str(npp, node, netdef_offset(match.driver), error);
+    } else if (node->type == YAML_SEQUENCE_NODE) {
+        for (yaml_node_item_t *iter = node->data.sequence.items.start; iter < node->data.sequence.items.top; iter++) {
+            elem = yaml_document_get_node(&npp->doc, *iter);
+            assert_type(npp, elem, YAML_SCALAR_NODE);
+            if (g_strrstr(scalar(elem), " "))
+                return yaml_error(npp, node, error, "A 'driver' glob cannot contain whitespace");
+
+            if (!sequence)
+                sequence = g_string_new(scalar(elem));
+            else
+                g_string_append_printf(sequence, "\t%s", scalar(elem)); /* tab separated */
+        }
+
+        if (!sequence)
+            return yaml_error(npp, node, error, "invalid sequence for 'driver'");
+
+        npp->current.netdef->match.driver = g_strdup(sequence->str);
+        g_string_free(sequence, TRUE);
+        ret = TRUE;
+    } else
+        return yaml_error(npp, node, error, "invalid type for 'driver': must be a scalar or a sequence of scalars");
+
+    return ret;
+}
+
 static const mapping_entry_handler match_handlers[] = {
-    {"driver", YAML_SCALAR_NODE, {.generic=handle_netdef_str}, netdef_offset(match.driver)},
+    {"driver", YAML_NO_NODE, {.generic=handle_match_driver}},
     {"macaddress", YAML_SCALAR_NODE, {.generic=handle_netdef_mac}, netdef_offset(match.mac)},
     {"name", YAML_SCALAR_NODE, {.generic=handle_netdef_id}, netdef_offset(match.original_name)},
     {NULL}
diff --git a/src/types.h b/src/types.h
index 4014c08..008b4ee 100644
--- a/src/types.h
+++ b/src/types.h
@@ -233,6 +233,7 @@ struct netplan_net_definition {
     /* these properties are only valid for physical interfaces (type < ND_VIRTUAL) */
     char* set_name;
     struct {
+        /* A glob (or tab-separated list of globs) to match a specific driver */
         char* driver;
         char* mac;
         char* original_name;
diff --git a/tests/generator/test_ethernets.py b/tests/generator/test_ethernets.py
index b98581b..46bf764 100644
--- a/tests/generator/test_ethernets.py
+++ b/tests/generator/test_ethernets.py
@@ -468,6 +468,58 @@ method=ignore
         driver: ixgbe''', expect_fail=True)
         self.assertIn('NetworkManager definitions do not support matching by driver', err)
 
+    def test_eth_match_by_drivers(self):
+        self.generate('''network:
+  version: 2
+  renderer: networkd
+  ethernets:
+    def1:
+      match:
+        driver: ["bcmgenet", "smsc*"]''')
+        self.assert_networkd({'def1.network': '''[Match]
+Driver=bcmgenet smsc*
+
+[Network]
+LinkLocalAddressing=ipv6
+'''})
+
+    def test_eth_match_by_drivers_whitespace(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    def1:
+      match:
+        driver: "bcmgenet smsc*"''', expect_fail=True)
+        self.assertIn('A \'driver\' glob cannot contain whitespace', err)
+
+    def test_eth_match_by_drivers_whitespace_sequence(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    def1:
+      match:
+        driver: ["ixgbe", "bcmgenet smsc*"]''', expect_fail=True)
+        self.assertIn('A \'driver\' glob cannot contain whitespace', err)
+
+    def test_eth_match_by_drivers_invalid_sequence(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    def1:
+      match:
+        driver: []''', expect_fail=True)
+        self.assertIn('invalid sequence for \'driver\'', err)
+
+    def test_eth_match_by_drivers_invalid_type(self):
+        err = self.generate('''network:
+  version: 2
+  ethernets:
+    def1:
+      match:
+        driver:
+            some_mapping: true''', expect_fail=True)
+        self.assertIn('invalid type for \'driver\': must be a scalar or a sequence of scalars', err)
+
     def test_eth_match_by_driver_rename(self):
         # in this case udev will rename the device so that NM can use the name
         self.generate('''network:
diff --git a/tests/test_utils.py b/tests/test_utils.py
index e3116e7..e2ee73a 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -162,6 +162,15 @@ class TestUtils(unittest.TestCase):
         iface = utils.find_matching_iface(DEVICES, match)
         self.assertEqual(iface, 'ens4')
 
+    @patch('netplan.cli.utils.get_interface_driver_name')
+    def test_find_matching_iface_name_and_drivers(self, gidn):
+        # we mock-out get_interface_driver_name to return useful values for the test
+        gidn.side_effect = lambda x: 'foo' if x == 'ens4' else 'bar'
+
+        match = {'name': 'ens?', 'driver': ['baz', 'f*', 'quux']}
+        iface = utils.find_matching_iface(DEVICES, match)
+        self.assertEqual(iface, 'ens4')
+
     @patch('netifaces.ifaddresses')
     def test_interface_macaddress(self, ifaddr):
         ifaddr.side_effect = lambda _: {netifaces.AF_LINK: [{'addr': '00:01:02:03:04:05'}]}