[Freeipa-devel] [PATCHES] jderose 0009-0019

[Rob Crittenden]

Rob Crittenden wrote:
> Jason Gerard DeRose wrote:
>> These patches finish my (initial) cleanup and testing of Andrew's
>> request authority plugins.  These have been tested against a CA server
>> Andrew has running (only accessible inside Red Hat firewall).  The tests
>> I ran aren't exhaustive by any means, but it's a start.
>>
>> A few other noteworthy changes:
>>
>> 1) I added a mechanism for an entire plugin module to be conditionally
>> loaded.  Previously individual plugins (classes) could be conditionally
>> registered, but there wasn't a way to conditionally skip the entire
>> module.  Now you can raise errors2.SkipPluginModule.
>>
>> For example, if your plugin is included in the built-in freeIPA plugins
>> but is only optionally enabled, you can use SkipPluginModule so that
>> nothing in your module after the point where you raise SkipPluginModule
>> gets processed:
>>
>>         from ipalib import api, SkipPluginModule
>>         if api.env.enable_ra is not True:
>>             # In this case, abort loading this plugin module...
>>             raise SkipPluginModule(reason='env.enable_ra is not True')
>>                 # The remaining is only processed when env.enable_ra 
>> is True...
>>                 # So we don't get an import error here!
>>         import not_installed_python_package
>>
>> 2) I added a place-holder directory for integration tests: checks/
>> Inside is my still rather hacky check-ra.py script I was using to test
>> the ra backend plugin.  I'll send another email shortly with some
>> thoughts about integration testing.
>>
>>
>> P.S.: I put the patches in a tarball because it seems that Evolution is
>> Windows-ifying the line endings in my attachments.
> 
> I'm having a really hard time reviewing these since the patches aren't 
> cumulative but build upon each other. Since patches 0001-0007 haven't 
> been applied yet it isn't possible to cleanly apply these either.
> 
> I'll see if I can slurp them all in together and make a single, unified 
> patch so I can see what is going on.
> 
> In the meantime we need to get the 1-7 patches imported. I had just a 
> couple of comments.
> 
> rob

Ok, that actually worked out fairy well but I'll have to address things 
indirectly.

I think that all command-arguments should have a help option to describe 
what it does.  This affects all the functions in cert.py.

ipaserver/plugins/ra.py:_request() doesn't close the connection on an 
exception

ipaserver/plugins/ra.py:__create_* should probably verify that 
permissions are appropriate (probably 600).

I think that a lot of things in ra.py will be pulled out at some point 
as they are run-once type functions that will be executed at install time.

rob