Alon Bar-Lev has posted comments on this change. Change subject: aaa: Extensions tester tool ......................................................................
Patch Set 7: (14 comments) missing change in Makefile to create the symlink at bin http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsTool.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsTool.java: Line 70: System.out.print("Please enter user: "); Line 71: user = System.console().readLine(); Line 72: } Line 73: Line 74: if (args.contains(ExtensionsToolArguments.ARG_PASSWORD_ENV_KEY)) { startswith? Line 75: password = System.getenv(args.get(ExtensionsToolArguments.ARG_PASSWORD_ENV_KEY)); Line 76: } else if (args.contains(ExtensionsToolArguments.ARG_PASSWORD_FILE)) { Line 77: try (BufferedReader reader = new BufferedReader(new InputStreamReader( new FileInputStream(args.get(ExtensionsToolArguments.ARG_PASSWORD_FILE))))) { Line 78: password = reader.readLine(); Line 76: } else if (args.contains(ExtensionsToolArguments.ARG_PASSWORD_FILE)) { Line 77: try (BufferedReader reader = new BufferedReader(new InputStreamReader( new FileInputStream(args.get(ExtensionsToolArguments.ARG_PASSWORD_FILE))))) { Line 78: password = reader.readLine(); Line 79: } Line 80: } else { else should be exception... check pass: explicitly. Line 81: System.out.print("Please enter password: "); Line 82: password = new String(System.console().readPassword()); Line 83: } Line 84: http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolArguments.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolArguments.java: Line 29: // Auth arguments Line 30: public static final String ARG_USER = "--user"; Line 31: public static final String ARG_AUTHN_NAME = "--authn"; Line 32: public static final String ARG_PASSWORD_FILE = "--password-file"; Line 33: public static final String ARG_PASSWORD_ENV_KEY = "--password-env-key"; please use the env: file: pass: prefixes of password value and have single --password argument Line 34: Line 35: // Query arguments Line 36: public static final String ARG_AUTHZ_NAME = "--authz"; Line 37: public static final String ARG_QUERY_ENTITY = "--query-entity"; http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolExecutor.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/ExtensionsToolExecutor.java: Line 24: System.out.println(t.getMessage()); Line 25: System.exit(1); Line 26: } Line 27: Line 28: if (testerArgs.contains(ACTION_HELP)) { not sure I understand how you check help at the end... Line 29: testerArgs.printHelp(); Line 30: System.exit(0); Line 31: } Line 32: } http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/modules/module.xml File backend/manager/extension-tool/src/main/modules/module.xml: Line 12: <module name="org.apache.commons.collections"/> Line 13: <module name="org.apache.commons.configuration"/> Line 14: <module name="org.apache.commons.lang"/> Line 15: <module name="org.apache.commons.logging"/> Line 16: <module name="org.apache.log4j"/> you do not need log4j, please make sure you need all the above Line 17: <module name="org.ovirt.engine.api.ovirt-engine-extensions-api"/> Line 18: <module name="org.ovirt.engine.core.aaa"/> Line 19: <module name="org.ovirt.engine.core.common"/> Line 20: <module name="org.ovirt.engine.core.compat"/> Line 22: <module name="org.ovirt.engine.core.utils"/> Line 23: <module name="org.ovirt.engine.core.uutils"/> Line 24: <module name="org.postgresql"/> Line 25: <module name="org.snmp4j"/> Line 26: <module name="sun.jdk"/> you do not need most of the above Line 27: </dependencies> http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/modules/org/ovirt/engine/core/extension-tool/main/module.xml File backend/manager/extension-tool/src/main/modules/org/ovirt/engine/core/extension-tool/main/module.xml: Line 9: <dependencies> Line 10: <module name="org.ovirt.engine.api.ovirt-engine-extensions-api"/> Line 11: <module name="org.ovirt.engine.core.extensions-manager"/> Line 12: <module name="org.ovirt.engine.core.utils"/> Line 13: <module name="sun.jdk"/> you do not need the above two Line 14: </dependencies> http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/resources/extensions-tool/help.properties File backend/manager/extension-tool/src/main/resources/extensions-tool/help.properties: Line 1: help.usage=usage: extensions-tool <action> [<args>] please do not add man pages we should be able to generate these from the usage --help should be sufficient for now. Line 2: Line 3: help.actions=\ Line 4: Available actions:\ Line 5: \n\tauth-sequence runs an authentication sequence (authenticate, fetch principal, logout)\ http://gerrit.ovirt.org/#/c/27814/7/backend/manager/extension-tool/src/main/resources/extensions-tool/log4j.xml File backend/manager/extension-tool/src/main/resources/extensions-tool/log4j.xml: Line 12: command line argument is specified. Line 13: --> Line 14: <appender-ref ref="null-appender" /> Line 15: </root> Line 16: </log4j:configuration> you are not using log4j no need to add http://gerrit.ovirt.org/#/c/27814/7/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthzUtils.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthzUtils.java: please do not use this in tool, and if change is required, please move it to separate patch. Line 1: package org.ovirt.engine.core.aaa; Line 2: Line 3: import java.util.ArrayList; Line 4: import java.util.Arrays; http://gerrit.ovirt.org/#/c/27814/7/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/SearchQueryParsingUtils.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/SearchQueryParsingUtils.java: please do not use this in tool, and if change is required, please move it to separate patch. Line 1: package org.ovirt.engine.core.aaa; Line 2: Line 3: import java.util.ArrayList; Line 4: import java.util.Arrays; http://gerrit.ovirt.org/#/c/27814/7/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 594: %{engine_jboss_modules}/org/ovirt/engine/api/ovirt-engine-extensions-api/main/ovirt-engine-extensions-api.jar Line 595: %{engine_jboss_modules}/org/ovirt/engine/core/aaa/main/aaa.jar Line 596: %{engine_jboss_modules}/org/ovirt/engine/core/common/main/common.jar Line 597: %{engine_jboss_modules}/org/ovirt/engine/core/compat/main/compat.jar Line 598: %{engine_jboss_modules}/org/ovirt/engine/core/extension-tool/main/extension-tool.jar please sort() Line 599: %{engine_jboss_modules}/org/ovirt/engine/core/dal/main/dal.jar Line 600: %{engine_jboss_modules}/org/ovirt/engine/core/extensions-manager/main/extensions-manager.jar Line 601: %{engine_jboss_modules}/org/ovirt/engine/core/searchbackend/main/searchbackend.jar Line 602: %{engine_jboss_modules}/org/ovirt/engine/core/tools/main/tools.jar http://gerrit.ovirt.org/#/c/27814/7/packaging/bin/ovirt-engine-extensions-tool.sh File packaging/bin/ovirt-engine-extensions-tool.sh: Line 19: # Line 20: Line 21: exec "${JAVA_HOME}/bin/java" \ Line 22: -Djboss.modules.write-indexes=false \ Line 23: -Djava.security.auth.login.config="${ENGINE_USR}/conf/jaas.conf" kerneros? Line 24: -jar "${JBOSS_HOME}/jboss-modules.jar" \ Line 25: -dependencies org.ovirt.engine.core.extension-tool \ Line 26: -class org.ovirt.engine.exttool.ExtensionsToolExecutor \ Line 27: -cp "${ENGINE_USR}/conf/commons-logging.properties" Line 23: -Djava.security.auth.login.config="${ENGINE_USR}/conf/jaas.conf" Line 24: -jar "${JBOSS_HOME}/jboss-modules.jar" \ Line 25: -dependencies org.ovirt.engine.core.extension-tool \ Line 26: -class org.ovirt.engine.exttool.ExtensionsToolExecutor \ Line 27: -cp "${ENGINE_USR}/conf/commons-logging.properties" set using -Djava.util.logging.config.file ? also this should be specific to tool... no? -- To view, visit http://gerrit.ovirt.org/27814 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ea2f9c62ced5bdd3801c9f6d8087a35e3c21886 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
