[Freeipa-devel] [PATCHES 0114-0115] DNS: allow to add root zone '.'

[Martin Kosek]

On 09/26/2014 10:20 AM, Martin Basti wrote:
> On 25/09/14 17:13, Martin Kosek wrote:
>> On 09/25/2014 04:39 PM, Petr Viktorin wrote:
>>> On 09/25/2014 04:32 PM, Petr Spacek wrote:
>>>> On 25.9.2014 10:31, Martin Basti wrote:
>>>>> On 24/09/14 16:24, Martin Basti wrote:
>>>>>> On 24/09/14 16:05, Martin Basti wrote:
>>>>>>> On 23/09/14 17:45, Petr Vobornik wrote:
>>>>>>>> On 25.8.2014 14:52, Martin Basti wrote:
>>>>>>>>> Patches attached.
>>>>>>>>>
>>>>>>>>> Ticket: https://fedorahosted.org/freeipa/ticket/4149
>>>>>>>>>
>>>>>>>>> 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
>>>>>>>>>
>>>>>>>>>
>>>>>>>> Review of:
>>>>>>>> http://www.redhat.com/archives/freeipa-devel/2014-September/msg00484.html
>>>>>>>>
>>>>>>>>
>>>>>>>> 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
>>>>>>>
>>>>>>>> Patch 120:
>>>>>>>>
>>>>>>>> 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?
>>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/4566
>>>>>>>> 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.
>>>>>>>> Patch 123:
>>>>>>>>
>>>>>>>> 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
>>>>>>>
>>>>>> I accidentally removed one line in previous patchset.
>>>>>> Updated patches attached
>>>>>>
>>>>> Sorry my IDE was too smart, and somehow added its configuration file
>>>>> to commit
>>>>> and I didn't notice it.
>>>>> Patches attached.
>>>> ACK, it works for me. Replica installation and deletion properly adds
>>>> and deletes records as necessary.
>>>>
>>>> I would defer further improvements to
>>>> https://fedorahosted.org/freeipa/ticket/3343
>>>>
>>> Pushed to:
>>> ipa-4-1: b7e3a990369d85dfd12165891cf9142d669a0259
>>> master: bc2eaa145637e1947449ee53548243ab22059805
>>>
>> I reopened the ticket, we missed update to DNS help page (ipa help dns):
>>
>> https://fedorahosted.org/freeipa/ticket/4149#comment:18
>>
>> Martin
>
> Thanks!
> Patch attached.

ACK!

Pushed to:
master: 3f8cfdab269490e4935db7d296c3fc7f2fa552f5
ipa-4-1: 0f2eb65f008777ebdee9b35f5f69bada66066484

Martin