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

[Rob Crittenden]

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.

rob