[Freeipa-devel] [PATCH 0063] Notify DNS slaves if zone serial number modification was detected.

[Petr Spacek]

On 09/24/2012 03:41 PM, Adam Tkac wrote:
> On Mon, Sep 24, 2012 at 03:21:23PM +0200, Petr Spacek wrote:
>> On 09/24/2012 03:09 PM, Adam Tkac wrote:
>>> On Mon, Sep 17, 2012 at 02:55:06PM +0200, Petr Spacek wrote:
>>>> Hello,
>>>>
>>>> this patch adds missing notification to DNS slaves if zone serial
>>>> number modification was detected.
>>>
>>> Hi,
>>>
>>> please check my comment below.
>>>
>>>>  From eb8d7fc0c02e03b9c7c90e497225536c449fab1c Mon Sep 17 00:00:00 2001
>>>> From: Petr Spacek <pspacek at redhat.com>
>>>> Date: Mon, 17 Sep 2012 14:29:45 +0200
>>>> Subject: [PATCH] Notify DNS slaves if zone serial number modification was
>>>>   detected.
>>>>
>>>> Signed-off-by: Petr Spacek <pspacek at redhat.com>
>>>> ---
>>>>   src/ldap_helper.c | 22 ++++++++++++++--------
>>>>   1 file changed, 14 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/src/ldap_helper.c b/src/ldap_helper.c
>>>> index 658b960f50b461fa602edf51e955f3bdd4769e1d..d22e8714df57edaad4cf45e6cc26ec0dbbd59108 100644
>>>> --- a/src/ldap_helper.c
>>>> +++ b/src/ldap_helper.c
>>>> @@ -1035,9 +1035,10 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>>>>   	isc_task_t *task = inst->task;
>>>>   	isc_uint32_t ldap_serial;
>>>>   	isc_uint32_t zr_serial;	/* SOA serial value from in-memory zone register */
>>>> -	unsigned char ldap_digest[RDLIST_DIGESTLENGTH];
>>>> +	unsigned char ldap_digest[RDLIST_DIGESTLENGTH] = {0};
>>>>   	unsigned char *zr_digest = NULL;
>>>>   	ldapdb_rdatalist_t rdatalist;
>>>> +	isc_boolean_t zone_dynamic = ISC_FALSE;
>>>>
>>>>   	REQUIRE(entry != NULL);
>>>>   	REQUIRE(inst != NULL);
>>>> @@ -1131,13 +1132,13 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>>>>   		&& result != DNS_R_DYNAMIC && result != DNS_R_CONTINUE)
>>>>   		goto cleanup;
>>>>
>>>> +	zone_dynamic = (result == DNS_R_DYNAMIC);
>>>>   	result = ISC_R_SUCCESS;
>>>>
>>>>   	/* initialize serial in zone register and always increment serial
>>>>   	 * for a new zone (typically after BIND start)
>>>>   	 * - the zone was possibly changed in meanwhile */
>>>>   	if (publish) {
>>>> -		memset(ldap_digest, 0, RDLIST_DIGESTLENGTH);
>>>>   		CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
>>>>   		CHECK(zr_set_zone_serial_digest(inst->zone_register, &name, ldap_serial,
>>>>   				ldap_digest));
>>>> @@ -1154,23 +1155,28 @@ ldap_parse_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst)
>>>>   	 * 3c) The old and new serials are same: autoincrement only if something
>>>>   	 *     else was changed.
>>>>   	 */
>>>> +	CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
>>>> +	CHECK(zr_get_zone_serial_digest(inst->zone_register, &name, &zr_serial,
>>>> +			&zr_digest));
>>>>   	if (inst->serial_autoincrement) {
>>>> -		CHECK(ldap_get_zone_serial(inst, &name, &ldap_serial));
>>>>   		CHECK(ldapdb_rdatalist_get(inst->mctx, inst, &name,
>>>>   				&name, &rdatalist));
>>>>   		CHECK(rdatalist_digest(inst->mctx, &rdatalist, ldap_digest));
>>>>
>>>> -		CHECK(zr_get_zone_serial_digest(inst->zone_register, &name, &zr_serial,
>>>> -				&zr_digest));
>>>>   		if (ldap_serial == zr_serial) {
>>>>   			/* serials are same - increment only if something was changed */
>>>>   			if (memcmp(zr_digest, ldap_digest, RDLIST_DIGESTLENGTH) != 0)
>>>>   				CHECK(soa_serial_increment(inst->mctx, inst, &name));
>>>> -		} else { /* serial in LDAP was changed - update zone register */
>>>> -			CHECK(zr_set_zone_serial_digest(inst->zone_register, &name,
>>>> -					ldap_serial, ldap_digest));
>>>>   		}
>>>>   	}
>>>> +	if (ldap_serial != zr_serial) {
>>>> +		/* serial in LDAP was changed - update zone register */
>>>> +		CHECK(zr_set_zone_serial_digest(inst->zone_register, &name,
>>>> +				ldap_serial, ldap_digest));
>>>> +
>>>> +		if (zone_dynamic)
>>>> +			dns_zone_notify(zone);
>>>
>>> This if() doesn't seem fully correct to me. Although in most FreeIPA scenarios
>>> it will work, consider situation when someone disables DDNS updates for zone and
>>> modifies records in LDAP. In this case zone_dynamic is FALSE but SOA serial
>>> changes.
>>>
>>> I would recommend to simpy send notify every time when serial changes.
>>
>> Two lines above start of the patch is following code:
>>
>> result = dns_zone_load(zone);
>> 	if (result != ISC_R_SUCCESS && result != DNS_R_UPTODATE
>> 		&& result != DNS_R_DYNAMIC && result != DNS_R_CONTINUE)
>> 		goto cleanup;
>>
>> 	zone_dynamic = (result == DNS_R_DYNAMIC);
>>
>> For non-dynamic zones "dns_zone_load()" sends notifies. Zone_dynamic
>> condition only prevents doubled dns_zone_notify() calls for
>> non-dynamic zones. I can remove this condition if doubled calls are
>> not problem.
>
> Ah, good point. Then ack for the original version.

Pushed to master:
c7c18b2e565ddf45d3f20cbab9e4cc2828ec9d5f

Petr^2 Spacek