libnm-core: use jansson to compare and check team configurations

Optionally link libnm-core against jansson JSON library and use it to
validate and compare team configurations.

.travis.yml

@@ -5,7 +5,7 @@ before_install:
     libnl-3-dev libnl-route-3-dev libnl-genl-3-dev ppp-dev libpolkit-gobject-1-dev libgnutls28-dev libgcrypt11-dev
     uuid-dev libudev-dev libgudev-1.0-dev libgirepository1.0-dev gobject-introspection libsoup2.4-dev gtk-doc-tools
     libglib2.0-doc libreadline-dev libnewt-dev libnss3-dev iptables make python-software-properties python-gi
-    python-dbus dbus dbus-x11
+    python-dbus dbus dbus-x11 libjansson4 libjansson-dev
   - sudo dbus-uuidgen --ensure
   - sudo apt-add-repository 'deb http://archive.ubuntu.com/ubuntu trusty main'
   - sudo apt-add-repository 'deb http://archive.ubuntu.com/ubuntu trusty-backports main restricted universe multiverse'

configure.ac

@@ -553,6 +553,18 @@ else
 fi
 AM_CONDITIONAL(WITH_TEAMDCTL, test "${enable_teamdctl}" = "yes")
 
+PKG_CHECK_MODULES(JANSSON, jansson, [have_jansson=yes], [have_jansson=no])
+AC_ARG_ENABLE(json-validation, AS_HELP_STRING([--enable-json-validation], [Enable JSON validation in libnm]),
+		[enable_json_validation=${enableval}], [enable_json_validation=${have_jansson}])
+if (test "${enable_json_validation}" = "yes"); then
+	if test x"$have_jansson" = x"no"; then
+		AC_MSG_ERROR(Jansson is required for JSON validation)
+	fi
+	AC_DEFINE(WITH_JANSSON, 1, [Define if JANSSON is enabled])
+else
+	AC_DEFINE(WITH_JANSSON, 0, [Define if JANSSON is enabled])
+fi
+
 # we usually compile with polkit support. --enable-polkit=yes|no only sets the
 # default configuration for main.auth-polkit. User can always enable/disable polkit
 # autorization via config. Only when specifying --enable-polkit=disabled, we do
@@ -1178,4 +1190,5 @@ echo "  more-asserts: $more_asserts"
 echo "  valgrind: $with_valgrind   $with_valgrind_suppressions"
 echo "  code coverage: $enable_code_coverage"
 echo "  LTO: $enable_lto"
+echo "  JSON validation: $enable_json_validation"
 echo

libnm-core/Makefile.am

@@ -13,7 +13,8 @@ AM_CPPFLAGS = \
 	-DNMLIBDIR=\"$(nmlibdir)\" \
 	-DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_LIB \
 	$(GLIB_CFLAGS) \
-	$(CODE_COVERAGE_CFLAGS)
+	$(CODE_COVERAGE_CFLAGS) \
+	$(JANSSON_CFLAGS)
 
 noinst_LTLIBRARIES = libnm-core.la
 
@@ -35,7 +36,8 @@ GLIB_MKENUMS_C_FLAGS = --identifier-prefix NM
 
 libnm_core_la_LIBADD =			\
 	$(GLIB_LIBS)			\
-	$(UUID_LIBS)
+	$(UUID_LIBS)			\
+	$(JANSSON_LIBS)
 
 libnm_core_la_LDFLAGS =			\
 	$(CODE_COVERAGE_LDFLAGS)

libnm-core/nm-setting-team-port.c

