[Freeipa-devel] [PATCH] 0059-0063 Update DNSSEC attributes/record types

[Petr Vobornik]

On 20.6.2014 15:30, Petr Vobornik wrote:
> On 20.6.2014 14:35, Martin Basti wrote:
>> On Thu, 2014-06-19 at 18:37 +0200, Martin Basti wrote:
>>> On Fri, 2014-06-13 at 09:55 +0200, Martin Basti wrote:
>>>> On Thu, 2014-06-12 at 16:20 +0200, Martin Basti wrote:
>>>>> On Thu, 2014-06-12 at 13:17 +0200, Petr Vobornik wrote:
>>>>>> On 9.6.2014 17:28, Martin Basti wrote:
>>>>>>> Ticket: https://fedorahosted.org/freeipa/ticket/4328
>>>>>>>
>>>>>>> Petr please make the WebUI patch review (0062) :-)
>>>>>>>
>>>>>>> Patches attached.
>>>>>>>
>>>>>>
>>>>>> Patch #0059: LGTM
>>>>>>
>>>>>> Patch #0060:
>>>>>>
>>>>>> 1. Please add `pattern_errmsg` to `salt` part of nsec3param.
>>>>>> Otherwise
>>>>>> you get general "Text does not match field pattern" error message
>>>>>> in Web UI.
>>>>>>
>>>>> I will add the message.
>>>>>
>>>>>> 2. Could be in one if:
>>>>>> +            if nsec3params is not None:
>>>>>> +                if len(nsec3params) > 1:
>>>>>>
>>>>>> Maybe I'm missing something. But why does the dns plugin code use
>>>>>> following all over the place:
>>>>>>
>>>>>>           try:
>>>>>>               nsec3params = rrattrs['nsec3paramrecord']
>>>>>>           except KeyError:
>>>>>>               pass
>>>>>>           else:
>>>>>>               if nsec3params is not None:
>>>>>>
>>>>>> instead of:
>>>>>>
>>>>>>       nsec3params = rrattrs.get('nsec3paramrecord')
>>>>>>       if nsec3params is not None:
>>>>>>
>>>>>> btw you use both patterns in the patch.
>>>>> I will use shorten form, I wrote it in the same way as the other
>>>>> code in
>>>>> the block was written, that's why.
>>>>>
>>>>>>
>>>>>> Patch #0061: ACK
>>>>>>
>>>>>>
>>>>>> Patch #0062:
>>>>>>
>>>>>> 3. Why are there the idnafsdbrec1 variables?
>>>>>>
>>>>> It was replace for NSEC records, which are not supported anymore.
>>>>> Now I realize, there is wrong description, so the idnafsbrec1 variable
>>>>> is not needed.
>>>>> I will remove it.
>>>>>
>>>>>> 4. related to ^^:
>>>>>> ./ipatests/test_xmlrpc/test_dns_plugin.py:199:33: E231 missing
>>>>>> whitespace after ','
>>>>>>
>>>>>>
>>>>>> Patch #0063: LGTM
>>>>>> IDK if they work because I'm experiencing some weird issues with
>>>>>> xmlrpc_tests in general.
>>>>> I had troubles only with permission tests, but all DNS test worked
>>>>> fine for me.
>>>>>
>>>>> Thank you for review.
>>>>>
>>>> Updated patches attached.
>>>>
>>>
>>> Rebased patches attached
>>
>> Updated patch attached, fixed missing update permission
>>
>
> ACK

Pushed to master:
* 48865aed5f15ae94db664c4cebed125ef8f223cc DNSSEC: remove unsuported records
* 5b95be802c6aa12b9464813441f85eaee3e3e82b DNSSEC: added NSEC3PARAM 
record type
* 4d90d3d572caedfd8813ccef0fe44551aed80d2b DNSSEC: webui update DNSSEC 
attributes
* cbc64454b026993d3724c9640e6ec738f549fdcd Tests: remove unused records 
from tests
* 4c88fdd9046c682c4b2cdce760e4c5440f2d41de Tests: tests for NSEC3PARAM 
records
-- 
Petr Vobornik