On 04/29/2014 04:27 PM, Rob Crittenden wrote:
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
than inter-diff since it is so small.
Not really. Having a patch file with a sequence+revision number
refer to has its merits. Especially in a hairy thread like this
Also one of our MUAs wrapped the lines, I had to undo that
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
IPA stores the principal in the request context so using that
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.
ACK on the functionality.
Sorry for not catching that last time, but your patch doesn't
*versioned* BuildRequres on python-kerberos, instead it adds a
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
exception now, and open a ticket to remove it later. That way the
succeeds despite the older version, and the new python-kerberos is
needed when installing freeipa-server-foreman-smartproxy.
That should make everyone happy, including Martin.
Unfortunately our lint exception mechanism doesn't work on
this needs a somewhat nastier hack.
The attaching a patch that does this (and I'm pasting a simple
below). Does that look okay to push?
I'm trying to find a better solution to all this. I may end up
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
any IPA-enrolled client, so you can run it directly on the Foreman
with the IPA server somewhere else. What this means is that someone
could probably fairly easily package this up for other
if we end up with a Fedora-only python-kerberos patch then
Fedora-only as well.
So I'm trying to get some movement out of upstream on this but it's
crickets for weeks. I think in the context of the calendar server
PyKerberos is small potatoes so doesn't get much lovin'. I'll
nagging to get some sort of response, even if it is "stop nagging
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
to see if we can get the patch accepted their until (if) upstream
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
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
It would be nice to put the whole path here so people don't have to
search for the file.
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
I also added a bit on testing so you can confirm that things are
Side note, cherrypy's routing makes requests like this possible:
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.
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
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.
I've eyeballed the changes, no testing yet. Here are my thoughts.
- 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.
Please use a bare `raise` for simple re-raising of exceptions in an
except: clause. It'll preserve the traceback.
The control flow for the masking/re-raising seems convoluted. I've seen
code like the following before:
nonmasked_exceptions = Exception
nonmasked_exceptions = ()
except nonmasked_exceptions as e:
except ConcreteException as e:
Also it might be useful to factor raise_masked_exception() out; see below.
- Rather than returning an DuplicateEntry error when POSTing to an
existing host, do a host_mod instead.
Could you add a test for this?
It looks like ip_address has no effect in this case, should we fail if
it's given instead of silently ignoring it? Same for rebuilding new hosts.
- Add config option to pass updatedns=True when deleting a host.
In DELETE, the exception handling seems overly broad. With a
raise_masked_exception() function and nomaskexception, you could handle
the specific error. It should help debugging if nothing else.
This way of masking errors seems clumsy when nesting commands; I wonder
if they can be handled better at a higher level ('request.error_response'?)
Freeipa-devel mailing list