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

[Martin Kosek]

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.

Martin