Sending the patch with renamed function.
Standa
On 12/01/2015 09:57 AM, Jan Cholasta wrote:
On 1.12.2015 09:37, Petr Spacek wrote:
On 30.11.2015 20:00, Martin Basti wrote:
On 27.11.2015 16:06, Stanislav Laznicka wrote:
Please, see the modified patch attached.
Standa
On 11/27/2015 03:48 PM, Martin Basti wrote:
On 27.11.2015 15:33, Petr Spacek wrote:
On 27.11.2015 15:32, Martin Basti wrote:
On 25.11.2015 17:18, Stanislav Laznicka wrote:
There were two functions for the same purpose. Removed one.
Hello,
I would like to have "log" param of is_host_resolvable as optional
Is there an immediate need for the optional param? If not, I
would not
clutter
the code.
So at least I would like to move log param as the last param, in
case of
need it can be modified to optional parameter.
Or log can be default as root_logger, IMO we us only root_logger
everywhere, but this need investigation.
Martin
It works, but I would like to have Honza's opinion if ipalib/util.py
is the
right place for the new method and if we really need to use exceptions.
ipalib/util.py is fine for the function, since it is not used anywhere
in ipapython.
I would rename the function to verify_host_resolvable() though, since
the is_ prefix suggests the function returns a boolean value rather
than raises an exception or not.
IMO is_record_resolvable() should return only True/False and then it
can be
located in ipapython module
Hmm, I do not see a reason why it should not throw an exception. IMHO
the
exception should contain the original error/result from resolver, so
it is
possible to print meaningful error message like "DNS query timed out"
instead
of "it does not work for some reason, trust us".
From d259e555bf096ff35397de80d273b97d829a45ea Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Wed, 25 Nov 2015 16:38:00 +0100
Subject: [PATCH] Removed duplicate domain name validating function
---
ipa-client/ipa-install/ipa-client-install | 9 +++++---
ipalib/plugins/dns.py | 22 ++++++++++++-------
ipalib/plugins/host.py | 2 +-
ipalib/plugins/service.py | 2 +-
ipalib/util.py | 35 +++++++++++++++----------------
ipapython/ipautil.py | 12 -----------
6 files changed, 39 insertions(+), 43 deletions(-)
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 05a550b11e74db84e46a126798c4db728226865c..974dd1da8bf3f5836170ca67d2f4c298e7ec6844 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -54,6 +54,7 @@ try:
from ipapython.config import IPAOptionParser
from ipalib import api, errors
from ipalib import x509, certstore
+ from ipalib.util import verify_host_resolvable
from ipalib.constants import CACERT
from ipapython.dn import DN
from ipapython.ssh import SSHPublicKey
@@ -1761,11 +1762,13 @@ def get_server_connection_interface(server):
def client_dns(server, hostname, options):
- dns_ok = ipautil.is_host_resolvable(hostname)
-
- if not dns_ok:
+ try:
+ verify_host_resolvable(hostname, root_logger)
+ dns_ok = True
+ except errors.DNSNotARecordError:
root_logger.warning("Hostname (%s) does not have A/AAAA record.",
hostname)
+ dns_ok = False
if (options.dns_updates or options.all_ip_addresses or options.ip_addresses
or not dns_ok):
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 830a70fa5aab7eef5f56f14050921eb2dc160449..67947360eb207de31ed114bb630705c409b2f9a9 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -51,9 +51,10 @@ from ipalib.util import (normalize_zonemgr,
DNSSECSignatureMissingError, UnresolvableRecordError,
EDNS0UnsupportedError, DNSSECValidationError,
validate_dnssec_zone_forwarder_step1,
- validate_dnssec_zone_forwarder_step2)
+ validate_dnssec_zone_forwarder_step2,
+ verify_host_resolvable)
-from ipapython.ipautil import CheckedIPAddress, is_host_resolvable
+from ipapython.ipautil import CheckedIPAddress
from ipapython.dnsutil import DNSName
if six.PY3:
@@ -1554,7 +1555,7 @@ _dns_record_options = tuple(__dns_record_options_iter())
_dns_supported_record_types = tuple(record.rrtype for record in _dns_records \
if record.supported)
-def check_ns_rec_resolvable(zone, name):
+def check_ns_rec_resolvable(zone, name, log):
assert isinstance(zone, DNSName)
assert isinstance(name, DNSName)
@@ -1563,7 +1564,9 @@ def check_ns_rec_resolvable(zone, name):
elif not name.is_absolute():
# this is a DNS name relative to the zone
name = name.derelativize(zone.make_absolute())
- if not is_host_resolvable(name):
+ try:
+ verify_host_resolvable(name, log)
+ except errors.DNSNotARecordError:
raise errors.NotFound(
reason=_('Nameserver \'%(host)s\' does not have a corresponding '
'A/AAAA record') % {'host': name}
@@ -2734,7 +2737,8 @@ class dnszone_add(DNSZoneBase_add):
# verify if user specified server is resolvable
if not options['force']:
- check_ns_rec_resolvable(keys[0], entry_attrs['idnssoamname'])
+ check_ns_rec_resolvable(keys[0], entry_attrs['idnssoamname'],
+ self.log)
# show warning about --name-server option
context.show_warning_nameserver_option = True
else:
@@ -2833,7 +2837,7 @@ class dnszone_mod(DNSZoneBase_mod):
nameserver = entry_attrs['idnssoamname']
if nameserver:
if not nameserver.is_empty() and not options['force']:
- check_ns_rec_resolvable(keys[0], nameserver)
+ check_ns_rec_resolvable(keys[0], nameserver, self.log)
context.show_warning_nameserver_option = True
else:
# empty value, this option is required by ldap
@@ -3004,7 +3008,7 @@ class dnsrecord(LDAPObject):
if options.get('force', False) or nsrecords is None:
return
for nsrecord in nsrecords:
- check_ns_rec_resolvable(keys[0], DNSName(nsrecord))
+ check_ns_rec_resolvable(keys[0], DNSName(nsrecord), self.log)
def _idnsname_pre_callback(self, ldap, dn, entry_attrs, *keys, **options):
assert isinstance(dn, DN)
@@ -4196,7 +4200,9 @@ class dns_resolve(Command):
def execute(self, *args, **options):
query=args[0]
- if not is_host_resolvable(query):
+ try:
+ verify_host_resolvable(query, self.log)
+ except errors.DNSNotARecordError:
raise errors.NotFound(
reason=_('Host \'%(host)s\' not found') % {'host': query}
)
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index bceab314b8acb496e885a889311e026cabfb2a47..fa867f3703210cf3008c0e67381883453ce99ce4 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -625,7 +625,7 @@ class host_add(LDAPCreate):
check_forward=True,
check_reverse=check_reverse)
if not options.get('force', False) and not 'ip_address' in options:
- util.validate_host_dns(self.log, keys[-1])
+ util.verify_host_resolvable(keys[-1], self.log)
if 'locality' in entry_attrs:
entry_attrs['l'] = entry_attrs['locality']
entry_attrs['cn'] = keys[-1]
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index d63e00bea0dd9a69fd550916337a8cc1a88a93fb..4752e198b169e49a582ba45b433fcb99d98fae46 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -554,7 +554,7 @@ class service_add(LDAPCreate):
# We know the host exists if we've gotten this far but we
# really want to discourage creating services for hosts that
# don't exist in DNS.
- util.validate_host_dns(self.log, hostname)
+ util.verify_host_resolvable(hostname, self.log)
if not 'managedby' in entry_attrs:
entry_attrs['managedby'] = hostresult['dn']
diff --git a/ipalib/util.py b/ipalib/util.py
index 89d67e67afb2edd5ff75ab369430892063564ddc..c9a0237fb020b3288e91ef7af60496f4870853ae 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -66,32 +66,31 @@ def json_serialize(obj):
return ''
return json_serialize(obj.__json__())
-def validate_host_dns(log, fqdn):
+def verify_host_resolvable(fqdn, log):
"""
See if the hostname has a DNS A/AAAA record.
"""
- try:
- answers = resolver.query(fqdn, rdatatype.A)
- log.debug(
- 'IPA: found %d A records for %s: %s' % (len(answers), fqdn,
- ' '.join(str(answer) for answer in answers))
- )
- except DNSException as e:
- log.debug(
- 'IPA: DNS A record lookup failed for %s' % fqdn
- )
- # A record not found, try to find AAAA record
+ if not isinstance(fqdn, DNSName):
+ fqdn = DNSName(fqdn)
+
+ fqdn = fqdn.make_absolute()
+ for rdtype in ('A', 'AAAA'):
try:
- answers = resolver.query(fqdn, rdatatype.AAAA)
+ answers = resolver.query(fqdn, rdtype)
log.debug(
- 'IPA: found %d AAAA records for %s: %s' % (len(answers), fqdn,
- ' '.join(str(answer) for answer in answers))
+ 'IPA: found %d %s records for %s: %s' % (len(answers),
+ rdtype, fqdn, ' '.join(str(answer) for answer in answers))
)
- except DNSException as e:
+ except DNSException:
log.debug(
- 'IPA: DNS AAAA record lookup failed for %s' % fqdn
+ 'IPA: DNS %s record lookup failed for %s' %
+ (rdtype, fqdn)
)
- raise errors.DNSNotARecordError()
+ continue
+ else:
+ return
+ # dns lookup failed in both tries
+ raise errors.DNSNotARecordError()
def has_soa_or_ns_record(domain):
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4551ea5c4025223dcff5cdc8998fedeccd14c3c2..104f9d180d9589556992363d090d22f15549bf9f 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -911,18 +911,6 @@ def bind_port_responder(port, socket_type=socket.SOCK_STREAM, socket_timeout=Non
if s is None and last_socket_error is not None:
raise last_socket_error # pylint: disable=E0702
-def is_host_resolvable(fqdn):
- if not isinstance(fqdn, DNSName):
- fqdn = DNSName(fqdn)
- for rdtype in (rdatatype.A, rdatatype.AAAA):
- try:
- resolver.query(fqdn.make_absolute(), rdtype)
- except DNSException:
- continue
- else:
- return True
-
- return False
def host_exists(host):
"""
--
2.5.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
