Hello,

This patch set attempts to move ldap_parse_master_zoneentry() a little bit closer to sane code.


It is preparation for
https://fedorahosted.org/bind-dyndb-ldap/ticket/56

--
Petr^2 Spacek
From bfa03960c700bedda454bb7cef5c89bbfce1bbba Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Tue, 15 Apr 2014 18:57:40 +0200
Subject: [PATCH] Refactor empty zone handling.

https://fedorahosted.org/bind-dyndb-ldap/ticket/56

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/ldap_helper.c | 79 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 6c2ead85459eaf531311a15c5817acb4117e7618..790ac4a597dca8772ddb3eed8a6f91d43c7f6b1f 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -715,10 +715,12 @@ destroy_ldap_connection(ldap_connection_t **ldap_connp)
 
 /* Test if the existing zone is 'empty zone' per RFC 6303. */
 static isc_boolean_t ATTR_NONNULLS ATTR_CHECKRESULT
-zone_isempty(isc_mem_t *mctx, dns_zone_t *zone) {
+zone_isempty(dns_zone_t *zone) {
 	char **argv = NULL;
+	isc_mem_t *mctx = NULL;
 	isc_boolean_t result = ISC_FALSE;
 
+	mctx = dns_zone_getmctx(zone);
 	if (dns_zone_getdbtype(zone, &argv, mctx) != ISC_R_SUCCESS)
 		CLEANUP_WITH(ISC_FALSE);
 
@@ -833,6 +835,42 @@ cleanup:
 	return result;
 }
 
+/**
+ * Unload empty zone from given view.
+ *
+ * @retval ISC_R_EXISTS   if a zone with given name is not an empty zone
+ * @retval ISC_R_SUCCESS  if name was an empty zone
+ *                        and it was unloaded successfully
+ * @retval ISC_R_NOTFOUND if name does not match any zone in given view
+ * @retval other errors
+ */
+static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
+zone_unload_ifempty(dns_view_t *view, dns_name_t *name) {
+	isc_result_t result;
+	dns_zone_t *zone = NULL;
+	char zone_name[DNS_NAME_FORMATSIZE];
+
+	CHECK(dns_view_findzone(view, name, &zone));
+
+	if (zone_isempty(zone) == ISC_TRUE) {
+		dns_name_format(name, zone_name, DNS_NAME_FORMATSIZE);
+		result = delete_bind_zone(view->zonetable, &zone);
+		if (result != ISC_R_SUCCESS)
+			log_error_r("unable to unload automatic empty zone "
+				    "%s", zone_name);
+		else
+			log_info("automatic empty zone %s unloaded",
+				 zone_name);
+	} else {
+		result = ISC_R_EXISTS;
+	}
+
+cleanup:
+	if (zone != NULL)
+		dns_zone_detach(&zone);
+	return result;
+}
+
 /*
  * Create a new zone with origin 'name'. The zone will be added to the
  * ldap_inst->view.
@@ -845,44 +883,18 @@ create_zone(ldap_instance_t *ldap_inst, dns_name_t *name, dns_zone_t **zonep)
 	const char *argv[2];
 	sync_state_t sync_state;
 	isc_task_t *task = NULL;
+	char zone_name[DNS_NAME_FORMATSIZE];
 
 	REQUIRE(ldap_inst != NULL);
 	REQUIRE(name != NULL);
 	REQUIRE(zonep != NULL && *zonep == NULL);
 
 	argv[0] = ldapdb_impname;
 	argv[1] = ldap_inst->db_name;
 
-	result = dns_view_findzone(ldap_inst->view, name, &zone);
-	if (result != ISC_R_NOTFOUND) {
-		char zone_name[DNS_NAME_FORMATSIZE];
-		dns_name_format(name, zone_name, DNS_NAME_FORMATSIZE);
-
-		if (result != ISC_R_SUCCESS) {
-			log_error_r("dns_view_findzone() failed while "
-				    "searching for zone '%s'", zone_name);
-		} else { /* zone already exists */
-			if (zone_isempty(ldap_inst->mctx, zone) == ISC_TRUE) {
-				result = delete_bind_zone(ldap_inst->view->zonetable,
-							  &zone);
-				if (result != ISC_R_SUCCESS)
-					log_error_r("failed to create new zone "
-						    "'%s': unable to unload "
-						    "automatic empty zone",
-						    zone_name);
-				else
-					log_info("automatic empty zone %s "
-						 "unloaded", zone_name);
-
-			} else {
-				result = ISC_R_EXISTS;
-				log_error_r("failed to create new zone '%s'",
-					    zone_name);
-			}
-		}
-		if (result != ISC_R_SUCCESS)
-			goto cleanup;
-	}
+	result = zone_unload_ifempty(ldap_inst->view, name);
+	if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND)
+		goto cleanup;
 
 	CHECK(dns_zone_create(&zone, ldap_inst->mctx));
 	CHECK(dns_zone_setorigin(zone, name));
@@ -901,6 +913,9 @@ create_zone(ldap_instance_t *ldap_inst, dns_name_t *name, dns_zone_t **zonep)
 	return ISC_R_SUCCESS;
 
 cleanup:
