Re: [Freeipa-devel] [PATCHES 0111-0113] Fix NS record coexistence validation

2014-09-05 Thread Martin Kosek
On 09/04/2014 01:12 PM, Petr Spacek wrote:
> On 3.9.2014 16:51, Martin Basti wrote:
>> On 03/09/14 12:30, Martin Kosek wrote:
>>> On 09/02/2014 05:38 PM, Petr Spacek wrote:
 On 21.8.2014 19:21, Martin Basti wrote:
> During work on DNSSEC we found a wrong validation of NS records
> Patch 0113 fixes an error in tests caused by bind-dyndb-ldap bug
> https://fedorahosted.org/bind-dyndb-ldap/ticket/123
> Patches attached.
 Functional ACK. It can be pushed if Python gurus don't see any problem.

>>> I think the patches will need a rebase before push, I cannot apply them to 
>>> my
>>> tree. The Python part itself looked good to me.
>>>
>>> Martin
>>>
>> Rebased patch attached, due changes in freeipa-mbasti-0109,
>> patches mbasti-0109.2, mbasti-0110.2 are required.
> 
> Rebased versions work for me. Functional ACK.
> 

Ok, LGTM. Pushed to master, ipa-4-1.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES 0111-0113] Fix NS record coexistence validation

2014-09-04 Thread Petr Spacek

On 3.9.2014 16:51, Martin Basti wrote:

On 03/09/14 12:30, Martin Kosek wrote:

On 09/02/2014 05:38 PM, Petr Spacek wrote:

On 21.8.2014 19:21, Martin Basti wrote:

During work on DNSSEC we found a wrong validation of NS records
Patch 0113 fixes an error in tests caused by bind-dyndb-ldap bug
https://fedorahosted.org/bind-dyndb-ldap/ticket/123
Patches attached.

Functional ACK. It can be pushed if Python gurus don't see any problem.


I think the patches will need a rebase before push, I cannot apply them to my
tree. The Python part itself looked good to me.

Martin


Rebased patch attached, due changes in freeipa-mbasti-0109,
patches mbasti-0109.2, mbasti-0110.2 are required.


Rebased versions work for me. Functional ACK.

--
Petr^2 Spacek

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES 0111-0113] Fix NS record coexistence validation

2014-09-03 Thread Martin Basti

On 03/09/14 12:30, Martin Kosek wrote:

On 09/02/2014 05:38 PM, Petr Spacek wrote:

On 21.8.2014 19:21, Martin Basti wrote:

During work on DNSSEC we found a wrong validation of NS records
Patch 0113 fixes an error in tests caused by bind-dyndb-ldap bug
https://fedorahosted.org/bind-dyndb-ldap/ticket/123
Patches attached.

Functional ACK. It can be pushed if Python gurus don't see any problem.


I think the patches will need a rebase before push, I cannot apply them to my
tree. The Python part itself looked good to me.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Rebased patch attached, due changes in freeipa-mbasti-0109,
patches mbasti-0109.2, mbasti-0110.2 are required.

--
Martin Basti

From be5dd565252ccf089e52a378656f070e889b3eaa Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 21 Aug 2014 18:08:10 +0200
Subject: [PATCH 1/3] DNS fix NS record coexistence validator

NS can coexistent only with A, , DS, NS record
---
 ipalib/plugins/dns.py | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index e4822003fd8ca2b2caa9c4db9b812488b2023bdc..c7039bbf9c4c52a5a12ba6b806d3a0d414144fb8 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2916,11 +2916,23 @@ class dnsrecord(LDAPObject):
 error=_('only one DNAME record is allowed per name '
 '(RFC 6672, section 2.4)'))
 # DNAME must not coexist with CNAME, but this is already checked earlier
