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

@@ -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 ! openssl version >& /dev/null; then
-    if [ "$?" != "0" ]; 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 <<EOF | ${sudo} tee "${dest_dir}"/${client_id}.kubeconfig > /dev/null
+    cat <<EOF | ${sudo} tee "${dest_dir}"/"${client_id}".kubeconfig > /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