@@ -85,6 +85,18 @@ nm_setting_team_port_get_config (NMSettingTeamPort *setting)
 static gboolean
 verify (NMSetting *setting, NMConnection *connection, GError **error)
 {
+	NMSettingTeamPortPrivate *priv = NM_SETTING_TEAM_PORT_GET_PRIVATE (setting);
+
+	if (priv->config) {
+		if (!_nm_utils_check_valid_json (priv->config, error)) {
+			g_prefix_error (error,
+			                "%s.%s: ",
+			                NM_SETTING_TEAM_PORT_SETTING_NAME,
+			                NM_SETTING_TEAM_PORT_CONFIG);
+			return FALSE;
+		}
+	}
+
 	if (connection) {
 		NMSettingConnection *s_con;
 		const char *slave_type;
@@ -116,6 +128,25 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	return TRUE;
 }
 
+static gboolean
+compare_property (NMSetting *setting,
+                  NMSetting *other,
+                  const GParamSpec *prop_spec,
+                  NMSettingCompareFlags flags)
+{
+	NMSettingClass *parent_class;
+
+	if (nm_streq0 (prop_spec->name, NM_SETTING_TEAM_PORT_CONFIG)) {
+		return _nm_utils_team_config_equal (NM_SETTING_TEAM_PORT_GET_PRIVATE (setting)->config,
+		                                    NM_SETTING_TEAM_PORT_GET_PRIVATE (other)->config,
+		                                    TRUE);
+	}
+
+	/* Otherwise chain up to parent to handle generic compare */
+	parent_class = NM_SETTING_CLASS (nm_setting_team_port_parent_class);
+	return parent_class->compare_property (setting, other, prop_spec, flags);
+}
+
 static void
 nm_setting_team_port_init (NMSettingTeamPort *setting)
 {
@@ -173,10 +204,11 @@ nm_setting_team_port_class_init (NMSettingTeamPortClass *setting_class)
 	g_type_class_add_private (setting_class, sizeof (NMSettingTeamPortPrivate));
 
 	/* virtual methods */
-	object_class->set_property = set_property;
+	object_class->set_property     = set_property;
-	object_class->get_property = get_property;
+	object_class->get_property     = get_property;
-	object_class->finalize = finalize;
+	object_class->finalize         = finalize;
-	parent_class->verify       = verify;
+	parent_class->compare_property = compare_property;
+	parent_class->verify           = verify;
 
 	/* Properties */
 	/**

libnm-core/nm-setting-team.c

@@ -27,6 +27,7 @@
 #include "nm-utils.h"
 #include "nm-utils-private.h"
 #include "nm-connection-private.h"
+#include "nm-utils-private.h"
 
 /**
  * SECTION:nm-setting-team
@@ -82,7 +83,41 @@ nm_setting_team_get_config (NMSettingTeam *setting)
 static gboolean
 verify (NMSetting *setting, NMConnection *connection, GError **error)
 {
-	return _nm_connection_verify_required_interface_name (connection, error);
+	NMSettingTeamPrivate *priv = NM_SETTING_TEAM_GET_PRIVATE (setting);
+
+	if (!_nm_connection_verify_required_interface_name (connection, error))
+		return FALSE;
+
+	if (priv->config) {
+		if (!_nm_utils_check_valid_json (priv->config, error)) {
+			g_prefix_error (error,
+			                "%s.%s: ",
+			                NM_SETTING_TEAM_SETTING_NAME,
+			                NM_SETTING_TEAM_CONFIG);
+			return FALSE;
+		}
+	}
+
+	return TRUE;
+}
+
+static gboolean
+compare_property (NMSetting *setting,
+                  NMSetting *other,
+                  const GParamSpec *prop_spec,
+                  NMSettingCompareFlags flags)
+{
+	NMSettingClass *parent_class;
+
+	if (nm_streq0 (prop_spec->name, NM_SETTING_TEAM_CONFIG)) {
+		return _nm_utils_team_config_equal (NM_SETTING_TEAM_GET_PRIVATE (setting)->config,
+		                                    NM_SETTING_TEAM_GET_PRIVATE (other)->config,
+		                                    FALSE);
+	}
+
+	/* Otherwise chain up to parent to handle generic compare */
+	parent_class = NM_SETTING_CLASS (nm_setting_team_parent_class);
+	return parent_class->compare_property (setting, other, prop_spec, flags);
 }
 
 static void
@@ -142,10 +177,11 @@ nm_setting_team_class_init (NMSettingTeamClass *setting_class)
 	g_type_class_add_private (setting_class, sizeof (NMSettingTeamPrivate));
 
 	/* virtual methods */
-	object_class->set_property = set_property;
+	object_class->set_property     = set_property;
-	object_class->get_property = get_property;
+	object_class->get_property     = get_property;
-	object_class->finalize     = finalize;
+	object_class->finalize         = finalize;
-	parent_class->verify       = verify;
+	parent_class->compare_property = compare_property;
+	parent_class->verify           = verify;
 
 	/* Properties */
 	/**

libnm-core/nm-utils-private.h

@@ -31,6 +31,9 @@
 gboolean    _nm_utils_string_slist_validate (GSList *list,
                                              const char **valid_values);
 
+gboolean    _nm_utils_check_valid_json  (const char *json, GError **error);
+gboolean    _nm_utils_team_config_equal (const char *conf1, const char *conf2, gboolean port);
+
 /* D-Bus transform funcs */
 
 GVariant *  _nm_utils_hwaddr_to_dbus   (const GValue *prop_value);

libnm-core/nm-utils.c

@@ -33,6 +33,10 @@
 #include <gmodule.h>
 #include <sys/stat.h>
 
+#if WITH_JANSSON
+#include <jansson.h>
+#endif
+
 #include "nm-utils-private.h"
 #include "nm-setting-private.h"
 #include "crypto.h"
@@ -4090,3 +4094,140 @@ const char **nm_utils_enum_get_values (GType type, gint from, gint to)
 	return (const char **) g_ptr_array_free (array, FALSE);
 }
 
