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

[Martin Babinsky]

On 11/06/2015 10:10 AM, Petr Spacek wrote:
> 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!
>

NACKed by commitee until further notice.

-- 
Martin^3 Babinsky