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

[Petr Viktorin]

On 02/27/2014 10:18 PM, Rob Crittenden wrote:
> Rob Crittenden wrote:
>> Dmitri Pal wrote:
>>> On 02/17/2014 04:57 PM, Rob Crittenden wrote:
>>>> Dmitri Pal wrote:
>>>>> On 02/17/2014 04:13 PM, Rob Crittenden wrote:
>>>>>> Dmitri Pal wrote:
>>>>>>> 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:
>>>>>>>>>>> +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.
>>>>>>
>>>>>> Just having a hard time seeing the value. This would be like making
>>>>>> each of the IPA plugins a separate package and somehow installing
>>>>>> them
>>>>>> individually.
>>>>>>
>>>>>> To do this you'd need at least 2 packages, a high level one with the
>>>>>> "framework" and then a separate one for each data type.
>>>>>>
>>>>>> This could also be achieved, and likely more cleanly, without all
>>>>>> these extra packages, as a simple configuration file. Once a package,
>>>>>> always a package.
>>>>>>
>>>>>> Maybe I'm too close to the problem, but to me this is a
>>>>>> Foreman-specific solution. The "smartproxy" is a Foreman creation, I
>>>>>> don't know that anything else is using it. If you want a RESTful
>>>>>> server, then we enable REST in IPA directly, but selectively enabling
>>>>>> and disabling APIs is not something we've done before. It has been
>>>>>> controlled by ACIs instead.
>>>>>>
>>>>>> rob
>>>>>>
>>>>>
>>>>> We are trying to predict future here. Say we release it as you
>>>>> suggest.
>>>>> Tomorrow we point someone upstream who wants to add users to IPA
>>>>> from a
>>>>> similar proxy to this implementation and say use this as a model.
>>>>> And then Rich needs the same but for DNS for Designate.
>>>>>
>>>>> What would be the best? Rob if you knew that tomorrow two other
>>>>> developers will create similar proxies for users and DNS would you
>>>>> change anything? Would you provide some guidelines to them?
>>>>> If you are close to the problem you should be able to at least tell
>>>>> them: "copy and paste" vs. "add more APIs to the same proxy".
>>>>> If your recommendation is copy and paste then I suspect there will be
>>>>> challenges of running these proxies on the same machine - they will
>>>>> collide on ports and sockets. If we say "extend" shouldn't we use a
>>>>> more
>>>>> generic name?
>>>>>
>>>>
>>>> I'd say "use the existing IPA API". The only reason we have to write
>>>> the smartproxy at all is to interoperate with another service that
>>>> already has a well-defined remote server API which uses REST.
>>>>
>>>> rob
>>> OK, so you think that proxy is one off. OK I am fine with it.
>>> I was under assumption that it would be a beginning of a trend but I
>>> might be wrong as I do not have valid arguments to prove or disprove one
>>> way or another.
>>> I guess time would show.
>>>
>>> Any objections to Rob's approach?
>>>
>>
>> Ok, so try to summarize this long-running thread, I'll rename the
>> subpackage to freeipa-server-foreman-smartproxy to make it clearer what
>> it is/does. Right now it requires manual configuration so having the
>> package installed should have no negative impacts (other than
>> potentially pulling in additional dependencies).
>>
>> I'll leave it in smartproxy for now, it's just cleaner and better
>> integrates with ipatests IMHO.
>>
>> Foreman supports SSL client auth which is great, by cherrypy does not
>> yet. There is a pull request to add this,
>> https://bitbucket.org/cherrypy/cherrypy/pull-request/15/added-support-for-client-certificate/activity
>>
>> . Foreman otherwise supports no other authentication method, so we're
>> blocked with this. The certs for this would initially come out of
>> Foreman/puppet.
>>
>> I'll submit a new patch with an updated spec but I think otherwise I've
>> addressed the isuses Petr has raised. This thread has taken a lot of
>> turns so it is very possible I missed something though :-)
>
> Updated patch based on feedback from Foreman team. I added a new URI,
> /features, which Foreman uses to determine what capabilities a proxy has.
>
> rob

On my VMs, where the first request is handled properly but the server 
hangs on the second one. I gave you access to the machines for 
investigation.


Please add the Python libraries (python-cherrypy, python-requests, 
python-kerberos) to BuildRequires. Otherwise the build fails due to 
pylint errors.


In the man page:

> +Create a host or user whose credentials will be used by the server to make requests and add it to the role:
> +
> + $ ipa user\-add \-\-first=Smartproxy \-\-last=Serversmartproxy
> + $ ipa role\-add\-member \-\-users=smartproxy 'Smartproxy management'

the first command should be
     ipa user-add smartproxy --first=Smartproxy --last=Serversmartproxy
since by default the username is 'sserversmartproxy'.


A nitpick regarding the systemd service: according to [0], Type=forking 
should be avoided. Is there a reason against setting Type=simple, and 
removing the PID file?


[0] 
http://www.freedesktop.org/software/systemd/man/daemon.html#Writing%20Systemd%20Unit%20Files

-- 
Petr³