Introduce cfgmgr validator

Add validator support. The validator takes a "tree"-ish structure
describing the configuration model, composed of clauses (repeatable or
not) and properties (mandatory or not, as well as their types).

The validator walk in this structure and check for each clause if it
exists, if it is repeated (and allowed or not), if the mandatory
properties are present and what's their type.

What is not handled by the validator (and probably okay) are
non-described properties and non-described clauses (so it is possible to
add more which is not in the configuration file).

lib/isc/cfgmgr.c

@@ -15,6 +15,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include <isc/buffer.h>
 #include <isc/cfgmgr.h>
 #include <isc/list.h>
 #include <isc/lmdb.h>
@@ -53,7 +54,7 @@ typedef struct {
 	bool builtinon;
 } context_t;
 
-static const char *isc__cfgmgr_lasterror = "";
+static const isc_cfgmgr_clause_t **isc__cfgmgr_configformat = NULL;
 static isc_mem_t *isc__cfgmgr_mctx = NULL;
 static MDB_env *isc__cfgmgr_env = NULL;
 static thread_local context_t isc__cfgmgr_ctx =
@@ -65,6 +66,13 @@ static thread_local context_t isc__cfgmgr_ctx =
 		     .readonly = false,
 		     .builtinon = false };
 
+/*
+ * lasterror is not in the context_t object because it must outlives a
+ * transaction (i.e. a commit error occurs, the context is flushed, but user
+ * still needs to know what was the error)
+ */
+static thread_local char isc__cfgmgr_lasterror[BUFLEN];
+
 static unsigned long
 isc__cfgmgr_parseid(const char *dbkey) {
 	uint32_t id = 0;
@@ -105,7 +113,8 @@ isc__cfgmgr_parseid(const char *dbkey) {
 }
 
 isc_result_t
-isc_cfgmgr_init(isc_mem_t *mctx, const char *dbpath) {
+isc_cfgmgr_init(isc_mem_t *mctx, const char *dbpath,
+		const isc_cfgmgr_clause_t **configformat) {
 	int result = ISC_R_SUCCESS;
 	char dbname[BUFLEN];
 	char dblockname[BUFLEN];
@@ -166,6 +175,8 @@ isc_cfgmgr_init(isc_mem_t *mctx, const char *dbpath) {
 		remove(dblockname);
 	}
 
+	isc__cfgmgr_configformat = configformat;
+
 	goto out;
 
 cleanup:
@@ -317,6 +328,15 @@ isc__cfgmgr_freectx(void) {
 	isc__cfgmgr_ctx.builtinon = false;
 }
 
+static void
+isc__cfgmgr_setlasterror(const char *fmt, ...) {
+	va_list ap;
+
+	va_start(ap, fmt);
+	REQUIRE(vsnprintf(isc__cfgmgr_lasterror, BUFLEN, fmt, ap) < BUFLEN);
+	va_end(ap);
+}
+
 static isc_result_t
 isc__cfgmgr_starttransaction(bool readonly) {
 	MDB_dbi dbi;
@@ -328,6 +348,8 @@ isc__cfgmgr_starttransaction(bool readonly) {
 	REQUIRE(isc__cfgmgr_ctx.txn == NULL);
 	REQUIRE(isc__cfgmgr_ctx.cursor == NULL);
 
+	isc__cfgmgr_setlasterror("");
+
 	if (mdb_txn_begin(isc__cfgmgr_env, NULL, readonly ? MDB_RDONLY : 0,
 			  &isc__cfgmgr_ctx.txn) != 0)
 	{
@@ -379,10 +401,162 @@ isc_cfgmgr_lasterror(void) {
 	return isc__cfgmgr_lasterror;
 }
 
+static const char *
+isc__cfgmgr_typetostring(isc_cfgmgr_type_t type) {
+	switch (type) {
+	case ISC_CFGMGR_UNDEFINED:
+		return "undefined";
+	case ISC_CFGMGR_STRING:
+		return "string";
+	case ISC_CFGMGR_BOOLEAN:
+		return "boolean";
+	case ISC_CFGMGR_NONE:
+		return "none";
+	case ISC_CFGMGR_SOCKADDR:
+		return "sockaddr";
+	case ISC_CFGMGR_UINT32:
+		return "uint32";
+	}
+
+	UNREACHABLE();
+}
+
+static void
+isc__cfgmgr_validate_buildprefix(const char *name) {
+	isc_buffer_t b;
+	const openedclause_t *clause;
+
+	isc_buffer_init(&b, isc__cfgmgr_ctx.buffer, BUFLEN);
+	clause = ISC_LIST_TAIL(isc__cfgmgr_ctx.openedclauses);
+
+	while (clause != NULL) {
+		isc_buffer_putstr(&b, clause->name);
+		clause = ISC_LIST_PREV(clause, link);
+
+		if (clause != NULL || name != NULL) {
+			isc_buffer_putstr(&b, ".");
+		}
+	}
+
+	if (name != NULL) {
+		isc_buffer_putstr(&b, name);
+	}
+
+	isc_buffer_putuint8(&b, 0);
+}
+
+static isc_result_t
+isc__cfgmgr_validate_prop(const isc_cfgmgr_prop_t *prop) {
+	isc_result_t result;
+	isc_cfgmgr_val_t val;
+
+	result = isc_cfgmgr_getval(prop->name, &val);
+	if (result != ISC_R_SUCCESS) {
+		if (result == ISC_R_NOTFOUND) {
+			if (prop->optional) {
+				result = ISC_R_SUCCESS;
+			} else {
+				isc__cfgmgr_validate_buildprefix(prop->name);
+				isc__cfgmgr_setlasterror(
+					"mandatory property %s is "
+					"missing",
+					isc__cfgmgr_ctx.buffer);
+			}
+		}
+		goto out;
+	}
+
+	if (val.type != prop->type) {
+		isc__cfgmgr_validate_buildprefix(prop->name);
+		isc__cfgmgr_setlasterror(
+			"property %s has wrong type. given %s, "
+			"expected %s",
+			isc__cfgmgr_ctx.buffer,
+			isc__cfgmgr_typetostring(val.type),
+			isc__cfgmgr_typetostring(prop->type));
+		result = ISC_R_UNEXPECTED;
+	}
+
+out:
+	return result;
+}
+
+static isc_result_t
+isc__cfgmgr_validate_clause(const isc_cfgmgr_clause_t *clause) {
+	isc_result_t result;
+
+	result = isc_cfgmgr_open(clause->name);
+	if (result == ISC_R_SUCCESS) {
+	again:
+		for (size_t i = 0; clause->props[i] != NULL; i++) {
+			result = isc__cfgmgr_validate_prop(clause->props[i]);
+			if (result != ISC_R_SUCCESS) {
+				break;
+			}
+		}
+
+		if (clause->clauses != NULL) {
+			for (size_t i = 0; clause->clauses[i] != NULL; i++) {
+				result = isc__cfgmgr_validate_clause(
+					clause->clauses[i]);
+				if (result != ISC_R_SUCCESS) {
+					break;
+				}
+			}
+		}
+
+		if (result == ISC_R_SUCCESS) {
+			result = isc_cfgmgr_nextclause();
+
+			if (result == ISC_R_SUCCESS) {
+				if (clause->repeat == false) {
+					result = ISC_R_MULTIPLE;
+					isc__cfgmgr_validate_buildprefix(NULL);
+					isc__cfgmgr_setlasterror(
+						"clause %s is repeated but "
+						"it's not "
+						"repeatable",
+						isc__cfgmgr_ctx.buffer);
+				} else {
+					goto again;
+				}
+			} else if (result == ISC_R_NOMORE) {
+				result = ISC_R_SUCCESS;
+			}
+		}
+		isc_cfgmgr_close();
+	} else if (result == ISC_R_NOTFOUND) {
+		if (clause->optional) {
+			result = ISC_R_SUCCESS;
+		} else {
+			isc__cfgmgr_validate_buildprefix(clause->name);
+			isc__cfgmgr_setlasterror("mandatory clause %s is "
+						 "missing",
+						 isc__cfgmgr_ctx.buffer);
+		}
+	}
+
+	return result;
+}
+
 static isc_result_t
 isc__cfgmgr_validate(void) {
 	isc_result_t result = ISC_R_SUCCESS;
 
+	if (isc__cfgmgr_configformat != NULL) {
+		for (size_t i = 0; isc__cfgmgr_configformat[i] != NULL; i++) {
+			result = isc__cfgmgr_validate_clause(
+				isc__cfgmgr_configformat[i]);
+			if (result != ISC_R_SUCCESS) {
+				break;
+			}
+		}
+	}
+
+	/*
+	 * An empty format bypass the validation, so it's a success in this case
+	 * as well.
+	 */
 	return result;
 }
 
@@ -435,7 +609,8 @@ isc_cfgmgr_commit(void) {
 	if (result == ISC_R_SUCCESS) {
 		mdb_cursor_close(isc__cfgmgr_ctx.cursor);
 		if (mdb_txn_commit(isc__cfgmgr_ctx.txn) != 0) {
-			isc__cfgmgr_lasterror = "configuration database error";
+			isc__cfgmgr_setlasterror(
+				"configuration database error");
 			result = ISC_R_FAILURE;
 		}
 		isc__cfgmgr_freectx();
@@ -667,10 +842,12 @@ isc__cfgmgr_getvalparent(const char *name, isc_cfgmgr_val_t *value) {
 		 * the parent clauses until we reach the top-level (and attempt
 		 * the builtin mode above).
 		 *
-		 * isc_cfgmgr_open/close could be used instead of directly
-		 * tweaking the openedclauses stack, but this would require
-		 * extra LMDB lookup to retreive the current clause id once
-		 * we're done.
+		 * It is not possible to directly use isc_cfgmgr_open/close
+		 * here, becuase if the clause is repeated, there is no garantee
+		 * that we find the same instance when re-opening it (if it was
+		 * opened using isc_cfgmgr_nextclause).
+		 *
+		 * Also it's quicker this way as avoid extra LMDB lookups.
 		 */
 		ISC_LIST_UNLINK(isc__cfgmgr_ctx.openedclauses, clause, link);
 		isc__cfgmgr_updateprefix();

lib/isc/include/isc/cfgmgr.h

@@ -26,7 +26,32 @@ typedef enum isc_cfgmgr_type {
 	ISC_CFGMGR_NONE,
 	ISC_CFGMGR_SOCKADDR,
 	ISC_CFGMGR_UINT32
-} __attribute__ ((__packed__)) isc_cfgmgr_type_t;
+} __attribute__((__packed__)) isc_cfgmgr_type_t;
+
+/*
+ * Configuration validation and printing. The validator is able to verify that
+ * mandatory properties (or subclauses) of a clause are present, as well as
+ * their respective types, as well as a non-repeatable clause is not repeated or
+ * a mandatory clause is not present. However it won't verify if a non-expected
+ * clause does exists. (This actually might handy if non-user configurable data
+ * needs to be stored in cfgmgr.)
+ */
+typedef struct isc_cfgmgr_clause   isc_cfgmgr_clause_t;
+typedef struct isc_cfgmgr_property isc_cfgmgr_prop_t;
+
+struct isc_cfgmgr_clause {
+	const char		   *name;
+	bool			    optional;
+	bool			    repeat;
+	const isc_cfgmgr_clause_t **clauses;
+	const isc_cfgmgr_prop_t	  **props;
+};
+
+struct isc_cfgmgr_property {
+	const char	 *name;
+	bool		  optional;
+	isc_cfgmgr_type_t type;
+};
 
 /*
  * Generic value holding the actual value and type value for
@@ -212,9 +237,15 @@ isc_cfgmgr_rollback(void);
  * isc_cfgmgr_deinit (this drops all the data written in
  * cfgmgr). Returns ISC_R_SUCCESS or ISC_R_FAILURE if there is an
  * issue initializing the internal database.
+ *
+ * "configformat" is a NULL-terminated array and its life-cycle must outlive
+ * cfgmgr (it is not copied). It is used to validate and to dump the
+ * configuration. NULL can be passed, in which case the validation is bypassed
+ * and the dump functions won't return anything.
  */
 isc_result_t
-isc_cfgmgr_init(isc_mem_t *mctx, const char *dbpath);
+isc_cfgmgr_init(isc_mem_t *mctx, const char *dbpath,
+		const isc_cfgmgr_clause_t **configformat);
 
 /*
  * Destroy all cfgmgr data and free memory. Must be called only after

tests/isc/cfgmgr_test.c

@@ -34,8 +34,8 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * Invalid cfgmgr initialization
 	 */
-	expect_assert_failure(isc_cfgmgr_init(NULL, TEST_DBPATH));
-	expect_assert_failure(isc_cfgmgr_init(mctx, NULL));
+	expect_assert_failure(isc_cfgmgr_init(NULL, TEST_DBPATH, NULL));
+	expect_assert_failure(isc_cfgmgr_init(mctx, NULL, NULL));
 	expect_assert_failure(isc_cfgmgr_close());
 	expect_assert_failure(isc_cfgmgr_getval("foo", &dummyval));
 	expect_assert_failure(isc_cfgmgr_getnextlistval(&dummyval));
@@ -54,7 +54,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * operations which must be used under transaction
 	 */
-	(void)isc_cfgmgr_init(mctx, TEST_DBPATH);
+	(void)isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	expect_assert_failure(isc_cfgmgr_delclause());
 	expect_assert_failure(isc_cfgmgr_getval("foo", &dummyval));
 	expect_assert_failure(isc_cfgmgr_getnextlistval(&dummyval));
@@ -82,7 +82,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * One close for one open
 	 */
-	(void)isc_cfgmgr_init(mctx, TEST_DBPATH);
+	(void)isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(isc_cfgmgr_rwtransaction(), ISC_R_SUCCESS);
 	isc_cfgmgr_newclause("foo");
 	isc_cfgmgr_close();
@@ -93,7 +93,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * One close for one open (and failed open as not existent)
 	 */
-	(void)isc_cfgmgr_init(mctx, TEST_DBPATH);
+	(void)isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(isc_cfgmgr_rwtransaction(), ISC_R_SUCCESS);
 	isc_cfgmgr_newclause("foo");
 	assert_int_equal(isc_cfgmgr_open("bar"), ISC_R_NOTFOUND);
@@ -105,7 +105,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * (Implicit) two closes for two open
 	 */
-	(void)isc_cfgmgr_init(mctx, TEST_DBPATH);
+	(void)isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(isc_cfgmgr_rwtransaction(), ISC_R_SUCCESS);
 	isc_cfgmgr_newclause("foo");
 	isc_cfgmgr_newclause("bar");
@@ -115,7 +115,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * can't write on non-write open clause nor open a subclause as write
 	 */
-	(void)isc_cfgmgr_init(mctx, TEST_DBPATH);
+	(void)isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(isc_cfgmgr_rwtransaction(), ISC_R_SUCCESS);
 	(void)isc_cfgmgr_newclause("foo");
 	(void)isc_cfgmgr_setval("bar",
@@ -141,7 +141,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * reading values parameters
 	 */
-	(void)isc_cfgmgr_init(mctx, TEST_DBPATH);
+	(void)isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(isc_cfgmgr_rwtransaction(), ISC_R_SUCCESS);
 	isc_cfgmgr_newclause("foo");
 	(void)isc_cfgmgr_setval("bar",
@@ -153,7 +153,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_assertions) {
 	/*
 	 * can't de-init while having opened transaction
 	 */
-	(void)isc_cfgmgr_init(mctx, TEST_DBPATH);
+	(void)isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(isc_cfgmgr_rwtransaction(), ISC_R_SUCCESS);
 	isc_cfgmgr_newclause("foo");
 	expect_assert_failure(isc_cfgmgr_deinit());
@@ -171,7 +171,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_rw) {
 	isc_cfgmgr_val_t val1;
 	isc_cfgmgr_val_t val2 = { .type = ISC_CFGMGR_UNDEFINED };
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -384,7 +384,8 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_rw) {
 ISC_RUN_TEST_IMPL(isc_cfgmgr_rollback) {
 	isc_cfgmgr_val_t val;
 
-	assert_int_equal(isc_cfgmgr_init(mctx, TEST_DBPATH), ISC_R_SUCCESS);
+	assert_int_equal(isc_cfgmgr_init(mctx, TEST_DBPATH, NULL),
+			 ISC_R_SUCCESS);
 
 	assert_int_equal(isc_cfgmgr_rwtransaction(), ISC_R_SUCCESS);
 	isc_cfgmgr_newclause("foo");
@@ -429,7 +430,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_parseid) {
 	 * exercise the nested clause and repeatable clauses with such
 	 * odd names
 	 */
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -533,7 +534,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_override) {
 	isc_cfgmgr_val_t val1;
 	isc_cfgmgr_val_t val2;
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -592,7 +593,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_rw_string) {
 	isc_cfgmgr_val_t val1;
 	isc_cfgmgr_val_t val2 = { .type = ISC_CFGMGR_UNDEFINED };
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -658,7 +659,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_list) {
 	isc_result_t result;
 	isc_cfgmgr_val_t val;
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -777,7 +778,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_repeatable_clauses) {
 	isc_cfgmgr_val_t val1;
 	isc_cfgmgr_val_t val2 = { .type = ISC_CFGMGR_UNDEFINED };
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -876,7 +877,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_nested_clauses) {
 	 * Let's start by writting then reading
 	 * foo { bar { baz { gee: none; }; }; };
 	 */
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -1042,7 +1043,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_delete) {
 	isc_result_t result;
 	isc_cfgmgr_val_t val;
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -1267,7 +1268,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_threads) {
 	pthread_t thread;
 	sem_t sems[2];
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	REQUIRE(sem_init(&sems[0], 0, 0) == 0);
@@ -1304,16 +1305,203 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_threads) {
 	isc_cfgmgr_deinit();
 }
 
-ISC_RUN_TEST_IMPL(isc_cfgmgr_validation_error) {
+ISC_RUN_TEST_IMPL(isc_cfgmgr_validationtest) {
 	isc_result_t result;
+	isc_cfgmgr_val_t val;
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	/*
+	 * foo clause is mandatory and non-repeatable, it has one property prop1
+	 * (string, mandatory)
+	 */
+	const isc_cfgmgr_prop_t prop1 = { "prop1", false, ISC_CFGMGR_STRING };
+	const isc_cfgmgr_prop_t prop2 = { "prop2", true, ISC_CFGMGR_UINT32 };
+	const isc_cfgmgr_prop_t *foo_props[] = { &prop1, &prop2, NULL };
+	const isc_cfgmgr_clause_t foo = { "foo", false, false, NULL,
+					  foo_props };
+
+	/*
+	 * bar clause is optional and repratable, it has one property prop3
+	 * (string, mandatory), another prop4 (string, optional)
+	 */
+	const isc_cfgmgr_prop_t prop3 = { "prop3", false, ISC_CFGMGR_STRING };
+	const isc_cfgmgr_prop_t prop4 = { "prop4", true, ISC_CFGMGR_STRING };
+	const isc_cfgmgr_prop_t *bar_props[] = { &prop3, &prop4, NULL };
+	const isc_cfgmgr_clause_t bar = { "bar", true, true, NULL, bar_props };
+
+	const isc_cfgmgr_clause_t *format[] = { &foo, &bar, NULL };
+
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, format);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	result = isc_cfgmgr_rwtransaction();
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	/*
-	 * To be done - but first it needs a clear pattern to walk accross the
-	 * whole configuration.
+	 * let's add just the mandatory property to foo first
 	 */
+	isc_cfgmgr_newclause("foo");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "ab" };
+	result = isc_cfgmgr_setval("prop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_cfgmgr_close();
+
+	/*
+	 * two instances of bar as it's a repeatable clause
+	 */
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "cd" };
+	isc_cfgmgr_newclause("bar");
+	result = isc_cfgmgr_setval("prop3", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_cfgmgr_close();
+
+	isc_cfgmgr_newclause("bar");
+	val.string = "ef";
+	result = isc_cfgmgr_setval("prop4", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_cfgmgr_close();
+
+	/*
+	 * validation fails, as the second instance of bar doesn't have the
+	 * mandatory property prop3. (the first instance doesn't have prop4, but
+	 * it doesn't matter as it's optional)
+	 */
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_NOTFOUND);
+	assert_string_equal(isc_cfgmgr_lasterror(),
+			    "mandatory property bar.prop3 is missing");
+
+	/*
+	 * another attempt, with only foo
+	 */
+	result = isc_cfgmgr_rwtransaction();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("foo");
+
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_UINT32, .uint32 = 42 };
+	result = isc_cfgmgr_setval("prop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	/*
+	 * validation fails here as well, because the type of prop1 is wrong
+	 */
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_UNEXPECTED);
+	assert_string_equal(isc_cfgmgr_lasterror(),
+			    "property foo.prop1 has wrong type. given uint32, "
+			    "expected string");
+
+	/*
+	 * validation fails again, because foo is not repeatable
+	 */
+	result = isc_cfgmgr_rwtransaction();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("foo");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_cfgmgr_close();
+
+	isc_cfgmgr_newclause("foo");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+	isc_cfgmgr_close();
+
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_MULTIPLE);
+	assert_string_equal(isc_cfgmgr_lasterror(),
+			    "clause foo is repeated but it's not repeatable");
+
+	/*
+	 * validation fails again, becuase foo is mandatory clause but not
+	 * provided
+	 */
+	result = isc_cfgmgr_rwtransaction();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("bar");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop3", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_NOTFOUND);
+	assert_string_equal(isc_cfgmgr_lasterror(),
+			    "mandatory clause foo is missing");
+
+	/*
+	 * Test validation of subclauses. Now bar is a mandatory child clause of
+	 * foo.
+	 */
+	const isc_cfgmgr_clause_t bar2 = { "bar", false, true, NULL,
+					   bar_props };
+	const isc_cfgmgr_clause_t *foo2subclauses[] = { &bar2, NULL };
+	const isc_cfgmgr_clause_t foo2 = { "foo", false, false, foo2subclauses,
+					   foo_props };
+	const isc_cfgmgr_clause_t *format2[] = { &foo2, NULL };
+
+	isc_cfgmgr_deinit();
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, format2);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	result = isc_cfgmgr_rwtransaction();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("foo");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	/*
+	 * validation fails because bar subclause is missing from foo
+	 */
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_NOTFOUND);
+	assert_string_equal(isc_cfgmgr_lasterror(),
+			    "mandatory clause foo.bar is missing");
+
+	/*
+	 * Let's try again... This time it fails because of a missing mandatory
+	 * property in bar
+	 */
+	result = isc_cfgmgr_rwtransaction();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("foo");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("bar");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop4", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_NOTFOUND);
+	assert_string_equal(isc_cfgmgr_lasterror(),
+			    "mandatory property foo.bar.prop3 is missing");
+
+	/*
+	 * And again... This time validation passes.
+	 */
+	result = isc_cfgmgr_rwtransaction();
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("foo");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop1", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	isc_cfgmgr_newclause("bar");
+	val = (isc_cfgmgr_val_t){ .type = ISC_CFGMGR_STRING, .string = "b" };
+	result = isc_cfgmgr_setval("prop3", &val);
+	assert_int_equal(result, ISC_R_SUCCESS);
+
+	result = isc_cfgmgr_commit();
+	assert_int_equal(result, ISC_R_SUCCESS);
 
 	isc_cfgmgr_deinit();
 }
@@ -1322,7 +1510,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_getparent) {
 	isc_result_t result;
 	isc_cfgmgr_val_t val;
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -1458,7 +1646,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_opentests) {
 	isc_result_t result;
 	isc_cfgmgr_val_t val = { .type = ISC_CFGMGR_UINT32, .uint32 = 1234 };
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -1535,7 +1723,7 @@ ISC_RUN_TEST_IMPL(isc_cfgmgr_builtintests) {
 	isc_result_t result;
 	isc_cfgmgr_val_t val;
 
-	result = isc_cfgmgr_init(mctx, TEST_DBPATH);
+	result = isc_cfgmgr_init(mctx, TEST_DBPATH, NULL);
 	assert_int_equal(result, ISC_R_SUCCESS);
 
 	result = isc_cfgmgr_rwtransaction();
@@ -1631,7 +1819,7 @@ ISC_TEST_ENTRY(isc_cfgmgr_repeatable_clauses)
 ISC_TEST_ENTRY(isc_cfgmgr_nested_clauses)
 ISC_TEST_ENTRY(isc_cfgmgr_threads)
 ISC_TEST_ENTRY(isc_cfgmgr_parseid)
-ISC_TEST_ENTRY(isc_cfgmgr_validation_error)
+ISC_TEST_ENTRY(isc_cfgmgr_validationtest)
 ISC_TEST_ENTRY(isc_cfgmgr_getparent)
 ISC_TEST_ENTRY(isc_cfgmgr_opentests)
 ISC_TEST_ENTRY(isc_cfgmgr_builtintests)