On 02/22/2012 12:42 PM, Petr Spacek wrote:
Hello,

this patch fixes https://fedorahosted.org/bind-dyndb-ldap/ticket/43 - hold bind and plugin global settings in LDAP.

Currently it's not optimized for performance. Patch for avoiding unnecessary locking will follow tomorrow or on Friday.


After discussion with Martin we decided to defer support for changing persistent search option & support for LDAP configuration override (https://fedorahosted.org/bind-dyndb-ldap/ticket/48).

Both deferred cases require bigger changes and it's no so simple to test it, so it is risky before a release.

Hello Petr,

thanks for your patch. Please see my comment below, after you fix the patch please push it to master.

Regards, Adam



bind-dyndb-ldap-pspacek-0006-Support-for-plugin-configuration-from-LDAP-without-p.patch


 From 01296f949ec10f77ac1285993dc4b6c9b2358e02 Mon Sep 17 00:00:00 2001
From: Petr Spacek<pspa...@redhat.com>
Date: Wed, 22 Feb 2012 11:01:33 +0100
Subject: [PATCH] Support for plugin configuration from LDAP, without
  performance optimization. Patched BIND with postponed
  plugin start is required for forwarders option to work.
  Signed-off-by: Petr Spacek<pspa...@redhat.com>

---
  README             |   18 ++++++++++++-
  src/ldap_helper.c  |   61 +++++++++++++++++++++++++++++++++++++++++---
  src/zone_manager.c |   71 ++++++++++++++++++++++++++++++++++++++--------------
  src/zone_manager.h |    4 +++
  4 files changed, 129 insertions(+), 25 deletions(-)

diff --git a/README b/README
index 9cd1493..2770c46 100644
--- a/README
+++ b/README
@@ -15,7 +15,7 @@ Hopefully, the patch will once be included in the official 
BIND release.
  ===========
  * support for dynamic updates
  * SASL authentication
-* persistent search for zones
+* persistent search for zones (experimental)


  3. Installation
@@ -245,6 +245,22 @@ idnsZoneActive attribute is set to True. For each entry it 
will find, it
  will register a new zone with BIND. The LDAP back-end will keep each
  record it gets from LDAP in its cache for 5 minutes.

+5.3 Configuration in LDAP
+-------------------------
+Some options can be configured in LDAP as idnsConfigObject attributes.
+Value configured in LDAP has priority over value in configuration file.
+(This behavior will change in future versions!)
+Configuration is updated at same time as zone refresh.
+
+Following options are supported (option = attribute equivalent):
+
+forwarders = idnsForwarders (BIND native option)
+forward = idnsForwardPolicy (BIND native option)
+sync_ptr = idnsAllowSyncPTR
+zone_refresh = idnsZoneRefresh
+
+Forward policy option cannot be set without setting forwarders at the same 
time.
+

  6. License
  ==========
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index b26c9c9..0027eee 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -45,6 +45,8 @@
  #include<isc/time.h>
  #include<isc/util.h>
  #include<isc/netaddr.h>
+#include<isc/parseint.h>
+#include<isc/timer.h>

  #include<alloca.h>
  #define LDAP_DEPRECATED 1
@@ -93,8 +95,10 @@
   * Note about locking in this source.
   *
   * ldap_instance_t structure is equal to dynamic-db {}; statement in 
named.conf.
- * Attributes in ldap_instance_t can be modified only in new_ldap_instance
- * function, which means server is started or reloaded.
+ * Attributes in ldap_instance_t are be modified in new_ldap_instance function,
+ * which means server is started or reloaded (running single-thread).
+ * Before modifying at other places, switch to single-thread mode via
+ * isc_task_beginexclusive() and then return back via isc_task_endexclusive()!
   *
   * ldap_connection_t structure represents connection to the LDAP database and
   * per-connection specific data. Access is controlled via
@@ -856,6 +860,7 @@ parse_nameserver(char *token, struct sockaddr *sa)
        return result;
  }

