[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Freeipa-devel] [PATCH 0206] Publish zones only after all LDAP events have been processed



On 12.11.2013 16:08, Petr Spacek wrote:
Hello,

Publish zones only after all LDAP events have been processed.

Zones are not exposed in _default DNS view until all events
generated before LDAP intermediate message have been processed.

This prevents BIND from returning NXDOMAIN for some names from
a zone but NOERROR answers for other names in the same zone.
It would be pretty confusing and not easy to debug.

I'm attaching rebased patch.

This patch should go to master branch.

--
Petr^2 Spacek

From c758bbbbbd868ce44d80b8b90a4f7f90a9e128fe Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspacek redhat com>
Date: Mon, 11 Nov 2013 17:53:12 +0100
Subject: [PATCH] Publish zones only after all LDAP events have been processed.

Zones are not exposed in _default DNS view until all events
generated before LDAP intermediate message have been processed.

This prevents BIND from returning NXDOMAIN for some names from
a zone but NOERROR answers for other names in the same zone.
It would be pretty confusing and not easy to debug.

Signed-off-by: Petr Spacek <pspacek redhat com>
---
 src/ldap_helper.c | 68 +++++++++++++++++++++++++++++++++++++++++--------------
 src/ldap_helper.h |  2 ++
 src/syncrepl.c    |  1 +
 3 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index a5bebdced70fefe25e3a3779ee024fdd37d3cdd0..9b6b4b2173138af2c79a3222cd9b99596038f302 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -763,6 +763,8 @@ delete_bind_zone(dns_zt_t *zt, dns_zone_t **zonep) {
 	}
 
 	result = dns_zt_unmount(zt, zone);
+	if (result == ISC_R_NOTFOUND) /* zone wasn't part of a view */
+		result = ISC_R_SUCCESS;
 	zmgr = dns_zone_getmgr(zone);
 	if (zmgr != NULL)
 		dns_zonemgr_releasezone(zmgr, zone);
@@ -873,6 +875,40 @@ cleanup:
 	return result;
 }
 
+isc_result_t
+publish_zones(ldap_instance_t *inst) {
+	isc_result_t result;
+	rbt_iterator_t *iter = NULL;
+	dns_zone_t *zone = NULL;
+	DECLARE_BUFFERED_NAME(name);
+	unsigned int published_cnt = 0;
+	unsigned int total_cnt = 0;
+
+	INIT_BUFFERED_NAME(name);
+	CHECK(zr_rbt_iter_init(inst->zone_register, &iter, &name));
+	do {
+		++total_cnt;
+		CHECK(zr_get_zone_ptr(inst->zone_register, &name, &zone));
+		result = publish_zone(inst, zone);
+		if (result != ISC_R_SUCCESS)
+			dns_zone_log(zone, ISC_LOG_ERROR,
+				     "cannot add zone to view: %s",
+				     dns_result_totext(result));
+		else
+			++published_cnt;
+		dns_zone_detach(&zone);
+
+		INIT_BUFFERED_NAME(name);
+		CHECK(rbt_iter_next(&iter, &name));
+	} while (result == ISC_R_SUCCESS);
+
+cleanup:
+	log_info("%u zones from LDAP instance '%s' loaded (%u zones defined)",
+		 published_cnt, inst->db_name, total_cnt);
+	return result;
+}
+
+
 static isc_result_t ATTR_NONNULLS
 configure_zone_acl(isc_mem_t *mctx, dns_zone_t *zone,
 		void (acl_setter)(dns_zone_t *zone, dns_acl_t *acl),
@@ -1011,15 +1047,17 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t *name, isc_boolean_t lock,
 	} else if (result != ISC_R_SUCCESS)
 		goto cleanup;
 
-	CHECK(dns_view_findzone(inst->view, name, &foundzone));
-	/* foundzone != zone indicates a bug */
-	RUNTIME_CHECK(foundzone == zone);
-	dns_zone_detach(&foundzone);
+	result = dns_view_findzone(inst->view, name, &foundzone);
+	if (result == ISC_R_SUCCESS) {
+		/* foundzone != zone indicates a bug */
+		RUNTIME_CHECK(foundzone == zone);
+		dns_zone_detach(&foundzone);
 
-	if (lock) {
-		dns_view_thaw(inst->view);
-		freeze = ISC_TRUE;
-	}
+		if (lock) {
+			dns_view_thaw(inst->view);
+			freeze = ISC_TRUE;
+		}
+	} /* else: zone wasn't in a view */
 
 	CHECK(delete_bind_zone(inst->view->zonetable, &zone));
 	CHECK(zr_del_zone(inst->zone_register, name));
@@ -1620,7 +1658,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 	isc_result_t result;
 	isc_boolean_t unlock = ISC_FALSE;
 	isc_boolean_t publish = ISC_FALSE;
-	isc_boolean_t published = ISC_FALSE;
+	isc_boolean_t configured = ISC_FALSE;
 	isc_boolean_t ssu_changed;
 	isc_task_t *task = inst->task;
 	ldapdb_rdatalist_t rdatalist;
@@ -1750,11 +1788,10 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
 		dns_zone_clearxfracl(zone);
 	}
 
-	if (publish) {
-		/* Everything is set correctly, publish zone */
+	sync_state_get(inst->sctx, &sync_state);
+	if (publish == ISC_TRUE && sync_state == sync_finished)
 		CHECK(publish_zone(inst, zone));
-		published = ISC_TRUE;
-	}
+	configured = ISC_TRUE;
 
 	/*
 	 * Don't bother if load fails, server will return
@@ -1893,7 +1930,7 @@ cleanup:
 		dns_journal_destroy(&journal);
 	if (ldapdb != NULL)
 		dns_db_detach(&ldapdb);
-	if (publish && !published) { /* Failure in ACL parsing or so. */
+	if (publish && !configured) { /* Failure in ACL parsing or so. */
 		log_error_r("zone '%s': publishing failed, rolling back due to",
 			    entry->dn);
 		result = delete_forwarding_table(inst, &name, "zone", entry->dn);
@@ -4403,9 +4440,6 @@ int ldap_sync_intermediate (
 		if (result != ISC_R_SUCCESS)
 			log_error_r("sync_barrier_wait() failed for instance '%s'",
 				    inst->db_name);
-		else
-			log_info("all zones from LDAP instance '%s' loaded",
-				 inst->db_name);
 	}
 	return LDAP_SUCCESS;
 }
diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index cbed09c0edad4ed4e2cf8ba3420d2c75a442ab83..bab8bf556ef97612790b7fcddf28a0df7626e8d6 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -100,4 +100,6 @@ remove_entry_from_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst) ATTR_NONNU
 
 settings_set_t * ldap_instance_getsettings_local(ldap_instance_t *ldap_inst) ATTR_NONNULLS;
 
+isc_result_t publish_zones(ldap_instance_t *inst);
+
 #endif /* !_LD_LDAP_HELPER_H_ */
diff --git a/src/syncrepl.c b/src/syncrepl.c
index b1a74617292aa7afeb4c012f50bbc26700021122..967557c395ead0d579c8e49a9196225d3084cf7b 100644
--- a/src/syncrepl.c
+++ b/src/syncrepl.c
@@ -126,6 +126,7 @@ barrier_decrement(isc_task_t *task, isc_event_t *event) {
 		bev->sctx->state = sync_finished;
 		isc_condition_broadcast(&bev->sctx->cond);
 		UNLOCK(&bev->sctx->mutex);
+		publish_zones(inst);
 	}
 
 cleanup:
-- 
1.8.3.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]