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

Fri Feb 14 22:07:23 UTC 2014

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