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.

--
Martin^3 Babinsky

--
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