+#if WITH_JANSSON
+gboolean
+_nm_utils_check_valid_json (const char *str, GError **error)
+{
+	json_t *json;
+	json_error_t jerror;
+
+	g_return_val_if_fail (!error || !*error, FALSE);
+
+	if (!str || !str[0]) {
+		g_set_error_literal (error,
+		                     NM_CONNECTION_ERROR,
+		                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		                     "value is NULL or empty");
+		return FALSE;
+	}
+
+	json = json_loads (str, 0, &jerror);
+	if (!json) {
+		g_set_error (error,
+		             NM_CONNECTION_ERROR,
+		             NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		             "%s at position %d",
+		             jerror.text,
+		             jerror.position);
+		return FALSE;
+	}
+
+	json_decref (json);
+	return TRUE;
+}
+
+/* json_object_foreach_safe() is only available since Jansson 2.8,
+ * reimplement it */
+#define _json_object_foreach_safe(object, n, key, value) \
+    for (key = json_object_iter_key (json_object_iter (object)), \
+         n = json_object_iter_next (object, json_object_iter_at (object, key)); \
+         key && (value = json_object_iter_value (json_object_iter_at (object, key))); \
+         key = json_object_iter_key (n), \
+         n = json_object_iter_next (object, json_object_iter_at (object, key)))
+
+gboolean
+_nm_utils_team_config_equal (const char *conf1,
+                             const char *conf2,
+                             gboolean port_config)
+{
+	json_t *json1 = NULL, *json2 = NULL, *json;
+	gs_free char *dump1 = NULL, *dump2 = NULL;
+	json_t *value, *property;
+	json_error_t jerror;
+	const char *key;
+	gboolean ret;
+	void *tmp;
+	int i;
+
+	if (nm_streq0 (conf1, conf2))
+		return TRUE;
+
+	/* A NULL configuration is equivalent to default value '{}' */
+	json1 = json_loads (conf1 ?: "{}", 0, &jerror);
+	if (json1)
+		json2 = json_loads (conf2 ?: "{}", 0, &jerror);
+
+	if (!json1 || !json2) {
+		ret = FALSE;
+		goto out;
+	}
+
+	/* Some properties are added by teamd when missing from the initial
+	 * configuration.  Add them with the default value if necessary, depending
+	 * on the configuration type.
+	 */
+	for (i = 0, json = json1; i < 2; i++, json = json2) {
+		if  (port_config) {
+			property = json_object_get (json, "link_watch");
+			if (!property) {
+				property = json_object ();
+				json_object_set_new (property, "name", json_string ("ethtool"));
+				json_object_set_new (json, "link_watch", property);
+			}
+		} else {
+			property = json_object_get (json, "runner");
+			if (!property) {
+				property = json_object ();
+				json_object_set_new (property, "name", json_string ("roundrobin"));
+				json_object_set_new (json, "runner", property);
+			}
+		}
+	}
+
+	/* Only consider a given subset of nodes, others can change depending on
+	 * current state */
+	for (i = 0, json = json1; i < 2; i++, json = json2) {
+		_json_object_foreach_safe (json, tmp, key, value) {
+			if (!NM_IN_STRSET (key, "runner", "link_watch"))
+				json_object_del (json, key);
+		}
+	}
+
+	dump1 = json_dumps (json1, JSON_INDENT(0) | JSON_ENSURE_ASCII | JSON_SORT_KEYS);
+	dump2 = json_dumps (json2, JSON_INDENT(0) | JSON_ENSURE_ASCII | JSON_SORT_KEYS);
+
+	ret = nm_streq0 (dump1, dump2);
+out:
+
+	if (json1)
+		json_decref (json1);
+	if (json2)
+		json_decref (json2);
+
+	return ret;
+}
+
+#else /* WITH_JANSSON */
+
+gboolean
+_nm_utils_check_valid_json (const char *str, GError **error)
+{
+	if (!str || !str[0]) {
+		g_set_error_literal (error,
+		                     NM_CONNECTION_ERROR,
+		                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		                     "value is NULL or empty");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+gboolean
+_nm_utils_team_config_equal (const char *conf1,
+                             const char *conf2,
+                             gboolean port_config)
+{
+	return nm_streq0 (conf1, conf2);
+}
+#endif

libnm-core/tests/test-general.c

@@ -4382,6 +4382,110 @@ test_nm_utils_dns_option_find_idx (void)
 
 /******************************************************************************/
 
+static void
+_json_config_check_valid (const char *conf, gboolean expected)
+{
+	GError *error = NULL;
+	gboolean res;
+
+	res = _nm_utils_check_valid_json (conf, &error);
+	g_assert_cmpint (res, ==, expected);
+	g_assert (res || error);
+}
+
+static void
+test_nm_utils_check_valid_json (void)
+{
+	_json_config_check_valid (NULL, FALSE);
+	_json_config_check_valid ("", FALSE);
+#if WITH_JANSSON
+	_json_config_check_valid ("{ }", TRUE);
+	_json_config_check_valid ("{ \"a\" : 1 }", TRUE);
+	_json_config_check_valid ("{ \"a\" : }", FALSE);
+#else
+	/* Without JSON library everything except empty string is considered valid */
+	_json_config_check_valid ("{ }", TRUE);
+	_json_config_check_valid ("{'%!-a1", TRUE);
+#endif
+}
+
+static void
+_team_config_equal_check (const char *conf1,
+                          const char *conf2,
+                          gboolean port_config,
+                          gboolean expected)
+{
+	g_assert_cmpint (_nm_utils_team_config_equal (conf1, conf2, port_config), ==, expected);
+}
+
+static void
+test_nm_utils_team_config_equal (void)
+{
+#if WITH_JANSSON
+	_team_config_equal_check ("", "", TRUE, TRUE);
+	_team_config_equal_check ("{}",
+	                          "{ }",
+	                          TRUE,
+	                          TRUE);
+	_team_config_equal_check ("{}",
+	                          "{",
+	                          TRUE,
+	                          FALSE);
+
+	/* team config */
+	_team_config_equal_check ("{ }",
+	                          "{ \"runner\" :  { \"name\" : \"roundrobin\"} }",
+	                          FALSE,
+	                          TRUE);
+	_team_config_equal_check ("{ }",
+	                          "{ \"runner\" :  { \"name\" : \"random\"} }",
+	                          FALSE,
+	                          FALSE);
+	_team_config_equal_check ("{ \"runner\" :  { \"name\" : \"roundrobin\"} }",
+	                          "{ \"runner\" :  { \"name\" : \"random\"} }",
+	                          FALSE,
+	                          FALSE);
+	_team_config_equal_check ("{ \"runner\" :  { \"name\" : \"random\"} }",
+	                          "{ \"runner\" :  { \"name\" : \"random\"} }",
+	                          FALSE,
+	                          TRUE);
+	_team_config_equal_check ("{ \"runner\" :  { \"name\" : \"random\"}, \"ports\" : { \"eth0\" : {} } }",
+	                          "{ \"runner\" :  { \"name\" : \"random\"}, \"ports\" : { \"eth1\" : {} } }",
+	                          FALSE,
+	                          TRUE);
+
+	/* team port config */
+	_team_config_equal_check ("{ }",
+	                          "{ \"link_watch\" :  { \"name\" : \"ethtool\"} }",
+	                          TRUE,
+	                          TRUE);
+	_team_config_equal_check ("{ }",
+	                          "{ \"link_watch\" :  { \"name\" : \"arp_ping\"} }",
+	                          TRUE,
+	                          FALSE);
+	_team_config_equal_check ("{ \"link_watch\" :  { \"name\" : \"ethtool\"} }",
+	                          "{ \"link_watch\" :  { \"name\" : \"arp_ping\"} }",
+	                          TRUE,
+	                          FALSE);
+	_team_config_equal_check ("{ \"link_watch\" :  { \"name\" : \"arp_ping\"} }",
+	                          "{ \"link_watch\" :  { \"name\" : \"arp_ping\"} }",
+	                          TRUE,
+	                          TRUE);
+	_team_config_equal_check ("{ \"link_watch\" :  { \"name\" : \"arp_ping\"}, \"ports\" : { \"eth0\" : {} } }",
+	                          "{ \"link_watch\" :  { \"name\" : \"arp_ping\"}, \"ports\" : { \"eth1\" : {} } }",
+	                          TRUE,
+	                          TRUE);
+#else
+	/* Without JSON library, strings are compared for equality */
+	_team_config_equal_check ("", "", TRUE, TRUE);
+	_team_config_equal_check ("", " ", TRUE, FALSE);
+	_team_config_equal_check ("{ \"a\": 1 }", "{ \"a\": 1 }", TRUE, TRUE);
+	_team_config_equal_check ("{ \"a\": 1 }", "{ \"a\":   1 }", TRUE, FALSE);
+#endif
+}
+
+/******************************************************************************/
+
 enum TEST_IS_POWER_OF_TWP_ENUM_SIGNED {
 	_DUMMY_1 = -1,
 };
@@ -5066,7 +5170,8 @@ int main (int argc, char **argv)
 
 	g_test_add_func ("/core/general/_nm_utils_dns_option_validate", test_nm_utils_dns_option_validate);
 	g_test_add_func ("/core/general/_nm_utils_dns_option_find_idx", test_nm_utils_dns_option_find_idx);
-
+	g_test_add_func ("/core/general/_nm_utils_validate_json", test_nm_utils_check_valid_json);
+	g_test_add_func ("/core/general/_nm_utils_team_config_equal", test_nm_utils_team_config_equal);
 	g_test_add_func ("/core/general/test_nm_utils_enum", test_nm_utils_enum);
 
 	return g_test_run ();