-if rrattrs.get('nsrecord') and not keys[1].is_empty():
-raise errors.ValidationError(name='dnamerecord',
-  error=_('DNAME record is not allowed to coexist with an '
-  'NS record except when located in a zone root '
-  'record (RFC 6672, section 2.3)'))
+
+# NS record validation
+# NS record can coexist only with A, , DS, and other NS records (except zone apex)
+# RFC 2181 section 6.1,
+allowed_records = ['', 'A', 'DS', 'NS']
+nsrecords = rrattrs.get('nsrecord')
+if nsrecords and not self.is_pkey_zone_record(*keys):
+for r_type in _record_types:
+if (r_type not in allowed_records
+and rrattrs.get('%srecord' % r_type.lower())
+):
+raise errors.ValidationError(
+name='nsrecord',
+error=_('NS record is not allowed to coexist with an '
+'%(type)s record except when located in a '
+'zone root record (RFC 2181, section 6.1)') %
+{'type': r_type})
 
 def check_record_type_dependencies(self, keys, rrattrs):
 # Test that all record type dependencies are satisfied
@@ -2936,7 +2948,6 @@ class dnsrecord(LDAPObject):
 error=_('DS record requires to coexist with an '
  'NS record (RFC 4592 section 4.6, RFC 4035 section 2.4)'))
 
-
 def _entry2rrsets(self, entry_attrs, dns_name, dns_domain):
 '''Convert entry_attrs to a dictionary {rdtype: rrset}.
 
-- 
1.8.3.1

From 02b3613c6cc42ad741293442f50fc7a9a8425a09 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 21 Aug 2014 18:09:22 +0200
Subject: [PATCH 2/3] Test: DNS NS validation

---
 ipatests/test_xmlrpc/test_dns_plugin.py | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 4b4595580b0058b25bb0552bd4f8f72a3d606d03..1784e0b17e87cffc037184c1ab06d2dfe0454ac1 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -123,6 +123,10 @@ name1_dn = DN(('idnsname',name1), zone1_dn)
 name1_renamed = u'testdnsres-renamed'
 name1_renamed_dnsname = DNSName(name1_renamed)
 
+name_ns = u'testdnsres-ns'
+name_ns_dnsname = DNSName(name_ns)
+name_ns_dn = DN(('idnsname',name_ns), zone1_dn)
+
 revname1 = u'80'
 revname1_dnsname = DNSName(revname1)
 revname1_ip = revzone1_ipprefix + revname1
@@ -1168,9 +1172,10 @@ class test_dns(Declarative):
 desc='Try to add NS record to %r using dnsrecord_add' % (dname),
 command=('dnsrecord_add', [zone1, dname],
 {'nsrecord': u'%s.%s.' % (name1, zone1)}),
-expected=errors.ValidationError(name='dnamerecord',
-error=u'DNAME record is not allowed to coexist with an NS '
-  u'record except when located in a zone root record (RFC 6672, section 2.3)'),
+expected=errors.ValidationError(name='nsrecord',
+error=u'NS record is not allowed to coexist with an DNAME '
+  u'record except

Re: [Freeipa-devel] [PATCHES 0111-0113] Fix NS record coexistence validation

2014-09-03 Thread Martin Kosek
On 09/02/2014 05:38 PM, Petr Spacek wrote:
> On 21.8.2014 19:21, Martin Basti wrote:
>> During work on DNSSEC we found a wrong validation of NS records
>> Patch 0113 fixes an error in tests caused by bind-dyndb-ldap bug
>> https://fedorahosted.org/bind-dyndb-ldap/ticket/123
>> Patches attached.
> 
> Functional ACK. It can be pushed if Python gurus don't see any problem.
> 

I think the patches will need a rebase before push, I cannot apply them to my
tree. The Python part itself looked good to me.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCHES 0111-0113] Fix NS record coexistence validation

2014-09-02 Thread Petr Spacek

On 21.8.2014 19:21, Martin Basti wrote:

During work on DNSSEC we found a wrong validation of NS records
Patch 0113 fixes an error in tests caused by bind-dyndb-ldap bug
https://fedorahosted.org/bind-dyndb-ldap/ticket/123
Patches attached.


Functional ACK. It can be pushed if Python gurus don't see any problem.

--
Petr^2 Spacek

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel