ifcfg-rh: don't let complex routes (rule files) prevent writing connection

... if the connection has no static routes, there is no reason to
reject writing to these files, we don't touch the route file.

4 files changed, 92 insertions, 48 deletions

diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 779f5261d7..0433e6f6c4 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -1175,6 +1175,7 @@ make_proxy_setting (shvarFile *ifcfg, GError **error)
 static NMSetting *
 make_ip4_setting (shvarFile *ifcfg,
                   const char *network_file,
+                  gboolean routes_read,
                   gboolean *out_has_defroute,
                   GError **error)
 {
@@ -1424,8 +1425,8 @@ make_ip4_setting (shvarFile *ifcfg,
 	/* Static routes  - route-<name> file */
 	route_path = utils_get_route_path (svFileGetName (ifcfg));
 
-	if (utils_has_complex_routes (route_path)) {
-		PARSE_WARNING ("'rule-' or 'rule6-' file is present; you will need to use a dispatcher script to apply these routes");
+	if (!routes_read) {
+		/* NOP */
 	} else if (utils_has_route_file_new_syntax (route_path)) {
 		/* Parse route file in new syntax */
 		route_ifcfg = utils_get_route_ifcfg (svFileGetName (ifcfg), FALSE);
@@ -1591,6 +1592,7 @@ read_aliases (NMSettingIPConfig *s_ip4, gboolean read_defroute, const char *file
 static NMSetting *
 make_ip6_setting (shvarFile *ifcfg,
                   const char *network_file,
+                  gboolean routes_read,
                   GError **error)
 {
 	NMSettingIPConfig *s_ip6 = NULL;
@@ -1847,12 +1849,13 @@ make_ip6_setting (shvarFile *ifcfg,
 
 	/* DNS searches ('DOMAIN' key) are read by make_ip4_setting() and included in NMSettingIPConfig */
 
-	if (!utils_has_complex_routes (svFileGetName (ifcfg))) {
+	if (!routes_read) {
+		/* NOP */
+	} else {
 		/* Read static routes from route6-<interface> file */
 		route6_path = utils_get_route6_path (svFileGetName (ifcfg));
 		if (!read_route_file (AF_INET6, route6_path, s_ip6, error))
 			goto error;
-
 		g_free (route6_path);
 	}
 
@@ -5158,6 +5161,8 @@ connection_from_file_full (const char *filename,
 	NMSetting *s_ip4, *s_ip6, *s_proxy, *s_port, *s_dcb = NULL, *s_user;
 	const char *ifcfg_name = NULL;
 	gboolean has_ip4_defroute = FALSE;
+	gboolean has_complex_routes_v4;
+	gboolean has_complex_routes_v6;
 
 	g_return_val_if_fail (filename != NULL, NULL);
 	g_return_val_if_fail (out_unhandled && !*out_unhandled, NULL);
@@ -5369,13 +5374,32 @@ connection_from_file_full (const char *filename,
 	if (!connection)
 		return NULL;
 
-	s_ip6 = make_ip6_setting (parsed, network_file, error);
+	has_complex_routes_v4 = utils_has_complex_routes (filename, AF_INET);
+	has_complex_routes_v6 = utils_has_complex_routes (filename, AF_INET6);
+
+	if (has_complex_routes_v4 || has_complex_routes_v6) {
+		if (has_complex_routes_v4 && !has_complex_routes_v6)
+			PARSE_WARNING ("'rule-' file is present; you will need to use a dispatcher script to apply these routes");
+		else if (has_complex_routes_v6 && !has_complex_routes_v4)
+			PARSE_WARNING ("'rule6-' file is present; you will need to use a dispatcher script to apply these routes");
+		else
+			PARSE_WARNING ("'rule-' and 'rule6-' files are present; you will need to use a dispatcher script to apply these routes");
+	}
+
+	s_ip6 = make_ip6_setting (parsed,
+	                          network_file,
+	                          !has_complex_routes_v4 && !has_complex_routes_v6,
+	                          error);
 	if (!s_ip6)
 		return NULL;
 	else
 		nm_connection_add_setting (connection, s_ip6);
 
-	s_ip4 = make_ip4_setting (parsed, network_file, &has_ip4_defroute, error);
+	s_ip4 = make_ip4_setting (parsed,
+	                          network_file,
+	                          !has_complex_routes_v4 && !has_complex_routes_v6,
+	                          &has_ip4_defroute,
+	                          error);
 	if (!s_ip4)
 		return NULL;
 	else {
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
index e82ef60c63..c58169e4c3 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c
@@ -280,25 +280,22 @@ gone:
 }
 
 gboolean
-utils_has_complex_routes (const char *filename)
+utils_has_complex_routes (const char *filename, int addr_family)
 {
-	char *rules;
+	g_return_val_if_fail (filename, TRUE);
 
-	g_return_val_if_fail (filename != NULL, TRUE);
+	if (NM_IN_SET (addr_family, AF_UNSPEC, AF_INET)) {
+		gs_free char *rules = utils_get_extra_path (filename, RULE_TAG);
 
-	rules = utils_get_extra_path (filename, RULE_TAG);
-	if (g_file_test (rules, G_FILE_TEST_EXISTS)) {
-		g_free (rules);
-		return TRUE;
+		if (g_file_test (rules, G_FILE_TEST_EXISTS))
+			return TRUE;
 	}
-	g_free (rules);
 
-	rules = utils_get_extra_path (filename, RULE6_TAG);
-	if (g_file_test (rules, G_FILE_TEST_EXISTS)) {
-		g_free (rules);
-		return TRUE;
+	if (NM_IN_SET (addr_family, AF_UNSPEC, AF_INET6)) {
+		gs_free char *rules = utils_get_extra_path (filename, RULE6_TAG);
+		if (g_file_test (rules, G_FILE_TEST_EXISTS))
+			return TRUE;
 	}
-	g_free (rules);
 
 	return FALSE;
 }
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h
index 8e003ccf89..e7abf4d8d7 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h
@@ -48,7 +48,7 @@ shvarFile *utils_get_route_ifcfg (const char *parent, gboolean should_create);
 shvarFile *utils_get_route6_ifcfg (const char *parent, gboolean should_create);
 
 gboolean utils_has_route_file_new_syntax (const char *filename);
-gboolean utils_has_complex_routes (const char *filename);
+gboolean utils_has_complex_routes (const char *filename, int addr_family);
 
 gboolean utils_is_ifcfg_alias_file (const char *alias, const char *ifcfg);
 
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index 8806212c5c..ccb8e72ac3 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -2121,6 +2121,9 @@ write_ip4_setting (NMConnection *connection,
 	const char *method = NULL;
 	gboolean has_netmask;
 
+	NM_SET_OUT (out_route_content_svformat, NULL);
+	NM_SET_OUT (out_route_content, NULL);
+
 	s_ip4 = nm_connection_get_setting_ip4_config (connection);
 	if (!s_ip4) {
 		/* slave-type: clear IPv4 settings.
@@ -2721,6 +2724,9 @@ nms_ifcfg_rh_writer_write_connection (NMConnection *connection,
 	gs_free char *route_path = NULL;
 	gs_free char *route6_path = NULL;
 	nm_auto_free_gstring GString *route_content = NULL;
+	gboolean route_ignore = FALSE;
+	gboolean has_complex_routes_v4;
+	gboolean has_complex_routes_v6;
 	nm_auto_shvar_file_close shvarFile *route_content_svformat = NULL;
 	nm_auto_free_gstring GString *route6_content = NULL;
 	gs_unref_hashtable GHashTable *secrets = NULL;
@@ -2730,13 +2736,6 @@ nms_ifcfg_rh_writer_write_connection (NMConnection *connection,
 	nm_assert (_nm_connection_verify (connection, NULL) == NM_SETTING_VERIFY_SUCCESS);
 	nm_assert (!out_reread || !*out_reread);
 
-	if (   filename
-	    && utils_has_complex_routes (filename)) {
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-		             "Cannot modify a connection that has an associated 'rule-' or 'rule6-' file");
-		return FALSE;
-	}
-
 	if (!nms_ifcfg_rh_writer_can_write_connection (connection, error))
 		return FALSE;
 
@@ -2868,16 +2867,36 @@ nms_ifcfg_rh_writer_write_connection (NMConnection *connection,
 
 	route_path_is_svformat = utils_has_route_file_new_syntax (route_path);
 
+	has_complex_routes_v4 = utils_has_complex_routes (ifcfg_name, AF_INET);
+	has_complex_routes_v6 = utils_has_complex_routes (ifcfg_name, AF_INET6);
+
+	if (has_complex_routes_v4 || has_complex_routes_v6) {
+		NMSettingIPConfig *s_ip4, *s_ip6;
+
+		s_ip4 = nm_connection_get_setting_ip4_config (connection);
+		s_ip6 = nm_connection_get_setting_ip6_config (connection);
+		if (   (   s_ip4
+		        && nm_setting_ip_config_get_num_routes (s_ip4) > 0)
+		    || (   s_ip6
+		        && nm_setting_ip_config_get_num_routes (s_ip6) > 0)) {
+			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+			             "Cannot configure static routes on a connection that has an associated 'rule%s-' file",
+			             has_complex_routes_v4 ? "" : "6");
+			return FALSE;
+		}
+		route_ignore = TRUE;
+	}
+
 	if (!write_ip4_setting (connection,
 	                        ifcfg,
-	                        route_path_is_svformat ? &route_content_svformat : NULL,
-	                        route_path_is_svformat ? NULL                      :&route_content,
+	                        !route_ignore && route_path_is_svformat ? &route_content_svformat : NULL,
+	                        !route_ignore && route_path_is_svformat ? NULL                      :&route_content,
 	                        error))
 		return FALSE;
 
 	if (!write_ip6_setting (connection,
 	                        ifcfg,
-	                        &route6_content,
+	                        !route_ignore ? &route6_content : NULL,
 	                        error))
 		return FALSE;
 
@@ -2901,28 +2920,32 @@ nms_ifcfg_rh_writer_write_connection (NMConnection *connection,
 	if (!write_secrets (ifcfg, secrets, error))
 		return FALSE;
 
-	if (!route_content && !route_content_svformat)
-		(void) unlink (route_path);
-	else {
-		if (route_path_is_svformat) {
-			if (!svWriteFile (route_content_svformat, 0644, error))
-				return FALSE;
-		} else {
-			if (!g_file_set_contents (route_path, route_content->str, route_content->len, NULL)) {
-				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-				             "Writing route file '%s' failed", route_path);
-				return FALSE;
+	if (!route_ignore) {
+		if (!route_content && !route_content_svformat)
+			(void) unlink (route_path);
+		else {
+			if (route_path_is_svformat) {
+				if (!svWriteFile (route_content_svformat, 0644, error))
+					return FALSE;
+			} else {
+				if (!g_file_set_contents (route_path, route_content->str, route_content->len, NULL)) {
+					g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+					             "Writing route file '%s' failed", route_path);
+					return FALSE;
+				}
 			}
 		}
 	}
 
-	if (!route6_content)
-		(void) unlink (route6_path);
-	else {
-		if (!g_file_set_contents (route6_path, route6_content->str, route6_content->len, NULL)) {
-			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
-			             "Writing route6 file '%s' failed", route6_path);
-			return FALSE;
+	if (!route_ignore) {
+		if (!route6_content)
+			(void) unlink (route6_path);
+		else {
+			if (!g_file_set_contents (route6_path, route6_content->str, route6_content->len, NULL)) {
+				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
+				             "Writing route6 file '%s' failed", route6_path);
+				return FALSE;
+			}
 		}
 	}