+/* TODO: Code hardering. */
  static void *
  get_in_addr(struct sockaddr *sa)
  {
@@ -960,7 +965,22 @@ ldap_parse_configentry(ldap_entry_t *entry, 
ldap_instance_t *inst)
  {
        isc_result_t result;
        ldap_valuelist_t values;
+       isc_boolean_t unlock_required;
You should set unlock_required to ISC_FALSE here, otherwise it can be undefined (and used in cleanup:) when isc_task_beginexclusive() below returns ISC_R_LOCKBUSY.
+       isc_timer_t *timer_inst;
+       isc_interval_t timer_interval;
+       isc_uint32_t interval_sec;
+       isc_timertype_t timer_type;
+
+       /* Before changing parameters transit to single-thread operation. */
+       result = isc_task_beginexclusive(inst->task);
+       RUNTIME_CHECK(result == ISC_R_SUCCESS ||
+                                       result == ISC_R_LOCKBUSY);
+       if (result == ISC_R_SUCCESS)
+               unlock_required = ISC_TRUE;
+
+       log_debug(3, "Parsing configuration object");

+       /* idnsForwardPolicy change is handled by configure_zone_forwarders() */
        result = ldap_entry_getvalues(entry, "idnsForwarders",&values);
        if (result == ISC_R_SUCCESS) {
                log_debug(2, "Setting global forwarders");
@@ -969,7 +989,38 @@ ldap_parse_configentry(ldap_entry_t *entry, 
ldap_instance_t *inst)
                        log_error("Global forwarder could not be set up.");
                }
        }
-       /* TODO: handle updates of other config attrs */
+
+       result = ldap_entry_getvalues(entry, "idnsAllowSyncPTR",&values);
+       if (result == ISC_R_SUCCESS) {
+               log_debug(2, "Setting global AllowSyncPTR = %s", 
HEAD(values)->value);
+               inst->sync_ptr = (strcmp(HEAD(values)->value, "TRUE") == 0)
+                                               ? ISC_TRUE : ISC_FALSE;
+       }
+
+       result = ldap_entry_getvalues(entry, "idnsZoneRefresh",&values);
+       if (result == ISC_R_SUCCESS) {
+               log_debug(2, "Setting global ZoneRefresh timer = %s", 
HEAD(values)->value);
+               RUNTIME_CHECK(manager_get_db_timer(inst->db_name,&timer_inst) 
== ISC_R_SUCCESS);
+
+               result = isc_parse_uint32(&interval_sec, HEAD(values)->value, 
10);
+               if (result != ISC_R_SUCCESS) {
+                       log_error("Could not parse ZoneRefresh interval");
+                       goto cleanup;
+               }
+               isc_interval_set(&timer_interval, interval_sec, 0);
+               /* update interval only, not timer type */
+               timer_type = isc_timer_gettype(timer_inst);
+               result = isc_timer_reset(timer_inst, timer_type, NULL,
+                               &timer_interval, ISC_TRUE);
+               if (result != ISC_R_SUCCESS) {
+                       log_error("Could not adjust ZoneRefresh timer");
+                       goto cleanup;
+               }
+       }
+
+cleanup:
+       if (unlock_required == ISC_TRUE)
+               isc_task_endexclusive(inst->task);

        return ISC_R_SUCCESS;
  }
@@ -1103,14 +1154,14 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst)
        int zone_count = 0;
        ldap_entry_t *entry;
        dns_rbt_t *rbt = NULL;
