[Freeipa-devel] [PATCH 0157] Prohibit deletion of active subdomain range

[Alexander Bokovoy]

On Thu, 13 Mar 2014, Petr Spacek wrote:
>On 13.3.2014 13:20, Martin Kosek wrote:
>>On 03/13/2014 01:10 PM, Alexander Bokovoy wrote:
>>>On Thu, 13 Mar 2014, Martin Kosek wrote:
>>>>On 03/13/2014 01:01 PM, Alexander Bokovoy wrote:
>>>>>On Thu, 13 Mar 2014, Martin Kosek wrote:
>>>>>>On 03/13/2014 12:45 PM, Tomas Babej wrote:
>>>>>>>Hi,
>>>>>>>
>>>>>>>Changes the code in the idrange_del method to not only check for
>>>>>>>the root domains that match the SID in the IDRange, but for the
>>>>>>>SIDs of subdomains of trusts as well.
>>>>>>>
>>>>>>>https://fedorahosted.org/freeipa/ticket/4247
>>>>>>
>>>>>>This is a very complicated validation procedure IMO. Lot of subcommands,
>>>>>>lot of
>>>>>>LDAP searches.
>>>>>>
>>>>>>Why can't we do just one LDAP search with
>>>>>>- base api.env.container_trusts
>>>>>>- scope SUB
>>>>>>- filter (&(objectclass=ipaNTTrustedDomain)(ipanttrusteddomainsid=range_sid))
>>>>>>
>>>>>>When errors.NotFound is raised, we are OK. When it is not raised, we have a
>>>>>>problem.
>>>>>>
>>>>>>Wouldn't it be simpler?
>>>>>
>>>>>No. Please do not do optimization here. It is a code that is called very
>>>>>rarely and expressiveness is more important here than optimizing access
>>>>>to couple of entries in LDAP.
>>>>>
>>>>
>>>>I am not optimizing - I am actually making the validation much simpler. What is
>>>>more simple and straightforward?
>>>>
>>>>A) One ldap.find_entries call
>>>>B) A loop, numerous subcommands and LDAP searches
>>>
>>>So far I've been successful in keeping details on how trust objects are
>>>represented in LDAP hidden from the rest of the framework code by
>>>encapsulating it all in trust.py. The change you propose will
>>>make it leaking to idrange.py. If we start changing the structure (which
>>>is maintained by ipasam module, not the framework), we will have more
>>>maintenance problems with the code spread out.
>>
>>Ok, I can see your point, but I am still not sure it warrants that complicated
>>validation and a new dependency between commands. You cannot that easily change
>>the structure of the subdomains anyway as it would break all older servers
>>which expect the subdomains to be where they are.
>>
>>In plugins, we do LDAP searches in cases like this one quite regularly IMO, it
>>is not something unprecendented. And there is a good reason, simple LDAP call
>>is much easier and less error prone to changes in our frameworks than calling
>>subcommands. One glitch or a change in the subcommand can break not only the
>>subcommand, but it's all callers.
>
>Note that you can encapsulate the search proposed by Martin in a 
>function defined in trusts.py so it doesn't need to be implemented 
>idrange.py.
>
>It is just a matter of finding the right name for it.
I wanted to propose that as well.

We already have conditional import of ipaserver.dcerpc, we can use
ipalibs.plugins.trust import for that helper, just don't export it
through API.

-- 
/ Alexander Bokovoy