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

Tue Apr 29 14:27:13 UTC 2014

Petr Viktorin wrote:
> On 04/23/2014 08:52 PM, Rob Crittenden wrote:
>> Petr Viktorin wrote:
>>> On 04/09/2014 11:29 PM, Rob Crittenden wrote:
>>>> Rob Crittenden wrote:
>>>>> Petr Viktorin wrote:
>>>>>> On 03/14/2014 07:58 PM, Rob Crittenden wrote:
>>>>>>> Petr Viktorin wrote:
>>>>>>>> On 03/12/2014 07:48 PM, Rob Crittenden wrote:
>>>>>> [...]
>>>>>>>>>
>>>>>>>>> Here are a couple more enhancements I'm considering, this seems
>>>>>>>>> simpler
>>>>>>>>> than inter-diff since it is so small.
>>>>>>>>
>>>>>>>> Not really. Having a patch file with a sequence+revision number you
>>>>>>>> can
>>>>>>>> refer to has its merits. Especially in a hairy thread like this
>>>>>>>> one.
>>>>>>>> Also one of our MUAs wrapped the lines, I had to undo that
>>>>>>>> manually.
>>>>>>>>
>>>>>>>>> Here is why I made the changes, in order:
>>>>>>>>>
>>>>>>>>> I doubled the calls to create the connection but one isn't in a
>>>>>>>>> try/except!? Remove the obvious one.
>>>>>>>>>
>>>>>>>>> We currently completely eat GSSAPI errors, I figure we should log
>>>>>>>>> failures.
>>>>>>>>>
>>>>>>>>> IPA stores the principal in the request context so using that will
>>>>>>>>> save
>>>>>>>>> a GSSAPI call (and as we learned, a lock in gssproxy).
>>>>>>>>>
>>>>>>>>> I included your content-type change.
>>>>>>>>
>>>>>>>> These changes look good.
>>>>>>>> I'm almost done testing but I need to call it a week.
>>>>>>>
>>>>>>> Awesome, thanks.
>>>>>>
>>>>>> ACK on the functionality.
>>>>>>
>>>>>>>> Sorry for not catching that last time, but your patch doesn't add a
>>>>>>>> *versioned* BuildRequres on python-kerberos, instead it adds a
>>>>>>>> duplicate
>>>>>>>> unversioned one. So lint (and thus the build) will fail if the old
>>>>>>>> python-kerberos version is installed.
>>>>>>>>
>>>>>>>> A possible a solution to the build trouble would be to just add a
>>>>>>>> lint
>>>>>>>> exception now, and open a ticket to remove it later. That way the
>>>>>>>> build
>>>>>>>> succeeds despite the older version, and the new python-kerberos is
>>>>>>>> only
>>>>>>>> needed when installing freeipa-server-foreman-smartproxy.
>>>>>>>> That should make everyone happy, including Martin.
>>>>>>>> Unfortunately our lint exception mechanism doesn't work on
>>>>>>>> modules, so
>>>>>>>> this needs a somewhat nastier hack.
>>>>>>>> The attaching a patch that does this (and I'm pasting a simple diff
>>>>>>>> below). Does that look okay to push?
>>>>>>>
>>>>>>> I'm trying to find a better solution to all this. I may end up
>>>>>>> taking
>>>>>>> Martin's suggestion of rawhide-only to avoid this sort of thing.
>>>>>>
>>>>>> Looks like you'll still need to silence pylint on f20 in that case.
>>>>>>
>>>>>>> The deal with the smartproxy is that you can/should be able to run
>>>>>>> it on
>>>>>>> any IPA-enrolled client, so you can run it directly on the Foreman
>>>>>>> box,
>>>>>>> with the IPA server somewhere else. What this means is that someone
>>>>>>> could probably fairly easily package this up for other distributions
>>>>>>> and
>>>>>>> if we end up with a Fedora-only python-kerberos patch then
>>>>>>> smartproxy is
>>>>>>> Fedora-only as well.
>>>>>>>
>>>>>>> So I'm trying to get some movement out of upstream on this but it's
>>>>>>> been
>>>>>>> crickets for weeks. I think in the context of the calendar server
>>>>>>> PyKerberos is small potatoes so doesn't get much lovin'. I'll amp up
>>>>>>> the
>>>>>>> nagging to get some sort of response, even if it is "stop nagging
>>>>>>> us!"
>>>>>>>
>>>>>>> rob
>>>>>>
>>>>>> Good luck!
>>>>>
>>>>> Ok, taking a different tack on this. Rather than running it as a
>>>>> separate server process, run it as a WSGI app inside Apache. This
>>>>> required a fair bit of re-tooling and complicates the set up a little
>>>>> bit. I think I've got it all covered in the man page.
>>>>>
>>>>> On the python-kerberos front I've got bugs opened in Ubuntu and Debian
>>>>> to see if we can get the patch accepted their until (if) upstream ever
>>>>> takes a look.
>>>>
>>>> I decided to run the new WSGI app in a different process group, using
>>>> the smartproxy we use for delegation. This simplifies the connection
>>>> code, rather than using ldap2 like I was using, we use the RPC
>>>> interface. And it provides to process separation. As a side-effect it
>>>> will make running this code on platforms without GSSProxy a bit easier.
>>>>
>>>> rob
>>>>
>>>
>>> Works great here!
>>>
>>>
>>> The python-kerberos dependency issue still needs to be solved.
>>
>> Build is on the way to updates-testing if you can give it a go.
>>
>>>
>>> The man page says:
>>>     Copy ipa-smartproxy-apache.conf to
>>> /etc/httpd/conf.d/ipa-smartproxy.conf.
>>> It would be nice to put the whole path here so people don't have to
>>> search for the file.
>>
>> Done.
>>
>>>
>>> The "Configure Apache to use smartproxy" line looks like a step to be
>>> performed. It could use some emphasis to make it look like a header.
>>
>> I combined it with the subsequent sentence so hopefully it is a bit
>> clearer.
>>
>> I also added a bit on testing so you can confirm that things are working.
>>
>>
>>> Side note, cherrypy's routing makes requests like this possible:
>>>      http POST
>>> :8090/ipa/smartproxy/host/testhost.idm.lab.eng.brq.redhat.com/some_description/True/06-00-00-00-00-00/some_userclass
>>>
>>>
>>>
>>>
>>> Should that be allowed?
>>
>> It is definitely ugly but AFAICT it isn't illegal. The zero
>> content-length bothers me more than this horrible-looking URI. It
>> definitely requires some understanding of the ordering of parameters to
>> get this call right.
>>
>> rob
>
> Everything works now!
> Except one thing: json_encode_binary recently got a mandatory version
> argument. For code that's part of IPA, it should be fine to just use
> API_VERSION here.
>
> I tested with that added; ACK if you agree.

ACK on your change.

Sorry, one more set of changes, some fairly significant. This brings our 
proxy in line with the Foreman proxy functionality-wise. I tested this 
against a Foremane 1.5.0 RC1 build and it is fine. I didn't go as far as 
actually deploying any hosts but did confirm that the create/delete 
functions work ok.

Significant changes:
- random removed as an option. It is always true unless a user-provided 
password is sent.
- Added an option to Command so that the real IPA error can be raised to 
make internal error handling possible.
- Rather than returning an DuplicateEntry error when POSTing to an 
existing host, do a host_mod instead.
- Add config option to pass updatedns=True when deleting a host.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-1106-11-rest.patch
Type: text/x-patch
Size: 49931 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140429/5f8035a0/attachment.bin>