Alon Bar-Lev has posted comments on this change. Change subject: aaa: Change builtin authenticators and directories initialization ......................................................................
Patch Set 1: (11 comments) http://gerrit.ovirt.org/#/c/25741/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java: Line 11: Line 12: import org.ovirt.engine.core.aaa.AuthenticationProfileRepository; Line 13: import org.ovirt.engine.extensions.aaa.builtin.kerberosldap.KerberosManager; Line 14: import org.ovirt.engine.extensions.aaa.builtin.kerberosldap.UsersDomainsCacheManagerService; Line 15: import org.ovirt.engine.extensions.aaa.builtin.kerberosldap.LdapBrokerUtils; this should not be added Line 16: import org.ovirt.engine.core.bll.dwh.DwhHeartBeat; Line 17: import org.ovirt.engine.core.bll.gluster.GlusterJobsManager; Line 18: import org.ovirt.engine.core.bll.job.ExecutionHandler; Line 19: import org.ovirt.engine.core.bll.network.MacPoolManager; Line 56: // TODO: remove this later, and rely only on the custom and built in extensions directories configuration Line 57: Line 58: List<Properties> configurations = createInternalConfigurations(); Line 59: configurations.addAll(createKerberosLdapConfigurations()); Line 60: ExtensionManager.getInstance().load(configurations); better to call load twice... and drop the temp var Line 61: AuthenticationProfileRepository.getInstance(); Line 62: KerberosManager.getInstance(); Line 63: UsersDomainsCacheManagerService.getInstance().init(); Line 64: DbUserCacheManager.getInstance().init(); Line 108: Line 109: new DwhHeartBeat().init(); Line 110: } Line 111: Line 112: private List<Properties> createInternalConfigurations() { no need, please put it in configuration file at /usr Line 113: List<Properties> results = new ArrayList<>(); Line 114: Properties authConfig = new Properties(); Line 115: authConfig.put(ExtensionManager.CLASS, "org.ovirt.extensions.builtin.ldapkerberos.LdapKerberosAuthenticator"); Line 116: authConfig.put(ExtensionManager.PROVIDES, "org.ovirt.engine.core.authentication"); Line 135: Line 136: private List<Properties> createKerberosLdapConfigurations() { Line 137: Line 138: List<Properties> results = new ArrayList<>(); Line 139: for (String domain : LdapBrokerUtils.getDomainsList(true)) { you should go into vdc_options directly. Line 140: Properties authConfig = new Properties(); Line 141: authConfig.put(ExtensionManager.CLASS, "org.ovirt.extensions.builtin.ldapkerberos.LdapKerberosAuthenticator"); Line 142: authConfig.put(ExtensionManager.PROVIDES, "org.ovirt.engine.core.authentication"); Line 143: authConfig.put(ExtensionManager.ENABLED, true); Line 139: for (String domain : LdapBrokerUtils.getDomainsList(true)) { Line 140: Properties authConfig = new Properties(); Line 141: authConfig.put(ExtensionManager.CLASS, "org.ovirt.extensions.builtin.ldapkerberos.LdapKerberosAuthenticator"); Line 142: authConfig.put(ExtensionManager.PROVIDES, "org.ovirt.engine.core.authentication"); Line 143: authConfig.put(ExtensionManager.ENABLED, true); string? Line 144: authConfig.put(ExtensionManager.MODULE, "org.ovirt.engine.extensions.builtin"); Line 145: authConfig.put(ExtensionManager.NAME, String.format("builtin-authn-%1$s", domain)); Line 146: authConfig.put("org.ovirt.engine.aaa.authn.profile.name", domain); Line 147: authConfig.put("ovirt.engine.aaa.authn.authz.plugin", String.format("builitin-authz-%1$s", domain)); Line 149: Line 150: Properties dirConfig = new Properties(); Line 151: dirConfig.put(ExtensionManager.CLASS, "org.ovirt.extensions.builtin.ldapkerberos.LdapKerberosDirectory"); Line 152: dirConfig.put(ExtensionManager.PROVIDES, "org.ovirt.engine.core.authorization"); Line 153: dirConfig.put(ExtensionManager.ENABLED, true); string? Line 154: dirConfig.put(ExtensionManager.MODULE, "org.ovirt.engine.extensions.builtin"); Line 155: dirConfig.put(ExtensionManager.NAME, String.format("builtin-authz-%1$s", domain)); Line 156: dirConfig.put("org.ovirt.engine.aaa.authz.profile.name", domain); Line 157: results.add(authConfig); http://gerrit.ovirt.org/#/c/25741/1/backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionManager.java File backend/manager/modules/extensions-manager/src/main/java/org/ovirt/engine/core/extensions/mgr/ExtensionManager.java: Line 40 Line 41 Line 42 Line 43 Line 44 initialize here? private Map<Extension.ExtensionProperties, Object> context = new EnumMap<>(ExtensionProperties.class); Line 88: return (Properties) context.get(ExtensionProperties.CONFIGURATION); Line 89: } Line 90: Line 91: private void load(Properties props) { Line 92: enabled = props.get(ENABLED) != null ? Boolean.parseBoolean((String) props.get(ENABLED)) : true; just put strings... Line 93: context.put(ExtensionProperties.CONFIGURATION, props); Line 94: context.put(ExtensionProperties.NAME, props.getProperty(NAME)); Line 95: context.put(ExtensionProperties.PROVIDES, props.getProperty(PROVIDES)); Line 96: } Line 134: Line 135: public void load(Collection<Properties> configurations) { Line 136: if (configurations == null) { Line 137: return; Line 138: } hmmm... so why anyone will call it? better just to wait for null pointer exceptions... Line 139: for (Properties configuration : configurations) { Line 140: ExtensionEntry entry = Line 141: new ExtensionEntry(configuration); Line 142: ExtensionEntry alreadyLoaded = loadedEntries.get(entry.getName()); Line 137: return; Line 138: } Line 139: for (Properties configuration : configurations) { Line 140: ExtensionEntry entry = Line 141: new ExtensionEntry(configuration); why is the new line? :) Line 142: ExtensionEntry alreadyLoaded = loadedEntries.get(entry.getName()); Line 143: if (alreadyLoaded != null) { Line 144: throw new ConfigurationException(String.format("Could not load the configuration '%1$s'. %2%s", Line 145: entry.getName(), Line 139: for (Properties configuration : configurations) { Line 140: ExtensionEntry entry = Line 141: new ExtensionEntry(configuration); Line 142: ExtensionEntry alreadyLoaded = loadedEntries.get(entry.getName()); Line 143: if (alreadyLoaded != null) { this and bellow are common to both file and properties.... merge? Line 144: throw new ConfigurationException(String.format("Could not load the configuration '%1$s'. %2%s", Line 145: entry.getName(), Line 146: alreadyLoaded.file != null ? String.format("The already loaded file %1$s contains a configuration with the same name", Line 147: alreadyLoaded.file.getAbsolutePath()) -- To view, visit http://gerrit.ovirt.org/25741 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8513cb992c5becef7e83c04a8da8bc7f1622348 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@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
