[Freeipa-devel] [PATCH 0325] Add Domain Level feature

[Tomas Babej]

On 05/22/2015 12:36 PM, Petr Vobornik wrote:
> On 05/22/2015 07:08 AM, Jan Cholasta wrote:
>> Dne 21.5.2015 v 18:18 Tomas Babej napsal(a):
>>>
>>>
>>> On 05/19/2015 04:07 PM, Tomas Babej wrote:
>>>>
>>>>
>>>> On 05/19/2015 03:59 PM, Martin Kosek wrote:
>>>>> On 05/19/2015 03:56 PM, Tomas Babej wrote:
>>>>>>
>>>>>> On 05/19/2015 03:51 PM, Martin Kosek wrote:
>>>>>>> On 05/19/2015 03:49 PM, Ludwig Krispenz wrote:
>>>>>>>> On 05/19/2015 03:36 PM, Martin Kosek wrote:
>>>>>>>>> On 05/19/2015 03:22 PM, Tomas Babej wrote:
>>>>>>>>> ...
>>>>>>>>>>> 3) Domain level is just a single integer and it should be
>>>>>>>>>>> treated as such,
>>>>>>>>>>> there's no need for an LDAPObject plugin and other unnecessary
>>>>>>>>>>> complexities.
>>>>>>>>>>> The implemetation could be as simple as (from top of my head,
>>>>>>>>>>> untested):
>>>>>>>>>> That's right, I also considered this approach, but as far as I
>>>>>>>>>> know you do
>>>>>>>>>> not
>>>>>>>>>> get the permission handling for the global DomainLevel entry
>>>>>>>>>> otherwise.
>>>>>>>>>>
>>>>>>>>>> Ludwig, I changed the path for the global entry to 
>>>>>>>>>> cn=DomainLevel.
>>>>>>>>> I know this particular DN was added to the design by Simo, but
>>>>>>>>> why do we want
>>>>>>>>> to use CamelCase with LDAP object?
>>>>>>>>>
>>>>>>>>> Wouldn't "cn=Domain Level,cn=ipa,cn=etc,SUFFIX" be a better place
>>>>>>>>> for it? This
>>>>>>>>> is the last time we can change it, so I am asking now. Then, we
>>>>>>>>> will be stuck
>>>>>>>>> with this DN forever.
>>>>>>>> I don't mind using ""cn=Domain Level" ,
>>>>>>>>
>>>>>>>> but where does the entry live, here you say
>>>>>>>>
>>>>>>>> cn=Domain Level,cn=ipa,cn=etc,SUFFIX"
>>>>>>>>
>>>>>>>> and in the design page it is:
>>>>>>>>
>>>>>>>> cn=DomainLevel,cn=etc,SUFFIX
>>>>>>>>
>>>>>>>> The current version of the topology plugin is looking for
>>>>>>>>
>>>>>>>> cn=DomainLevel,cn=ipa,cn=etc,SUFFIX"
>>>>>>>> but I want to change it to do a search on
>>>>>>>> objectclass=ipaDomainLevelConfig
>>>>>>> I see - we all need to unify the location apparently. I updated the
>>>>>>> design page
>>>>>>> to use "cn=Domain Level,cn=ipa,cn=etc,SUFFIX". Tomas, please send
>>>>>>> the updated
>>>>>>> patch set, it should be an extremely simple change :-)
>>>>>> I prefer the ipa parent and the space in the name, so I'm glad we
>>>>>> could agree
>>>>>> on this without much bikeshedding.
>>>>>>
>>>>>> Updated patch attaced.
>>>>>>
>>>>>> Tomas
>>>>>>
>>>>>>
>>>>> I still see
>>>>>
>>>>> +# Create default Domain Level entry if it does not exist
>>>>> +dn: cn=DomainLevel,cn=ipa,cn=etc,$SUFFIX
>>>>> +default: objectClass: top
>>>>> +default: objectClass: nsContainer
>>>>> +default: objectClass: ipaDomainLevelConfig
>>>>> +default: ipaDomainLevel: 0
>>>>>
>>>>> ...
>>>>
>>>> Right, the space eluded me there, thanks for the catch.
>>>>
>>>> Tomas
>>>
>>> A new iteration of the patch, including the server-side checks for the
>>> installers.
>>>
>>> Tomas
>>
>> 1) https://www.redhat.com/archives/freeipa-devel/2015-May/msg00228.html
>> - I still don't agree that the plugin should be based on LDAPObject.
>
> On the other hand, with LDAPObject base, Web UI for this feature is 
> much more simpler because it can rely on existing conventions.

IMHO we can swap the approach in a later patch, if we decide it's 
necessary. It does not block or relate to other features much.

>
>>
>> 2) Use api domainlevel-show call to get the current domain level in
>> ipa-replica-install instead of duplicating the code.
I chose the former approach since the domainlevel_show command doesn't 
need to be available, but yeah, this can be properly detected and worked 
around too. Fixed.

>>
>> 3) Set the domain level in DSInstance.create_instance instead of a
>> separate call in ipa-server-install. It should be done about the same
>> time as the master entry is added.
>>
>> 4) I think the option should be named --domain-level (with a dash), for
>> consistency.
>>
>
>
All other issues fixed.

Updated patch atttached.

Tomas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0325-6-Add-Domain-Level-feature.patch
Type: text/x-patch
Size: 21212 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150522/b61efeff/attachment.bin>