On 15/01/15 17:13, David Kupka wrote:
On 01/15/2015 03:22 PM, David Kupka wrote:
On 01/15/2015 12:43 PM, David Kupka wrote:
On 01/12/2015 06:34 PM, Martin Basti wrote:
On 09/01/15 14:43, David Kupka wrote:
On 01/07/2015 04:15 PM, Martin Basti wrote:
On 07/01/15 12:27, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4249

Thank you for patch:

1)
-        root_logger.error("Cannot update DNS records! "
-                          "Failed to connect to server '%s'.",
server)
+        ips = get_local_ipaddresses()
+    except CalledProcessError as e:
+        root_logger.error("Cannot update DNS records. %s" % e)

IMO the error message should be more specific,  add there something
like
"Unable to get local IP addresses". at least in log.debug()

2)
+    lines = ipresult[0].replace('\\', '').split('\n')

.replace() is not needed

3)
+    if len(ips) == 0:

if not ips:

is more pythonic by PEP8


Thanks for catching these. Updated patch attached.

merciful NACK

Thank you for the patch, unfortunately I hit one issue which needs
to be
resolved.

If "sync PTR" is activated in zone settings, and reverse zone doesn't
exists, nsupdate/BIND returns SERVFAIL and ipa-client-install print
Error message, 'DNS update failed'. In fact, all A/AAAA records was
succesfully updated, only PTR records failed.

Bind log:
named-pkcs11[28652]: updating zone 'example.com/IN': adding an RR at
'vm-101.example.com' AAAA

named-pkcs11[28652]: PTR record synchronization (addition) for A/AAAA
'vm-101.example.com.' refused: unable to find active reverse zone
for IP
address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found

With IPv6 we have several addresses from different reverse zones and
this situation may happen often.
I suggest following:
1) Print list of addresses which will be updated. (Now if update fails,
user needs to read log, which addresses installer tried to update)
2) Split nsupdates per A/AAAA record.
3a) If failed, check with DNS query if A/AAAA and PTR record are there
and print proper error message
3b) Just print A/AAAA (or PTR) record may not be updated for particular
IP address.

Any other suggestions are welcome.


After long discussion with DNS and UX guru I've implemented it this way:
1. Call nsupdate only once with all updates.
2. Verify that the expected records are resolvable.
3. If no print list of missing A/AAAA, list of missing PTR records and
list to mismatched PTR record.

As this is running inside client we can't much more and it's up to user
to check what's rotten in his DNS setup.

Updated patch attached.


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



One more change to behave well in -crazy- exotic environments that
resolves more PTR records for single IP.



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


Yet another change to make language nerds and our UX guru happy :-)


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


Rebased patch attached.
--
David Kupka
From 3ae6959cfd08c34cfcb0eaf29d057b5ea4ebbac4 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Sun, 4 Jan 2015 15:04:18 -0500
Subject: [PATCH] client: Update DNS with all available local IP addresses.

Detect all usable IP addresses assigned to any interface and create
coresponding DNS records on server.

https://fedorahosted.org/freeipa/ticket/4249
---
 ipa-client/ipa-install/ipa-client-install | 173 +++++++++++++++++++-----------
 1 file changed, 112 insertions(+), 61 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 91323ae115a27d221bcbc43fee887c56d99c8635..eab20e6c44954834b736d3477db88c7708912002 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -32,6 +32,7 @@ try:
     from optparse import SUPPRESS_HELP, OptionGroup, OptionValueError
     import shutil
     from krbV import Krb5Error
+    import dns
 
     import nss.nss as nss
     import SSSDConfig
@@ -1500,40 +1501,22 @@ def unconfigure_nisdomain():
     if not enabled:
         services.knownservices.domainname.disable()
 
