[Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type

[Martin Kosek]

On 02/21/2013 04:24 PM, Sumit Bose wrote:
> Hi,
> 
> this series of patches fix https://fedorahosted.org/freeipa/ticket/2960
> The related design page is
> http://freeipa.org/page/V3/Read_and_use_per_service_pac_type .
> 
> It was agreed while discussing the design page that the currently
> hardcoded value for the NFS service will be dropped completely, hence
> the first patch reverts to patch which added the hardcoded value.
> 
> The second patch adds the needed attribute to compensate the now missing
> hardcoded value.
> 
> In the following three patches the PAC type is read and used
> accordingly. The last patch adds a unit test for a new function.
> 
> bye,
> Sumit

I did only sanity testing to the C part of the RFE so far, but by reading it it
looks ok. It may be beneficial if Simo or Alexander checked if the ipa-kdb part
is OK.

I will comment on the Python parts:

1) Your update check testing if there is any NFS service with ipakrbauthzdata
looks like a good heuristics to prevent admin frustration. Though an ideal
solution would require
https://fedorahosted.org/freeipa/ticket/2961
to prevent multiple updates when admin purposefully removes this attribute. We
may want to reference the ticket in a comment in the update plugin...


2) The upgrade plugin may get into infinite loop if ldap.find_entries returns
truncated result. As you do not update entries directly but only return update
instructions when you have no truncated results, you will loop.

To simulate this, I just created 2 NFS principals and set size_limit=1 in your
find_entries call.


3) Ok, we set ipakrbauthzdata to NONE on upgrades. But what about new NFS
service principals added by service-add? Shouldn't we set ipakrbauthzdata for
new services too? As a default when no user ipakrhbauthzdata is set...
Otherwise admins could be surprised why their new NFS services do not work
while the others do.

Also, I think we should have this NFS culprit documented somewhere (i.e. why we
set ipakrbauthzdata to NONE by default). At least as a small section in the
online help for services plugin...

Martin