[Freeipa-devel] [PATCH 0090] show optionally configured components in server-find/show command output

[Martin Babinsky]

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.

-- 
Martin^3 Babinsky