[Freeipa-devel] [PATCH] 260 Replace DNS client based on acutil with python-dns

[Martin Kosek]

On Wed, 2012-05-23 at 14:24 +0200, Martin Kosek wrote:
> On Tue, 2012-05-22 at 14:41 +0200, Petr Viktorin wrote:
> > On 05/16/2012 09:44 AM, Martin Kosek wrote:
> > > On Tue, 2012-05-15 at 14:02 +0200, Petr Viktorin wrote:
> > >> >  On 05/11/2012 06:52 PM, Martin Kosek wrote:
> > >>> >  >  python-dns is very feature-rich and it can help us a lot with our DNS
> > >>> >  >  related code. This patch does the first step, i.e. replaces acutil use
> > >>> >  >  with python-dns, which is more convenient to use as you will see in the
> > >>> >  >  patch. More integration will follow in the future.
> > >>> >  >
> > >>> >  >  I send this patch rather early, so that I can get responses to this
> > >>> >  >  patch early and also so that we are able to catch issues in a safe
> > >>> >  >  distance from the next release.
> > >> >
> > >>> >  >  ---
> > >>> >  >  IPA client and server tool set used authconfig acutil module to
> > >>> >  >  for client DNS operations. This is not optimal DNS interface for
> > >>> >  >  several reasons:
> > >>> >  >  - does not provide native Python object oriented interface
> > >>> >  >      but but rather C-like interface based on functions and
> > >>> >  >      structures which is not easy to use and extend
> > >>> >  >  - acutil is not meant to be used by third parties besides
> > >>> >  >      authconfig and thus can break without notice
> > >>> >  >
> > >>> >  >  Replace the acutil with python-dns package which has a feature rich
> > >>> >  >  interface for dealing with all different aspects of DNS including
> > >>> >  >  DNSSEC. The main target of this patch is to replace all uses of
> > >>> >  >  acutil DNS library with a use python-dns. In most cases, even
> > >>> >  >  though the larger parts of the code are changed, the actual
> > >>> >  >  functionality is changed only in the following cases:
> > >>> >  >  - redundant DNS checks were removed from verify_fqdn function
> > >>> >  >      in installutils to make the whole DNS check simpler and
> > >>> >  >      less error-prone. Logging was improves for the remaining
> > >>> >  >      checks
> > >>> >  >  - improved logging for ipa-client-install DNS discovery
> > >>> >  >
> > >>> >  >  https://fedorahosted.org/freeipa/ticket/2730
> > >> >
> > 
> > [...]
> > 
> > > Fixed.
> > >
> > > Martin
> > 
> > I've been testing the patches in various setups and haven't found a 
> > regression so far. ACK on 261, for 260 I have a comment below.
> > 
> > 
> > > diff --git a/ipa-client/ipaclient/ipadiscovery.py b/ipa-client/ipaclient/ipadiscovery.py
> > > index 86bef28b2d7fdfc8111b493bddec7ac6888f021a..1889d1918c01fe0c80a3d56b9a7ef304c5a7d97c 100644
> > > --- a/ipa-client/ipaclient/ipadiscovery.py
> > > +++ b/ipa-client/ipaclient/ipadiscovery.py
> > > @@ -310,84 +313,74 @@ class IPADiscovery:
> > >               os.rmdir(temp_ca_dir)
> > >
> > >
> > > -    def ipadnssearchldap(self, tdomain):
> > > -        servers = ""
> > > -        rserver = ""
> > > +    def ipadns_search_srv(self, domain, srv_record_name, default_port,
> > > +                          get_first=True):
> > > +        """
> > > +        Search for SRV records in given domain. When no record is found,
> > > +        en empty string is returned
> > >
> > > -        qname = "_ldap._tcp."+tdomain
> > > -        # terminate the name
> > > -        if not qname.endswith("."):
> > > -            qname += "."
> > > -        results = ipapython.dnsclient.query(qname, ipapython.dnsclient.DNS_C_IN, ipapython.dnsclient.DNS_T_SRV)
> > > +        :param domain: Search domain name
> > > +        :param srv_record_name: SRV record name, e.g. "_ldap._tcp"
> > > +        :param default_port: When default_port is not None, it is being
> > > +                    checked with the port in SRV record and if they don't
> > > +                    match, the port from SRV record is appended to
> > > +                    found hostname in this format: "hostname:port"
> > > +        :param get_first: break on first find, otherwise multiple finds
> > > +                    separated by ":" may be returned
> > 
> > They are separated by ",".
> > In the calling code, for splitting a comma-separated string it is better 
> > to use servers.split(',') than ipautil.parse_items(servers). Or, return 
> > a list directly from here.
> > 
> 
> I did not want to get too intrusive with the patch, but I took your
> advice and rather return now a list of servers - its more effective than
> to returning a comma-joined list and then splitting it back to standard
> list :-) That made parse_items function redundant.
> 
> Martin

I forgot to include a change in the spec file - authconfig should be no
longer needed for build.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-260-4-replace-dns-client-based-on-acutil-with-python-dns.patch
Type: text/x-patch
Size: 46786 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120523/553cae2c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-261-4-fix-default_server-configuration-in-ipapython.config.patch
Type: text/x-patch
Size: 1007 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120523/553cae2c/attachment-0001.bin>