[Freeipa-devel] IPA client realm/domain autodiscovery improvements

[Jan Cholasta]

On 25.2.2016 10:35, Petr Spacek wrote:
> On 24.2.2016 16:30, Sumit Bose wrote:
>> On Wed, Feb 24, 2016 at 04:08:14PM +0100, David Kupka wrote:
>>> On 24/02/16 15:55, Sumit Bose wrote:
>>>> On Wed, Feb 24, 2016 at 03:30:40PM +0100, Martin Babinsky wrote:
>>>>> On 02/24/2016 03:20 PM, Sumit Bose wrote:
>>>>>> On Wed, Feb 24, 2016 at 01:31:55PM +0100, Petr Vobornik wrote:
>>>>>>> On 02/16/2016 02:23 PM, Martin Babinsky wrote:
>>>>>>>> Hi list,
>>>>>>>>
>>>>>>>> WARNING: huge brain dump ahead.
>>>>>>>>
>>>>>>>> During investigation of https://fedorahosted.org/freeipa/ticket/4305 me
>>>>>>>> and Petr Spaced (CC'ed) came to a conclusion that the IPA realm
>>>>>>>> autodiscovery code used by ipa-client-install is so convoluted, complex
>>>>>>>> and hard to understand that the cost of fixing a bug/adding a behavioral
>>>>>>>> change (there are some other tickets dealing with ipadiscovery, e.g. see
>>>>>>>> https://fedorahosted.org/freeipa/ticket/5270
>>>>>>>> https://fedorahosted.org/freeipa/ticket/3912 ) can potentially be higher
>>>>>>>> that a more large-scale rewrite of the module and related codebase.
>>>>>>>>
>>>>>>>> Since we plan to do some general refactoring work anyway, I would like
>>>>>>>> to discuss the possible course of action we may take to tackle this
>>>>>>>> issue. I would like to present the following options we have been
>>>>>>>> discussing so far:
>>>>>>>>
>>>>>>>> 1.) Do a substantial rewrite of existing code ("ipaclient/ipadiscovery.py")
>>>>>>>>
>>>>>>>> We may take the existing IPADiscovery class and try rewrite it in order
>>>>>>>> to get a more concise and deterministic code, which will:
>>>>>>>>
>>>>>>>> * rely more on python-dns and answers provided by resolver (e.g. we are
>>>>>>>> directly parsing resolv.conf for available domains when we can ask the
>>>>>>>> resolver to get domains for us)
>>>>>>>> * be more pythonic (replace error codes with thrown exceptions, clean up
>>>>>>>> numerous C-isms etc.)
>>>>>>>> * not try to outsmart user when server/realm/domain is specified
>>>>>>>> beforehand. Currently, even if you specify all three options, there is
>>>>>>>> still some DNS discovery performed, hence bug #4305. I think that one
>>>>>>>> you specify server(s), not magic should be performed and the discovery
>>>>>>>> process should be reduced to checking whether they are IPA servers and
>>>>>>>> pull all other info (like realm) from them.
>>>>>>>>
>>>>>>>> This may be a considerable effort requiring some time to implement and
>>>>>>>> get right, but IMHO still comparable to the time spent iteratively
>>>>>>>> placing 'if' statements into the existing code and hoping to not break
>>>>>>>> anything. I would even argue it is not worth the effort because we can
>>>>>>>>
>>>>>>>> 2.) Use realmd dbus interface to do DNS discovery
>>>>>>>>
>>>>>>>> I have attached aquick and dirty script I have slapped together to
>>>>>>>> leverage 'org.freedesktop.realmd.Discover' interface to do IPA realm
>>>>>>>> discovery for us. This has a lot of appeal to me since we are leveraging
>>>>>>>> existing codebase for DNS discovery and will have to handle only cases
>>>>>>>> when the user manually specifies a list of IPA servers for the client.
>>>>>>>>
>>>>>>>> This however pulls in realmd as a (potentially circular) dependency for
>>>>>>>> ipa client, and when we find bug in the discovery code, we will have to
>>>>>>>> poke upstream (Stef or Sumit I think) to fix it.
>>>>>>>>
>>>>>>>> So from the long-term point of view, Jan Cholasta's (CC'ed) suggestion to:
>>>>>>>>
>>>>>>>> 3.) Split out IPA discovery portion of realmd to a separate C library
>>>>>>>> shared between IPA and realmd
>>>>>>>>
>>>>>>>> may be best. Both projects will have shared codebase (maintained either
>>>>>>>> by us or realmd devs), which can be reused also by other people to
>>>>>>>> create their own discovery/enrollment solution. This would however
>>>>>>>> require sustained and concerted efforts of both teams to create the
>>>>>>>> library and possibly rewrite realmd to accommodate this change.
>>>>>>>>
>>>>>>>> There may be some other options viable for us, if so please mention them
>>>>>>>> in a reply. Also please correct any piece of information I got wrong.
>>>>>>>>
>>>>>>>> TL;DR: IPA realm/domain discovery is a mess, please suggest a way to fix
>>>>>>>> it.
>>>>>>>>
>>>>>>>
>>>>>>> #3 sounds good from long term perspective.
>>>>>>>
>>>>>>> In short term, i.e., #4305, we should skip discovery when --on-master is
>>>>>>> used.
>>>>>>
>>>>>> I would prefer #2. Because as seen from the patch it is already working
>>>>>> and can easily be used from python. I think this was also one of the
>>>>>> reasons why Stef used a DBus service for this instead of a C library
>>>>>> which then needs various bindings for python, Java, ruby, Go you name
>>>>>> it.
>>>>>>
>>>>>
>>>>> This approach is also my favorite. However, my (and several of my
>>>>> colleagues) concern is that
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1271551 will break it in
>>>>> kickstart environment. I don't know how much of an issue is this, I guess it
>>>>> completely precludes automatic enrollment during machine provisioning.
>>>>
>>>> realmd has the --dbus-peer option. So I guess it might be possible that
>>>> realmd can be started by ipa-client-install directly in this case and
>>>> allow communication over a socket pair. I'm not sure how hard or easy
>>>> this would be in python.
>>>>
>>>> Stef, do you have some pointers how to use dbus-peer from python?
>>>>
>>>> bye,
>>>> Sumit
>>>>
>>>
>>> We have already done something similar with certmonger. You can look into
>>> ipapython/certmonger.py
>>
>> Great, so it should be possible to use realmd in the same way in a
>> kickstart environment.
>
> Then I vote for #2 as it is easy enough.
>
> If necessary, realmd can be a weak dependency so if it is not present
> ipa-client-install will work as long as --option are explicitly specified.

+1.

My personal favorite would be to deprecate autodiscovery in 
ipa-client-install and tell people to use realmd instead. What you 
describe is all that is needed to retain backward compatibility.

-- 
Jan Cholasta