cli: fix handling of VPN names in nmcli by using libnm functions

At various places, nmcli requires to specify a VPN type by name, for example

  $ nmcli connection add type vpn ifname '*' vpn-type $VPN_TYPE

This $VPN_TYPE used to be a hard-coded list of known VPN plugin names.
But actually, it should be a VPN service-type. A service-type used to be
the D-Bus name of the VPN plugin. Now, with multiple VPN support that
is no longer the case, but it still has the form of a D-Bus bus name.
Alternativley, it could be an alias, which is just a way for plugins
to support multiple service-types.

Fix that, to support fully qualified service-types in the form
of D-Bus bus names. Also, support lookup by name, in which case
the present plugin-info instances are searched.
Finally, support a list of hard-code short-names.

All the logic how to translate a short-name to a fully qualified
service-type is now inside libnm, so that various user agree on
those names and don't have to hard-code them each.

3 files changed, 29 insertions, 106 deletions

diff --git a/clients/cli/connections.c b/clients/cli/connections.c
index 5b0261c4d3..506bfe0fd0 100644
--- a/clients/cli/connections.c
+++ b/clients/cli/connections.c
@@ -5886,16 +5886,13 @@ cleanup_bridge_slave:
 		/* Build up the settings required for 'vpn' */
 		gboolean success = FALSE;
 		const char *vpn_type = NULL;
-		char *vpn_type_ask = NULL;
+		gs_free char *vpn_type_ask = NULL;
 		const char *user_c = NULL;
 		char *user = NULL;
-		const char *st;
 		gs_free char *service_type_free = NULL;
-		const char *service_type = NULL;
 		nmc_arg_t exp_args[] = { {"vpn-type", TRUE, &vpn_type, !ask},
 		                         {"user",     TRUE, &user_c,   FALSE},
 		                         {NULL} };
-		gs_free const char **plugin_names = NULL;
 
 		if (!nmc_parse_args (exp_args, FALSE, &argc, &argv, error))
 			return FALSE;
@@ -5910,15 +5907,11 @@ cleanup_bridge_slave:
 		if (vpn_type_ask)
 			vpn_type = g_strstrip (vpn_type_ask);
 
-		plugin_names = nm_vpn_get_plugin_names (FALSE);
-		if (!(st = nmc_string_is_valid (vpn_type, plugin_names, NULL))) {
+		service_type_free = nm_vpn_plugin_info_list_find_service_type (nm_vpn_get_plugin_infos (), vpn_type);
+		if (!service_type_free)
 			g_print (_("Warning: 'vpn-type': %s not known.\n"), vpn_type);
-			st = vpn_type;
-		}
-
-		service_type = nm_vpn_get_service_for_name (st);
-		if (!service_type)
-			service_type = service_type_free = nm_vpn_get_service_for_name_default (st);
+		else
+			vpn_type = service_type_free;
 
 		/* Also ask for all optional arguments if '--ask' is specified. */
 		user = g_strdup (user_c);
@@ -5929,12 +5922,11 @@ cleanup_bridge_slave:
 		s_vpn = (NMSettingVpn *) nm_setting_vpn_new ();
 		nm_connection_add_setting (connection, NM_SETTING (s_vpn));
 
-		g_object_set (s_vpn, NM_SETTING_VPN_SERVICE_TYPE, service_type, NULL);
+		g_object_set (s_vpn, NM_SETTING_VPN_SERVICE_TYPE, vpn_type, NULL);
 		g_object_set (s_vpn, NM_SETTING_VPN_USER_NAME, user, NULL);
 
 		success = TRUE;
 cleanup_vpn:
-		g_free (vpn_type_ask);
 		g_free (user);
 		if (!success)
 			return FALSE;
