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

[Petr Spacek]

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.

-- 
Petr^2 Spacek