From 24b5c6772352d52d6475d2a63e91d0dae7fd332a Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Mon, 18 Feb 2019 15:33:44 +0200 Subject: [PATCH] hack/lib/util.sh: various shellcheck-reported cleanups. Use "command -v" instead of "which". Also remove the redirections, since "command -v" does not return an error message if the command isn't found. Also use "read -r" instead of "read" and quote variables properly. Do some error handling if "pushd" or "popd" fail. Read values properly into arrays. However, one shellcheck error is ignored in trap mechanism. The logic in trap_add function requires the trap command to be expanded when run. Just storing the variable into trap doesn't work. Add a shellcheck disable directive to ignore the error. An alternative to ignoring could be tricking shellcheck with: trap ''"${new_cmd}" "${trap_add_name}" --- hack/lib/util.sh | 49 ++++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/hack/lib/util.sh b/hack/lib/util.sh index 9d058c205e..a3ead2c2eb 100755 --- a/hack/lib/util.sh +++ b/hack/lib/util.sh @@ -39,7 +39,7 @@ kube::util::wait_for_url() { local times=${4:-30} local maxtime=${5:-1} - which curl >/dev/null || { + command -v curl >/dev/null || { kube::log::usage "curl must be installed" exit 1 } @@ -69,7 +69,7 @@ kube::util::trap_add() { local new_cmd # Grab the currently defined trap commands for this trap - existing_cmd=`trap -p "${trap_add_name}" | awk -F"'" '{print $2}'` + existing_cmd=$(trap -p "${trap_add_name}" | awk -F"'" '{print $2}') if [[ -z "${existing_cmd}" ]]; then new_cmd="${trap_add_cmd}" @@ -77,7 +77,11 @@ kube::util::trap_add() { new_cmd="${trap_add_cmd};${existing_cmd}" fi - # Assign the test + # Assign the test. Disable the shellcheck warning telling that trap + # commands should be single quoted to avoid evaluating them at this + # point instead evaluating them at run time. The logic of adding new + # commands to a single trap requires them to be evaluated right away. + # shellcheck disable=SC2064 trap "${new_cmd}" "${trap_add_name}" done } @@ -222,10 +226,10 @@ kube::util::gen-docs() { "${genyaml}" "${dest}/docs/yaml/kubectl/" # create the list of generated files - pushd "${dest}" > /dev/null + pushd "${dest}" > /dev/null || return 1 touch docs/.generated_docs find . -type f | cut -sd / -f 2- | LC_ALL=C sort > docs/.generated_docs - popd > /dev/null + popd > /dev/null || return 1 } # Removes previously generated docs-- we don't want to check them in. $KUBE_ROOT @@ -233,7 +237,7 @@ kube::util::gen-docs() { kube::util::remove-gen-docs() { if [ -e "${KUBE_ROOT}/docs/.generated_docs" ]; then # remove all of the old docs; we don't want to check them in. - while read file; do + while read -r file; do rm "${KUBE_ROOT}/${file}" 2>/dev/null || true done <"${KUBE_ROOT}/docs/.generated_docs" # The docs/.generated_docs file lists itself, so we don't need to explicitly @@ -322,7 +326,7 @@ kube::util::git_upstream_remote_name() { kube::util::create-fake-git-tree() { local -r target_dir=${1:-$(pwd)} - pushd "${target_dir}" >/dev/null + pushd "${target_dir}" >/dev/null || return 1 git init >/dev/null git config --local user.email "nobody@k8s.io" git config --local user.name "$0" @@ -331,7 +335,7 @@ kube::util::create-fake-git-tree() { if (( ${KUBE_VERBOSE:-5} >= 6 )); then kube::log::status "${target_dir} is now a git tree." fi - popd >/dev/null + popd >/dev/null || return 1 } # Checks whether godep restore was run in the current GOPATH, i.e. that all referenced repos exist @@ -344,7 +348,7 @@ kube::util::godep_restored() { local root local old_rev="" - while read path rev; do + while read -r path rev; do rev=$(echo "${rev}" | sed "s/['\"]//g") # remove quotes which are around revs sometimes if [[ "${rev}" == "${old_rev}" ]] && [[ "${path}" == "${root}"* ]]; then @@ -353,7 +357,7 @@ kube::util::godep_restored() { fi root="${path}" - while [ "${root}" != "." -a ! -d "${gopath}/src/${root}/.git" -a ! -d "${gopath}/src/${root}/.hg" ]; do + while [ "${root}" != "." ] && [ ! -d "${gopath}/src/${root}/.git" ] && [ ! -d "${gopath}/src/${root}/.hg" ]; do root=$(dirname "${root}") done if [ "${root}" == "." ]; then @@ -387,7 +391,7 @@ kube::util::ensure_clean_working_dir() { exit 1 fi | sed 's/^/ /' echo -e "\nCommit your changes in another terminal and then continue here by pressing enter." - read + read -r done 1>&2 } @@ -479,7 +483,8 @@ kube::util::has_changes() { local -r pattern=$2 local -r not_pattern=${3:-totallyimpossiblepattern} - local base_ref=$(kube::util::base_ref "${git_branch}") + local base_ref + base_ref=$(kube::util::base_ref "${git_branch}") echo "Checking for '${pattern}' changes against '${base_ref}'" # notice this uses ... to find the first shared ancestor @@ -499,11 +504,11 @@ kube::util::download_file() { local -r url=$1 local -r destination_file=$2 - rm ${destination_file} 2&> /dev/null || true + rm "${destination_file}" 2&> /dev/null || true for i in $(seq 5) do - if ! curl -fsSL --retry 3 --keepalive-time 2 ${url} -o ${destination_file}; then + if ! curl -fsSL --retry 3 --keepalive-time 2 "${url}" -o "${destination_file}"; then echo "Downloading ${url} failed. $((5-i)) retries left." sleep 1 else @@ -518,8 +523,7 @@ kube::util::download_file() { # Sets: # OPENSSL_BIN: The path to the openssl binary to use function kube::util::test_openssl_installed { - openssl version >& /dev/null - if [ "$?" != "0" ]; then + if ! openssl version >& /dev/null; then echo "Failed to run openssl. Please ensure openssl is installed" exit 1 fi @@ -602,7 +606,7 @@ function kube::util::write_client_kubeconfig { local api_port=$5 local client_id=$6 local token=${7:-} - cat < /dev/null + cat < /dev/null apiVersion: v1 kind: Config clusters: @@ -635,7 +639,8 @@ EOF # Determines if docker can be run, failures may simply require that the user be added to the docker group. function kube::util::ensure_docker_daemon_connectivity { - DOCKER=(docker ${DOCKER_OPTS}) + IFS=" " read -ra DOCKER <<< "${DOCKER_OPTS}" + DOCKER=(docker "${DOCKER[@]}") if ! "${DOCKER[@]}" info > /dev/null 2>&1 ; then cat <<'EOF' >&2 Can't connect to 'docker' daemon. please fix and retry. @@ -713,7 +718,7 @@ function kube::util::ensure-cfssl { fi mkdir -p "${cfssldir}" - pushd "${cfssldir}" > /dev/null + pushd "${cfssldir}" > /dev/null || return 1 echo "Unable to successfully run 'cfssl' from ${PATH}; downloading instead..." kernel=$(uname -s) @@ -742,7 +747,7 @@ function kube::util::ensure-cfssl { echo "Hint: export PATH=\$PATH:\$GOPATH/bin; go get -u github.com/cloudflare/cfssl/cmd/..." exit 1 fi - popd > /dev/null + popd > /dev/null || return 1 } # kube::util::ensure_dockerized @@ -766,7 +771,7 @@ function kube::util::ensure_dockerized { function kube::util::ensure-gnu-sed { if LANG=C sed --help 2>&1 | grep -q GNU; then SED="sed" - elif which gsed &>/dev/null; then + elif command -v gsed &>/dev/null; then SED="gsed" else kube::log::error "Failed to find GNU sed as sed or gsed. If you are on Mac: brew install gnu-sed." >&2 @@ -794,7 +799,7 @@ function kube::util::check-file-in-alphabetical-order { # kube::util::require-jq # Checks whether jq is installed. function kube::util::require-jq { - if ! which jq &>/dev/null; then + if ! command -v jq &>/dev/null; then echo "jq not found. Please install." 1>&2 return 1 fi