config: warn about unknown keys in config files

Emit a warning when we find an unsupported option in a configuration
file.

Makefile.am

@@ -3521,6 +3521,7 @@ $(src_tests_config_test_config_OBJECTS): $(libnm_core_lib_h_pub_mkenums)
 
 EXTRA_DIST += \
 	src/tests/config/NetworkManager.conf \
+	src/tests/config/NetworkManager-warn.conf \
 	src/tests/config/NetworkManager.state \
 	src/tests/config/bad.conf \
 	src/tests/config/global-dns-invalid.conf \

src/main.c

@@ -155,7 +155,7 @@ nm_main_config_reload (int signal)
 	 *
 	 * Hence, a NMConfig singleton instance must always be
 	 * available. */
-	nm_config_reload (nm_config_get (), reload_flags);
+	nm_config_reload (nm_config_get (), reload_flags, TRUE);
 }
 
 static void
@@ -232,6 +232,7 @@ main (int argc, char *argv[])
 	NMConfigCmdLineOptions *config_cli;
 	guint sd_id = 0;
 	GError *error_invalid_logging_config = NULL;
+	const char *const *warnings;
 
 	/* Known to cause a possible deadlock upon GDBus initialization:
 	 * https://bugzilla.gnome.org/show_bug.cgi?id=674885 */
@@ -376,6 +377,11 @@ main (int argc, char *argv[])
 		nm_clear_g_free (&bad_domains);
 	}
 
+	warnings = nm_config_get_warnings (config);
+	for ( ; warnings && *warnings; warnings++)
+		nm_log_warn (LOGD_CORE, "config: %s", *warnings);
+	nm_config_clear_warnings (config);
+
 	/* the first access to State causes the file to be read (and possibly print a warning) */
 	nm_config_state_get (config);
 

src/nm-config.c

