[Freeipa-devel] [PATCH] 1106 IPA REST smart proxy

[Dmitri Pal]

On 02/17/2014 02:33 PM, Rob Crittenden wrote:
> Dmitri Pal wrote:
>> On 02/17/2014 01:21 PM, Rob Crittenden wrote:
>>> Martin Kosek wrote:
>>>> On 02/14/2014 11:26 PM, Dmitri Pal wrote:
>>>>> On 02/14/2014 05:07 PM, Rob Crittenden wrote:
>>>>>> Dmitri Pal wrote:
>>>>>>> On 02/14/2014 04:52 PM, Rob Crittenden wrote:
>>>>>>>> Dmitri Pal wrote:
>>>>>>>>> On 02/14/2014 03:09 PM, Rob Crittenden wrote:
>>>>>>>>>> Dmitri Pal wrote:
>>>>>>>>>>> On 02/14/2014 03:43 AM, Martin Kosek wrote:
>>>>>>>>>>>> On 02/14/2014 12:07 AM, Rob Crittenden wrote:
>>>>>>>>>>>>> Martin Kosek wrote:
>>>>>>>>>>>>>> On 01/28/2014 09:35 PM, Rob Crittenden wrote:
>>>>>>>>>>>>>>> Petr Viktorin wrote:
>>>>>>>>>>>>>>>> On 01/23/2014 02:17 PM, Petr Viktorin wrote:
>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>>>> The URL endpoint /ipa/rest suggests that if we implement a
>>>>>>>>>>>>>>>> complete REST
>>>>>>>>>>>>>>>> API for IPA it would live here. Is the API supposed to be
>>>>>>>>>>>>>>>> future-compatible? (The API itself seems to be a good
>>>>>>>>>>>>>>>> subset of a
>>>>>>>>>>>>>>>> complete REST API, but can we easily add an frontend with
>>>>>>>>>>>>>>>> authentication, i18n, etc. here later, and keep the
>>>>>>>>>>>>>>>> limitations for
>>>>>>>>>>>>>>>> unauthenticated access?)
>>>>>>>>>>>>>>>> Perhaps /ipa/smartproxy would be a better choice?
>>>>>>>>>>>>>>> It was future-proofing. I'm fine with changing the URI, 
>>>>>>>>>>>>>>> it is
>>>>>>>>>>>>>>> probably a good
>>>>>>>>>>>>>>> thing to save that name.
>>>>>>>>>>>>>> +1 for moving to /ipa/smartproxy/rest, we will want a
>>>>>>>>>>>>>> complete REST
>>>>>>>>>>>>>> interface
>>>>>>>>>>>>>> in ipa/rest/ in the future. I rather opened a ticket to
>>>>>>>>>>>>>> track that:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/4168
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Martin
>>>>>>>>>>>>>>
>>>>>>>>>>>>> I think I've addressed most of Petr's suggestions with the
>>>>>>>>>>>>> exception
>>>>>>>>>>>>> of the
>>>>>>>>>>>>> global ipa handle and I stuck with *args, **options as 
>>>>>>>>>>>>> this is
>>>>>>>>>>>>> pretty
>>>>>>>>>>>>> much
>>>>>>>>>>>>> standard in IPA calls.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The gssproxy.conf.snippet just makes it easier to
>>>>>>>>>>>>> copy/paste. I can
>>>>>>>>>>>>> drop it if
>>>>>>>>>>>>> you want, I suppose it is duplication.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Note that I ran this past the Foreman design again and as a
>>>>>>>>>>>>> result
>>>>>>>>>>>>> added
>>>>>>>>>>>>> another interface, /realm. It was my understanding that this
>>>>>>>>>>>>> Foreman
>>>>>>>>>>>>> design
>>>>>>>>>>>>> wasn't set into stone but a patch is working its way through
>>>>>>>>>>>>> their
>>>>>>>>>>>>> system that
>>>>>>>>>>>>> followed the spec so I went ahead and added it. It isn't a
>>>>>>>>>>>>> big deal,
>>>>>>>>>>>>> the Host()
>>>>>>>>>>>>> class handles it out of the box.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I also updated the design page a bit to reflect some of the
>>>>>>>>>>>>> changes
>>>>>>>>>>>>> made.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Right now there are no plans to backport python-kerberos to
>>>>>>>>>>>>> F20.
>>>>>>>>>>>>>
>>>>>>>>>>>>> rob
>>>>>>>>>>>> I will leave functional testing to others, I just read the
>>>>>>>>>>>> code. I am
>>>>>>>>>>>> still
>>>>>>>>>>>> quite concerned about the positioning, naming and "branding"
>>>>>>>>>>>> of this
>>>>>>>>>>>> feature.
>>>>>>>>>>>>
>>>>>>>>>>>> 1) Package name
>>>>>>>>>>>>
>>>>>>>>>>>> Currently, it is a host/hostgroup smartproxy, primarily for
>>>>>>>>>>>> Foreman
>>>>>>>>>>>> integration
>>>>>>>>>>>> use case.
>>>>>>>>>>>>
>>>>>>>>>>>> Packaging it as freeipa-server-smartproxy may be ok, but only
>>>>>>>>>>>> if we
>>>>>>>>>>>> plan to use
>>>>>>>>>>>> this proxy for all other projects. I.e. if we one day
>>>>>>>>>>>> implement user
>>>>>>>>>>>> smartproxy, it would need to be part of this otherwise it
>>>>>>>>>>>> would lead
>>>>>>>>>>>> to strange
>>>>>>>>>>>> organization, like having freeipa-server-smartproxy and
>>>>>>>>>>>> freeipa-server-smartproxy-users packages. Maybe it should be
>>>>>>>>>>>> named
>>>>>>>>>>>> differently?
>>>>>>>>>>>>
>>>>>>>>>>>> freeipa-server-foreman-smartproxy
>>>>>>>>>>>> freeipa-server-smartproxy-hosts
>>>>>>>>>>>>
>>>>>>>>>>>> 2) ipa-rest stuff
>>>>>>>>>>>>
>>>>>>>>>>>> We have ipa-rest script, ipa-rest.conf, ipa-rest.service,
>>>>>>>>>>>> ipa-rest
>>>>>>>>>>>> man.
>>>>>>>>>>>> However, I have the same concerns as above. This is too 
>>>>>>>>>>>> general
>>>>>>>>>>>> and it
>>>>>>>>>>>> may
>>>>>>>>>>>> conflict with future core server needs (like when we
>>>>>>>>>>>> implement core
>>>>>>>>>>>> IPA REST
>>>>>>>>>>>> interface - #4168). Let us name it consistently with package
>>>>>>>>>>>> name:
>>>>>>>>>>>>
>>>>>>>>>>>> ipa-smartproxy.service
>>>>>>>>>>>> ipa-smartproxy-hosts.service
>>>>>>>>>>>> ipa-foreman-smartproxy.service
>>>>>>>>>>>>
>>>>>>>>>>>> The same for binary, man, ...
>>>>>>>>>>>>
>>>>>>>>>>>> 3) Man pages
>>>>>>>>>>>>
>>>>>>>>>>>> The same point, you brand it as "IPA REST server". This is too
>>>>>>>>>>>> general.
>>>>>>>>>>>>
>>>>>>>>>>>> To sum it up - let us chose one name/brand of this feature
>>>>>>>>>>>> and let's
>>>>>>>>>>>> use it
>>>>>>>>>>>> consistently in FreeIPA infrastructure - subpackage name,
>>>>>>>>>>>> subdirectory
>>>>>>>>>>>> in git,
>>>>>>>>>>>> subdirectory in ipatests, man, conf, script, names in man 
>>>>>>>>>>>> pages.
>>>>>>>>>>>>
>>>>>>>>>>>> Martin
>>>>>>>>>>> +1
>>>>>>>>>>>
>>>>>>>>>>> I think it should be "host"
>>>>>>>>>>>
>>>>>>>>>>> ipa-host-smartproxy
>>>>>>>>>>>
>>>>>>>>>>> then we will be able to add other smart proxies and then
>>>>>>>>>>> combine them
>>>>>>>>>>> into one ipa-smartproxy package later if needed.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> This would imply we actually run separate servers for the 
>>>>>>>>>> various
>>>>>>>>>> commands. Given that right now we're focused on just the
>>>>>>>>>> Foreman use
>>>>>>>>>> cases I think ipa-smartproxy is sufficient.
>>>>>>>>>>
>>>>>>>>>> For our purposes the smartproxy is just a thin wrapper around
>>>>>>>>>> the IPA
>>>>>>>>>> API. It is extensible for our needs, we just don't need to yet.
>>>>>>>>>> But if
>>>>>>>>>> we did, we'd do so within the cherrypy server and everything
>>>>>>>>>> would be
>>>>>>>>>> self-contained.
>>>>>>>>>>
>>>>>>>>>> rob
>>>>>>>>>
>>>>>>>>> Are you saying that we will just extend this smart proxy for
>>>>>>>>> other use
>>>>>>>>> cases like users and others?
>>>>>>>>>
>>>>>>>>
>>>>>>>> It all depends on the use case. If we're talking Foreman then
>>>>>>>> yes. If
>>>>>>>> something else, then we'll discuss it at the time, but it still
>>>>>>>> may be
>>>>>>>> able to be included.
>>>>>>>>
>>>>>>>> The IPA Foreman smart proxy is distinguished by a couple of 
>>>>>>>> things:
>>>>>>>>
>>>>>>>> - listens on port 8090, only on localhost
>>>>>>>> - is unauthenticated
>>>>>>>> - uses the /realm URI namespace
>>>>>>>>
>>>>>>>> I'm exposing hosts and hostgroups as well, but per the spec that
>>>>>>>> Dominic came up with only /realm/:domain is required and affects
>>>>>>>> only
>>>>>>>> hosts.
>>>>>>>>
>>>>>>>> We can stick this behind Apache to get authentication, even on a
>>>>>>>> specific URI if we want/need to, but the basic REST stuff can
>>>>>>>> still be
>>>>>>>> handled by cherrypy.
>>>>>>>>
>>>>>>>> We'll need to balance complexity of mixing things vs the
>>>>>>>> complexity of
>>>>>>>> code duplication in multiple servers, unless we come up with some
>>>>>>>> sort
>>>>>>>> of REST server class where we just instantiate whatever we 
>>>>>>>> need. But
>>>>>>>> that is for the future.
>>>>>>>>
>>>>>>>> rob
>>>>>>>
>>>>>>> But if it is specific for Foreman then it should have a generic
>>>>>>> name. I
>>>>>>> agree with Martin here.
>>>>>>
>>>>>> I have the feeling we're in basic agreement.
>>>>>>
>>>>>> smartproxy is the Foreman name. I'm not pushing back against
>>>>>> renaming, I'll
>>>>>> have a new patch out shortly doing just that. I'm just pushing back
>>>>>> against
>>>>>> renaming it too deeply. I'm going with ipa-smartproxy.
>>>>>>
>>>>>> rob
>>>>> The point is that smartproxy is a good name to be reused outside
>>>>> foreman. So
>>>>> rather than assuming that "smartproxy" is foreman specific we can go
>>>>> with:
>>>>> host proxy or host smart proxy - for this use case
>>>>> and then
>>>>> user proxy or user smart proxy - for use cases like User
>>>>> provisioning systems
>>>>> and then
>>>>> DNS proxy or DNS smart proxy - ...
>>>>> and then
>>>>> DHCP proxy or DHCP smart proxy - ...
>>>>> and so on
>>>>>
>>>>> We need to establish a good pattern that we will be able to easily
>>>>> extend.
>>>>
>>>> +1, this was exactly my goal. It is easy to name and organize things
>>>> now,
>>>> difficult to do when it is live upstream and/or integrated with 
>>>> Foreman.
>>>>
>>>> I think the key for the right naming is a fact if this is really
>>>> specific to
>>>> Foreman or it is a truly general design usable also in other use
>>>> cases. If
>>>> Foreman-specific, I would go with freeipa-server-smartproxy-host or
>>>> similar.
>>>>
>>>> If general, we can go with
>>>>
>>>> freeipa-server-proxy-host
>>>> freeipa-server-proxy-user
>>>> freeipa-server-proxy-dhcp
>>>>
>>>> The proxies may share the framework and only expose the requested
>>>> part of the
>>>> tree - which in advance gives you an option for an API separation, as
>>>> compared
>>>> to general freeipa-server-smartproxy.
>>>
>>> I still don't get the point of this. Are you proposing having 3
>>> different servers, each providing a unique service? Or one service
>>> that one can turn on/off features, or something else? Do you envision
>>> this as 3 separate subpackages?
>>>
>>> There is no "framework" in my current patch, it is a cherrypy server
>>> that exposes parts of IPA on a given URI. It is the thinnest of layers.
>>
>>
>> Then it seems to make sense to have 3 different packages that can be
>> optionally coninstalled and would probably share the same principal,
>> keytab and local REST API socket/port.
>>
>
> Well, what you are designing is a central framework with plugins. What 
> I designed is a quick-and-dirty get something up quickly. We need to 
> pick one. The plugable method is going to require a fair bit of 
> rework, though it will potentially pay dividends in the future.

I think that we can start where you are but drive towards this 
architecture via refactoring. But we need to pick the right name now.
Even if the architecture is not there yet we should name the package in 
such way that it does not preclude us from moving towards a clean 
architecture I described during the next iteration.

>
> rob


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/