On 6.11.2015 10:00, Martin Babinsky wrote:
> On 11/02/2015 12:56 PM, Martin Babinsky wrote:
>> On 10/30/2015 11:53 AM, Martin Babinsky wrote:
>>> On 10/26/2015 01:41 PM, Martin Babinsky wrote:
>>>> On 10/22/2015 04:13 PM, Martin Basti wrote:
>>>>>
>>>>>
>>>>> On 22.10.2015 10:44, Martin Babinsky wrote:
>>>>>> https://fedorahosted.org/freeipa/ticket/5181
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Thank you for the patch.
>>>>>
>>>>> 1)
>>>>> +OPTIONAL_SERVICES = {
>>>>> +    'DNS',
>>>>> +    'CA',
>>>>> +    'KRA',
>>>>> +    'ADTRUST',
>>>>> +    'EXTID',
>>>>> +    'DNSKeyExporter',
>>>>> +    'DNSSEC',
>>>>> +    'DNSKeySync',
>>>>> +}
>>>>>
>>>>> This did not scale well, maybe we should improve it to use some general
>>>>> solution for whole IPA to distinct mandratory and optionl service,
>>>>> but I
>>>>> do not know how (or if it is possible)
>>>>>
>>>> Yes this does not scale well. After some playing around with relocating
>>>> the SERVICE_LIST object in 'ipaserver/install/service.py' I found out
>>>> that more refactoring would be needed to improve the layout and
>>>> availability of LDAP service names to both server and client code. I
>>>> have put the list of core services to ipalib/constants.py for now, and I
>>>> suggest to open a separate ticket for more general solution.
>>>>
>>>>> 2)
>>>>> +        search_filter=('(&(objectclass=ipaConfigObject)'
>>>>> +                       '(ipaConfigString=enabledService))')
>>>>>
>>>>> Common user cannot read ipaConfigString, so this will work only for
>>>>> admins, I do not see any limitations of access in code for other users.
>>>>>
>>>>
>>>> I think that you agreed with Petr^2 that this filter is OK. I left it as
>>>> it is but I have rewritten it as a call to ldap.make_filter to improve
>>>> readability and/or potential extensibility a bit.
>>>>
>>>>> 3)
>>>>> +        opt_components = [
>>>>> +            r['cn'][0] for r in result if r['cn'][0] in
>>>>> OPTIONAL_SERVICES
>>>>> +        ]
>>>>> Probably instead of indexing, you may use result.single_value['cn']
>>>>>
>>>>> Martin^2
>>>>
>>>> Attaching updated patch.
>>>>
>>>>
>>>>
>>> Self-NACK, I found a bug in the patch during work on topology management
>>> stuff.
>>>
>>
>> Attaching updated patch.
>>
>>
>>
> Bump for review.

I apologize for the delay, ACK!

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to