On Wed, Jan 12, 2011 at 01:15:36PM -0500, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 01/12/2011 07:37 AM, Adam Tkac wrote:
> > Hello,
> >
> > bind-dyndb-ldap currently leaves empty nodes in LDAP when the last
> > DNS resource record associated with the node was removed:
> >
> > Before DDNS update:
> >
> > dn: idnsName=test,idnsName=example.com,ou=dns,dc=example,dc=com
> > aRecord: 1.1.1.1
> > dNSTTL: 1111
> > objectClass: idnsRecord
> > idnsName: test
> >
> > After DDNS update (removal of "test.example.com A 1.1.1.1" record):
> >
> > dn: idnsName=test,idnsName=example.com,ou=dns,dc=example,dc=com
> > dNSTTL: 1111
> > objectClass: idnsRecord
> > idnsName: test
> >
> > As you can see this node is empty and useless.
> >
> > With the patch the whole node is removed.
> >
> > Comments are welcomed.
> >
> > Regards, Adam
>
>
>
> Nack.
>
> Your prototype for ldap_modify_do() includes 'isc_result_t delete_node',
> but the actual implementation expects 'isc_boolean_t delete_node'. I'm
> guessing that by coincidence these typedefs are the same primitive type,
> but I'd rather they both use isc_boolean_t which is more correct.
>
> Otherwise it looks good to me.
Good catch! Fixed patch is attached.
Regards, Adam
--
Adam Tkac, Red Hat, Inc.
-------------- next part --------------
>From 5a1ddebaa55d5fcf97e4a10401d4339adcd29aab Mon Sep 17 00:00:00 2001
From: Adam Tkac <atkac at redhat.com>
Date: Mon, 10 Jan 2011 15:25:40 +0100
Subject: [PATCH] Delete node from LDAP if there is no RR associated with the name.
If the last DNS resource record associated with the name is removed
then remove the whole node from LDAP.
Solves https://fedorahosted.org/bind-dyndb-ldap/ticket/1.
Signed-off-by: Adam Tkac <atkac at redhat.com>
---
src/ldap_driver.c | 14 +++++++++++++-
src/ldap_helper.c | 29 ++++++++++++++++++-----------
src/ldap_helper.h | 2 +-
3 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 965877c..9c1da40 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -787,6 +787,7 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
dns_rdatalist_t *rdlist;
dns_rdatalist_t diff;
isc_result_t result;
+ isc_boolean_t delete_node = ISC_FALSE;
REQUIRE(version == ldapdb_version);
@@ -822,7 +823,18 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
goto cleanup;
}
- CHECK(remove_from_ldap(&ldapdbnode->owner, ldapdb->ldap_inst, &diff));
+ /*
+ * If there is only one rdatalist in the node with no rdata
+ * it means all resource records associated with the node's DNS
+ * name (owner) was deleted. So delete the whole node from the
+ * LDAP.
+ */
+ if (HEAD(ldapdbnode->rdatalist) == TAIL(ldapdbnode->rdatalist) &&
+ HEAD((HEAD(ldapdbnode->rdatalist))->rdata) == NULL)
+ delete_node = ISC_TRUE;
+
+ CHECK(remove_from_ldap(&ldapdbnode->owner, ldapdb->ldap_inst, &diff,
+ delete_node));
CHECK(discard_from_cache(ldapdb->ldap_cache, &ldapdbnode->owner));
if (newrdataset != NULL) {
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index e29bd04..aee3f52 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -259,7 +259,7 @@ static isc_result_t ldap_query(ldap_connection_t *ldap_conn, const char *base,
/* Functions for writing to LDAP. */
static isc_result_t ldap_modify_do(ldap_connection_t *ldap_conn, const char *dn,
- LDAPMod **mods);
+ LDAPMod **mods, isc_boolean_t delete_node);
static isc_result_t ldap_rdttl_to_ldapmod(isc_mem_t *mctx,
dns_rdatalist_t *rdlist, LDAPMod **changep);
static isc_result_t ldap_rdatalist_to_ldapmod(isc_mem_t *mctx,
@@ -269,7 +269,7 @@ static isc_result_t ldap_rdata_to_char_array(isc_mem_t *mctx,
dns_rdata_t *rdata_head, char ***valsp);
static void free_char_array(isc_mem_t *mctx, char ***valsp);
static isc_result_t modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
- dns_rdatalist_t *rdlist, int mod_op);
+ dns_rdatalist_t *rdlist, int mod_op, isc_boolean_t delete_node);
isc_result_t
new_ldap_instance(isc_mem_t *mctx, const char *db_name,
@@ -1740,7 +1740,8 @@ handle_connection_error(ldap_connection_t *ldap_conn, isc_result_t *result)
/* FIXME: Handle the case where the LDAP handle is NULL -> try to reconnect. */
static isc_result_t
-ldap_modify_do(ldap_connection_t *ldap_conn, const char *dn, LDAPMod **mods)
+ldap_modify_do(ldap_connection_t *ldap_conn, const char *dn, LDAPMod **mods,
+ isc_boolean_t delete_node)
{
int ret;
int err_code;
@@ -1750,9 +1751,14 @@ ldap_modify_do(ldap_connection_t *ldap_conn, const char *dn, LDAPMod **mods)
REQUIRE(dn != NULL);
REQUIRE(mods != NULL);
- log_debug(2, "writing to '%s'", dn);
+ if (delete_node) {
+ log_debug(2, "deleting whole node: '%s'", dn);
+ ret = ldap_delete_ext_s(ldap_conn->handle, dn, NULL, NULL);
+ } else {
+ log_debug(2, "writing to '%s'", dn);
+ ret = ldap_modify_ext_s(ldap_conn->handle, dn, mods, NULL, NULL);
+ }
- ret = ldap_modify_ext_s(ldap_conn->handle, dn, mods, NULL, NULL);
if (ret == LDAP_SUCCESS)
return ISC_R_SUCCESS;
@@ -1998,14 +2004,14 @@ modify_soa_record(ldap_connection_t *ldap_conn, const char *zone_dn,
dns_rdata_freestruct((void *)&soa);
- return ldap_modify_do(ldap_conn, zone_dn, changep);
+ return ldap_modify_do(ldap_conn, zone_dn, changep, ISC_FALSE);
#undef SET_LDAP_MOD
}
static isc_result_t
modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
- dns_rdatalist_t *rdlist, int mod_op)
+ dns_rdatalist_t *rdlist, int mod_op, isc_boolean_t delete_node)
{
isc_result_t result;
isc_mem_t *mctx = ldap_inst->mctx;
@@ -2033,7 +2039,7 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
CHECK(ldap_rdttl_to_ldapmod(mctx, rdlist, &change[1]));
}
- CHECK(ldap_modify_do(ldap_conn, str_buf(owner_dn), change));
+ CHECK(ldap_modify_do(ldap_conn, str_buf(owner_dn), change, delete_node));
cleanup:
put_connection(ldap_conn);
@@ -2047,12 +2053,13 @@ cleanup:
isc_result_t
write_to_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst, dns_rdatalist_t *rdlist)
{
- return modify_ldap_common(owner, ldap_inst, rdlist, LDAP_MOD_ADD);
+ return modify_ldap_common(owner, ldap_inst, rdlist, LDAP_MOD_ADD, ISC_FALSE);
}
isc_result_t
remove_from_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst,
- dns_rdatalist_t *rdlist)
+ dns_rdatalist_t *rdlist, isc_boolean_t delete_node)
{
- return modify_ldap_common(owner, ldap_inst, rdlist, LDAP_MOD_DELETE);
+ return modify_ldap_common(owner, ldap_inst, rdlist, LDAP_MOD_DELETE,
+ delete_node);
}
diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index 594af43..887a059 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -105,6 +105,6 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst, isc_boolean_t create);
isc_result_t write_to_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst,
dns_rdatalist_t *rdlist);
isc_result_t remove_from_ldap(dns_name_t *owner, ldap_instance_t *ldap_inst,
- dns_rdatalist_t *rdlist);
+ dns_rdatalist_t *rdlist, isc_boolean_t delete_node);
#endif /* !_LD_LDAP_HELPER_H_ */
--
1.7.3.4