[Freeipa-devel] [PATCH] 0159-0160 Support ID views in compat tree

[thierry bordaz]

On 10/07/2014 05:00 PM, Alexander Bokovoy wrote:
> On Tue, 07 Oct 2014, thierry bordaz wrote:
>> On 10/01/2014 06:16 PM, Alexander Bokovoy wrote:
>>> Hi!
>>>
>>> Attached are patches to add support of FreeIPA ID views to Schema
>>> compatibility plugin (slapi-nis). There are two patches for FreeIPA and
>>> a separate patch for slapi-nis. Patches can be applied 
>>> independently; if
>>> old slapi-nis is installed, it will simply work with new configuration
>>> but do nothing with respect to answering to requests using 
>>> host-specific
>>> ID views.
>>>
>>> I included documentation on how slapi-nis ID views feature supposed to
>>> work, available in slapi-nis/doc/ipa/ipa-sch.txt. Any comments and 
>>> fixes
>>> are welcome. There are no additional tests in slapi-nis to cover compat
>>> trees, we have multiple tests in FreeIPA for this purpose, will be run
>>> as part of FreeIPA CI effort.
>>>
>>> FreeIPA patches add ACIs for accessing ID view-applied entries over
>>> compat tree. They also include additional configuration; this
>>> configuration is needed to properly resolve ID view overrides when
>>> creating compat entries.
>>>
>>> A second FreeIPA patch adds support to override login shell. This part
>>> was missing from the original patchset by Tomas.
>>>
>>> For trusted AD users one needs patches to SSSD 1.12.2, made by Sumit
>>> Bose. There is also a regression (fixed by Sumit as well) that prevents
>>> authentication of AD users over PAM which affects authentication over
>>> compat tree. With the patch from Sumit authentication works again, both
>>> with ID view and without it.
>>>
>>>
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>> Hi Alexander,
>>
>>   Just a minor remark about idview_get_overrides. It does an internal
>>   search of the views and keep the returned views into
>>   cbdata->overrides. Later the entries will be freed. Internal search
>>   may also return referral arrays (usually freed by
>>   slapi_free_search_results_internal). In case cn=views container
>>   contains referral entries you may need to free them.
> What would be a reason to return these referrals in IPA case? This code
> is IPA-specific and we don't have any referrals for the tree or backend
> we are dealing with. I agree it would be good to do in general but how
> that is relevant to the specific use case?

You are right, this is IPA-specific and we control the fact that there 
is no referral entries there.
>
>>   Also, I have a concern about performance. My understanding is that
>>   for each search, backend_search_cb allocates a new cbdata with
>>   uninitialized 'overrides'. So it will trigger the internal search
>>   idview_get_overrides.
>>   Most of the time those overrides rules have not been updated, so
>>   idview_get_overrides will retrieve the same entries.
>>   Wouldn't it be possible to 'cache' those entries (to prevent the
>>   internal search), and just clear those entries if a mod/add/del
>>   happen one them ?
> I was thinking about using this approach but rejected it for several
> reasons, primary due to lack of development time. We can do a lot of
> refactoring on how slapi-nis currently uses RAM to store all compat
> entries at the same time; the current approach with ID overrides tries
> to not complicate that logic. We agreed with Simo to do performance
> optimizations at a later stage.
Ok. I fully agree.