Re: [Freeipa-devel] [PATCH] 0025-0028 Implement SOA serial number increments for external changes
On 07/13/2012 03:42 PM, Adam Tkac wrote: On Tue, Jul 10, 2012 at 03:57:24PM +0200, Petr Spacek wrote: Hello, these patches provides SOA serial auto-increment feature for external changes. Related ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/67 It is necessary to set "psearch" AND "serial_autoincrement" to "yes" in /etc/named.conf to enable this feature. In replicated environment idnsSOAserial attribute has to be declared as non-replicated. It is done by mkosek's patch 281 for 389 DS & FreeIPA. For testing purposes it is enough to add "idnsSOAserial" to end of exclude list in nsDS5ReplicatedAttributeList attribute for each replication agreement located in cn=mapping tree,cn=config subtree. My patch 28 contains "trick" necessary for replicated environments with 389 DS. 389 sends entry change notification (ECN) in cases when non-replicated attribute idnsSOAserial was changed on *other side*. In that case no change is visible in DNS attributes, but ECN is sent by 389. (Attribute modifyTimestamp is changed also.) Patch 28 computes digest/hash from all resource records in idnsZone object and compares old and new digest after each received ECN. This approach eliminates "false changes". Each patch depends on all preceding patches, but each patch implements visible (and testable) part of functionality. Hello Peter, please check my comments below. Regards, Adam Hello, I did all changes except this one: +unsigned int +rdatalist_length(const dns_rdatalist_t *rdlist) There is no reason to have this function exported, please mark it as static (and probably also as inline). rdatalist_length() is used from rdlist.c and ldap_driver.c. Rebased patches were pushed to master: https://fedorahosted.org/bind-dyndb-ldap/changeset/99663b6d65cf5dc166b3cb6f83be1878b8de3163 https://fedorahosted.org/bind-dyndb-ldap/changeset/cd37fba03c5c86a766d1a9f893036ac3540e8b7c https://fedorahosted.org/bind-dyndb-ldap/changeset/9a3f29c12db99597222ffa2bf0713d0b00cb4699 https://fedorahosted.org/bind-dyndb-ldap/changeset/c379d81508fbfa00ecb5da727ff7b097ebb29a3d https://fedorahosted.org/bind-dyndb-ldap/changeset/93ae7491a80ba8c4789f8770e14c053b67176de4 Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0025-0028 Implement SOA serial number increments for external changes
On Tue, Jul 10, 2012 at 03:57:24PM +0200, Petr Spacek wrote:
> Hello,
>
> these patches provides SOA serial auto-increment feature for external changes.
> Related ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/67
>
> It is necessary to set "psearch" AND "serial_autoincrement" to "yes"
> in /etc/named.conf to enable this feature.
>
> In replicated environment idnsSOAserial attribute has to be declared
> as non-replicated. It is done by mkosek's patch 281 for 389 DS &
> FreeIPA.
>
> For testing purposes it is enough to add "idnsSOAserial" to end of
> exclude list in nsDS5ReplicatedAttributeList attribute for each
> replication agreement located in cn=mapping tree,cn=config subtree.
>
>
> My patch 28 contains "trick" necessary for replicated environments
> with 389 DS. 389 sends entry change notification (ECN) in cases when
> non-replicated attribute idnsSOAserial was changed on *other side*.
> In that case no change is visible in DNS attributes, but ECN is sent
> by 389. (Attribute modifyTimestamp is changed also.)
>
> Patch 28 computes digest/hash from all resource records in idnsZone
> object and compares old and new digest after each received ECN. This
> approach eliminates "false changes".
>
> Each patch depends on all preceding patches, but each patch
> implements visible (and testable) part of functionality.
Hello Peter,
please check my comments below.
Regards, Adam
> From 0ed1d3dd4910e1c94617b0209420b1c6598de68e Mon Sep 17 00:00:00 2001
> From: Petr Spacek
> Date: Wed, 27 Jun 2012 10:36:26 +0200
> Subject: [PATCH] Increment SOA serial for each ordinary record received
> through psearch
>
> Signed-off-by: Petr Spacek
> ---
> src/ldap_helper.c | 98
> -
> 1 files changed, 97 insertions(+), 1 deletions(-)
>
> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
> index
> 7f0a6f4b37171a6fa4db79cd32fdd8bc62288e0f..5c509443dfb176607d8ee4714d196efaa4afa66b
> 100644
> --- a/src/ldap_helper.c
> +++ b/src/ldap_helper.c
> @@ -34,6 +34,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include
> #include
> @@ -172,6 +174,7 @@ struct ldap_instance {
> isc_boolean_t exiting;
> isc_boolean_t sync_ptr;
> isc_boolean_t dyn_update;
> + isc_boolean_t serial_autoincrement;
> };
>
> struct ldap_pool {
> @@ -343,6 +346,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> { "ldap_hostname", default_string("") },
> { "sync_ptr",default_boolean(ISC_FALSE) },
> { "dyn_update", default_boolean(ISC_FALSE) },
> + { "serial_autoincrement",default_boolean(ISC_FALSE) },
> end_of_settings
> };
>
> @@ -401,6 +405,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> ldap_settings[i++].target = ldap_inst->ldap_hostname;
> ldap_settings[i++].target = &ldap_inst->sync_ptr;
> ldap_settings[i++].target = &ldap_inst->dyn_update;
> + ldap_settings[i++].target = &ldap_inst->serial_autoincrement;
> CHECK(set_settings(ldap_settings, argv));
>
> /* Set timer for deadlock detection inside semaphore_wait_timed . */
> @@ -463,6 +468,13 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
> "increasing limit");
> ldap_inst->connections = 3;
> }
> + if (ldap_inst->serial_autoincrement == ISC_TRUE
> + && ldap_inst->psearch != ISC_TRUE) {
> + log_error("SOA serial number auto-increment feature requires "
> + "persistent search");
> + result = ISC_R_FAILURE;
> + goto cleanup;
> + }
>
> CHECK(new_ldap_cache(mctx, argv, &ldap_inst->cache,
> ldap_inst->psearch));
> CHECK(ldap_pool_create(mctx, ldap_inst->connections, &ldap_inst->pool));
> @@ -2741,6 +2753,86 @@ ldap_pscontrol_destroy(isc_mem_t *mctx, LDAPControl
> **ctrlp)
> *ctrlp = NULL;
> }
>
> +isc_result_t
Better is "static inline isc_result_t"
> +soa_serial_get(ldap_instance_t *inst, dns_name_t *zone_name,
> + isc_uint32_t *serial) {
> + isc_result_t result;
> + dns_zone_t *zone = NULL;
> +
> + REQUIRE(inst != NULL);
> + REQUIRE(zone_name != NULL);
> + REQUIRE(serial != NULL);
All those REQUIREs are redundant, please remove them. The "inst" parameter is
checked before function call in soa_serial_increment and last two params are
checked by functions below.
> +
> + CHECK(zr_get_zone_ptr(inst->zone_register, zone_name, &zone));
> + CHECK(dns_zone_getserial2(zone, serial));
> + dns_zone_detach(&zone);
> +
> +cleanup:
> + return result;
> +}
> +
> +isc_result_t
> +soa_serial_increment(isc_mem_t *mctx, ldap_instance_t *inst,
> + dns_name_t *zone_name) {
> + isc_result_t result = ISC_R_FAILURE;
> + ldap_connection_t * conn = NULL;
> +
[Freeipa-devel] [PATCH] 0025-0028 Implement SOA serial number increments for external changes
Hello,
these patches provides SOA serial auto-increment feature for external changes.
Related ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/67
It is necessary to set "psearch" AND "serial_autoincrement" to "yes" in
/etc/named.conf to enable this feature.
In replicated environment idnsSOAserial attribute has to be declared as
non-replicated. It is done by mkosek's patch 281 for 389 DS & FreeIPA.
For testing purposes it is enough to add "idnsSOAserial" to end of exclude
list in nsDS5ReplicatedAttributeList attribute for each replication agreement
located in cn=mapping tree,cn=config subtree.
My patch 28 contains "trick" necessary for replicated environments with 389
DS. 389 sends entry change notification (ECN) in cases when non-replicated
attribute idnsSOAserial was changed on *other side*. In that case no change is
visible in DNS attributes, but ECN is sent by 389. (Attribute modifyTimestamp
is changed also.)
Patch 28 computes digest/hash from all resource records in idnsZone object and
compares old and new digest after each received ECN. This approach eliminates
"false changes".
Each patch depends on all preceding patches, but each patch implements visible
(and testable) part of functionality.
Petr^2 Spacek
From 0ed1d3dd4910e1c94617b0209420b1c6598de68e Mon Sep 17 00:00:00 2001
From: Petr Spacek
Date: Wed, 27 Jun 2012 10:36:26 +0200
Subject: [PATCH] Increment SOA serial for each ordinary record received
through psearch
Signed-off-by: Petr Spacek
---
src/ldap_helper.c | 98 -
1 files changed, 97 insertions(+), 1 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 7f0a6f4b37171a6fa4db79cd32fdd8bc62288e0f..5c509443dfb176607d8ee4714d196efaa4afa66b 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -34,6 +34,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -172,6 +174,7 @@ struct ldap_instance {
isc_boolean_t exiting;
isc_boolean_t sync_ptr;
isc_boolean_t dyn_update;
+ isc_boolean_t serial_autoincrement;
};
struct ldap_pool {
@@ -343,6 +346,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
{ "ldap_hostname", default_string("") },
{ "sync_ptr", default_boolean(ISC_FALSE) },
{ "dyn_update", default_boolean(ISC_FALSE) },
+ { "serial_autoincrement", default_boolean(ISC_FALSE) },
end_of_settings
};
@@ -401,6 +405,7 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
ldap_settings[i++].target = ldap_inst->ldap_hostname;
ldap_settings[i++].target = &ldap_inst->sync_ptr;
ldap_settings[i++].target = &ldap_inst->dyn_update;
+ ldap_settings[i++].target = &ldap_inst->serial_autoincrement;
CHECK(set_settings(ldap_settings, argv));
/* Set timer for deadlock detection inside semaphore_wait_timed . */
@@ -463,6 +468,13 @@ new_ldap_instance(isc_mem_t *mctx, const char *db_name,
"increasing limit");
ldap_inst->connections = 3;
}
+ if (ldap_inst->serial_autoincrement == ISC_TRUE
+ && ldap_inst->psearch != ISC_TRUE) {
+ log_error("SOA serial number auto-increment feature requires "
+"persistent search");
+ result = ISC_R_FAILURE;
+ goto cleanup;
+ }
CHECK(new_ldap_cache(mctx, argv, &ldap_inst->cache, ldap_inst->psearch));
CHECK(ldap_pool_create(mctx, ldap_inst->connections, &ldap_inst->pool));
@@ -2741,6 +2753,86 @@ ldap_pscontrol_destroy(isc_mem_t *mctx, LDAPControl **ctrlp)
*ctrlp = NULL;
}
+isc_result_t
+soa_serial_get(ldap_instance_t *inst, dns_name_t *zone_name,
+isc_uint32_t *serial) {
+ isc_result_t result;
+ dns_zone_t *zone = NULL;
+
+ REQUIRE(inst != NULL);
+ REQUIRE(zone_name != NULL);
+ REQUIRE(serial != NULL);
+
+ CHECK(zr_get_zone_ptr(inst->zone_register, zone_name, &zone));
+ CHECK(dns_zone_getserial2(zone, serial));
+ dns_zone_detach(&zone);
+
+cleanup:
+ return result;
+}
+
+isc_result_t
+soa_serial_increment(isc_mem_t *mctx, ldap_instance_t *inst,
+ dns_name_t *zone_name) {
+ isc_result_t result = ISC_R_FAILURE;
+ ldap_connection_t * conn = NULL;
+ ld_string_t *zone_dn = NULL;
+ ldapdb_rdatalist_t rdatalist;
+ dns_rdatalist_t *rdlist = NULL;
+ dns_rdata_t *soa_rdata = NULL;
+ isc_uint32_t old_serial;
+ isc_uint32_t new_serial;
+ isc_time_t curr_time;
+
+ REQUIRE(inst != NULL);
+ REQUIRE(zone_name != NULL);
+
+ CHECK(str_new(mctx, &zone_dn));
+ CHECK(dnsname_to_dn(inst->zone_register, zone_name, zone_dn));
+ log_debug(5, "incrementing SOA serial number in zone '%s'",
+str_buf(zone_dn));
+
+ /* get original SOA rdata and serial value */
+ INIT_LIST(rdatalist);
+ CHECK(ldapdb_rdatalist_get(mctx, inst, zone_name, zone_name, &rdatalist));
+ CHECK(ldapdb_rdatalist_findrdatatype(&rdatalist, dns_rdatatype_soa, &rdlist));
+ soa_rdata = ISC_LIST_HEAD(rdlist->rdata);
+ CHECK(soa_serial_get(inst, zone_name, &old_serial));
+
+ /* Compute the new SOA serial - use actual timestamp.
+ * If timestamp <= oldSOAserial then increment old serial by one. */
+ isc_time_now(&c
