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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code