[Freeipa-devel] [PATCH 0393-0398] Unload automatic empty zones only if conflicting forward zone has policy 'only'Add ability to log warningsUnload automatic empty zones which are super/sub/equal domain as forward zoneDo not touch global forwarders from named.conf if there is no config in LDAPAdd missing includes to util.hMove zone_isempty() and delete_bind_zone() to zone_register.c

[Petr Spacek]

On 6.5.2016 16:41, Tomas Hozza wrote:
> On 04/06/2016 01:42 PM, Petr Spacek wrote:
>> Hello,
>>
>> attached patch set implements
>> https://fedorahosted.org/bind-dyndb-ldap/ticket/160
>> described in
>> https://fedorahosted.org/bind-dyndb-ldap/wiki/BIND9/Design/AutomaticEmptyZones
>>
>> It will be accompanied with upgrade code in FreeIPA which will automatically
>> convert forward zones as necessary.
>>
> 
> ACK.
> 
> I reviewed the patches on GitHub (https://github.com/pspacek/bind-dyndb-ldap/commits/empty_zones_unload_only)
> 
> I have these comments:
> 
> - commit 396 (https://github.com/pspacek/bind-dyndb-ldap/commit/a1bb7c0f802107d1fc67de8a58d0f33095accd06)
> 
>   - it uses "forwarders" in wrong context. In many places and also in the commit message you should use "forward zone" instead of "forwarder". "forwarder is the server to which the queries are forwarded, not the zone.
> 
>   - It wold be nice to explicitly note in the commit message, that conflicting empty zones are unloaded regardless of the forward policy ("only" or "first").

I've fixed this before push.

> - It would be good to document, that once you add forward zone, which causes removal of empty zone and then you remove the forward zone, the empty zone is not added back. The empty zone will appear again only after restart of BIND, when it loads all empty zones and only existing ones are removed. The situation is the same when you configure global forwarders and them remove them = all empty zones are unloaded and these are not added back until the restart of BIND. This is a limitation and IMHO the users should be informed about this behavior, as it may not be completely expected.
Good idea! This will be documented in follow-up patch.


> - Configuring global forwarders as forward first creates too many log messages (for each empty zone). This may not be a problem, but it is IMHO not necessary. Also when I configure global forwarders as forward "only" and then change it to forward "first", the log message is printed even though the empty zones are not there any more, which is a bit misleading.
Okay, I can improve this a little bit so zones which were once unloaded will
not be reported again. This is a matter for a follow-up patch.

On the other hand, I'm going to print message for every zone because this is
was BIND 9.11 is doing.


> - Warning log message about the configuration of global forward zone are logged every time on start-up regardless of the forward policy set in LDAP.
This is caused by forwarding policy configured in named.conf. I will think
about possible fix but it is not that easy. For now I will leave it as it is.
Maybe rebase to BIND 9.11 will fix it for us because they print warnings, too.

> Other than that the changes look good to me. The functionality works as documented with the shortcomings mentioned above.

Thanks, pushed to master:
3232aa4f35850c5164e7ec0b9cc523e3cf7bdb5d Unload automatic empty zones only if
conflicting forward zone has policy 'only'.
4174e96496aad3dc8b7040b552aa589157d9ede6 Add ability to log warnings.
5c3312cee1c080c5df45741da26ed801a9262d74 Unload automatic empty zones which
are super/sub/equal domain as forward zone.
998b3fbd055dd455c0c0f3ebf7a68efe12e5ce52 Do not touch global forwarders from
named.conf if there is no config in LDAP.
ffcfb9563ae97cdc345d5611ccc168101fdffeeb Add missing includes to util.h.
168ed7bb2145f1c515e0f6166e24797630949df2 Move zone_isempty() and
delete_bind_zone() to zone_register.c.


Thank you very much for your time!


> PS: There seems to be some kind of race condition when you configure forward zone and you have global forwarding turned off. In case you configure a forward zone with forward "only" and it conflicts with an empty zone, you can get SERVFAIL from the server for hostname from such zone. It is reproducible only if you are quick enough. The next query is forwarded correctly and the response from forwarder is returned.

I think that this is there since forever and I have no idea why it happens and
what we can do to fix it :-(

-- 
Petr^2 Spacek