+	dns_name_format(name, zone_name, DNS_NAME_FORMATSIZE);
+	log_error_r("failed to create new zone '%s'", zone_name);
+
 	if (zone != NULL) {
 		if (dns_zone_getmgr(zone) != NULL)
 			dns_zonemgr_releasezone(ldap_inst->zmgr, zone);
@@ -1403,7 +1418,7 @@ configure_zone_forwarders(ldap_entry_t *entry, ldap_instance_t *inst,
 		result = dns_zt_find(inst->view->zonetable, name, 0, NULL,
 				     &zone);
 		if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
-			if (zone_isempty(inst->mctx, zone)) {
+			if (zone_isempty(zone)) {
 				dns_zone_log(zone, ISC_LOG_INFO, "automatic "
 					     "empty zone will be shut down "
 					     "to enable forwarding");
-- 
1.9.0

From 4b7618495a05f80c0cd383be2e48cce6d36f4442 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Tue, 15 Apr 2014 19:21:56 +0200
Subject: [PATCH] Rename variables in ldap_parse_master_zoneentry() and
 create_zone().

This is preparation for future changes related to DNSSEC support.

https://fedorahosted.org/bind-dyndb-ldap/ticket/56
---
 src/ldap_helper.c | 76 +++++++++++++++++++++++++++----------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 790ac4a597dca8772ddb3eed8a6f91d43c7f6b1f..b62eb375dab51e9ea49ee8e661f02f1d61055570 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -876,50 +876,50 @@ cleanup:
  * ldap_inst->view.
  */
 static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
-create_zone(ldap_instance_t *ldap_inst, dns_name_t *name, dns_zone_t **zonep)
+create_zone(ldap_instance_t *ldap_inst, dns_name_t *name, dns_zone_t **rawp)
 {
 	isc_result_t result;
-	dns_zone_t *zone = NULL;
+	dns_zone_t *raw = NULL;
 	const char *argv[2];
 	sync_state_t sync_state;
 	isc_task_t *task = NULL;
 	char zone_name[DNS_NAME_FORMATSIZE];
 
 	REQUIRE(ldap_inst != NULL);
 	REQUIRE(name != NULL);
-	REQUIRE(zonep != NULL && *zonep == NULL);
+	REQUIRE(rawp != NULL && *rawp == NULL);
 
 	argv[0] = ldapdb_impname;
 	argv[1] = ldap_inst->db_name;
 
 	result = zone_unload_ifempty(ldap_inst->view, name);
 	if (result != ISC_R_SUCCESS && result != ISC_R_NOTFOUND)
 		goto cleanup;
 
-	CHECK(dns_zone_create(&zone, ldap_inst->mctx));
-	CHECK(dns_zone_setorigin(zone, name));
-	dns_zone_setclass(zone, dns_rdataclass_in);
-	dns_zone_settype(zone, dns_zone_master);
-	CHECK(dns_zone_setdbtype(zone, 2, argv));
-	CHECK(dns_zonemgr_managezone(ldap_inst->zmgr, zone));
+	CHECK(dns_zone_create(&raw, ldap_inst->mctx));
+	CHECK(dns_zone_setorigin(raw, name));
+	dns_zone_setclass(raw, dns_rdataclass_in);
+	dns_zone_settype(raw, dns_zone_master);
+	CHECK(dns_zone_setdbtype(raw, 2, argv));
+	CHECK(dns_zonemgr_managezone(ldap_inst->zmgr, raw));
 	sync_state_get(ldap_inst->sctx, &sync_state);
 	if (sync_state == sync_init) {
-		dns_zone_gettask(zone, &task);
+		dns_zone_gettask(raw, &task);
 		CHECK(sync_task_add(ldap_inst->sctx, task));
 		isc_task_detach(&task);
 	}
 
-	*zonep = zone;
+	*rawp = raw;
 	return ISC_R_SUCCESS;
 
 cleanup:
 	dns_name_format(name, zone_name, DNS_NAME_FORMATSIZE);
 	log_error_r("failed to create new zone '%s'", zone_name);
 
-	if (zone != NULL) {
-		if (dns_zone_getmgr(zone) != NULL)
-			dns_zonemgr_releasezone(ldap_inst->zmgr, zone);
-		dns_zone_detach(&zone);
+	if (raw != NULL) {
+		if (dns_zone_getmgr(raw) != NULL)
+			dns_zonemgr_releasezone(ldap_inst->zmgr, raw);
+		dns_zone_detach(&raw);
 	}
 	if (task != NULL)
 		isc_task_detach(&task);
@@ -1812,7 +1812,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 	const char *dn;
 	ldap_valuelist_t values;
 	dns_name_t name;
-	dns_zone_t *zone = NULL;
+	dns_zone_t *raw = NULL;
 	dns_zone_t *zone_raw = NULL;
 	isc_result_t result;
 	isc_boolean_t unlock = ISC_FALSE;
@@ -1880,13 +1880,13 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 	 * Load the zone. */
 
 	/* Check if we are already serving given zone */
-	result = zr_get_zone_ptr(inst->zone_register, &name, &zone);
+	result = zr_get_zone_ptr(inst->zone_register, &name, &raw);
 	if (result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH) {
-		CHECK(create_zone(inst, &name, &zone));
-		CHECK(configure_paths(inst->mctx, inst, zone, ISC_FALSE));
-		CHECK(zr_add_zone(inst->zone_register, zone, dn));
+		CHECK(create_zone(inst, &name, &raw));
+		CHECK(configure_paths(inst->mctx, inst, raw, ISC_FALSE));
+		CHECK(zr_add_zone(inst->zone_register, raw, dn));
 		new_zone = ISC_TRUE;
-		log_debug(2, "created zone %p: %s", zone, dn);
+		log_debug(2, "created zone %p: %s", raw, dn);
 	} else if (result != ISC_R_SUCCESS)
 		goto cleanup;
 
@@ -1912,44 +1912,44 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 		isc_boolean_t ssu_enabled;
 		const char *ssu_policy = NULL;
 
-		log_debug(2, "Setting SSU table for %p: %s", zone, dn);
+		log_debug(2, "Setting SSU table for %p: %s", raw, dn);
 		CHECK(setting_get_bool("dyn_update", zone_settings, &ssu_enabled));
 		if (ssu_enabled) {
 			/* Get the update policy and update the zone with it. */
 			CHECK(setting_get_str("update_policy", zone_settings,
 					      &ssu_policy));
-			CHECK(configure_zone_ssutable(zone, ssu_policy));
+			CHECK(configure_zone_ssutable(raw, ssu_policy));
 		} else {
 			/* Empty policy will prevent the update from reaching
 			 * LDAP driver and error will be logged. */
-			CHECK(configure_zone_ssutable(zone, ""));
+			CHECK(configure_zone_ssutable(raw, ""));
 		}
 	}
 
 	/* Fetch allow-query and allow-transfer ACLs */
-	log_debug(2, "Setting allow-query for %p: %s", zone, dn);
+	log_debug(2, "Setting allow-query for %p: %s", raw, dn);
 	result = ldap_entry_getvalues(entry, "idnsAllowQuery", &values);
 	if (result == ISC_R_SUCCESS) {
-		CHECK(configure_zone_acl(inst->mctx, zone, &dns_zone_setqueryacl,
+		CHECK(configure_zone_acl(inst->mctx, raw, &dns_zone_setqueryacl,
 					 HEAD(values)->value, acl_type_query));
 	} else {
 		log_debug(2, "allow-query not set");
-		dns_zone_clearqueryacl(zone);
+		dns_zone_clearqueryacl(raw);
 	}
 
-	log_debug(2, "Setting allow-transfer for %p: %s", zone, dn);
+	log_debug(2, "Setting allow-transfer for %p: %s", raw, dn);
 	result = ldap_entry_getvalues(entry, "idnsAllowTransfer", &values);
 	if (result == ISC_R_SUCCESS) {
-		CHECK(configure_zone_acl(inst->mctx, zone, &dns_zone_setxfracl,
+		CHECK(configure_zone_acl(inst->mctx, raw, &dns_zone_setxfracl,
 					 HEAD(values)->value, acl_type_transfer));
 	} else {
 		log_debug(2, "allow-transfer not set");
-		dns_zone_clearxfracl(zone);
+		dns_zone_clearxfracl(raw);
 	}
 
 	sync_state_get(inst->sctx, &sync_state);
 	if (new_zone == ISC_TRUE && sync_state == sync_finished)
-		CHECK(publish_zone(task, inst, zone));
+		CHECK(publish_zone(task, inst, raw));
 
 	/* synchronize zone origin with LDAP */
 	CHECK(setting_get_str("fake_mname", inst->local_settings,
@@ -2026,21 +2026,21 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 	dns_diff_print(&diff, NULL);
 #endif
 	if (ldap_writeback == ISC_TRUE) {
-		dns_zone_log(zone, ISC_LOG_DEBUG(5), "writing new zone serial "
+		dns_zone_log(raw, ISC_LOG_DEBUG(5), "writing new zone serial "
 			     "%u to LDAP", new_serial);
 		result = ldap_replace_serial(inst, &name, new_serial);
 		if (result != ISC_R_SUCCESS)
-			dns_zone_log(zone, ISC_LOG_ERROR,
+			dns_zone_log(raw, ISC_LOG_ERROR,
 				     "serial (%u) write back to LDAP failed",
 				     new_serial);
 	}
 
 	if (!EMPTY(diff.tuples)) {
 		if (sync_state == sync_finished && new_zone == ISC_FALSE) {
 			/* write the transaction to journal */
-			dns_zone_getraw(zone, &zone_raw);
+			dns_zone_getraw(raw, &zone_raw);
 			if (zone_raw == NULL)
-				journal_filename = dns_zone_getjournal(zone);
+				journal_filename = dns_zone_getjournal(raw);
 			else
 				journal_filename = dns_zone_getjournal(zone_raw);
 			CHECK(dns_journal_open(inst->mctx, journal_filename,
@@ -2060,7 +2060,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 
 	/* Do zone load only if the initial LDAP synchronization is done. */
 	if (sync_state == sync_finished && data_changed == ISC_TRUE)
-		CHECK(load_zone(zone));
+		CHECK(load_zone(raw));
 
 cleanup:
 	dns_diff_clear(&diff);
@@ -2089,8 +2089,8 @@ cleanup:
 		isc_task_endexclusive(task);
 	if (dns_name_dynamic(&name))
 		dns_name_free(&name, inst->mctx);
-	if (zone != NULL)
-		dns_zone_detach(&zone);
+	if (raw != NULL)
+		dns_zone_detach(&raw);
 	ldapdb_rdatalist_destroy(inst->mctx, &rdatalist);
 
 	return result;
-- 
1.9.0

From f6af8ce3b6e38352d9a418ed237144e3a170a923 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 16 Apr 2014 16:07:06 +0200
Subject: [PATCH] Add support for key-directory to configure_paths().

https://fedorahosted.org/bind-dyndb-ldap/ticket/56

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/ldap_helper.c   | 9 +++++++++
 src/zone_register.c | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b62eb375dab51e9ea49ee8e661f02f1d61055570..f6f4daa28bcebb134d734765133806f6e1e7f619 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1642,16 +1642,25 @@ configure_paths(isc_mem_t *mctx, ldap_instance_t *inst, dns_zone_t *zone,
 		isc_boolean_t issecure) {
 	isc_result_t result;
 	ld_string_t *file_name = NULL;
+	ld_string_t *key_dir = NULL;
 
 	CHECK(zr_get_zone_path(mctx, ldap_instance_getsettings_local(inst),
 			       dns_zone_getorigin(zone),
 			       (issecure ? "signed" : "raw"), &file_name));
 	CHECK(dns_zone_setfile(zone, str_buf(file_name)));
+	if (issecure == ISC_TRUE) {
+		CHECK(zr_get_zone_path(mctx,
+				       ldap_instance_getsettings_local(inst),
+				       dns_zone_getorigin(zone), "keys/",
+				       &key_dir));
+		dns_zone_setkeydirectory(zone, str_buf(key_dir));
+	}
 	CHECK(fs_file_remove(dns_zone_getfile(zone)));
 	CHECK(fs_file_remove(dns_zone_getjournal(zone)));
 
 cleanup:
 	str_destroy(&file_name);
+	str_destroy(&key_dir);
 	return result;
 }
 
diff --git a/src/zone_register.c b/src/zone_register.c
index d58159c3104f2624bfaa05de4515db5b2598d51f..e84664b3b5ea3a9306a592e82fa53f3dbc097de4 100644
--- a/src/zone_register.c
+++ b/src/zone_register.c
@@ -257,9 +257,9 @@ create_zone_info(isc_mem_t *mctx, dns_zone_t *zone, const char *dn,
 				  settings_name, global_settings,
 				  &zinfo->settings));
 
-	/* Prepate a directory for this zone */
+	/* Prepare a directory for this zone */
 	CHECK(zr_get_zone_path(mctx, global_settings, dns_zone_getorigin(zone),
-			       NULL, &zone_dir));
+			       "keys/", &zone_dir));
 	CHECK(fs_dirs_create(str_buf(zone_dir)));
 
 	DE_CONST(db_name, argv[0]);
-- 
1.9.0

From 990f10ac003ea8d1e9cac89c49cf41c45b29d1f6 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Thu, 17 Apr 2014 14:49:48 +0200
Subject: [PATCH] Refactor master zone configuration.

ldap_parse_master_zoneentry() is way too long and unmanageable.

https://fedorahosted.org/bind-dyndb-ldap/ticket/56
---
 src/ldap_helper.c | 140 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 83 insertions(+), 57 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index f6f4daa28bcebb134d734765133806f6e1e7f619..816080b6f3d8e5721a6b08939f280f89489cf9e4 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1813,21 +1813,101 @@ cleanup:
 #undef MAX_SERIAL_LENGTH
 }
 
+/**
+ * Reconfigure master zone according to configuration in LDAP object.
+ *
+ * @param[in]  raw Raw zone backed by LDAP database. In-line secure zone
+ *                 will be reconfigured as necessary.
+ */
+static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
+zone_master_reconfigure(ldap_entry_t *entry, settings_set_t *zone_settings,
+			dns_zone_t *raw, isc_task_t *task) {
+	isc_result_t result;
+	const char *dn = NULL;
+	ldap_valuelist_t values;
+	isc_mem_t *mctx = NULL;
+	isc_boolean_t ssu_changed;
+
+	REQUIRE(entry != NULL);
+	REQUIRE(zone_settings != NULL);
+	REQUIRE(raw != NULL);
+	REQUIRE(task != NULL);
+
+	dn = entry->dn;
+	mctx = dns_zone_getmctx(raw);
+
+	result = setting_update_from_ldap_entry("dyn_update", zone_settings,
+						"idnsAllowDynUpdate", entry, task);
+	if (result != ISC_R_SUCCESS && result != ISC_R_IGNORE)
+		goto cleanup;
+	ssu_changed = (result == ISC_R_SUCCESS);
+
+	result = setting_update_from_ldap_entry("sync_ptr", zone_settings,
+				       "idnsAllowSyncPTR", entry, task);
+	if (result != ISC_R_SUCCESS && result != ISC_R_IGNORE)
+		goto cleanup;
+
+	result = setting_update_from_ldap_entry("update_policy", zone_settings,
+						"idnsUpdatePolicy", entry, task);
+	if (result != ISC_R_SUCCESS && result != ISC_R_IGNORE)
+		goto cleanup;
+
+	if (result == ISC_R_SUCCESS || ssu_changed) {
+		isc_boolean_t ssu_enabled;
+		const char *ssu_policy = NULL;
+
+		log_debug(2, "Setting SSU table for %p: %s", raw, dn);
+		CHECK(setting_get_bool("dyn_update", zone_settings, &ssu_enabled));
+		if (ssu_enabled) {
+			/* Get the update policy and update the zone with it. */
+			CHECK(setting_get_str("update_policy", zone_settings,
+					      &ssu_policy));
+			CHECK(configure_zone_ssutable(raw, ssu_policy));
+		} else {
+			/* Empty policy will prevent the update from reaching
+			 * LDAP driver and error will be logged. */
+			CHECK(configure_zone_ssutable(raw, ""));
+		}
+	}
+
+	/* Fetch allow-query and allow-transfer ACLs */
+	log_debug(2, "Setting allow-query for %p: %s", raw, dn);
+	result = ldap_entry_getvalues(entry, "idnsAllowQuery", &values);
+	if (result == ISC_R_SUCCESS) {
+		CHECK(configure_zone_acl(mctx, raw, &dns_zone_setqueryacl,
+					 HEAD(values)->value, acl_type_query));
+	} else {
+		log_debug(2, "allow-query not set");
+		dns_zone_clearqueryacl(raw);
+	}
+
+	log_debug(2, "Setting allow-transfer for %p: %s", raw, dn);
+	result = ldap_entry_getvalues(entry, "idnsAllowTransfer", &values);
+	if (result == ISC_R_SUCCESS) {
+		CHECK(configure_zone_acl(mctx, raw, &dns_zone_setxfracl,
+					 HEAD(values)->value, acl_type_transfer));
+	} else {
+		log_debug(2, "allow-transfer not set");
+		dns_zone_clearxfracl(raw);
+	}
+
+cleanup:
+	return result;
+}
+
 /* Parse the master zone entry */
 static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
 ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 			    isc_task_t *task)
 {
 	const char *dn;
-	ldap_valuelist_t values;
 	dns_name_t name;
 	dns_zone_t *raw = NULL;
 	dns_zone_t *zone_raw = NULL;
 	isc_result_t result;
 	isc_boolean_t unlock = ISC_FALSE;
 	isc_boolean_t new_zone = ISC_FALSE;
 	isc_boolean_t configured = ISC_FALSE;
-	isc_boolean_t ssu_changed;
 	ldapdb_rdatalist_t rdatalist;
 	settings_set_t *zone_settings = NULL;
 	const char *fake_mname = NULL;
@@ -1900,61 +1980,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 		goto cleanup;
 
 	CHECK(zr_get_zone_settings(inst->zone_register, &name, &zone_settings));
-
-	result = setting_update_from_ldap_entry("dyn_update", zone_settings,
-				       "idnsAllowDynUpdate", entry, inst->task);
-	if (result != ISC_R_SUCCESS && result != ISC_R_IGNORE)
-		goto cleanup;
-	ssu_changed = (result == ISC_R_SUCCESS);
-
-	result = setting_update_from_ldap_entry("sync_ptr", zone_settings,
-				       "idnsAllowSyncPTR", entry, inst->task);
-	if (result != ISC_R_SUCCESS && result != ISC_R_IGNORE)
-		goto cleanup;
-
-	result = setting_update_from_ldap_entry("update_policy", zone_settings,
-				       "idnsUpdatePolicy", entry, inst->task);
-	if (result != ISC_R_SUCCESS && result != ISC_R_IGNORE)
-		goto cleanup;
-
-	if (result == ISC_R_SUCCESS || ssu_changed) {
-		isc_boolean_t ssu_enabled;
-		const char *ssu_policy = NULL;
-
-		log_debug(2, "Setting SSU table for %p: %s", raw, dn);
-		CHECK(setting_get_bool("dyn_update", zone_settings, &ssu_enabled));
-		if (ssu_enabled) {
-			/* Get the update policy and update the zone with it. */
-			CHECK(setting_get_str("update_policy", zone_settings,
-					      &ssu_policy));
-			CHECK(configure_zone_ssutable(raw, ssu_policy));
-		} else {
-			/* Empty policy will prevent the update from reaching
-			 * LDAP driver and error will be logged. */
-			CHECK(configure_zone_ssutable(raw, ""));
-		}
-	}
-
-	/* Fetch allow-query and allow-transfer ACLs */
-	log_debug(2, "Setting allow-query for %p: %s", raw, dn);
-	result = ldap_entry_getvalues(entry, "idnsAllowQuery", &values);
-	if (result == ISC_R_SUCCESS) {
-		CHECK(configure_zone_acl(inst->mctx, raw, &dns_zone_setqueryacl,
-					 HEAD(values)->value, acl_type_query));
-	} else {
-		log_debug(2, "allow-query not set");
-		dns_zone_clearqueryacl(raw);
-	}
-
-	log_debug(2, "Setting allow-transfer for %p: %s", raw, dn);
-	result = ldap_entry_getvalues(entry, "idnsAllowTransfer", &values);
-	if (result == ISC_R_SUCCESS) {
-		CHECK(configure_zone_acl(inst->mctx, raw, &dns_zone_setxfracl,
-					 HEAD(values)->value, acl_type_transfer));
-	} else {
-		log_debug(2, "allow-transfer not set");
-		dns_zone_clearxfracl(raw);
-	}
+	CHECK(zone_master_reconfigure(entry, zone_settings, raw, task));
 
 	sync_state_get(inst->sctx, &sync_state);
 	if (new_zone == ISC_TRUE && sync_state == sync_finished)
-- 
1.9.0

From a9403a9ca932448e6405c38a0040a750ad1b9b3a Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Thu, 17 Apr 2014 19:57:48 +0200
Subject: [PATCH] Refactor zone apex synchronization and serial maintenance.

ldap_parse_master_zoneentry() is way too long and unmanageable.

https://fedorahosted.org/bind-dyndb-ldap/ticket/56

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/ldap_helper.c | 231 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 130 insertions(+), 101 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 816080b6f3d8e5721a6b08939f280f89489cf9e4..d9f9adb93ca4e386ba05d8f7533a692b1c68f4e2 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1895,49 +1895,154 @@ cleanup:
 	return result;
 }
 
+/**
+ * Synchronize internal RBTDB with master zone object in LDAP and update serial
+ * as necessary.
+ *
+ * @param[in]  new_zone Is the RBTDB empty? (I.e. even without SOA record.)
+ * @param[in]  version  LDAP DB opened for reading and writing.
+ * @param[out] diff     Initialized diff. It will be filled with differences
+ *                      between RBTDB and LDAP object + SOA serial update.
+ * @param[out] new_serial     SOA serial after update;
+ *                            valid if ldap_writeback = ISC_TRUE.
+ * @param[out] ldap_writeback SOA serial was updated.
+ * @param[out] data_changed   Other data were updated.
+ *
+ */
+static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
+zone_sync_apex(const ldap_instance_t * const inst,
+	       ldap_entry_t * const entry, dns_name_t name,
+	       const sync_state_t sync_state, const isc_boolean_t new_zone,
+	       dns_db_t * const ldapdb, dns_db_t * const rbtdb,
+	       dns_dbversion_t * const version, dns_diff_t * const diff,
+	       isc_uint32_t * const new_serial,
+	       isc_boolean_t * const ldap_writeback,
+	       isc_boolean_t * const data_changed) {
+	isc_result_t result;
+	const char *fake_mname = NULL;
+	ldapdb_rdatalist_t rdatalist;
+	dns_rdatasetiter_t *rbt_rds_iterator = NULL;
+	/* RBTDB's origin node cannot be detached until the node is non-empty.
+	 * This is workaround for ISC-Bug #35080. */
+	dns_dbnode_t *node = NULL;
+	dns_difftuple_t *soa_tuple = NULL;
+	isc_boolean_t soa_tuple_alloc = ISC_FALSE;
+	isc_uint32_t curr_serial;
+
+	INIT_LIST(rdatalist);
+	CHECK(setting_get_str("fake_mname", inst->local_settings,
+			      &fake_mname));
+	CHECK(ldap_parse_rrentry(inst->mctx, entry, &name, fake_mname,
+				 &rdatalist));
+
+	CHECK(dns_db_getoriginnode(rbtdb, &node));
+	result = dns_db_allrdatasets(rbtdb, node, version, 0,
+				     &rbt_rds_iterator);
+	if (result == ISC_R_SUCCESS) {
+		CHECK(diff_ldap_rbtdb(inst->mctx, &name, &rdatalist,
+				      rbt_rds_iterator, diff));
+		dns_rdatasetiter_destroy(&rbt_rds_iterator);
+	} else if (result != ISC_R_NOTFOUND)
+		goto cleanup;
+
+	/* New zone doesn't have serial defined yet. */
+	if (new_zone != ISC_TRUE)
+		CHECK(dns_db_getsoaserial(rbtdb, version, &curr_serial));
+
+	/* Detect if SOA serial is affected by the update or not.
+	 * Always bump serial in case of re-synchronization. */
+	CHECK(diff_analyze_serial(diff, &soa_tuple, data_changed));
+	if (new_zone == ISC_TRUE || *data_changed == ISC_TRUE ||
+	    sync_state != sync_finished) {
+		if (soa_tuple == NULL) {
+			/* The diff doesn't contain new SOA serial
+			 * => generate new serial and write it back to LDAP. */
+			*ldap_writeback = ISC_TRUE;
+			soa_tuple_alloc = ISC_TRUE;
+			CHECK(dns_db_createsoatuple(ldapdb, version, inst->mctx,
+						    DNS_DIFFOP_DEL, &soa_tuple));
+			dns_diff_appendminimal(diff, &soa_tuple);
+			CHECK(dns_db_createsoatuple(ldapdb, version, inst->mctx,
+						    DNS_DIFFOP_ADD, &soa_tuple));
+			CHECK(update_soa_serial(dns_updatemethod_unixtime,
+						soa_tuple, new_serial));
+			dns_diff_appendminimal(diff, &soa_tuple);
+		} else if (new_zone == ISC_TRUE || sync_state != sync_finished ||
+			   isc_serial_le(dns_soa_getserial(&soa_tuple->rdata),
+					 curr_serial)) {
+			/* The diff tries to send SOA serial back!
+			 * => generate new serial and write it back to LDAP.
+			 * Force serial update if we are adding a new zone. */
+			*ldap_writeback = ISC_TRUE;
+			CHECK(update_soa_serial(dns_updatemethod_unixtime,
+						soa_tuple, new_serial));
+		} else {
+			/* The diff contains new serial already
+			 * => do nothing. */
+			*ldap_writeback = ISC_FALSE;
+		}
+
+	} else {/* if (data_changed == ISC_FALSE) */
+		*ldap_writeback = ISC_FALSE;
+		if (soa_tuple == NULL) {
+			/* The diff is empty => do nothing. */
+			INSIST(EMPTY(diff->tuples));
+		} else if (isc_serial_le(dns_soa_getserial(&soa_tuple->rdata),
+					 curr_serial)) {
+			/* Attempt to move serial backwards without any data
+			 * => ignore it. */
+			dns_diff_clear(diff);
+		}/* else:
+		  * The diff contains new serial already
+		  * => do nothing. */
+	}
+
+cleanup:
+	/* New zone has to have at least SOA record and NS record. */
+	if (new_zone == ISC_TRUE
+	    && (*data_changed == ISC_FALSE || soa_tuple == NULL)) {
+		if (result == ISC_R_SUCCESS)
+			result = ISC_R_FAILURE;
+	}
+	if (soa_tuple_alloc == ISC_TRUE && soa_tuple != NULL)
+		dns_difftuple_free(&soa_tuple);
+	if (node != NULL)
+		dns_db_detachnode(rbtdb, &node);
+	if (rbt_rds_iterator != NULL)
+		dns_rdatasetiter_destroy(&rbt_rds_iterator);
+	ldapdb_rdatalist_destroy(inst->mctx, &rdatalist);
+	return result;
+}
+
 /* Parse the master zone entry */
 static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
 ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 			    isc_task_t *task)
 {
 	const char *dn;
 	dns_name_t name;
 	dns_zone_t *raw = NULL;
-	dns_zone_t *zone_raw = NULL;
 	isc_result_t result;
 	isc_boolean_t unlock = ISC_FALSE;
 	isc_boolean_t new_zone = ISC_FALSE;
-	isc_boolean_t configured = ISC_FALSE;
-	ldapdb_rdatalist_t rdatalist;
 	settings_set_t *zone_settings = NULL;
-	const char *fake_mname = NULL;
-	isc_boolean_t data_changed;
 	isc_boolean_t ldap_writeback;
-	isc_uint32_t curr_serial;
+	isc_boolean_t data_changed;
 	isc_uint32_t new_serial;
 
 	dns_db_t *rbtdb = NULL;
 	dns_db_t *ldapdb = NULL;
 	dns_diff_t diff;
 	dns_dbversion_t *version = NULL;
-	/* RBTDB's origin node cannot be detached until the node is non-empty.
-	 * This is workaround for possible bug in bind-9.9.3-P2. */
-	dns_dbnode_t *node = NULL;
-	dns_rdatasetiter_t *rbt_rds_iterator = NULL;
-	dns_difftuple_t *soa_tuple = NULL;
-	isc_boolean_t soa_tuple_alloc = ISC_FALSE;
-
 	sync_state_t sync_state;
 	dns_journal_t *journal = NULL;
 	char *journal_filename = NULL;
 
+	REQUIRE(entry != NULL);
+	REQUIRE(inst != NULL);
+
 	dns_diff_init(inst->mctx, &diff);
-
-	REQUIRE(entry != NULL);
-	REQUIRE(inst != NULL);
-
 	dns_name_init(&name, NULL);
-	INIT_LIST(rdatalist);
 
 	/* Derive the dns name of the zone from the DN. */
 	dn = entry->dn;
@@ -1987,73 +2092,12 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 		CHECK(publish_zone(task, inst, raw));
 
 	/* synchronize zone origin with LDAP */
-	CHECK(setting_get_str("fake_mname", inst->local_settings,
-			      &fake_mname));
-	CHECK(ldap_parse_rrentry(inst->mctx, entry, &name, fake_mname,
-				 &rdatalist));
-
 	CHECK(zr_get_zone_dbs(inst->zone_register, &name, &ldapdb, &rbtdb));
 	CHECK(dns_db_newversion(ldapdb, &version));
-	CHECK(dns_db_getoriginnode(rbtdb, &node));
-	result = dns_db_allrdatasets(rbtdb, node, version, 0,
-				     &rbt_rds_iterator);
-	if (result == ISC_R_SUCCESS) {
-		CHECK(diff_ldap_rbtdb(inst->mctx, &name, &rdatalist,
-				      rbt_rds_iterator, &diff));
-		dns_rdatasetiter_destroy(&rbt_rds_iterator);
-	} else if (result != ISC_R_NOTFOUND)
-		goto cleanup;
-
-	/* New zone doesn't have serial defined yet. */
-	if (new_zone == ISC_FALSE)
-		CHECK(dns_db_getsoaserial(rbtdb, version, &curr_serial));
-
-	/* Detect if SOA serial is affected by the update or not.
-	 * Always bump serial in case of re-synchronization. */
-	CHECK(diff_analyze_serial(&diff, &soa_tuple, &data_changed));
-	if (data_changed == ISC_TRUE || sync_state != sync_finished) {
-		if (soa_tuple == NULL) {
-			/* The diff doesn't contain new SOA serial
-			 * => generate new serial and write it back to LDAP. */
-			ldap_writeback = ISC_TRUE;
-			soa_tuple_alloc = ISC_TRUE;
-			CHECK(dns_db_createsoatuple(ldapdb, version, inst->mctx,
-						    DNS_DIFFOP_DEL, &soa_tuple));
-			dns_diff_appendminimal(&diff, &soa_tuple);
-			CHECK(dns_db_createsoatuple(ldapdb, version, inst->mctx,
-						    DNS_DIFFOP_ADD, &soa_tuple));
-			CHECK(update_soa_serial(dns_updatemethod_unixtime,
-						soa_tuple, &new_serial));
-			dns_diff_appendminimal(&diff, &soa_tuple);
-		} else if (new_zone == ISC_TRUE || sync_state != sync_finished ||
-			   isc_serial_le(dns_soa_getserial(&soa_tuple->rdata),
-					 curr_serial)) {
-			/* The diff tries to send SOA serial back!
-			 * => generate new serial and write it back to LDAP.
-			 * Force serial update if we are adding a new zone. */
-			ldap_writeback = ISC_TRUE;
-			CHECK(update_soa_serial(dns_updatemethod_unixtime,
-						soa_tuple, &new_serial));
-		} else {
-			/* The diff contains new serial already
-			 * => do nothing. */
-			ldap_writeback = ISC_FALSE;
-		}
-
-	} else {/* if (data_changed == ISC_FALSE) */
-		ldap_writeback = ISC_FALSE;
-		if (soa_tuple == NULL) {
-			/* The diff is empty => do nothing. */
-			INSIST(EMPTY(diff.tuples));
-		} else if (isc_serial_le(dns_soa_getserial(&soa_tuple->rdata),
-					 curr_serial)) {
-			/* Attempt to move serial backwards without any data
-			 * => ignore it. */
-			dns_diff_clear(&diff);
-		}/* else:
-		  * The diff contains new serial already
-		  * => do nothing. */
-	}
+	CHECK(zone_sync_apex(inst, entry, name, sync_state, new_zone,
+			     ldapdb, rbtdb, version,
+			     &diff, &new_serial, &ldap_writeback,
+			     &data_changed));
 
 #if RBTDB_DEBUG >= 2
 	dns_diff_print(&diff, stdout);
@@ -2073,11 +2117,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 	if (!EMPTY(diff.tuples)) {
 		if (sync_state == sync_finished && new_zone == ISC_FALSE) {
 			/* write the transaction to journal */
-			dns_zone_getraw(raw, &zone_raw);
-			if (zone_raw == NULL)
-				journal_filename = dns_zone_getjournal(raw);
-			else
-				journal_filename = dns_zone_getjournal(zone_raw);
+			journal_filename = dns_zone_getjournal(raw);
 			CHECK(dns_journal_open(inst->mctx, journal_filename,
 					       DNS_JOURNAL_CREATE, &journal));
 			CHECK(dns_journal_write_transaction(journal, &diff));
@@ -2088,32 +2128,22 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 		dns_db_closeversion(ldapdb, &version, ISC_TRUE);
 	}
 
-	/* Make sure that zone has at least SOA record. */
-	if (new_zone == ISC_FALSE
-	    || (data_changed == ISC_TRUE && soa_tuple != NULL))
-		configured = ISC_TRUE;
-
 	/* Do zone load only if the initial LDAP synchronization is done. */
 	if (sync_state == sync_finished && data_changed == ISC_TRUE)
 		CHECK(load_zone(raw));
 
 cleanup:
 	dns_diff_clear(&diff);
-	if (soa_tuple_alloc == ISC_TRUE && soa_tuple != NULL)
-		dns_difftuple_free(&soa_tuple);
-	if (rbt_rds_iterator != NULL)
-		dns_rdatasetiter_destroy(&rbt_rds_iterator);
-	if (node != NULL)
-		dns_db_detachnode(rbtdb, &node);
 	if (rbtdb != NULL && version != NULL)
 		dns_db_closeversion(ldapdb, &version, ISC_FALSE); /* rollback */
 	if (rbtdb != NULL)
 		dns_db_detach(&rbtdb);
 	if (journal != NULL)
 		dns_journal_destroy(&journal);
 	if (ldapdb != NULL)
 		dns_db_detach(&ldapdb);
-	if (new_zone && !configured) { /* Failure in ACL parsing or so. */
+	if (new_zone && result != ISC_R_SUCCESS) {
+		/* Failure in ACL parsing or so. */
 		log_error_r("zone '%s': publishing failed, rolling back due to",
 			    entry->dn);
 		result = ldap_delete_zone2(inst, &name, ISC_TRUE, ISC_FALSE);
@@ -2126,7 +2156,6 @@ cleanup:
 		dns_name_free(&name, inst->mctx);
 	if (raw != NULL)
 		dns_zone_detach(&raw);
-	ldapdb_rdatalist_destroy(inst->mctx, &rdatalist);
 
 	return result;
 }
-- 
1.9.0

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to