On Mon, 2012-02-27 at 15:15 -0500, Rob Crittenden wrote:
> Martin Kosek wrote:
> > 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
> 
> A bit of rebasing is required on most of these.

Thanks. I rebased all of those and fixed reported errors and also a
misformatted except clause.

> 217 ACK

Pushed to master, ipa-2-2.

> 
> 218. NACK. The invalid values are still accepted and you get the same 
> error message "No options to add...". This is different than entering an 
> invalid type like XYZ. I think we should instead either not allow entry 
> at all or recognize that it is unsupported and say so.

Good catch. I just fixed the error message, not the actual check. Fixed.

> 
> 219. NACK. The error message doesn't include _ in the list of allowed 
> characters but it accepts it (it probably shouldn't). IPA hostnames do not.

Error message fixed. _ need to be allowed for DNS record names, they are
used for example for SRV records. They are just not allowed for
hostnames.

> 
> 220 NACK. The ticket number is wrong and the patch doesn't apply.
> 
> rob

Fixed.

Martin
>From dee38b829fda0f379fba4f032a9448fc919c9c22 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 28 Feb 2012 08:48:08 +0100
Subject: [PATCH 1/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 |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 1a75f068ed80ac5737772c46e0648d469673629c..ed3a6832e83f4e4fb315bd0fc887a45662cf86e5 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1376,6 +1376,8 @@ def __dns_record_options_iter():
             yield extra
 
 _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 = {
@@ -1938,9 +1940,12 @@ class dnsrecord_add(LDAPCreate):
 
                 if not isinstance(param, DNSRecord):
                     raise ValueError()
-            except KeyError, ValueError:
-                all_types = u', '.join(_record_types)
-                self.Backend.textui.print_plain(_(u'Invalid type. Allowed values are: %s') % all_types)
+
+                if not param.supported:
+                    raise ValueError()
+            except (KeyError, ValueError):
+                all_types = u', '.join(_dns_supported_record_types)
+                self.Backend.textui.print_plain(_(u'Invalid or unsupported type. Allowed values are: %s') % all_types)
                 continue
             ok = True
 
@@ -1964,7 +1969,7 @@ class dnsrecord_add(LDAPCreate):
                 # check if any record part was added
                 try:
                     rrparam = self.params[param.hint]
-                except KeyError, AttributeError:
+                except (KeyError, AttributeError):
                     continue
 
                 if rrparam.name in entry_attrs:
-- 
1.7.7.6

>From 8bd0d7941a90f858cfbe93338d9460724111e93f Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 28 Feb 2012 09:05:01 +0100
Subject: [PATCH 2/3] 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               |   21 ++++-----------
 ipalib/util.py                       |   47 ++++++++++++++++++++++++++-------
 tests/test_xmlrpc/test_dns_plugin.py |   20 ++++++++++++++
 5 files changed, 100 insertions(+), 43 deletions(-)

diff --git a/API.txt b/API.txt
index 548fc93d48128aab5cebd60dda7fd304b569785b..1b1104d33e2493cb1d562defb9f6cf2e5a933ca9 100644
--- a/API.txt
+++ b/API.txt
@@ -1626,7 +1626,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)
@@ -1648,7 +1648,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')
@@ -1658,21 +1658,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)
@@ -1709,7 +1709,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)
@@ -1734,7 +1734,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')
@@ -1744,7 +1744,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 ed3a6832e83f4e4fb315bd0fc887a45662cf86e5..ee757d87346c52a7c4ff9a3e764e23dc9eb9628f 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, CheckedIPAddress
 from ldap import explode_dn
@@ -299,7 +300,7 @@ def _normalize_bind_aci(bind_acis):
     acis += u';'
     return acis
 
-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>.
@@ -310,6 +311,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)
@@ -777,7 +794,7 @@ class AFSDBRecord(DNSRecord):
             maxvalue=65535,
         ),
         Str('hostname',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Hostname'),
         ),
     )
@@ -816,7 +833,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'),
         ),
@@ -837,7 +854,7 @@ class DNAMERecord(DNSRecord):
     rfc = 2672
     parts = (
         Str('target',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Target'),
         ),
     )
