On Thu, 2012-05-24 at 13:53 +0200, Petr Viktorin wrote:
> On 05/23/2012 02:30 PM, Martin Kosek wrote:
> > 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.
> >>
> Good riddance.
> >> Martin
> >
> > I forgot to include a change in the spec file - authconfig should be no
> > longer needed for build.
> >
> > Martin
> 
> I tested several installs and couldn't find a regression. ACK.
> 
> 

Thanks, pushed both to master.

Martin

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

Reply via email to