On Tue, 2012-02-21 at 17:27 +0100, Martin Kosek wrote:
> This set of 3 DNS patches fixes 2 minor issues found during DNS test day
> (217, 218) and there is slightly longer patch (219) which improves and
> consolidates hostname/domain name validation.
> 
> The testing should be pretty straightforward in case of these 3 tickets.
> 
> Martin

I forgot to generate API.txt in patch 219. Fixed version is included.

While at it I made another patch (220) to DNS/host plugins built on
217-219. It fixes the way we handle FQDNs with ending "." in both
plugins. It will also fix bug that Marco Pizzoli was experiencing.

Some clean ups in host plugin procedures for finding DNS zone for a host
were needed so that host's IP address is added correctly to DNS zones
with trailing dot.

Martin
>From 5a3bcc08575ae6eca3cc4e759c0dfb7e2fbce1f9 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 21 Feb 2012 10:37:08 +0100
Subject: [PATCH 1/3] Improve dns error message

Improve AttrValueNotFound exception error message raised in the DNS
module when a deleted (modified) attribute value is not found
in a record. In order to be consistent with previous DNS module
implementation this error message should include an attribute
label instead of an attribute name.

https://fedorahosted.org/freeipa/ticket/2377
---
 ipalib/plugins/dns.py |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index fe32efccdf9b0e0d7d8d58dd9896205ef369ec5a..ce03f3074e6374fb133a567abeafb60938915b81 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1761,7 +1761,8 @@ class dnsrecord_mod(LDAPUpdate):
                 old_dnsvalue, new_parts = updated_attrs[attr]
 
                 if old_dnsvalue not in old_entry.get(attr, []):
-                    raise errors.AttrValueNotFound(attr=attr,
+                    attr_name = unicode(param.label or param.name)
+                    raise errors.AttrValueNotFound(attr=attr_name,
                                                    value=old_dnsvalue)
                 old_entry[attr].remove(old_dnsvalue)
 
@@ -1906,7 +1907,12 @@ class dnsrecord_del(LDAPUpdate):
                 try:
                     old_entry[attr].remove(val)
                 except (KeyError, ValueError):
-                    raise errors.AttrValueNotFound(attr=attr,
+                    try:
+                        param = self.params[attr]
+                        attr_name = unicode(param.label or param.name)
+                    except:
+                        attr_name = attr
+                    raise errors.AttrValueNotFound(attr=attr_name,
                                                    value=val)
             entry_attrs[attr] = list(set(old_entry[attr]))
 
-- 
1.7.7.6

>From eed2a61dfdc897732121855228c36725f900b3a4 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 21 Feb 2012 10:51:58 +0100
Subject: [PATCH 2/3] Improve dnsrecord-add interactive mode

When an invalid record type is entered during dnsrecord-add
interactive mode, user is provided with a list of allowed values
(record types). However, the provided list contains also
unsupported record types (APL, DHCID, etc.) and any attempt to add
such records would end with error. This patch limits the list
to supported record types only.

https://fedorahosted.org/freeipa/ticket/2378
---
 ipalib/plugins/dns.py |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index ce03f3074e6374fb133a567abeafb60938915b81..1ceb7c45b4a0e9892af1dc31f848971e9a93fca9 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1092,6 +1092,8 @@ def __dns_record_options_iter():
             yield part
 
 _dns_record_options = tuple(__dns_record_options_iter())
+_dns_supported_record_types = tuple(record.rrtype for record in _dns_records \
+                                    if record.supported)
 
 # dictionary of valid reverse zone -> number of address components
 _valid_reverse_zones = {
@@ -1638,7 +1640,7 @@ class dnsrecord_add(LDAPCreate):
                 if not isinstance(param, DNSRecord):
                     raise ValueError()
             except KeyError, ValueError:
-                all_types = u', '.join(_record_types)
+                all_types = u', '.join(_dns_supported_record_types)
                 self.Backend.textui.print_plain(_(u'Invalid type. Allowed values are: %s') % all_types)
                 continue
             ok = True
-- 
1.7.7.6

>From 1bcac8851c757f11c540eed89a0ee8b46bc3eba7 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 21 Feb 2012 15:28:04 +0100
Subject: [PATCH 3/4] Improve hostname and domain name validation

DNS plugin did not check DNS zone and DNS record validity and
user was thus able to create domains like "foo bar" or other
invalid DNS labels which would really confuse both user and
bind-dyndb-ldap plugin.

This patch at first consolidates hostname/domain name validators
so that they use common functions and we don't have regular
expressions and other checks defined in several places. These
new cleaned validators are then used for zone/record name
validation.

https://fedorahosted.org/freeipa/ticket/2384
---
 API.txt                              |   16 ++++++------
 ipalib/plugins/dns.py                |   39 ++++++++++++++++++++++-------
 ipalib/plugins/host.py               |   19 +++-----------
 ipalib/util.py                       |   44 ++++++++++++++++++++++++++-------
 tests/test_xmlrpc/test_dns_plugin.py |   20 +++++++++++++++
 5 files changed, 95 insertions(+), 43 deletions(-)

diff --git a/API.txt b/API.txt
index 66713317c9b11057f12676c2afc7bc36d0ca3969..6b2babe7c25f8705007e22b802d5d28e5e0c6474 100644
--- a/API.txt
+++ b/API.txt
@@ -1658,7 +1658,7 @@ output: Output('error', (<type 'list'>, <type 'tuple'>, <type 'NoneType'>), None
 output: Output('value', <type 'bool'>, None)
 command: host_add
 args: 1,16,3
-arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, required=True)
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, required=True)
 option: Str('description', attribute=True, cli_name='desc', multivalue=False, required=False)
 option: Str('l', attribute=True, cli_name='locality', multivalue=False, required=False)
 option: Str('nshostlocation', attribute=True, cli_name='location', multivalue=False, required=False)
@@ -1680,7 +1680,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('value', <type 'unicode'>, None)
 command: host_add_managedby
 args: 1,4,3
-arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True)
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('version?', exclude='webui')
@@ -1690,21 +1690,21 @@ output: Output('failed', <type 'dict'>, None)
 output: Output('completed', <type 'int'>, None)
 command: host_del
 args: 1,1,3
-arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True)
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=True, primary_key=True, query=True, required=True)
 option: Flag('updatedns?', autofill=True, default=False)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('result', <type 'dict'>, None)
 output: Output('value', <type 'unicode'>, None)
 command: host_disable
 args: 1,0,3
-arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True)
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True)
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('result', <type 'bool'>, None)
 output: Output('value', <type 'unicode'>, None)
 command: host_find
 args: 1,31,4
 arg: Str('criteria?', noextrawhitespace=False)
-option: Str('fqdn', attribute=True, autofill=False, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=False)
+option: Str('fqdn', attribute=True, autofill=False, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=False)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, query=True, required=False)
 option: Str('l', attribute=True, autofill=False, cli_name='locality', multivalue=False, query=True, required=False)
 option: Str('nshostlocation', attribute=True, autofill=False, cli_name='location', multivalue=False, query=True, required=False)
@@ -1741,7 +1741,7 @@ output: Output('count', <type 'int'>, None)
 output: Output('truncated', <type 'bool'>, None)
 command: host_mod
 args: 1,19,3
-arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True)
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True)
 option: Str('description', attribute=True, autofill=False, cli_name='desc', multivalue=False, required=False)
 option: Str('l', attribute=True, autofill=False, cli_name='locality', multivalue=False, required=False)
 option: Str('nshostlocation', attribute=True, autofill=False, cli_name='location', multivalue=False, required=False)
@@ -1766,7 +1766,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('value', <type 'unicode'>, None)
 command: host_remove_managedby
 args: 1,4,3
-arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True)
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('version?', exclude='webui')
@@ -1776,7 +1776,7 @@ output: Output('failed', <type 'dict'>, None)
 output: Output('completed', <type 'int'>, None)
 command: host_show
 args: 1,5,3
-arg: Str('fqdn', attribute=True, cli_name='hostname', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9][a-zA-Z0-9-\\.]{0,254}$', pattern_errmsg='may only include letters, numbers, and -', primary_key=True, query=True, required=True)
+arg: Str('fqdn', attribute=True, cli_name='hostname', multivalue=False, primary_key=True, query=True, required=True)
 option: Flag('rights', autofill=True, default=False)
 option: Str('out?')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 1ceb7c45b4a0e9892af1dc31f848971e9a93fca9..97b9d2f23eae8dc8ef830232d82ef0bb6b925653 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -28,7 +28,8 @@ from ipalib import Command
 from ipalib.parameters import Flag, Bool, Int, Decimal, Str, StrEnum, Any
 from ipalib.plugins.baseldap import *
 from ipalib import _, ngettext
