Alon Bar-Lev has uploaded a new change for review. Change subject: packaging: remove bash usage from tools ......................................................................
packaging: remove bash usage from tools bash usage for scripts should be discouraged in favor of POSIX sh. remove the argument# validation as the java already validates it. Change-Id: I40064044a6da2b1088ee5d40eff874b9f8db66c3 Signed-off-by: Alon Bar-Lev <[email protected]> --- M packaging/bin/engine-config.sh M packaging/bin/engine-manage-domains.sh M packaging/bin/engine-prolog.sh.in M packaging/fedora/setup/engine-check-update 4 files changed, 152 insertions(+), 184 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/61/14661/1 diff --git a/packaging/bin/engine-config.sh b/packaging/bin/engine-config.sh index 76d7483..f7028a8 100755 --- a/packaging/bin/engine-config.sh +++ b/packaging/bin/engine-config.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh # # This script is designed to run the configuration tool. # The tool's configuration should be under the /etc directory. @@ -8,86 +8,71 @@ . "$(dirname "$(readlink -f "$0")")"/engine-prolog.sh usage () { - printf "engine-config: get/set/list configuration\n" - printf "USAGE:\n" - printf "\tengine-config ACTION [--cver=version] [-p | --properties=/path/to/alternate/property/file] [-c | --config=/path/to/alternate/config/file]\n" - printf "Where:\n" - printf "\tACTION action to perform, see details below\n" - printf "\tversion relevant configuration version to use.\n" - printf "\t-p, --properties= (optional) use the given alternate properties file.\n" - printf "\t-c, --config= (optional) use the given alternate configuration file.\n" - printf "\n" - printf "\tAvailable actions:\n" - printf "\t-l, --list\n" - printf "\t\tlist available configuration keys.\n" - printf "\t-a, --all\n" - printf "\t\tget all available configuration values.\n" - printf "\t-g key, --get=key [--cver=version]\n" - printf "\t\tget the value of the given key for the given version. If a version is not given, the values of all existing versions are returned.\n" - printf "\t-s key=val [--cver=version], --set key=val [--cver=version]\n" - printf "\t\tset the value of the given key for the given version. The cver version is required for this action only when the version is not 'general'.\n" - printf "\t-h, --help\n" - printf "\t\tdisplay this help and exit.\n" - printf "\n" - printf "### Notes: \n" - printf "### 1. Passwords: password can be set in interactive mode ie:\n" - printf "### engine-config -s PasswordEntry=interactive\n" - printf "### or via file with one of the following options:\n" - printf "### engine-config -s PasswordEntry --admin-pass-file=/tmp/mypass\n" - printf "### engine-config -s PasswordEntry=/tmp/mypass\n" - printf "### PasswordEntry varies between the different password options\n" - printf "### See engine-config -h <OptionName> for more specific details\n" - printf "### 2. In order for your change(s) to take effect,\n" - printf "### restart the oVirt engine service (using: 'service ovirt-engine restart').\n" - printf "################################################################################\n" + cat << __EOF__ +engine-config: get/set/list configuration +USAGE: + engine-config ACTION [--cver=version] [-p | --properties=/path/to/alternate/property/file] [-c | --config=/path/to/alternate/config/file] +Where: + ACTION action to perform, see details below + version relevant configuration version to use. + -p, --properties= (optional) use the given alternate properties file. + -c, --config= (optional) use the given alternate configuration file. - return 0 + Available actions: + -l, --list + list available configuration keys. + -a, --all + get all available configuration values. + -g key, --get=key [--cver=version] + get the value of the given key for the given version. If a version is not given, the values of all existing versions are returned. + -s key=val [--cver=version], --set key=val [--cver=version] + set the value of the given key for the given version. The cver version is required for this action only when the version is not 'general'. + -h, --help + display this help and exit. + +### Notes: +### 1. Passwords: password can be set in interactive mode ie: +### engine-config -s PasswordEntry=interactive +### or via file with one of the following options: +### engine-config -s PasswordEntry --admin-pass-file=/tmp/mypass +### engine-config -s PasswordEntry=/tmp/mypass +### PasswordEntry varies between the different password options +### See engine-config -h <OptionName> for more specific details +### 2. In order for your change(s) to take effect, +### restart the oVirt engine service (using: 'service ovirt-engine restart'). +################################################################################ +__EOF__ + return 0 } -# Support alternate configuration file +# TODO: +# why do we need CONF_FILE here? +# we do not use any vairable CONF_FILE="${ENGINE_ETC}/engine-config/engine-config.conf" +ARGS="" -found=0 -for ((i=1; i<=$# && ! found; i++)) -do - var="${!i}" - next=$[$i+1] - next="${!next}" - - if [ "-c" == "${var}" ]; then - CONF_FILE="${next}" - found=1 - elif [ `echo "${var}" | grep -i '\-\-config\='` ]; then - candidate=${var#--config=} - if [ -s $candidate ]; then - CONF_FILE=$candidate - else - CONF_FILE= - fi - found=1 - fi +while [ -n "$1" ]; do + ARGS="${ARGS} \"$(echo "$1" | sed 's/"/\\"/g')\"" + x="$1" + v="${x#*=}" + shift + case "${x}" in + -c) + CONF_FILE="$1" + shift + ;; + -configFile=*) + CONF_FILE="${v}" + ;; + -h|-help|--help) + usage + exit 0 + ;; + esac done -if [ ${found} -eq 1 -a "x" == "x$CONF_FILE" ]; then - die "Error! Alternate conf file '$candidate' is empty or does not exist!\n" -fi - -if [ ! -s $CONF_FILE ]; then - CONF_FILE=./engine-config.conf -fi -. $CONF_FILE - -# Check basic argument constraints -if [ "$#" -gt 8 -o "$#" -lt 1 ]; then - usage - die "Error: wrong argument number: $#.\n" -fi - - -if [ "$1" == "--help" -o "$1" == "-h" -a "$#" -eq 1 ]; then - usage - exit 0 -fi +[ -s "${CONF_FILE}" ] || die "Configuration file '${CONF_FILE}' is either empty or does not exist" +. "${CONF_FILE}" # # Add this option to the java command line to enable remote debugging in @@ -100,11 +85,10 @@ # not possible to debug the execution of the main method. # -# Run! -exec "${JAVA_HOME}/bin/java" \ - -Dlog4j.configuration="file:${ENGINE_ETC}/engine-config/log4j.xml" \ - -Djboss.modules.write-indexes=false \ - -jar "${JBOSS_HOME}/jboss-modules.jar" \ - -dependencies org.ovirt.engine.core.tools \ - -class org.ovirt.engine.core.config.EngineConfig \ - "$@" +eval exec "${JAVA_HOME}/bin/java" \ + -Dlog4j.configuration="file:${ENGINE_ETC}/engine-config/log4j.xml" \ + -Djboss.modules.write-indexes=false \ + -jar "${JBOSS_HOME}/jboss-modules.jar" \ + -dependencies org.ovirt.engine.core.tools \ + -class org.ovirt.engine.core.config.EngineConfig \ + ${ARGS} diff --git a/packaging/bin/engine-manage-domains.sh b/packaging/bin/engine-manage-domains.sh index 7ab96d5..51c9f19 100755 --- a/packaging/bin/engine-manage-domains.sh +++ b/packaging/bin/engine-manage-domains.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh # # This script is designed to run the manage domains utility. # The tool's configuration should be under the /etc directory. @@ -7,95 +7,85 @@ # Load the prolog: . "$(dirname "$(readlink -f "$0")")"/engine-prolog.sh -CONF_DIR="${ENGINE_ETC}/engine-manage-domains" -CONF_FILE="${CONF_DIR}/engine-manage-domains.conf" +usage() { + cat << __EOF__ +engine-manage-domains: add/edit/delete/validate/list domains +USAGE: + engine-manage-domains -action=ACTION [-domain=DOMAIN -provider=PROVIDER -user=USER -passwordFile=PASSWORD_FILE -interactive -configFile=PATH -addPermissions -forceDelete -ldapServers=LDAP_SERVERS] -report +Where: + ACTION action to perform (add/edit/delete/validate/list). See details below. + DOMAIN (mandatory for add, edit and delete) the domain you wish to perform the action on. + PROVIDER (mandatory for add, optional for edit) the LDAP provider type of server used for the domain. Among the supported providers IPA,RHDS and ActiveDirectory. + USER (optional for edit, mandatory for add) the domain user. + PASSWORD_FILE (optional for edit, mandatory for add) a file containing the password in the first line. + interactive alternative for using -passwordFile - read the password interactively. + PATH (optional) use the given alternate configuration file. + LDAP_SERVERS (optional) a comma delimited list of LDAP servers to be set to the domain. -found=0 -for ((i=1; i<=$# && ! found; i++)) -do - var="${!i}" - next=$[$i+1] - next="${!next}" - - if [ "-c" == "${var}" ]; then - CONF_FILE="${next}" - found=1 - elif [ `echo "${var}" | grep -i '\-configFile\='` ]; then - candidate=${var#-configFile=} - if [ -s $candidate ]; then - CONF_FILE=$candidate - else - die "Error: Alternate conf file $candidate is either empty or does not exist" - fi - found=1 - fi -done - -if [ ! -s $CONF_FILE ]; then - die "Error: Configuration file $CONF_FILE is either empty or does not exist" -fi - - -. $CONF_FILE - - -usage () { - printf "engine-manage-domains: add/edit/delete/validate/list domains\n" - printf "USAGE:\n" - printf "\tengine-manage-domains -action=ACTION [-domain=DOMAIN -provider=PROVIDER -user=USER -passwordFile=PASSWORD_FILE -interactive -configFile=PATH -addPermissions -forceDelete -ldapServers=LDAP_SERVERS] -report\n" - printf "Where:\n" - printf "\tACTION action to perform (add/edit/delete/validate/list). See details below.\n" - printf "\tDOMAIN (mandatory for add, edit and delete) the domain you wish to perform the action on.\n" - printf "\tPROVIDER (mandatory for add, optional for edit) the LDAP provider type of server used for the domain. Among the supported providers IPA,RHDS and ActiveDirectory.\n" - printf "\tUSER (optional for edit, mandatory for add) the domain user.\n" - printf "\tPASSWORD_FILE (optional for edit, mandatory for add) a file containing the password in the first line.\n" - printf "\tinteractive alternative for using -passwordFile - read the password interactively.\n" - printf "\tPATH (optional) use the given alternate configuration file.\n" - printf "\tLDAP_SERVERS (optional) a comma delimited list of LDAP servers to be set to the domain.\n" - printf "\n" - printf "\tAvailable actions:\n" - printf "\tadd\n" - printf "\tExamples:\n" - printf "\t\t-action=add -domain=example.com -user=admin -provider=IPA -passwordFile=/tmp/.pwd\n" - printf "\t\t\tAdd a domain called example.com, using user admin with ldap server type IPA and read the password from /tmp/.pwd.\n" - printf "\t\t-action=edit -domain=example.com -provider=ActiveDirectory -passwordFile=/tmp/.new_password\n" - printf "\t\t\tEdit the domain example.com, using another password file and updated the provider type to Active Directory.\n" - printf "\t\t-action=delete -domain=example.com [-forceDelete]\n" - printf "\t\t\tDelete the domain example.com.\n" - printf "\t\t-forceDelete Optional parameter used in combination with -action=delete to skip confirmation of operation.\n" - printf "\t\t\tDefault behaviour is prompt for confirmation of delete.\n" - printf "\t\t-action=validate\n" - printf "\t\t\tValidate the current configuration (go over all the domains, try to authenticate to each domain using the configured user/password.).\n" - printf "\t\t-report In combination with -action=validate will report all validation error, if occured.\n" - printf "\t\t\tDefault behaviour is to exit when a validation error occurs.\n" - printf "\t\t-addPermissions In combination with -action=add/edit will add engine superuser permissions to the user.\n" - printf "\t\t\tDefault behaviour is not to add permissions.\n" - printf "\t\t-action=list\n" - printf "\t\t\tLists the current configuration.\n" - printf "\t\t-h\n" - printf "\t\t\tShow this help.\n" - - return 0 + Available actions: + add + Examples: + -action=add -domain=example.com -user=admin -provider=IPA -passwordFile=/tmp/.pwd + Add a domain called example.com, using user admin with ldap server type IPA and read the password from /tmp/.pwd. + -action=edit -domain=example.com -provider=ActiveDirectory -passwordFile=/tmp/.new_password + Edit the domain example.com, using another password file and updated the provider type to Active Directory. + -action=delete -domain=example.com [-forceDelete] + Delete the domain example.com. + -forceDelete Optional parameter used in combination with -action=delete to skip confirmation of operation. + Default behaviour is prompt for confirmation of delete. + -action=validate + Validate the current configuration (go over all the domains, try to authenticate to each domain using the configured user/password.). + -report In combination with -action=validate will report all validation error, if occured. + Default behaviour is to exit when a validation error occurs. + -addPermissions In combination with -action=add/edit will add engine superuser permissions to the user. + Default behaviour is not to add permissions. + -action=list + Lists the current configuration. + -h + Show this help. +__EOF__ + return 0 } -if [ "$#" -gt 7 -o "$#" -lt 1 ]; then - usage - exit 1 -fi +# TODO: +# why do we need CONF_FILE here? +# we do not use any vairable +CONF_DIR="${ENGINE_ETC}/engine-manage-domains" +CONF_FILE="${CONF_DIR}/engine-manage-domains.conf" +ARGS="" +while [ -n "$1" ]; do + ARGS="${ARGS} \"$(echo "$1" | sed 's/"/\\"/g')\"" + x="$1" + v="${x#*=}" + shift + case "${x}" in + -c) + CONF_FILE="$1" + shift + ;; + -configFile=*) + CONF_FILE="${v}" + ;; + -h|-help|--help) + usage + exit 0 + ;; + esac +done -if [ "$1" == "--help" -o "$1" == "-h" ]; then - usage - exit 0 -fi +[ -s "${CONF_FILE}" ] || die "Configuration file '${CONF_FILE}' is either empty or does not exist" +. "${CONF_FILE}" -PROPERTIES_FILE=`mktemp` - -if [ ! -e $PROPERTIES_FILE ]; then - die "Error: Temporary properties file cannot be created\n" -fi - -cat << EOF > $PROPERTIES_FILE +# TODO: +# what is the benefit of creating this here +# and not at java code? +PROPERTIES_FILE="$(mktemp)" || die "Temporary properties file cannot be created" +cleanup() { + rm -fr "${PROPERTIES_FILE}" +} +trap cleanup 0 +cat << __EOF__ > "${PROPERTIES_FILE}" AdUserName= AdUserPassword.type=CompositePassword LDAPSecurityAuthentication= @@ -104,7 +94,7 @@ LdapServers= LDAPProviderTypes= LDAPServerPort= -EOF +__EOF__ # # Add this option to the java command line to enable remote debugging in @@ -117,17 +107,10 @@ # not possible to debug the execution of the main method. # -"${JAVA_HOME}/bin/java" \ - -Dlog4j.configuration="file:${ENGINE_ETC}/engine-manage-domains/log4j.xml" \ - -Djboss.modules.write-indexes=false \ - -jar "${JBOSS_HOME}/jboss-modules.jar" \ - -dependencies org.ovirt.engine.core.tools \ - -class org.ovirt.engine.core.domains.ManageDomains \ - "$@" -propertiesFile=$PROPERTIES_FILE - -RET_VAL=$? - -rm $PROPERTIES_FILE - -exit $RET_VAL - +eval exec "${JAVA_HOME}/bin/java" \ + -Dlog4j.configuration="file:${ENGINE_ETC}/engine-manage-domains/log4j.xml" \ + -Djboss.modules.write-indexes=false \ + -jar "${JBOSS_HOME}/jboss-modules.jar" \ + -dependencies org.ovirt.engine.core.tools \ + -class org.ovirt.engine.core.domains.ManageDomains \ + ${ARGS} -propertiesFile="${PROPERTIES_FILE}" diff --git a/packaging/bin/engine-prolog.sh.in b/packaging/bin/engine-prolog.sh.in index b5243b5..b5a2b91 100644 --- a/packaging/bin/engine-prolog.sh.in +++ b/packaging/bin/engine-prolog.sh.in @@ -1,6 +1,7 @@ # Print an error message to stderr and exit with an error code: die() { - printf >&2 "$@\n" + local m="$1" + echo "FATAL: ${m}" >&2 exit 1 } diff --git a/packaging/fedora/setup/engine-check-update b/packaging/fedora/setup/engine-check-update index d9f2c43..3f17250 100755 --- a/packaging/fedora/setup/engine-check-update +++ b/packaging/fedora/setup/engine-check-update @@ -1,2 +1,2 @@ -#!/bin/bash -/usr/share/ovirt-engine/scripts/engine-upgrade.py --check-update +#!/bin/sh +exec /usr/share/ovirt-engine/scripts/engine-upgrade.py --check-update -- To view, visit http://gerrit.ovirt.org/14661 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I40064044a6da2b1088ee5d40eff874b9f8db66c3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alon Bar-Lev <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