@@ -6702,10 +6694,10 @@ update_connection (gboolean persistent,
 static char *
 gen_func_vpn_types (const char *text, int state)
 {
-	gs_free const char **plugin_names = NULL;
+	gs_strfreev char **plugin_names = NULL;
 
-	plugin_names = nm_vpn_get_plugin_names (FALSE);
-	return nmc_rl_gen_func_basic (text, state, plugin_names);
+	plugin_names = nm_vpn_plugin_info_list_get_service_types (nm_vpn_get_plugin_infos (), FALSE, TRUE);
+	return nmc_rl_gen_func_basic (text, state, (const char **) plugin_names);
 }
 
 static char *
@@ -10625,6 +10617,7 @@ do_connection_import (NmCli *nmc, gboolean temporary, int argc, char **argv)
 	AddConnectionInfo *info;
 	NMConnection *connection = NULL;
 	NMVpnEditorPlugin *plugin;
+	gs_free char *service_type = NULL;
 
 	if (argc == 0) {
 		if (nmc->ask) {
@@ -10682,8 +10675,15 @@ do_connection_import (NmCli *nmc, gboolean temporary, int argc, char **argv)
 		goto finish;
 	}
 
+	service_type = nm_vpn_plugin_info_list_find_service_type (nm_vpn_get_plugin_infos (), type);
+	if (!service_type) {
+		g_string_printf (nmc->return_text, _("Error: failed to find VPN plugin for %s."), type);
+		nmc->return_value = NMC_RESULT_ERROR_UNKNOWN;
+		goto finish;
+	}
+
 	/* Import VPN configuration */
-	plugin = nm_vpn_lookup_plugin (type, NULL, &error);
+	plugin = nm_vpn_get_editor_plugin (service_type, &error);
 	if (!plugin) {
 		g_string_printf (nmc->return_text, _("Error: failed to load VPN plugin: %s."),
 		                 error->message);
@@ -10790,7 +10790,7 @@ do_connection_export (NmCli *nmc, int argc, char **argv)
 	type = nm_setting_vpn_get_service_type (nm_connection_get_setting_vpn (connection));
 
 	/* Export VPN configuration */
-	plugin = nm_vpn_lookup_plugin (NULL, type, &error);
+	plugin = nm_vpn_get_editor_plugin (type, &error);
 	if (!plugin) {
 		g_string_printf (nmc->return_text, _("Error: failed to load VPN plugin: %s."),
 		                 error->message);
diff --git a/clients/common/nm-vpn-helpers.c b/clients/common/nm-vpn-helpers.c
index 4eaa92d9f8..f2e6f4a662 100644
--- a/clients/common/nm-vpn-helpers.c
+++ b/clients/common/nm-vpn-helpers.c
@@ -19,8 +19,6 @@
 /**
  * SECTION:nm-vpn-helpers
  * @short_description: VPN-related utilities
- *
- * Some functions should probably eventually move into libnm.
  */
 
 #include "nm-default.h"
@@ -31,30 +29,23 @@
 
 #include "nm-utils.h"
 
-static gboolean plugins_loaded;
-static GSList *plugins = NULL;
+/*****************************************************************************/
 
 NMVpnEditorPlugin *
-nm_vpn_lookup_plugin (const char *name, const char *service, GError **error)
+nm_vpn_get_editor_plugin (const char *service_type, GError **error)
 {
 	NMVpnEditorPlugin *plugin = NULL;
 	NMVpnPluginInfo *plugin_info;
 	gs_free_error GError *local = NULL;
 
-	g_return_val_if_fail (!service ^ !name, NULL);
+	g_return_val_if_fail (service_type, NULL);
 	g_return_val_if_fail (error == NULL || *error == NULL, NULL);
 
-	if (G_UNLIKELY (!plugins_loaded))
-		nm_vpn_get_plugin_infos ();
-
-	if (service)
-		plugin_info = nm_vpn_plugin_info_list_find_by_service (plugins, service);
-	else
-		plugin_info = nm_vpn_plugin_info_list_find_by_name (plugins, name);
+	plugin_info = nm_vpn_plugin_info_list_find_by_service (nm_vpn_get_plugin_infos (), service_type);
 
 	if (!plugin_info) {
 		g_set_error (error, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_FAILED,
-		             _("unknown VPN plugin \"%s\""), service ?: name);
+		             _("unknown VPN plugin \"%s\""), service_type);
 		return NULL;
 	}
 	plugin = nm_vpn_plugin_info_get_editor_plugin (plugin_info);
@@ -88,6 +79,9 @@ nm_vpn_lookup_plugin (const char *name, const char *service, GError **error)
 GSList *
 nm_vpn_get_plugin_infos (void)
 {
+	static bool plugins_loaded;
+	static GSList *plugins = NULL;
+
 	if (G_LIKELY (plugins_loaded))
 		return plugins;
 	plugins_loaded = TRUE;
@@ -95,72 +89,6 @@ nm_vpn_get_plugin_infos (void)
 	return plugins;
 }
 
-const char **
-nm_vpn_get_plugin_names (gboolean only_available_plugins)
-{
-	GSList *p;
-	const char **list;
-	const char *known_names[] = {
-		"openvpn",
-		"vpnc",
-		"pptp",
-		"openconnect",
-		"openswan",
-		"libreswan",
-		"strongswan",
-		"ssh",
-		"l2tp",
-		"iodine",
-		"fortisslvpn",
-	};
-	guint i, j, k;
-
-	p = nm_vpn_get_plugin_infos ();
-	list = g_new0 (const char *, g_slist_length (p) + G_N_ELEMENTS (known_names) + 1);
-
-	i = 0;
-	for (i = 0; p; p = p->next)
-		list[i++] = nm_vpn_plugin_info_get_name (p->data);
-	if (!only_available_plugins) {
-		for (j = 0; j < G_N_ELEMENTS (known_names); j++)
-			list[i++] = known_names[j];
-	}
-
-	g_qsort_with_data (list, i, sizeof (gpointer), nm_strcmp_p_with_data, NULL);
-
-	/* remove duplicates */
-	for (k = 0, j = 1; j < i; j++) {
-		if (nm_streq (list[k], list[j]))
-			continue;
-		list[k++] = list[j];
-	}
-	list[k++] = NULL;
-
-	return list;
-}
-
-const char *
-nm_vpn_get_service_for_name (const char *name)
-{
-	NMVpnPluginInfo *plugin_info;
-
-	g_return_val_if_fail (name, NULL);
-
-	plugin_info = nm_vpn_plugin_info_list_find_by_name (nm_vpn_get_plugin_infos (), name);
-	if (plugin_info) {
-		/* this only means we have a .name file (NMVpnPluginInfo). Possibly the
-		 * NMVpnEditorPlugin is not loadable. */
-		return nm_vpn_plugin_info_get_service (plugin_info);
-	}
-	return NULL;
-}
-
-char *
-nm_vpn_get_service_for_name_default (const char *name)
-{
-	return g_strdup_printf ("%s.%s", NM_DBUS_INTERFACE, name);
-}
-
 gboolean
 nm_vpn_supports_ipv6 (NMConnection *connection)
 {
@@ -176,7 +104,7 @@ nm_vpn_supports_ipv6 (NMConnection *connection)
 	if (!service_type)
 		return FALSE;
 
-	plugin = nm_vpn_lookup_plugin (NULL, service_type, NULL);
+	plugin = nm_vpn_get_editor_plugin (service_type, NULL);
 	if (!plugin)
 		return FALSE;
 
diff --git a/clients/common/nm-vpn-helpers.h b/clients/common/nm-vpn-helpers.h
index 3d192f7fc8..e69e5b4e64 100644
--- a/clients/common/nm-vpn-helpers.h
+++ b/clients/common/nm-vpn-helpers.h
@@ -30,12 +30,7 @@ struct {
 
 GSList *nm_vpn_get_plugin_infos (void);
 
-const char **nm_vpn_get_plugin_names (gboolean only_available_plugins);
-
-const char *nm_vpn_get_service_for_name (const char *name);
-char *      nm_vpn_get_service_for_name_default (const char *name);
-
-NMVpnEditorPlugin *nm_vpn_lookup_plugin (const char *name, const char *service, GError **error);
+NMVpnEditorPlugin *nm_vpn_get_editor_plugin (const char *service_type, GError **error);
 
 gboolean nm_vpn_supports_ipv6 (NMConnection *connection);