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

[Dmitri Pal]

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.


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager for IdM portfolio
Red Hat Inc.


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