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

Reply via email to