-
-def resolve_ipaddress(server):
-    """ Connect to the server's LDAP port in order to determine what ip
-        address this machine uses as "public" ip (relative to the server).
-
-        Returns a tuple with the IP address and address family when
-        connection was successful. Socket error is raised otherwise.
-    """
-    last_socket_error = None
-
-    for res in socket.getaddrinfo(server, 389, socket.AF_UNSPEC,
-            socket.SOCK_STREAM):
-        af, socktype, proto, canonname, sa = res
-        try:
-            s = socket.socket(af, socktype, proto)
-        except socket.error, e:
-            last_socket_error = e
-            s = None
+def get_local_ipaddresses():
+    ipresult = ipautil.run([paths.IP, '-oneline', 'address', 'show'])
+    lines = ipresult[0].split('\n')
+    ips = []
+    for line in lines:
+        fields = line.split()
+        if len(fields) < 6:
+            continue
+        if fields[2] not in ['inet', 'inet6']:
             continue
-
+        (ip, mask) = fields[3].rsplit('/', 1)
         try:
-            s.connect(sa)
-            sockname = s.getsockname()
-
-            # For both IPv4 and IPv6 own IP address is always the first item
-            return (sockname[0], af)
-        except socket.error, e:
-            last_socket_error = e
-        finally:
-            if s:
-                s.close()
-
-    if last_socket_error is not None:
-        raise last_socket_error  # pylint: disable=E0702
+            ips.append(ipautil.CheckedIPAddress(ip))
+        except ValueError:
+            continue
+    return ips
 
 def do_nsupdate(update_txt):
     root_logger.debug("Writing nsupdate commands to %s:", UPDATE_FILE)
@@ -1558,21 +1541,24 @@ def do_nsupdate(update_txt):
 
     return result
 
-UPDATE_TEMPLATE_A = """
-debug
+DELETE_TEMPLATE_A = """
 update delete $HOSTNAME. IN A
 show
 send
-update add $HOSTNAME. $TTL IN A $IPADDRESS
-show
-send
 """
 
-UPDATE_TEMPLATE_AAAA = """
-debug
+DELETE_TEMPLATE_AAAA = """
 update delete $HOSTNAME. IN AAAA
 show
 send
+"""
+ADD_TEMPLATE_A = """
+update add $HOSTNAME. $TTL IN A $IPADDRESS
+show
+send
+"""
+
+ADD_TEMPLATE_AAAA = """
 update add $HOSTNAME. $TTL IN AAAA $IPADDRESS
 show
 send
@@ -1584,40 +1570,105 @@ CCACHE_FILE = paths.IPA_DNS_CCACHE
 def update_dns(server, hostname):
 
     try:
-        (ip, af) = resolve_ipaddress(server)
-    except socket.gaierror, e:
-        root_logger.debug("update_dns: could not connect to server: %s", e)
-        root_logger.error("Cannot update DNS records! "
-                          "Failed to connect to server '%s'.", server)
+        ips = get_local_ipaddresses()
+    except CalledProcessError as e:
+        root_logger.error("Cannot update DNS records. %s" % e)
+        root_logger.debug("Unable to get local IP addresses.")
         return
 
-    sub_dict = dict(HOSTNAME=hostname,
-                    IPADDRESS=ip,
-                    TTL=1200
-                )
-
-    if af == socket.AF_INET:
-        template = UPDATE_TEMPLATE_A
-    elif af == socket.AF_INET6:
-        template = UPDATE_TEMPLATE_AAAA
-    else:
-        root_logger.info("Failed to determine this machine's ip address.")
-        root_logger.warning("Failed to update DNS A record.")
+    if not ips:
+        root_logger.info("Failed to determine this machine's ip address(es).")
         return
 
-    update_txt = ipautil.template_str(template, sub_dict)
+    update_txt = "debug\n"
+    update_txt += ipautil.template_str(DELETE_TEMPLATE_A, dict(HOSTNAME=hostname))
+    update_txt += ipautil.template_str(DELETE_TEMPLATE_AAAA, dict(HOSTNAME=hostname))
+
+    for ip in ips:
+        sub_dict = dict(HOSTNAME=hostname,
+                        IPADDRESS=ip,
+                        TTL=1200
+                   )
+        if ip.version == 4:
+            template = ADD_TEMPLATE_A
+        elif ip.version == 6:
+            template = ADD_TEMPLATE_AAAA
+        update_txt += ipautil.template_str(template, sub_dict)
 
-    if do_nsupdate(update_txt):
-        root_logger.info("DNS server record set to: %s -> %s", hostname, ip)
-    else:
+    if not do_nsupdate(update_txt):
         root_logger.error("Failed to update DNS records.")
+    verify_dns_update(hostname, ips)
+
+def verify_dns_update(fqdn, ips):
+    """
+    Verify that the fqdn resolves to all IP addresses and
+    that there's matching PTR record for every IP address.
+    """
+    # verify A/AAAA records
+    missing_ips = [str(ip) for ip in ips]
+    extra_ips = []
+    for record_type in [dns.rdatatype.A, dns.rdatatype.AAAA]:
+        root_logger.debug('DNS resolver: Query: %s IN %s' %
+            (fqdn, dns.rdatatype.to_text(record_type)))
+        try:
+            answers = dns.resolver.query(fqdn, record_type)
+        except (dns.resolver.NoAnswer, dns.resolver.NXDOMAIN):
+            root_logger.debug('DNS resolver: No record.')
+        except dns.resolver.NoNameservers:
+            root_logger.debug('DNS resolver: No nameservers answered the query.')
+        except dns.exception.DNSException:
+            root_logger.debug('DNS resolver error.')
+        else:
+            for rdata in answers:
+                try:
+                    missing_ips.remove(rdata.address)
+                except ValueError:
+                    extra_ips.append(rdata.address)
+
+    # verify PTR records
+    fqdn_name = dns.name.from_text(fqdn)
+    wrong_reverse = {}
+    missing_reverse = [str(ip) for ip in ips]
+    for ip in ips:
+        ip_str = str(ip)
+        addr = dns.reversename.from_address(ip_str)
+        root_logger.debug('DNS resolver: Query: %s IN PTR' % addr)
+        try:
+            answers = dns.resolver.query(addr, dns.rdatatype.PTR)
+        except (dns.resolver.NoAnswer, dns.resolver.NXDOMAIN):
+            root_logger.debug('DNS resolver: No record.')
+        except dns.resolver.NoNameservers:
+            root_logger.debug('DNS resolver: No nameservers answered the query.')
+        except dns.exception.DNSException:
+            root_logger.debug('DNS resolver error.')
+        else:
+            missing_reverse.remove(ip_str)
+            for rdata in answers:
+                if not rdata.target == fqdn_name:
+                    wrong_reverse.setdefault(ip_str, []).append(rdata.target)
+
+    if missing_ips:
+        root_logger.warning('Missing A/AAAA record(s) for host %s: %s.' %
+            (fqdn, ', '.join(missing_ips)))
+    if extra_ips:
+        root_logger.warning('Extra A/AAAA record(s) for host %s: %s.' %
+            (fqdn, ', '.join(extra_ips)))
+    if missing_reverse:
+        root_logger.warning('Missing reverse record(s) for address(es): %s.' %
+            ', '.join(missing_reverse))
+    if wrong_reverse:
+        root_logger.warning('Incorrect reverse record(s):')
+        for ip in wrong_reverse:
+            for target in wrong_reverse[ip]:
+                root_logger.warning('%s is pointing to %s instead of %s' %
+                    (ip, target, fqdn_name))
 
 def client_dns(server, hostname, dns_updates=False):
 
     dns_ok = ipautil.is_host_resolvable(hostname)
 
     if not dns_ok:
-        root_logger.warning("Hostname (%s) not found in DNS", hostname)
+        root_logger.warning("Hostname (%s) does not have A/AAAA record.", hostname)
 
     if dns_updates or not dns_ok:
         update_dns(server, hostname)
-- 
2.4.3

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

Reply via email to