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

[Martin Babinsky]

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.

-- 
Martin^3 Babinsky
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0090.1-show-optionally-configured-components-in-server-find.patch
Type: text/x-patch
Size: 3421 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151026/f13aba5e/attachment.bin>