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