[Freeipa-devel] [PATCH] 0006 Hold bind and plugin global settings in LDAP

[Petr Spacek]

On 02/28/2012 02:25 PM, Adam Tkac wrote:
> 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
Fixed and pushed to master.

https://fedorahosted.org/bind-dyndb-ldap/changeset/8856c072c9373ede7662c90e0e0bc85d68188854/

-- 
Petr^2 Spacek

>
>>
>>
>> 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_ */
>