[Freeipa-devel] [PATCH 0032-0035]
Adam Tkac
atkac at redhat.com
Thu Jul 19 11:43:48 UTC 2012
On Wed, Jul 18, 2012 at 01:32:10PM +0200, Petr Spacek wrote:
> Hello,
>
> attached patch 0032 adds support for MODDN operation to persistent
> search implementation.
> Related ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/72
>
> Patches 0033-0035 does minor cleanup in old persistent search code.
Hello Peter,
please check one comment below. You don't have to resend modified patchset,
just go ahead and push it.
Regards, Adam
> From 79769c5ad71a10540cdd9b571eed9407e31da9e6 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Wed, 18 Jul 2012 13:01:28 +0200
> Subject: [PATCH] Add support for modify DN operation to persistent search.
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 108 +++++++++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 89 insertions(+), 19 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 8015db7018ddc9956471a99d6397ab95d83fbc3d..d49d72bc6ad23e8cec1c1f8a45135e79290b1422 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -239,6 +239,7 @@ struct ldap_psearchevent {
> isc_mem_t *mctx;
> char *dbname;
> char *dn;
> + char *prevdn;
> int chgtype;
> };
>
> @@ -316,6 +317,9 @@ static isc_result_t ldap_pscontrol_create(isc_mem_t *mctx, LDAPControl **ctrlp);
> static void ldap_pscontrol_destroy(isc_mem_t *mctx, LDAPControl **ctrlp);
>
> static isc_threadresult_t ldap_psearch_watcher(isc_threadarg_t arg);
> +static void psearch_update(ldap_instance_t *inst, ldap_entry_t *entry,
> + LDAPControl **ctrls);
> +
>
> /* Persistent updates watcher */
> static isc_threadresult_t
> @@ -789,6 +793,7 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t *name, isc_boolean_t lock)
> if (result == ISC_R_SUCCESS)
> unlock = ISC_TRUE;
>
> + /* TODO: flush cache records belonging to deleted zone */
> CHECK(discard_from_cache(inst->cache, name));
> }
>
> @@ -2957,37 +2962,69 @@ update_action(isc_task_t *task, isc_event_t *event)
> isc_result_t result ;
> ldap_instance_t *inst = NULL;
> ldap_connection_t *conn = NULL;
> - ldap_qresult_t *ldap_qresult = NULL;
> - ldap_entry_t *entry;
> + ldap_qresult_t *ldap_qresult_zone = NULL;
> + ldap_qresult_t *ldap_qresult_record = NULL;
> + ldap_entry_t *entry_zone = NULL;
> + ldap_entry_t *entry_record = NULL;
> isc_boolean_t delete = ISC_TRUE;
> isc_mem_t *mctx;
> - char *attrs[] = {
> + dns_name_t prevname;
> + char *attrs_zone[] = {
> "idnsName", "idnsUpdatePolicy", "idnsAllowQuery",
> "idnsAllowTransfer", "idnsForwardPolicy", "idnsForwarders", NULL
> };
> + char *attrs_record[] = {
> + "objectClass", "dn", NULL
> + };
>
> UNUSED(task);
>
> mctx = pevent->mctx;
> + dns_name_init(&prevname, NULL);
>
> result = manager_get_ldap_instance(pevent->dbname, &inst);
> /* TODO: Can it happen? */
> if (result != ISC_R_SUCCESS)
> goto cleanup;
>
> CHECK(ldap_pool_getconnection(inst->pool, &conn));
> -
> - CHECK(ldap_query(inst, conn, &ldap_qresult, pevent->dn,
> - LDAP_SCOPE_BASE, attrs, 0,
> + CHECK(ldap_query(inst, conn, &ldap_qresult_zone, pevent->dn,
> + LDAP_SCOPE_BASE, attrs_zone, 0,
> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
>
> - for (entry = HEAD(ldap_qresult->ldap_entries);
> - entry != NULL;
> - entry = NEXT(entry, link)) {
> + for (entry_zone = HEAD(ldap_qresult_zone->ldap_entries);
> + entry_zone != NULL;
> + entry_zone = NEXT(entry_zone, link)) {
> delete = ISC_FALSE;
> result = ldap_parse_zoneentry(entry, inst);
> if (result != ISC_R_SUCCESS)
> goto cleanup;
> +
> + if (PSEARCH_MODDN(pevent->chgtype)) {
> + if (dn_to_dnsname(inst->mctx, pevent->prevdn, &prevname, NULL)
> + == ISC_R_SUCCESS) {
> + CHECK(ldap_delete_zone(inst, pevent->prevdn, ISC_TRUE));
> + } else {
> + log_debug(5, "update_action: old zone wasn't managed "
> + "by plugin, dn '%s'", pevent->prevdn);
> + }
> +
> + /* fill the cache with records from renamed zone */
> + CHECK(ldap_query(inst, conn, &ldap_qresult_record, pevent->dn,
> + LDAP_SCOPE_ONELEVEL, attrs_record, 0,
> + "(objectClass=idnsRecord)"));
> +
> + /* LDAP schema requires SOA record (at least) */
> + INSIST(HEAD(ldap_qresult_record->ldap_entries) != NULL);
> + for (entry_record = HEAD(ldap_qresult_record->ldap_entries);
> + entry_record != NULL;
> + entry_record = NEXT(entry_record, link)) {
> +
> + psearch_update(inst, entry_record, NULL);
> + }
> + }
> +
> + INSIST(NEXT(entry_zone, link) == NULL); /* no multiple zones with same DN */
This INSIST seems like too strong for me. Imagine situation when administrator
wrongly modifies LDAP database and creates duplicated zone. In this case we will
crash. I would prefer to log error here, that multiple zones exist and only the
first occurence is used.
> }
>
> if (delete)
> @@ -2999,9 +3036,14 @@ cleanup:
> "Zones can be outdated, run `rndc reload`",
> pevent->dn, isc_result_totext(result));
>
> - ldap_query_free(ISC_FALSE, &ldap_qresult);
> + ldap_query_free(ISC_FALSE, &ldap_qresult_zone);
> + ldap_query_free(ISC_FALSE, &ldap_qresult_record);
> ldap_pool_putconnection(inst->pool, &conn);
> + if (dns_name_dynamic(&prevname))
> + dns_name_free(&prevname, inst->mctx);
> isc_mem_free(mctx, pevent->dbname);
> + if (pevent->prevdn != NULL)
> + isc_mem_free(mctx, pevent->prevdn);
> isc_mem_free(mctx, pevent->dn);
> isc_mem_detach(&mctx);
> isc_event_free(&event);
> @@ -3090,8 +3132,12 @@ update_record(isc_task_t *task, isc_event_t *event)
> /* Convert domain name from text to struct dns_name_t. */
> dns_name_t name;
> dns_name_t origin;
> + dns_name_t prevname;
> + dns_name_t prevorigin;
> dns_name_init(&name, NULL);
> dns_name_init(&origin, NULL);
> + dns_name_init(&prevname, NULL);
> + dns_name_init(&prevorigin, NULL);
> CHECK(dn_to_dnsname(mctx, pevent->dn, &name, &origin));
>
> if (PSEARCH_DEL(pevent->chgtype)) {
> @@ -3103,8 +3149,21 @@ update_record(isc_task_t *task, isc_event_t *event)
> cache = ldap_instance_getcache(inst);
> CHECK(discard_from_cache(cache, &name));
>
> + if (PSEARCH_MODDN(pevent->chgtype)) {
> + /* remove previous name only if it was inside DNS subtree */
> + if(dn_to_dnsname(mctx, pevent->prevdn, &prevname, &prevorigin)
> + == ISC_R_SUCCESS) {
> + log_debug(5, "psearch_update: removing name from cache, dn: '%s'",
> + pevent->prevdn);
> + CHECK(discard_from_cache(cache, &prevname));
> + } else {
> + log_debug(5, "psearch_update: old name wasn't managed "
> + "by plugin, dn '%s'", pevent->prevdn);
> + }
> + }
> +
> if (PSEARCH_ADD(pevent->chgtype) || PSEARCH_MOD(pevent->chgtype) ||
> - !PSEARCH_ANY(pevent->chgtype)) {
> + PSEARCH_MODDN(pevent->chgtype) || !PSEARCH_ANY(pevent->chgtype)) {
> /*
> * Find new data in LDAP. !PSEARCH_ANY indicates unchanged entry
> * found during initial lookup (i.e. database dump).
> @@ -3141,9 +3200,15 @@ cleanup:
>
> if (dns_name_dynamic(&name))
> dns_name_free(&name, inst->mctx);
> + if (dns_name_dynamic(&prevname))
> + dns_name_free(&prevname, inst->mctx);
> if (dns_name_dynamic(&origin))
> dns_name_free(&origin, inst->mctx);
> + if (dns_name_dynamic(&prevorigin))
> + dns_name_free(&prevorigin, inst->mctx);
> isc_mem_free(mctx, pevent->dbname);
> + if (pevent->prevdn != NULL)
> + isc_mem_free(mctx, pevent->prevdn);
> isc_mem_free(mctx, pevent->dn);
> isc_mem_detach(&mctx);
> isc_event_free(&event);
> @@ -3214,8 +3279,9 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
> isc_result_t result = ISC_R_SUCCESS;
> ldap_psearchevent_t *pevent = NULL;
> int chgtype = LDAP_ENTRYCHANGE_NONE;
> - char *moddn = NULL;
> char *dn = NULL;
> + char *prevdn_ldap = NULL;
> + char *prevdn = NULL;
> char *dbname = NULL;
> isc_mem_t *mctx = NULL;
> isc_taskaction_t action = NULL;
> @@ -3228,7 +3294,7 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
> }
>
> if (ctrls != NULL)
> - CHECK(ldap_parse_entrychangectrl(ctrls, &chgtype, &moddn));
> + CHECK(ldap_parse_entrychangectrl(ctrls, &chgtype, &prevdn_ldap));
>
> isc_mem_attach(inst->mctx, &mctx);
>
> @@ -3243,11 +3309,12 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
> goto cleanup;
> }
>
> - /* TODO: Handle moddn case. */
> if (PSEARCH_MODDN(chgtype)) {
> - log_error("psearch moddn change is not implemented");
> - result = ISC_R_FAILURE;
> - goto cleanup;
> + prevdn = isc_mem_strdup(mctx, prevdn_ldap);
> + if (prevdn == NULL) {
> + result = ISC_R_NOMEMORY;
> + goto cleanup;
> + }
> }
>
> /*
> @@ -3281,19 +3348,22 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
> pevent->mctx = mctx;
> pevent->dbname = dbname;
> pevent->dn = dn;
> + pevent->prevdn = prevdn;
> pevent->chgtype = chgtype;
> isc_task_send(inst->task, (isc_event_t **)&pevent);
>
> cleanup:
> if (result != ISC_R_SUCCESS) {
> if (dbname != NULL)
> isc_mem_free(mctx, dbname);
> if (dn != NULL)
> isc_mem_free(mctx, dn);
> + if (prevdn != NULL)
> + isc_mem_free(mctx, prevdn);
> if (mctx != NULL)
> isc_mem_detach(&mctx);
> - if (moddn != NULL)
> - ldap_memfree(moddn);
> + if (prevdn_ldap != NULL)
> + ldap_memfree(prevdn);
>
> log_error("psearch_update failed for %s zone. "
> "Zone can be outdated, run `rndc reload`",
> --
> 1.7.7.6
>
> From 40c54af3b1b6872a6eeac28624524adb09c4c9b7 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Wed, 18 Jul 2012 13:04:10 +0200
> Subject: [PATCH] Rename persistent search update_action() to update_zone().
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index d49d72bc6ad23e8cec1c1f8a45135e79290b1422..59429e21db7982edf795e0f8e12e22b9f5fa1b02 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -2956,7 +2956,7 @@ cleanup:
> * operation but zones don't change often.
> */
> static void
> -update_action(isc_task_t *task, isc_event_t *event)
> +update_zone(isc_task_t *task, isc_event_t *event)
> {
> ldap_psearchevent_t *pevent = (ldap_psearchevent_t *)event;
> isc_result_t result ;
> @@ -3327,7 +3327,7 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
> if ((class & LDAP_ENTRYCLASS_CONFIG) != 0)
> action = update_config;
> else if ((class & LDAP_ENTRYCLASS_ZONE) != 0)
> - action = update_action;
> + action = update_zone;
> else if ((class & LDAP_ENTRYCLASS_RR) != 0)
> action = update_record;
> else {
> --
> 1.7.7.6
>
> From b4bf985854391a68ad1c8394294edf64c80613d4 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Wed, 18 Jul 2012 13:05:59 +0200
> Subject: [PATCH] Minor code cleanup in persistent search error handling.
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 10 ++--------
> 1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 59429e21db7982edf795e0f8e12e22b9f5fa1b02..3a8ced88758d79719966ce80f97116d6cb2b84e2 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -2982,23 +2982,17 @@ update_zone(isc_task_t *task, isc_event_t *event)
> mctx = pevent->mctx;
> dns_name_init(&prevname, NULL);
>
> - result = manager_get_ldap_instance(pevent->dbname, &inst);
> - /* TODO: Can it happen? */
> - if (result != ISC_R_SUCCESS)
> - goto cleanup;
> -
> + CHECK(manager_get_ldap_instance(pevent->dbname, &inst));
> CHECK(ldap_pool_getconnection(inst->pool, &conn));
> CHECK(ldap_query(inst, conn, &ldap_qresult_zone, pevent->dn,
> LDAP_SCOPE_BASE, attrs_zone, 0,
> "(&(objectClass=idnsZone)(idnsZoneActive=TRUE))"));
>
> for (entry_zone = HEAD(ldap_qresult_zone->ldap_entries);
> entry_zone != NULL;
> entry_zone = NEXT(entry_zone, link)) {
> delete = ISC_FALSE;
> - result = ldap_parse_zoneentry(entry, inst);
> - if (result != ISC_R_SUCCESS)
> - goto cleanup;
> + CHECK(ldap_parse_zoneentry(entry_zone, inst));
>
> if (PSEARCH_MODDN(pevent->chgtype)) {
> if (dn_to_dnsname(inst->mctx, pevent->prevdn, &prevname, NULL)
> --
> 1.7.7.6
>
> From b538e9ca9784cc23105c8f90b6a12104407b27b9 Mon Sep 17 00:00:00 2001
> From: Petr Spacek <pspacek at redhat.com>
> Date: Wed, 18 Jul 2012 13:27:16 +0200
> Subject: [PATCH] Minor persistent search logging cleanup.
>
> Signed-off-by: Petr Spacek <pspacek at redhat.com>
> ---
> src/ldap_helper.c | 21 +++++++++++----------
> 1 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index 3a8ced88758d79719966ce80f97116d6cb2b84e2..c3499b8878af7ad8a4fc1f71406026b4d3d40aaa 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -3134,8 +3134,8 @@ update_record(isc_task_t *task, isc_event_t *event)
> dns_name_init(&prevorigin, NULL);
> CHECK(dn_to_dnsname(mctx, pevent->dn, &name, &origin));
>
> - if (PSEARCH_DEL(pevent->chgtype)) {
> - log_debug(5, "psearch_update: Removing item from cache (%s)",
> + if (PSEARCH_DEL(pevent->chgtype) || PSEARCH_MODDN(pevent->chgtype)) {
> + log_debug(5, "psearch_update: removing name from cache, dn: '%s'",
> pevent->dn);
> }
>
> @@ -3164,7 +3164,7 @@ update_record(isc_task_t *task, isc_event_t *event)
> *
> * @todo Change this to convert ldap_entry_t to ldapdb_rdatalist_t.
> */
> - log_debug(5, "psearch_update: Updating item in cache (%s)",
> + log_debug(5, "psearch_update: updating name in cache, dn: '%s'",
> pevent->dn);
> CHECK(ldapdb_rdatalist_get(mctx, inst, &name, &origin, &rdatalist));
>
> @@ -3177,18 +3177,13 @@ update_record(isc_task_t *task, isc_event_t *event)
> ldapdb_rdatalist_destroy(mctx, &rdatalist);
> }
>
> - log_debug(20,"psearch change type: none%d, add%d, del%d, mod%d, moddn%d",
> - !PSEARCH_ANY(pevent->chgtype), PSEARCH_ADD(pevent->chgtype),
> - PSEARCH_DEL(pevent->chgtype), PSEARCH_MOD(pevent->chgtype),
> - PSEARCH_MODDN(pevent->chgtype));
> -
> /* Do not bump serial during initial database dump. */
> if (inst->serial_autoincrement && PSEARCH_ANY(pevent->chgtype)) {
> CHECK(soa_serial_increment(mctx, inst, &origin));
> }
> cleanup:
> if (result != ISC_R_SUCCESS)
> - log_error("update_record (psearch) failed for %s. "
> + log_error("update_record (psearch) failed, dn '%s'. "
> "Records can be outdated, run `rndc reload`",
> pevent->dn);
>
> @@ -3282,14 +3277,20 @@ psearch_update(ldap_instance_t *inst, ldap_entry_t *entry, LDAPControl **ctrls)
>
> class = ldap_entry_getclass(entry);
> if (class == LDAP_ENTRYCLASS_NONE) {
> - log_error("psearch_update: ignoring unknown entry [dn %s]",
> + log_error("psearch_update: ignoring entry with unknown class, dn '%s'",
> entry->dn);
> return; /* ignore it, it's OK */
> }
>
> if (ctrls != NULL)
> CHECK(ldap_parse_entrychangectrl(ctrls, &chgtype, &prevdn_ldap));
>
> +
> + log_debug(20,"psearch change type: none%d, add%d, del%d, mod%d, moddn%d",
> + !PSEARCH_ANY(chgtype), PSEARCH_ADD(chgtype),
> + PSEARCH_DEL(chgtype), PSEARCH_MOD(chgtype),
> + PSEARCH_MODDN(chgtype));
> +
> isc_mem_attach(inst->mctx, &mctx);
>
> dn = isc_mem_strdup(mctx, entry->dn);
> --
> 1.7.7.6
>
--
Adam Tkac, Red Hat, Inc.
More information about the Freeipa-devel
mailing list