-from ipalib.util import validate_zonemgr, normalize_zonemgr, validate_hostname
+from ipalib.util import (validate_zonemgr, normalize_zonemgr,
+        validate_hostname, validate_dns_label, validate_domain_name)
 from ipapython import dnsclient
 from ipapython.ipautil import valid_ip
 from ldap import explode_dn
@@ -219,7 +220,7 @@ def _validate_ipnet(ugettext, ipnet):
         return _('invalid IP network format')
     return None
 
-def _domain_name_validator(ugettext, value):
+def _bind_hostname_validator(ugettext, value):
     try:
         # Allow domain name which is not fully qualified. These are supported
         # in bind and then translated as <non-fqdn-name>.<domain>.
@@ -230,6 +231,22 @@ def _domain_name_validator(ugettext, value):
 
     return None
 
+def _dns_record_name_validator(ugettext, value):
+    if value == _dns_zone_record:
+        return
+
+    try:
+        map(lambda label:validate_dns_label(label, allow_underscore=True), \
+            value.split(u'.'))
+    except ValueError, e:
+        return unicode(e)
+
+def _domain_name_validator(ugettext, value):
+    try:
+        validate_domain_name(value)
+    except ValueError, e:
+        return unicode(e)
+
 def _hostname_validator(ugettext, value):
     try:
         validate_hostname(value)
@@ -496,7 +513,7 @@ class AFSDBRecord(DNSRecord):
             maxvalue=65535,
         ),
         Str('hostname',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Hostname'),
         ),
     )
@@ -535,7 +552,7 @@ class CNAMERecord(DNSRecord):
     rfc = 1035
     parts = (
         Str('hostname',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Hostname'),
             doc=_('A hostname which this alias hostname points to'),
         ),
@@ -556,7 +573,7 @@ class DNAMERecord(DNSRecord):
     rfc = 2672
     parts = (
         Str('target',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Target'),
         ),
     )
@@ -635,7 +652,7 @@ class KXRecord(DNSRecord):
             maxvalue=65535,
         ),
         Str('exchanger',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Exchanger'),
             doc=_('A host willing to act as a key exchanger'),
         ),
@@ -776,7 +793,7 @@ class MXRecord(DNSRecord):
             maxvalue=65535,
         ),
         Str('exchanger',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Exchanger'),
             doc=_('A host willing to act as a mail exchanger'),
         ),
@@ -788,7 +805,7 @@ class NSRecord(DNSRecord):
 
     parts = (
         Str('hostname',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Hostname'),
         ),
     )
