Gateway fields deprecation and default routing support (FR-728) (LP: #1756590) (#216)

This patchset deprecates the gateway4/6 fields, advising the user to use a proper route instead. The rationale is that it groups all routing matters in the same place, making things more consistent. Along with this deprecation comes the support for the "default" keyword in the route to field, and an expansion of the gateway validation step to encompass all default routes.

Examples
```
network:
  ethernets:
    en1:
      addresses:
      - 1.2.3.4/8
      - fc00:123:4567:89ab:cdef::1234/64
      routes:
      - to: default
        via: 1.1.1.1
      - to: default
        via: "fc00:123:4567:89cb:cdef::1"
```

Addresses LP: #1756590

COMMITS:
* tests: split gateway4/6 into their own tests
* doc: add a paragraph regarding default routes
* netplan: deprecate gateway4/6
* tests: gw: add a multipass check
* parse: accept default routes as valid input
* validation: look at all routes for default route validation
This makes the default route validation much more comprehensive, looking
at all the routes. The underlying code is a bit more involved as a
result, though.
* networkd: support default route specification
* nm: support default route specification
* Rewording of error messages for consistency
Co-authored-by: Lukas Märdian <slyon@ubuntu.com>
* Whitespace fixes in string literals
Co-authored-by: Lukas Märdian <slyon@ubuntu.com>
* tests: add an integration test for default routes
* tests:integration:routing: not using DHCP in routes_default
Co-authored-by: Lukas Märdian <slyon@ubuntu.com>

7 files changed, 151 insertions, 17 deletions

diff --git a/src/networkd.c b/src/networkd.c
index c51e949..8884286 100644
--- a/src/networkd.c
+++ b/src/networkd.c
@@ -422,9 +422,14 @@ write_netdev_file(const NetplanNetDefinition* def, const char* rootdir, const ch
 static void
 write_route(NetplanIPRoute* r, GString* s)
 {
+    const char *to;
     g_string_append_printf(s, "\n[Route]\n");
 
-    g_string_append_printf(s, "Destination=%s\n", r->to);
+    if (g_strcmp0(r->to, "default") == 0)
+        to = get_global_network(r->family);
+    else
+        to = r->to;
+    g_string_append_printf(s, "Destination=%s\n", to);
 
     if (r->via)
         g_string_append_printf(s, "Gateway=%s\n", r->via);
diff --git a/src/nm.c b/src/nm.c
index ece3c28..aef15ac 100644
--- a/src/nm.c
+++ b/src/nm.c
@@ -216,10 +216,16 @@ write_routes(const NetplanNetDefinition* def, GKeyFile *kf, int family)
     if (def->routes != NULL) {
         for (unsigned i = 0, j = 1; i < def->routes->len; ++i) {
             const NetplanIPRoute *cur_route = g_array_index(def->routes, NetplanIPRoute*, i);
+            const char *destination;
 
             if (cur_route->family != family)
                 continue;
 
+            if (g_strcmp0(cur_route->to, "default") == 0)
+                destination = get_global_network(family);
+            else
+                destination = cur_route->to;
+
             if (cur_route->type && g_ascii_strcasecmp(cur_route->type, "unicast") != 0) {
                 g_fprintf(stderr, "ERROR: %s: NetworkManager only supports unicast routes\n", def->id);
                 exit(1);
@@ -238,7 +244,7 @@ write_routes(const NetplanNetDefinition* def, GKeyFile *kf, int family)
 
             tmp_key = g_strdup_printf("route%d", j);
             tmp_val = g_string_new(NULL);
-            g_string_printf(tmp_val, "%s,%s", cur_route->to, cur_route->via);
+            g_string_printf(tmp_val, "%s,%s", destination, cur_route->via);
             if (cur_route->metric != NETPLAN_METRIC_UNSPEC)
                 g_string_append_printf(tmp_val, ",%d", cur_route->metric);
             g_key_file_set_string(kf, group, tmp_key, tmp_val->str);
diff --git a/src/parse.c b/src/parse.c
index c1f8525..09cd1e2 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -1118,6 +1118,8 @@ handle_gateway4(yaml_document_t* doc, yaml_node_t* node, const void* _, GError**
     if (!is_ip4_address(scalar(node)))
         return yaml_error(node, error, "invalid IPv4 address '%s'", scalar(node));
     set_str_if_null(cur_netdef->gateway4, scalar(node));
+    g_warning("`gateway4` has been deprecated, use default routes instead.\n"
+              "See the 'Default routes' section of the documentation for more details.");
     return TRUE;
 }
 
@@ -1127,6 +1129,8 @@ handle_gateway6(yaml_document_t* doc, yaml_node_t* node, const void* _, GError**
     if (!is_ip6_address(scalar(node)))
         return yaml_error(node, error, "invalid IPv6 address '%s'", scalar(node));
     set_str_if_null(cur_netdef->gateway6, scalar(node));
+    g_warning("`gateway6` has been deprecated, use default routes instead.\n"
+              "See the 'Default routes' section of the documentation for more details.");
     return TRUE;
 }
 
@@ -1460,6 +1464,16 @@ handle_routes_ip(yaml_document_t* doc, yaml_node_t* node, const void* data, GErr
 }
 
 static gboolean
+handle_routes_destination(yaml_document_t *doc, yaml_node_t *node, const void *data, GError **error)
+{
+    const char *addr = scalar(node);
+    if (g_strcmp0(addr, "default") != 0)
+        return handle_routes_ip(doc, node, route_offset(to), error);
+    set_str_if_null(cur_route->to, addr);
+    return TRUE;
+}
+
+static gboolean
 handle_ip_rule_ip(yaml_document_t* doc, yaml_node_t* node, const void* data, GError** error)
 {
     guint offset = GPOINTER_TO_UINT(data);
@@ -1607,7 +1621,7 @@ static const mapping_entry_handler routes_handlers[] = {
     {"on-link", YAML_SCALAR_NODE, handle_routes_bool, NULL, route_offset(onlink)},
     {"scope", YAML_SCALAR_NODE, handle_routes_scope},
     {"table", YAML_SCALAR_NODE, handle_routes_guint, NULL, route_offset(table)},
-    {"to", YAML_SCALAR_NODE, handle_routes_ip, NULL, route_offset(to)},
+    {"to", YAML_SCALAR_NODE, handle_routes_destination},
     {"type", YAML_SCALAR_NODE, handle_routes_type},
     {"via", YAML_SCALAR_NODE, handle_routes_ip, NULL, route_offset(via)},
     {"metric", YAML_SCALAR_NODE, handle_routes_guint, NULL, route_offset(metric)},
@@ -1641,6 +1655,7 @@ handle_routes(yaml_document_t* doc, yaml_node_t* node, const void* _, GError** e
         cur_route->scope = g_strdup("global");
         cur_route->family = G_MAXUINT; /* 0 is a valid family ID */
         cur_route->metric = NETPLAN_METRIC_UNSPEC; /* 0 is a valid metric */
+        cur_route->table = NETPLAN_ROUTE_TABLE_UNSPEC;
         g_debug("%s: adding new route", cur_netdef->id);
 
         if (!process_mapping(doc, entry, routes_handlers, NULL, error))
@@ -2687,10 +2702,14 @@ GHashTable *
 netplan_finish_parse(GError** error)
 {
     if (netdefs) {
+        GError *recoverable = NULL;
         g_debug("We have some netdefs, pass them through a final round of validation");
-        if (!validate_gateway_consistency(netdefs, error))
-            g_warning("More than one global gateway specified, the routing is ambiguous! "
-                      "Please set up multiple routing tables and use `routing-policy` instead.");
+        if (!validate_default_route_consistency(netdefs, &recoverable)) {
+            g_warning("Problem encountered while validating default route consistency."
+                      "Please set up multiple routing tables and use `routing-policy` instead.\n"
+                      "Error: %s", (recoverable) ? recoverable->message : "");
+            g_clear_error(&recoverable);
+        }
         g_hash_table_foreach(netdefs, finish_iterator, error);
     }
 
diff --git a/src/util.c b/src/util.c
index 68d0484..a4c0dba 100644
--- a/src/util.c
+++ b/src/util.c
@@ -17,6 +17,7 @@
 
 #include <stdlib.h>
 #include <unistd.h>
+#include <arpa/inet.h>
 
 #include <glib.h>
 #include <glib/gprintf.h>
@@ -312,3 +313,17 @@ netplan_get_filename_by_id(const char* netdef_id, const char* rootdir)
     netplan_clear_netdefs();
     return filename;
 }
+
+/**
+ * Get a static string describing the default global network
+ * for a given address family.
+ */
+const char *
+get_global_network(int ip_family)
+{
+    g_assert(ip_family == AF_INET || ip_family == AF_INET6);
+    if (ip_family == AF_INET)
+        return "0.0.0.0/0";
+    else
+        return "::/0";
+}
diff --git a/src/util.h b/src/util.h
index 3a0dcc2..f34c601 100644
--- a/src/util.h
+++ b/src/util.h
@@ -27,6 +27,8 @@ void g_string_free_to_file(GString* s, const char* rootdir, const char* path, co
 void unlink_glob(const char* rootdir, const char* _glob);
 int find_yaml_glob(const char* rootdir, glob_t* out_glob);
 
+const char *get_global_network(int ip_family);
+
 int wifi_get_freq24(int channel);
 int wifi_get_freq5(int channel);
 
diff --git a/src/validation.c b/src/validation.c
index 27eb3b5..a0dca68 100644
--- a/src/validation.c
+++ b/src/validation.c
@@ -372,10 +372,73 @@ backend_rules_error:
     return valid;
 }
 
+struct _defroute_entry {
+    int family;
+    int table;
+    int metric;
+    const char *netdef_id;
+};
+
+static void
+defroute_err(struct _defroute_entry *entry, const char *new_netdef_id, GError **error) {
+    char table_name[128] = {};
+    char metric_name[128] = {};
+
+    g_assert(entry->family == AF_INET || entry->family == AF_INET6);
+
+    // XXX: handle 254 as an alias for main ?
+    if (entry->table == NETPLAN_ROUTE_TABLE_UNSPEC)
+        strncpy(table_name, "table: main", sizeof(table_name) - 1);
+    else
+        snprintf(table_name, sizeof(table_name) - 1, "table: %d", entry->table);
+
+    if (entry->metric == NETPLAN_METRIC_UNSPEC)
+        strncpy(metric_name, "metric: default", sizeof(metric_name) - 1);
+    else
+        snprintf(metric_name, sizeof(metric_name) - 1, "metric: %d", entry->metric);
+
+    g_set_error(error, G_MARKUP_ERROR, G_MARKUP_ERROR_INVALID_CONTENT,
+            "Conflicting default route declarations for %s (%s, %s), first declared in %s but also in %s",
+            (entry->family == AF_INET) ? "IPv4" : "IPv6",
+            table_name,
+            metric_name,
+            entry->netdef_id,
+            new_netdef_id);
+}
+
+static gboolean
+check_defroute(struct _defroute_entry *candidate,
+               GSList **entries,
+               GError **error)
+{
+    struct _defroute_entry *entry;
+    GSList *it;
+
+    g_assert(entries != NULL);
+    it = *entries;
+
+    while (it) {
+        struct _defroute_entry *e = it->data;
+        if (e->family == candidate->family &&
+                e->table == candidate->table &&
+                e->metric == candidate->metric) {
+            defroute_err(e, candidate->netdef_id, error);
+            return FALSE;
+        }
+        it = it->next;
+    }
+    entry = g_malloc(sizeof(*entry));
+    *entry = *candidate;
+    *entries = g_slist_prepend(*entries, entry);
+    return TRUE;
+}
+
 gboolean
-validate_gateway_consistency(GHashTable *netdefs, GError** error)
+validate_default_route_consistency(GHashTable *netdefs, GError ** error)
 {
-    const char *gw4 = NULL, *gw6 = NULL;
+    struct _defroute_entry candidate = {};
+    GSList *defroutes = NULL;
+    gboolean ret = TRUE;
     gpointer key, value;
     GHashTableIter iter;
 
@@ -383,17 +446,41 @@ validate_gateway_consistency(GHashTable *netdefs, GError** error)
     while (g_hash_table_iter_next (&iter, &key, &value))
     {
         NetplanNetDefinition *nd = value;
+        candidate.netdef_id = key;
+        candidate.metric = NETPLAN_METRIC_UNSPEC;
+        candidate.table = NETPLAN_ROUTE_TABLE_UNSPEC;
         if (nd->gateway4) {
-            if (gw4)
-                return FALSE;
-            gw4 = nd->gateway4;
+            candidate.family = AF_INET;
+            if (!check_defroute(&candidate, &defroutes, error)) {
+                ret = FALSE;
+                break;
+            }
         }
         if (nd->gateway6) {
-            if (gw6)
-                return FALSE;
-            gw6 = nd->gateway6;
+            candidate.family = AF_INET6;
+            if (!check_defroute(&candidate, &defroutes, error)) {
+                ret = FALSE;
+                break;
+            }
+        }
+
+        if (!nd->routes)
+            continue;
+
+        for (size_t i = 0; i < nd->routes->len; i++) {
+            NetplanIPRoute* r = g_array_index(nd->routes, NetplanIPRoute*, i);
+            char *suffix = strrchr(r->to, '/');
+            if (g_strcmp0(suffix, "/0") == 0 || g_strcmp0(r->to, "default") == 0) {
+                candidate.family = r->family;
+                candidate.table = r->table;
+                candidate.metric = r->metric;
+                if (!check_defroute(&candidate, &defroutes, error)) {
+                    ret = FALSE;
+                    break;
+                }
+            }
         }
     }
-    return TRUE;
+    g_slist_free_full(defroutes, g_free);
+    return ret;
 }
-
diff --git a/src/validation.h b/src/validation.h
index c29a30e..3f6e527 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -34,4 +34,4 @@ gboolean
 validate_backend_rules(NetplanNetDefinition* nd, GError** error);
 
 gboolean
-validate_gateway_consistency(GHashTable* netdefs, GError** error);
+validate_default_route_consistency(GHashTable* netdefs, GError** error);