Alon Bar-Lev has posted comments on this change. Change subject: aaa: Remove dependency at builtin on Common config ......................................................................
Patch Set 8: (4 comments) http://gerrit.ovirt.org/#/c/27607/8/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/Utils.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/Utils.java: Line 21: public static void putDefaultsIfAbsent(Properties conf) { Line 22: { Line 23: Properties defaults = new Properties(); Line 24: try { Line 25: defaults.load(Utils.class.getResourceAsStream("/builtin/defaults.properties")); > not necessarily. but then you have conflict with other jars, no? Line 26: for (Map.Entry<Object, Object> entry : defaults.entrySet()) { Line 27: putIfAbsent(conf, (String) entry.getKey(), (String) entry.getValue()); Line 28: } Line 29: Line 39: } Line 40: } Line 41: Line 42: public static void handleApplicationInit(String applicationName) throws Exception { Line 43: if (firstCall.compareAndSet(true, false)) { > I use atomic boolean. but nobody waits. Line 44: if (applicationName.equals(Base.ApplicationNames.OVIRT_ENGINE_AAA_EXTENSION_TOOL)) { Line 45: if (System.getProperty("java.security.auth.login.config") == null) { Line 46: InputStream stream = Utils.class.getResourceAsStream("/extensions-tester/jaas.conf"); Line 47: File file = File.createTempFile("jaas", "conf"); Line 60: if (!krb5File.exists()) { Line 61: throw new Exception(String.format("Failed loading kerberos settings from File %1$s.", Line 62: krb5File.getAbsolutePath())); Line 63: } else { Line 64: System.setProperty("java.security.krb5.conf", krb5File.getAbsolutePath()); > not sure i understood 2nd line, please elaborate. if you put at engine config something like: ENGINE_PROPERTIES="${ENGINE_PREOPRTIES} java.security.krb5.conf=\"${ENGINE_ETC}/...\"" then you can drop it from here... but you will need it when running as standalone Line 65: } Line 66: Configuration.getConfiguration().refresh(); Line 67: DirectoryManager.setObjectFactoryBuilder(new DirectoryContextFactoryBuilder()); Line 68: } http://gerrit.ovirt.org/#/c/27607/8/backend/manager/tools/src/main/resources/extensions-tester/jaas.conf File backend/manager/tools/src/main/resources/extensions-tester/jaas.conf: Line 1: EngineKerberosAuth { Line 2: com.sun.security.auth.module.Krb5LoginModule required client=TRUE; Line 3: }; > Not sure, why not? i should define where my jaas file is, and this is the j but it should be at the builtin not at the tool... no? we can have the tool initialize jaas context for us... but then you can just put a file at /usr/share/ovirt-engine/conf/jaas.conf with the oVirtKerb and oVirtKerbDebug and always have this available, so you can drop the specific logic from builtin extension initialization. but decide, either the tool establishes the context (similar to jboss), or the extension. -- To view, visit http://gerrit.ovirt.org/27607 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1384a99f73ab605b61bce8dcdfd63e222b0001fa Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches