[Freeipa-devel] [PATCH] 0006 Hold bind and plugin global settings in LDAP
Adam Tkac
atkac at redhat.com
Tue Feb 28 13:25:09 UTC 2012
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<pspacek at 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<pspacek at 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_ */
More information about the Freeipa-devel
mailing list