@@ -802,7 +819,7 @@ class NSECRecord(DNSRecord):
 
     parts = (
         Str('next',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Next Domain Name'),
         ),
         StrEnum('types+',
@@ -900,7 +917,7 @@ def _srv_target_validator(ugettext, value):
     if value == u'.':
         # service not available
         return
-    return _domain_name_validator(ugettext, value)
+    return _bind_hostname_validator(ugettext, value)
 
 class SRVRecord(DNSRecord):
     rrtype = 'SRV'
@@ -1153,6 +1170,7 @@ class dnszone(LDAPObject):
 
     takes_params = (
         Str('idnsname',
+            _domain_name_validator,
             cli_name='name',
             label=_('Zone name'),
             doc=_('Zone name (FQDN)'),
@@ -1434,6 +1452,7 @@ class dnsrecord(LDAPObject):
 
     takes_params = (
         Str('idnsname',
+            _dns_record_name_validator,
             cli_name='name',
             label=_('Record name'),
             doc=_('Record name'),
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 682b81420577d7f4e153867c7435803f841ab86a..775d999f43a43611ebc5ccd7e167ff1b71d9dc43 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -32,7 +32,7 @@ from ipalib.plugins.service import split_principal
 from ipalib.plugins.service import validate_certificate
 from ipalib.plugins.service import set_certificate_attrs
 from ipalib.plugins.dns import dns_container_exists, _record_types
-from ipalib.plugins.dns import add_forward_record
+from ipalib.plugins.dns import add_forward_record, _hostname_validator
 from ipalib import _, ngettext
 from ipalib import x509
 from ipalib.dn import *
@@ -97,14 +97,6 @@ EXAMPLES:
    ipa host-add-managedby --hosts=test2 test
 """)
 
-def validate_host(ugettext, fqdn):
-    """
-    Require at least one dot in the hostname (to support localhost.localdomain)
-    """
-    if fqdn.find('.') == -1:
-        return _('Fully-qualified hostname required')
-    return None
-
 def is_forward_record(zone, str_address):
     addr = netaddr.IPAddress(str_address)
     if addr.version == 4:
@@ -270,10 +262,7 @@ class host(LDAPObject):
     label_singular = _('Host')
 
     takes_params = (
-        Str('fqdn', validate_host,
-            pattern='^[a-zA-Z0-9][a-zA-Z0-9-\.]{0,254}$',
-            pattern_errmsg='may only include letters, numbers, and -',
-            maxlength=255,
+        Str('fqdn', _hostname_validator,
             cli_name='hostname',
             label=_('Host name'),
             primary_key=True,
@@ -555,7 +544,7 @@ class host_del(LDAPDelete):
 
     def pre_callback(self, ldap, dn, *keys, **options):
         # If we aren't given a fqdn, find it
-        if validate_host(None, keys[-1]) is not None:
+        if _hostname_validator(None, keys[-1]) is not None:
             hostentry = api.Command['host_show'](keys[-1])['result']
             fqdn = hostentry['fqdn'][0]
         else:
@@ -929,7 +918,7 @@ class host_disable(LDAPQuery):
         ldap = self.obj.backend
 
         # If we aren't given a fqdn, find it
-        if validate_host(None, keys[-1]) is not None:
+        if _hostname_validator(None, keys[-1]) is not None:
             hostentry = api.Command['host_show'](keys[-1])['result']
             fqdn = hostentry['fqdn'][0]
         else:
diff --git a/ipalib/util.py b/ipalib/util.py
index eb6702dc96a7b6a7c3f5f24ddb8be8d85665564a..412ba1634d497f648f8b88723e4de72787cc71a0 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -213,6 +213,33 @@ def normalize_zonemgr(zonemgr):
 
     return zonemgr
 
+def validate_dns_label(dns_label, allow_underscore=False):
+    label_chars = r'a-z0-9'
+    if allow_underscore:
+        label_chars += "_"
+    label_regex = r'^[%(chars)s]([%(chars)s-]?[%(chars)s])*$' % dict(chars=label_chars)
+    regex = re.compile(label_regex, re.IGNORECASE)
+
+    if len(dns_label) > 63:
+        raise ValueError(_('DNS label cannot be longer that 63 characters'))
+
+    if not regex.match(dns_label):
+        raise ValueError(_('only letters, numbers, and - are allowed. ' \
+                           '- must not be the last name character'))
+
+def validate_domain_name(domain_name):
+    if domain_name.endswith('.'):
+        domain_name = domain_name[:-1]
+
+    domain_name = domain_name.split(".")
+
+    # apply DNS name validator to every name part
+    map(lambda label:validate_dns_label(label), domain_name)
+
+    if not domain_name[-1].isalpha():
+        # see RFC 1123
+        raise ValueError(_('top level domain label must be alphabetic'))
+
 def validate_zonemgr(zonemgr):
     """ See RFC 1033, 1035 """
     regex_domain = re.compile(r'^[a-z0-9]([a-z0-9-]?[a-z0-9])*$', re.IGNORECASE)
@@ -252,8 +279,7 @@ def validate_zonemgr(zonemgr):
             if not regex_local_part.match(local_part):
                 raise ValueError(local_part_errmsg)
 
-    if not all(regex_domain.match(part) for part in domain.split(".")):
-        raise ValueError(_('domain name may only include letters, numbers, and -'))
+    validate_domain_name(domain)
 
 def validate_hostname(hostname, check_fqdn=True):
     """ See RFC 952, 1123
@@ -261,20 +287,18 @@ def validate_hostname(hostname, check_fqdn=True):
     :param hostname Checked value
     :param check_fqdn Check if hostname is fully qualified
     """
-    regex_name = re.compile(r'^[a-z0-9]([a-z0-9-]?[a-z0-9])*$', re.IGNORECASE)
-
     if len(hostname) > 255:
         raise ValueError(_('cannot be longer that 255 characters'))
 
     if hostname.endswith('.'):
         hostname = hostname[:-1]
 
-    if check_fqdn and '.' not in hostname:
-        raise ValueError(_('not fully qualified'))
-
-    if not all(regex_name.match(part) for part in hostname.split(".")):
-        raise ValueError(_('only letters, numbers, and - are allowed. ' \
-                           '- must not be the last name character'))
+    if '.' not in hostname:
+        if check_fqdn:
+            raise ValueError(_('not fully qualified'))
+        validate_dns_label(hostname)
+    else:
+        validate_domain_name(hostname)
 
 def validate_sshpubkey(ugettext, pubkey):
     try:
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index 9d83d924e924c7235df797a056cd7434a950bf1e..71e6ad40e82f0b756f6fa2604cab5d294a5cd7ba 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -90,6 +90,19 @@ class test_dns(Declarative):
 
 
         dict(
+            desc='Try to create zone with invalid name',
+            command=(
+                'dnszone_add', [u'invalid zone'], {
+                    'idnssoamname': dnszone1_mname,
+                    'idnssoarname': dnszone1_rname,
+                    'ip_address' : u'1.2.3.4',
+                }
+            ),
+            expected=errors.ValidationError(name='idnsname', error=''),
+        ),
+
+
+        dict(
             desc='Create zone %r' % dnszone1,
             command=(
                 'dnszone_add', [dnszone1], {
@@ -421,6 +434,13 @@ class test_dns(Declarative):
 
 
         dict(
+            desc='Try to create record with invalid name in zone %r' % dnszone1,
+            command=('dnsrecord_add', [dnszone1, u'invalid record'], {'arecord': u'127.0.0.1'}),
+            expected=errors.ValidationError(name='idnsname', error=''),
+        ),
+
+
+        dict(
             desc='Create record %r in zone %r' % (dnszone1, dnsres1),
             command=('dnsrecord_add', [dnszone1, dnsres1], {'arecord': u'127.0.0.1'}),
             expected={
-- 
1.7.7.6

>From 528c7aa66de38941756953764945142b67db340e Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Wed, 22 Feb 2012 10:43:03 +0100
Subject: [PATCH 4/4] Improve FQDN handling in DNS and host plugins

DNS and host plugin does not work well with domain names ending
with dot. host plugin creates a record with two fqdn attributes
when such hostname is created which then has to be manually fixed.
DNS plugin handled zones with and without trailing dot as two
distinct zones, which may lead to issues when both zones are
created.

This patch sanitizes approach to FQDNs in both DNS and host plugin.
Hostnames are now always normalized to the form without trailing
dot as this form did not work before and it would keep hostname
form consistent without changes in our server/client enrollment
process.

As DNS zones always worked in both forms this patch rather makes
sure that the plugin works with both forms of one zone and prevents
creating 2 identical zones with just different format.

https://fedorahosted.org/freeipa/ticket/2227
---
 ipalib/plugins/dns.py  |   21 +++++++++++++++++-
 ipalib/plugins/host.py |   56 +++++++++++++++++++----------------------------
 2 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 97b9d2f23eae8dc8ef830232d82ef0bb6b925653..4da5ec911c1602a1b7a5c3dfbfd0cc2171cef4f7 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1270,6 +1270,25 @@ class dnszone(LDAPObject):
         ),
     )
 
+    def get_dn(self, *keys, **options):
+        zone = keys[-1]
+        dn = super(dnszone, self).get_dn(zone, **options)
+        try:
+            self.backend.get_entry(dn, [''])
+        except errors.NotFound:
+            if zone.endswith(u'.'):
+                zone = zone[:-1]
+            else:
+                zone = zone + u'.'
+            test_dn = super(dnszone, self).get_dn(zone, **options)
+
+            try:
+                (dn, entry_attrs) = self.backend.get_entry(test_dn, [''])
+            except errors.NotFound:
+                pass
+
+        return dn
+
 api.register(dnszone)
 
 
@@ -1293,7 +1312,7 @@ class dnszone_add(LDAPCreate):
             self.obj.params['name_from_ip'](unicode(options['name_from_ip']))
         return super(dnszone_add, self).args_options_2_params(*args, **options)
 
-    def pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
+    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         if not dns_container_exists(self.api.Backend.ldap2):
             raise errors.NotFound(reason=_('DNS is not configured'))
 
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 775d999f43a43611ebc5ccd7e167ff1b71d9dc43..4439dfc69acdef90f41297ca743b0cda0c7417bd 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -219,6 +219,12 @@ def validate_ipaddr(ugettext, ipaddr):
         return unicode(e)
     return None
 
+def normalize_hostname(hostname):
+    """Use common fqdn form without the trailing dot"""
+    if hostname.endswith(u'.'):
+        hostname = hostname[:-1]
+    hostname = hostname.lower()
+    return hostname
 
 class host(LDAPObject):
     """
@@ -266,7 +272,7 @@ class host(LDAPObject):
             cli_name='hostname',
             label=_('Host name'),
             primary_key=True,
-            normalizer=lambda value: value.lower(),
+            normalizer=normalize_hostname,
         ),
         Str('description?',
             cli_name='desc',
@@ -337,8 +343,6 @@ class host(LDAPObject):
 
     def get_dn(self, *keys, **options):
         hostname = keys[-1]
-        if hostname.endswith('.'):
-            hostname = hostname[:-1]
         dn = super(host, self).get_dn(hostname, **options)
         try:
             self.backend.get_entry(dn, [''])
@@ -413,16 +417,12 @@ class host_add(LDAPCreate):
         if 'ip_address' in options and dns_container_exists(ldap):
             parts = keys[-1].split('.')
             domain = unicode('.'.join(parts[1:]))
-            result = api.Command['dnszone_find']()['result']
-            match = False
-            for zone in result:
-                if domain == zone['idnsname'][0]:
-                    match = True
-                    break
-            if not match:
-                raise errors.NotFound(
-                    reason=_('DNS zone %(zone)s not found') % dict(zone=domain)
-                )
+            try:
+                result = api.Command['dnszone_show'](domain)['result']
+                domain = result['idnsname'][0]
+            except errors.NotFound:
+                self.obj.handle_not_found(*keys)
+
             ip = CheckedIPAddress(options['ip_address'], match_local=False)
             if not options.get('no_reverse', False):
                 try:
@@ -575,16 +575,11 @@ class host_del(LDAPDelete):
             # Remove DNS entries
             parts = fqdn.split('.')
             domain = unicode('.'.join(parts[1:]))
-            result = api.Command['dnszone_find']()['result']
-            match = False
-            for zone in result:
-                if domain == zone['idnsname'][0]:
-                    match = True
-                    break
-            if not match:
-                raise errors.NotFound(
-                    reason=_('DNS zone %(zone)s not found') % dict(zone=domain)
-                )
+            try:
+                result = api.Command['dnszone_show'](domain)['result']
+                domain = result['idnsname'][0]
+            except errors.NotFound:
+                self.obj.handle_not_found(*keys)
             # Get all forward resources for this host
             records = api.Command['dnsrecord_find'](domain, idnsname=parts[0])['result']
             for record in records:
@@ -735,16 +730,11 @@ class host_mod(LDAPUpdate):
         if options.get('updatedns', False) and dns_container_exists(ldap):
             parts = keys[-1].split('.')
             domain = unicode('.'.join(parts[1:]))
-            result = api.Command['dnszone_find']()['result']
-            match = False
-            for zone in result:
-                if domain == zone['idnsname'][0]:
-                    match = True
-                    break
-            if not match:
-                raise errors.NotFound(
-                    reason=_('DNS zone %(zone)s not found') % dict(zone=domain)
-                )
+            try:
+                result = api.Command['dnszone_show'](domain)['result']
+                domain = result['idnsname'][0]
+            except errors.NotFound:
+                self.obj.handle_not_found(*keys)
             update_sshfp_record(domain, unicode(parts[0]), entry_attrs)
 
         if 'ipasshpubkey' in entry_attrs:
-- 
1.7.7.6

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

Reply via email to