[Freeipa-devel] [PATCH] 260 Replace DNS client based on acutil with python-dns
Petr Viktorin
pviktori at redhat.com
Thu May 24 11:53:16 UTC 2012
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.
--
Petr³
More information about the Freeipa-devel
mailing list