[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.
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:

              nsec3params = rrattrs['nsec3paramrecord']
          except KeyError:
              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


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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]