@@ -130,6 +130,8 @@ typedef struct {
 	 * that they are changed outside of NM (at least not while NM is running).
 	 * Hence, we read them once, that's it. */
 	GHashTable *device_states;
+
+	char **warnings;
 } NMConfigPrivate;
 
 struct _NMConfig {
@@ -281,6 +283,18 @@ nm_config_keyfile_set_string_list (GKeyFile *keyfile,
 
 /*****************************************************************************/
 
+const char *const*
+nm_config_get_warnings (NMConfig *config)
+{
+	return (const char *const *) NM_CONFIG_GET_PRIVATE (config)->warnings;
+}
+
+void
+nm_config_clear_warnings (NMConfig *config)
+{
+	g_clear_pointer (&NM_CONFIG_GET_PRIVATE (config)->warnings, g_strfreev);
+}
+
 NMConfigData *
 nm_config_get_data (NMConfig *config)
 {
@@ -722,8 +736,139 @@ _setting_is_string_list (const char *group, const char *key)
 #undef _IS
 }
 
+typedef struct {
+	char *group;
+	const char *const *keys;
+	bool is_prefix:1;
+	bool is_connection:1;
+} ConfigGroup;
+
+static const ConfigGroup config_groups[] = {
+	{
+		.group = NM_CONFIG_KEYFILE_GROUP_MAIN,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_MAIN_ASSUME_IPV6LL_ONLY,
+			NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT,
+			NM_CONFIG_KEYFILE_KEY_MAIN_AUTOCONNECT_RETRIES_DEFAULT,
+			NM_CONFIG_KEYFILE_KEY_MAIN_CONFIGURE_AND_QUIT,
+			NM_CONFIG_KEYFILE_KEY_MAIN_DEBUG,
+			NM_CONFIG_KEYFILE_KEY_MAIN_DHCP,
+			NM_CONFIG_KEYFILE_KEY_MAIN_DNS,
+			NM_CONFIG_KEYFILE_KEY_MAIN_HOSTNAME_MODE,
+			NM_CONFIG_KEYFILE_KEY_MAIN_IGNORE_CARRIER,
+			NM_CONFIG_KEYFILE_KEY_MAIN_MONITOR_CONNECTION_FILES,
+			NM_CONFIG_KEYFILE_KEY_MAIN_NO_AUTO_DEFAULT,
+			NM_CONFIG_KEYFILE_KEY_MAIN_PLUGINS,
+			NM_CONFIG_KEYFILE_KEY_MAIN_RC_MANAGER,
+			NM_CONFIG_KEYFILE_KEY_MAIN_SLAVES_ORDER,
+			NM_CONFIG_KEYFILE_KEY_MAIN_SYSTEMD_RESOLVED,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUP_LOGGING,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_LOGGING_AUDIT,
+			NM_CONFIG_KEYFILE_KEY_LOGGING_BACKEND,
+			NM_CONFIG_KEYFILE_KEY_LOGGING_DOMAINS,
+			NM_CONFIG_KEYFILE_KEY_LOGGING_LEVEL,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_CONNECTIVITY_ENABLED,
+			NM_CONFIG_KEYFILE_KEY_CONNECTIVITY_INTERVAL,
+			NM_CONFIG_KEYFILE_KEY_CONNECTIVITY_RESPONSE,
+			NM_CONFIG_KEYFILE_KEY_CONNECTIVITY_URI,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUP_KEYFILE,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_KEYFILE_HOSTNAME,
+			NM_CONFIG_KEYFILE_KEY_KEYFILE_PATH,
+			NM_CONFIG_KEYFILE_KEY_KEYFILE_UNMANAGED_DEVICES,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUP_IFUPDOWN,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_IFUPDOWN_MANAGED,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUPPREFIX_DEVICE,
+		.is_prefix = TRUE,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_DEVICE_CARRIER_WAIT_TIMEOUT,
+			NM_CONFIG_KEYFILE_KEY_DEVICE_IGNORE_CARRIER,
+			NM_CONFIG_KEYFILE_KEY_DEVICE_MANAGED,
+			NM_CONFIG_KEYFILE_KEY_DEVICE_SRIOV_NUM_VFS,
+			NM_CONFIG_KEYFILE_KEY_DEVICE_WIFI_BACKEND,
+			NM_CONFIG_KEYFILE_KEY_DEVICE_WIFI_SCAN_RAND_MAC_ADDRESS,
+			NM_CONFIG_KEYFILE_KEY_MATCH_DEVICE,
+			NM_CONFIG_KEYFILE_KEY_STOP_MATCH,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUP_GLOBAL_DNS,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_GLOBAL_DNS_OPTIONS,
+			NM_CONFIG_KEYFILE_KEY_GLOBAL_DNS_SEARCHES,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUPPREFIX_GLOBAL_DNS_DOMAIN,
+		.is_prefix = TRUE,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_GLOBAL_DNS_DOMAIN_SERVERS,
+			NM_CONFIG_KEYFILE_KEY_GLOBAL_DNS_DOMAIN_OPTIONS,
+		),
+	},
+	{
+		.group = NM_CONFIG_KEYFILE_GROUPPREFIX_CONNECTION,
+		.is_prefix = TRUE,
+		.is_connection = TRUE,
+		.keys = NM_MAKE_STRV (
+			NM_CONFIG_KEYFILE_KEY_MATCH_DEVICE,
+			NM_CONFIG_KEYFILE_KEY_STOP_MATCH,
+		),
+	},
+	{ } /* sentinel */
+};
+
 static gboolean
-read_config (GKeyFile *keyfile, gboolean is_base_config, const char *dirname, const char *path, GError **error)
+check_config_key (const char *group, const char *key)
+{
+	const ConfigGroup *g;
+	const char *const *k;
+
+	for (g = config_groups; g->group; g++) {
+		if (   (!g->is_prefix && nm_streq (group, g->group))
+		    || (g->is_prefix && g_str_has_prefix (group, g->group)))
+			break;
+	}
+
+	if (!g->group)
+		return FALSE;
+
+	for (k = g->keys; *k; k++) {
+		if (nm_streq (key, *k))
+			return TRUE;
+	}
+
+	if (g->is_connection) {
+		/* For now just accept everything */
+		return TRUE;
+	}
+
+	return FALSE;
+}
+
+static gboolean
+read_config (GKeyFile *keyfile, gboolean is_base_config,
+             const char *dirname, const char *path,
+             GPtrArray *warnings, GError **error)
 {
 	GKeyFile *kf;
 	char **groups, **keys;
@@ -892,6 +1037,12 @@ read_config (GKeyFile *keyfile, gboolean is_base_config, const char *dirname, co
 
 			new_value = g_key_file_get_value (kf, group, key, NULL);
 			g_key_file_set_value (keyfile, group, key, new_value);
+
+			if (!check_config_key (group, key)) {
+				g_ptr_array_add (warnings,
+				                 g_strdup_printf ("unknown key '%s' in section [%s] of file '%s'",
+				                                  key, group, path));
+			}
 			g_free (new_value);
 		}
 		g_strfreev (keys);
@@ -906,6 +1057,7 @@ static gboolean
 read_base_config (GKeyFile *keyfile,
                   const char *cli_config_main_file,
                   char **out_config_main_file,
+                  GPtrArray *warnings,
                   GError **error)
 {
 	GError *my_error = NULL;
@@ -917,7 +1069,7 @@ read_base_config (GKeyFile *keyfile,
 	/* Try a user-specified config file first */
 	if (cli_config_main_file) {
 		/* Bad user-specific config file path is a hard error */
-		if (read_config (keyfile, TRUE, NULL, cli_config_main_file, error)) {
+		if (read_config (keyfile, TRUE, NULL, cli_config_main_file, warnings, error)) {
 			*out_config_main_file = g_strdup (cli_config_main_file);
 			return TRUE;
 		} else
@@ -932,7 +1084,7 @@ read_base_config (GKeyFile *keyfile,
 	 */
 
 	/* Try deprecated nm-system-settings.conf first */
-	if (read_config (keyfile, TRUE, NULL, DEFAULT_CONFIG_MAIN_FILE_OLD, &my_error)) {
+	if (read_config (keyfile, TRUE, NULL, DEFAULT_CONFIG_MAIN_FILE_OLD, warnings, &my_error)) {
 		*out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE_OLD);
 		return TRUE;
 	}
@@ -944,7 +1096,7 @@ read_base_config (GKeyFile *keyfile,
 	g_clear_error (&my_error);
 
 	/* Try the standard config file location next */
-	if (read_config (keyfile, TRUE, NULL, DEFAULT_CONFIG_MAIN_FILE, &my_error)) {
+	if (read_config (keyfile, TRUE, NULL, DEFAULT_CONFIG_MAIN_FILE, warnings, &my_error)) {
 		*out_config_main_file = g_strdup (DEFAULT_CONFIG_MAIN_FILE);
 		return TRUE;
 	}
@@ -1022,6 +1174,7 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
                     const char *system_config_dir,
                     char **out_config_main_file,
                     char **out_config_description,
+                    char ***out_warnings,
                     GError **error)
 {
 	gs_unref_keyfile GKeyFile *keyfile = NULL;
@@ -1031,12 +1184,14 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
 	guint i;
 	gs_free char *o_config_main_file = NULL;
 	const char *run_config_dir = "";
+	gs_unref_ptrarray GPtrArray *warnings = NULL;
 
 	g_return_val_if_fail (config_dir, NULL);
 	g_return_val_if_fail (system_config_dir, NULL);
 	g_return_val_if_fail (!out_config_main_file || !*out_config_main_file, FALSE);
 	g_return_val_if_fail (!out_config_description || !*out_config_description, NULL);
 	g_return_val_if_fail (!error || !*error, FALSE);
+	g_return_val_if_fail (out_warnings && !*out_warnings, FALSE);
 
 	if (   (""RUN_CONFIG_DIR)[0] == '/'
 	    && !nm_streq (RUN_CONFIG_DIR, system_config_dir)
@@ -1045,6 +1200,7 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
 
 	/* create a default configuration file. */
 	keyfile = nm_config_create_keyfile ();
+	warnings = g_ptr_array_new_with_free_func (g_free);
 
 	system_confs = _get_config_dir_files (system_config_dir);
 	confs = _get_config_dir_files (config_dir);
@@ -1060,7 +1216,7 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
 			continue;
 		}
 
-		if (!read_config (keyfile, FALSE, system_config_dir, filename, error))
+		if (!read_config (keyfile, FALSE, system_config_dir, filename, warnings, error))
 			return NULL;
 		i++;
 	}
@@ -1074,19 +1230,19 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
 			continue;
 		}
 
-		if (!read_config (keyfile, FALSE, run_config_dir, filename, error))
+		if (!read_config (keyfile, FALSE, run_config_dir, filename, warnings, error))
 			return NULL;
 		i++;
 	}
 
 	/* First read the base config file */
-	if (!read_base_config (keyfile, cli ? cli->config_main_file : NULL, &o_config_main_file, error))
+	if (!read_base_config (keyfile, cli ? cli->config_main_file : NULL, &o_config_main_file, warnings, error))
 		return NULL;
 
 	g_assert (o_config_main_file);
 
 	for (i = 0; i < confs->len; i++) {
-		if (!read_config (keyfile, FALSE, config_dir, confs->pdata[i], error))
+		if (!read_config (keyfile, FALSE, config_dir, confs->pdata[i], warnings, error))
 			return NULL;
 	}
 
@@ -1133,6 +1289,11 @@ read_entire_config (const NMConfigCmdLineOptions *cli,
 		*out_config_description = g_string_free (str, FALSE);
 	}
 	NM_SET_OUT (out_config_main_file, g_steal_pointer (&o_config_main_file));
+
+	g_ptr_array_add (warnings, NULL);
+	*out_warnings = (char **) g_ptr_array_free (warnings, warnings->len == 1);
+	g_steal_pointer (&warnings);
+
 	return g_steal_pointer (&keyfile);
 }
 
@@ -2356,7 +2517,7 @@ nm_config_device_state_get (NMConfig *self,
 /*****************************************************************************/
 
 void
-nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags)
+nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags, gboolean emit_warnings)
 {
 	NMConfigPrivate *priv;
 	GError *error = NULL;
@@ -2366,6 +2527,8 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags)
 	char *config_description = NULL;
 	gs_strfreev char **no_auto_default = NULL;
 	gboolean intern_config_needs_rewrite;
+	gs_strfreev char **warnings = NULL;
+	guint i;
 
 	g_return_if_fail (NM_IS_CONFIG (self));
 	g_return_if_fail (   reload_flags
@@ -2390,6 +2553,7 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags)
 	                              priv->system_config_dir,
 	                              &config_main_file,
 	                              &config_description,
+	                              &warnings,
 	                              &error);
 	if (!keyfile) {
 		_LOGE ("Failed to reload the configuration: %s", error->message);
@@ -2398,6 +2562,11 @@ nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags)
 		return;
 	}
 
+	if (emit_warnings && warnings) {
+		for (i = 0; warnings[i]; i++)
+			_LOGW ("%s", warnings[i]);
+	}
+
 	no_auto_default = no_auto_default_from_file (priv->no_auto_default_file);
 
 	keyfile_intern = intern_config_read (priv->intern_config_file,
@@ -2594,6 +2763,7 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error)
 	                              priv->system_config_dir,
 	                              &config_main_file,
 	                              &config_description,
+	                              &priv->warnings,
 	                              error);
 	if (!keyfile)
 		return FALSE;
@@ -2685,6 +2855,7 @@ finalize (GObject *gobject)
 	g_free (priv->log_level);
 	g_free (priv->log_domains);
 	g_strfreev (priv->atomic_section_prefixes);
+	g_strfreev (priv->warnings);
 
 	_nm_config_cmd_line_options_clear (&priv->cli);
 

src/nm-config.h

@@ -181,7 +181,7 @@ void nm_config_set_no_auto_default_for_device  (NMConfig *config, NMDevice *devi
 
 NMConfig *nm_config_new (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error);
 NMConfig *nm_config_setup (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error);
-void nm_config_reload (NMConfig *config, NMConfigChangeFlags reload_flags);
+void nm_config_reload (NMConfig *config, NMConfigChangeFlags reload_flags, gboolean emit_warnings);
 
 const NMConfigState *nm_config_state_get (NMConfig *config);
 
@@ -278,6 +278,9 @@ const GHashTable *nm_config_device_state_get_all (NMConfig *self);
 const NMConfigDeviceStateData *nm_config_device_state_get (NMConfig *self,
                                                            int ifindex);
 
+const char *const *nm_config_get_warnings (NMConfig *config);
+void nm_config_clear_warnings (NMConfig *config);
+
 /*****************************************************************************/
 
 #endif /* __NETWORKMANAGER_CONFIG_H__ */

src/nm-manager.c

@@ -1181,7 +1181,7 @@ _reload_auth_cb (NMAuthChain *chain,
 		goto out;
 	}
 
-	nm_config_reload (priv->config, reload_type);
+	nm_config_reload (priv->config, reload_type, TRUE);
 	g_dbus_method_invocation_return_value (context, NULL);
 
 out:

src/tests/config/NetworkManager-warn.conf

@@ -0,0 +1,15 @@
+[main]
+dhcp=dhclient
+plugin=foo,bar,baz
+no-auto-default=11:11:11:11:11:11
+rc-managed=unmanaged
+dns=none
+
+[logging]
+level=INFO
+
+[connectivity]
+uri=http://example.com
+interval=100
+response=Hello
+audit=true

src/tests/config/test-config.c

@@ -552,6 +552,34 @@ test_config_confdir_parse_error (void)
 	g_clear_error (&error);
 }
 
+static void
+test_config_warnings (void)
+{
+	gs_unref_object NMConfig *config = NULL;
+	const char *const *warnings;
+
+	config = setup_config (NULL, TEST_DIR "/NetworkManager-warn.conf", "", NULL, "/no/such/dir", "", NULL);
+
+	warnings = nm_config_get_warnings (config);
+
+#define check_warning(str, group, key) \
+	{ \
+		gs_free char *expected = NULL; \
+		\
+		expected = g_strdup_printf ("unknown key '%s' in section [%s] of file '" TEST_DIR "/NetworkManager-warn.conf'", \
+		                            key, group); \
+		g_assert_cmpstr (str, ==, expected); \
+	}
+
+	g_assert (warnings);
+	g_assert_cmpint (g_strv_length ((char **) warnings), ==, 3);
+	check_warning (warnings[0], "main", "plugin");
+	check_warning (warnings[1], "main", "rc-managed");
+	check_warning (warnings[2], "connectivity", "audit");
+
+#undef check_warning
+}
+
 /*****************************************************************************/
 
 typedef void (*TestSetValuesUserSetFcn) (NMConfig *config, gboolean is_user, GKeyFile *keyfile_user, NMConfigChangeFlags *out_expected_changes);
@@ -622,7 +650,7 @@ _set_values_user (NMConfig *config,
 	else
 		NMTST_EXPECT_NM_INFO ("config: signal: SIGHUP (no changes from disk)*");
 
-	nm_config_reload (config, NM_CONFIG_CHANGE_CAUSE_SIGHUP);
+	nm_config_reload (config, NM_CONFIG_CHANGE_CAUSE_SIGHUP, FALSE);
 
 	g_test_assert_expected_messages ();
 
@@ -926,15 +954,15 @@ test_config_signal (void)
 
 	expected = NM_CONFIG_CHANGE_CAUSE_SIGUSR1;
 	NMTST_EXPECT_NM_INFO ("config: signal: SIGUSR1");
-	nm_config_reload (config, expected);
+	nm_config_reload (config, expected, FALSE);
 
 	expected = NM_CONFIG_CHANGE_CAUSE_SIGUSR2;
 	NMTST_EXPECT_NM_INFO ("config: signal: SIGUSR2");
-	nm_config_reload (config, expected);
+	nm_config_reload (config, expected, FALSE);
 
 	expected = NM_CONFIG_CHANGE_CAUSE_SIGHUP;
 	NMTST_EXPECT_NM_INFO ("config: signal: SIGHUP (no changes from disk)*");
-	nm_config_reload (config, expected);
+	nm_config_reload (config, expected, FALSE);
 
 	/* test with subscribing two signals...
 	 *
@@ -946,7 +974,7 @@ test_config_signal (void)
 	                  &expected);
 	expected = NM_CONFIG_CHANGE_CAUSE_SIGUSR2;
 	NMTST_EXPECT_NM_INFO ("config: signal: SIGUSR2");
-	nm_config_reload (config, NM_CONFIG_CHANGE_CAUSE_SIGUSR2);
+	nm_config_reload (config, NM_CONFIG_CHANGE_CAUSE_SIGUSR2, FALSE);
 	g_signal_handlers_disconnect_by_func (config, _test_signal_config_changed_cb2, &expected);
 
 	g_signal_handlers_disconnect_by_func (config, _test_signal_config_changed_cb, &expected);
@@ -1064,6 +1092,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/config/no-auto-default", test_config_no_auto_default);
 	g_test_add_func ("/config/confdir", test_config_confdir);
 	g_test_add_func ("/config/confdir-parse-error", test_config_confdir_parse_error);
+	g_test_add_func ("/config/warnings", test_config_warnings);
 
 	g_test_add_func ("/config/set-values", test_config_set_values);
 	g_test_add_func ("/config/global-dns", test_config_global_dns);