-       char *config_attrs[] = {
+       char *config_attrs[] = { /* Corresponds to idnsConfigObject attributes. 
*/
                "idnsForwardPolicy", "idnsForwarders",
                "idnsAllowSyncPTR", "idnsZoneRefresh",
                "idnsPersistentSearch", NULL
        };
        char *zone_attrs[] = {
                "idnsName", "idnsUpdatePolicy", "idnsAllowQuery",
-               "idnsAllowTransfer", "idnsForwardPolicy",
+               "idnsAllowTransfer", "idnsForwardPolicy",
                "idnsForwarders", NULL
        };

diff --git a/src/zone_manager.c b/src/zone_manager.c
index 80e0cd7..5ca47fd 100644
--- a/src/zone_manager.c
+++ b/src/zone_manager.c
@@ -23,6 +23,7 @@
  #include<isc/result.h>
  #include<isc/task.h>
  #include<isc/timer.h>
+#include<isc/boolean.h>
  #include<isc/util.h>

  #include<dns/dynamic_db.h>
@@ -112,6 +113,9 @@ manager_create_db_instance(isc_mem_t *mctx, const char 
*name,
        isc_result_t result;
        db_instance_t *db_inst = NULL;
        unsigned int zone_refresh;
+       isc_timermgr_t *timer_mgr;
+       isc_interval_t interval;
+       isc_timertype_t timer_type = isc_timertype_inactive;
        isc_task_t *task;
        setting_t manager_settings[] = {
                { "zone_refresh", default_uint(0) },
@@ -147,6 +151,28 @@ manager_create_db_instance(isc_mem_t *mctx, const char 
*name,
        CHECK(new_ldap_instance(mctx, db_inst->name, argv, dyndb_args, task,
                                &db_inst->ldap_inst));

+       /* Add a timer to periodically refresh the zones. Create inactive timer 
if
+        * zone refresh is disabled. (For simplifying configuration change.)
+        *
+        * Timer must exist before refresh_zones_from_ldap() is called. */
+       timer_mgr = dns_dyndb_get_timermgr(dyndb_args);
+       isc_interval_set(&interval, zone_refresh, 0);
+
+       if (zone_refresh) {
+               timer_type = isc_timertype_ticker;
+       } else {
+               timer_type = isc_timertype_inactive;
+       }
+
+       CHECK(isc_timer_create(timer_mgr, timer_type, NULL,
+                                       &interval, task, refresh_zones_action,
+                                          db_inst,&db_inst->timer));
+
+       /* instance must be in list while calling refresh_zones_from_ldap() */
+       LOCK(&instance_list_lock);
+       APPEND(instance_list, db_inst, link);
+       UNLOCK(&instance_list_lock);
+
        result = refresh_zones_from_ldap(db_inst->ldap_inst);
        if (result != ISC_R_SUCCESS) {
                /* In case we don't find any zones, we at least return
@@ -154,31 +180,24 @@ manager_create_db_instance(isc_mem_t *mctx, const char 
*name,
                result = ISC_R_SUCCESS;
                log_error("no valid zones found");
                /*
-                * Do not jump to cleanup. Rather create timer for zone refresh.
+                * Do not jump to cleanup. Rather start timer for zone refresh.
                 * This is just a workaround when the LDAP server is not 
available
                 * during the initialization process.
                 *
-                * goto cleanup;
+                * If no period is set (i.e. refresh is disabled in config), 
use 30 sec.
+                * Timer is already started for cases where period != 0.
                 */
+               if (!zone_refresh) { /* Enforce zone refresh in emergency 
situation. */
+                       isc_interval_set(&interval, 30, 0);
+                       result = isc_timer_reset(db_inst->timer, 
isc_timertype_ticker, NULL,
+                                               &interval, ISC_TRUE);
+                       if (result != ISC_R_SUCCESS) {
+                                       log_error("Could not adjust ZoneRefresh 
timer while init");
+                                       goto cleanup;
+                       }
+               }
        }

-       /* Add a timer to periodically refresh the zones. */
-       if (zone_refresh) {
-               isc_timermgr_t *timer_mgr;
-               isc_interval_t interval;
-
-               timer_mgr = dns_dyndb_get_timermgr(dyndb_args);
-               isc_interval_set(&interval, zone_refresh, 0);
-
-               CHECK(isc_timer_create(timer_mgr, isc_timertype_ticker, NULL,
-                               &interval, task, refresh_zones_action,
-                                      db_inst,&db_inst->timer));
-       }
-
-       LOCK(&instance_list_lock);
-       APPEND(instance_list, db_inst, link);
-       UNLOCK(&instance_list_lock);
-
        return ISC_R_SUCCESS;

  cleanup:
@@ -244,3 +263,17 @@ find_db_instance(const char *name, db_instance_t 
**instance)

        return ISC_R_NOTFOUND;
  }
+
+isc_result_t
+manager_get_db_timer(const char *name, isc_timer_t **timer) {
+       isc_result_t result;
+       db_instance_t *db_inst = NULL;
+
+       REQUIRE(name != NULL);
+
+       result = find_db_instance(name,&db_inst);
+       if (result == ISC_R_SUCCESS)
+               *timer = db_inst->timer;
+
+       return result;
+}
diff --git a/src/zone_manager.h b/src/zone_manager.h
index f080d0d..5a688b5 100644
--- a/src/zone_manager.h
+++ b/src/zone_manager.h
@@ -39,4 +39,8 @@ isc_result_t
  manager_get_ldap_instance(const char *name,
                          ldap_instance_t **ldap_inst);

+isc_result_t
+manager_get_db_timer(const char *name,
+                         isc_timer_t **timer);
+
  #endif /* !_LD_ZONE_MANAGER_H_ */

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

Reply via email to