On 23/09/14 17:45, Petr Vobornik wrote:
On 25.8.2014 14:52, Martin Basti wrote:
There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the
named service is stopped after deleting zone.
Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138
1. Please follow pep8 for the new code.
# git diff HEAD~7 -U0 | pep8 --diff --ignore=E501
Produces 25 erros.
Only E124 and E128 could be ignored if they are part of old code.
I left there some pep8 errors. They don't decrease readability
3. This patch uses term 'deprecated' in a different meaning than a
DeprecatedParam. It creates inconsistency -> future confusion. IMHO this
usage is correct since the usual understanding of deprecation is that the
param is still usable but user should be prepared that it will be removed
in a future. IMHO DeprecatedParam is badly designed but that's not an
issue of this patch.
I think we can leave this as is and create a ticket to rename
DeprecatedParam e.g. to RemovedParam. What do you think?
5. You've removed 'idnssoamname' and 'force' from Web UI but dnszone-add
precallback still uses these params. What is the intended purpose?
User should use modify dialog in webUI for zones.
Precallback fills default value for idnsmname from LDAP.
with --force there will be no validation of user specified soa mname
Purpose is a user should let IPA to fill mname with safe value.
10. In `normalize_zonemgr(zonemgr)`, if zonemgr contains '@' shouldn't it
be normalized to contain '.' at the end? Or is it handled by bind-dyndb-ldap?
Zone manager (SOA RNAME) can eb relative name, BIND will append zone name.
Currently we cant validate if email address is reachable, it doestn matter
if it is filled with nonexistent absolute name, or nonexistent relative name.
Unrelated to this patch set:
a. One is able to run:
# ipa dnszone-remove-permission $zone
multiple times and it always returns success
Is it intentional?
No, it isn't. I will inspect it and I will send additional patch
b. Web UI doesn't have means to call dnszone-mod with --force option
I'm not sure what you mean, it didn't do that before my patches.
Updated patches attached