@@ -916,7 +933,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'),
         ),
@@ -1057,7 +1074,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'),
         ),
@@ -1069,7 +1086,7 @@ class NSRecord(DNSRecord):
 
     parts = (
         Str('hostname',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Hostname'),
         ),
     )
@@ -1083,7 +1100,7 @@ class NSECRecord(DNSRecord):
 
     parts = (
         Str('next',
-            _domain_name_validator,
+            _bind_hostname_validator,
             label=_('Next Domain Name'),
         ),
         StrEnum('types+',
@@ -1181,7 +1198,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'
@@ -1426,6 +1443,7 @@ class dnszone(LDAPObject):
 
     takes_params = (
         Str('idnsname',
+            _domain_name_validator,
             cli_name='name',
             label=_('Zone name'),
             doc=_('Zone name (FQDN)'),
@@ -1742,6 +1760,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 df9ad7370925a3c716383ca9da27baf7368b1893..0ff5237fa8ce753df8190bbd1ddb9fb98b2974dd 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -31,7 +31,9 @@ from ipalib.plugins.baseldap import *
 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, add_records_for_host_validation, add_records_for_host
+from ipalib.plugins.dns import (dns_container_exists, _record_types,
+        add_records_for_host_validation, add_records_for_host,
+        _hostname_validator, get_reverse_zone)
 from ipalib.plugins.dns import get_reverse_zone
 from ipalib import _, ngettext
 from ipalib import x509
@@ -97,14 +99,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 remove_fwd_ptr(ipaddr, host, domain, recordtype):
     api.log.debug('deleting ipaddr %s' % ipaddr)
     try:
@@ -225,10 +219,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,
@@ -481,7 +472,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:
@@ -856,7 +847,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 395bf0cf01a0ec35d3d6c7cab0a629038307aac1..bbc0fa6743b1c7322900b2b115a9d022065f8d4d 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -213,6 +213,36 @@ def normalize_zonemgr(zonemgr):
 
     return zonemgr
 
+def validate_dns_label(dns_label, allow_underscore=False):
+    label_chars = r'a-z0-9'
+    underscore_err_msg = ''
+    if allow_underscore:
+        label_chars += "_"
+        underscore_err_msg = u' _,'
+    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,%(underscore)s and - are allowed. ' \
+                           '- must not be the DNS label character') \
+                           % dict(underscore=underscore_err_msg))
+
+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 +282,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 +290,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 7b1a45321801be20ae90b71ba9bbbf0c4bce03c5..e3958d23f5b656b9c7a4a87fb23d5fa1051daafc 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -93,6 +93,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], {
@@ -444,6 +457,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 f258a6c8e3f75af621e553b56d4c501d56b7352a Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Tue, 28 Feb 2012 09:13:13 +0100
Subject: [PATCH 3/3] 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/2420
---
 ipalib/plugins/dns.py  |   31 +++++++++++++++++++++++--------
 ipalib/plugins/host.py |   40 +++++++++++++++++-----------------------
 2 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index ee757d87346c52a7c4ff9a3e764e23dc9eb9628f..07d44dde76d513881eeb5762626af229549f8e52 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -401,13 +401,9 @@ def get_reverse_zone(ipaddr, prefixlen=None):
     return revzone, revname
 
 def add_records_for_host_validation(option_name, host, domain, ip_addresses, check_forward=True, check_reverse=True):
-    result = api.Command['dnszone_find']()['result']
-    match = False
-    for zone in result:
-        if domain == zone['idnsname'][0]:
-            match = True
-            break
-    if not match:
+    try:
+        api.Command['dnszone_show'](domain)['result']
+    except errors.NotFound:
         raise errors.NotFound(
             reason=_('DNS zone %(zone)s not found') % dict(zone=domain)
         )
@@ -1578,6 +1574,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)
 
 
@@ -1601,7 +1616,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 0ff5237fa8ce753df8190bbd1ddb9fb98b2974dd..3814215cb62bc2376c817b3163e8acf07f5a294b 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -176,6 +176,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):
     """
@@ -223,7 +229,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',
@@ -294,8 +300,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, [''])
@@ -504,16 +508,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:
@@ -664,16 +663,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