From faf27331bcb19b9723e59a3d69c41d9efa41104d Mon Sep 17 00:00:00 2001 From: Guangming Wang Date: Wed, 4 Sep 2019 23:45:38 +0800 Subject: [PATCH] fix shell checks errors in cluster/common.sh handle local variable in cluster/common.sh add comment for shellcheck disable line, and fix error change fix error: "read" parameters -r position update function comment refactor function find-release-tars rm lines in .shellcheck_failures Continue to cleanup common.sh --- cluster/common.sh | 103 ++++++++++++++++++++++++++------------ hack/.shellcheck_failures | 1 - 2 files changed, 72 insertions(+), 32 deletions(-) diff --git a/cluster/common.sh b/cluster/common.sh index 72353568a3b..d0570ed9afc 100755 --- a/cluster/common.sh +++ b/cluster/common.sh @@ -20,7 +20,7 @@ set -o errexit set -o nounset set -o pipefail -KUBE_ROOT=$(cd $(dirname "${BASH_SOURCE[0]}")/.. && pwd) +KUBE_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")"/.. && pwd) DEFAULT_KUBECONFIG="${HOME:-.}/.kube/config" @@ -29,15 +29,21 @@ source "${KUBE_ROOT}/hack/lib/util.sh" # # NOTE This must match the version_regex in build/common.sh # kube::release::parse_and_validate_release_version() -KUBE_RELEASE_VERSION_REGEX="^v(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)(-([a-zA-Z0-9]+)\\.(0|[1-9][0-9]*))?$" -KUBE_RELEASE_VERSION_DASHED_REGEX="v(0|[1-9][0-9]*)-(0|[1-9][0-9]*)-(0|[1-9][0-9]*)(-([a-zA-Z0-9]+)-(0|[1-9][0-9]*))?" +# +# KUBE_RELEASE_VERSION_REGEX is used in hack/get-build.sh and cluster/gce/util.sh and KUBE_RELEASE_VERSION_DASHED_REGEX is used in cluster/gce/util.sh, +# make sure to remove these vars when not used anymore +export KUBE_RELEASE_VERSION_REGEX="^v(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)(-([a-zA-Z0-9]+)\\.(0|[1-9][0-9]*))?$" +export KUBE_RELEASE_VERSION_DASHED_REGEX="v(0|[1-9][0-9]*)-(0|[1-9][0-9]*)-(0|[1-9][0-9]*)(-([a-zA-Z0-9]+)-(0|[1-9][0-9]*))?" # KUBE_CI_VERSION_REGEX matches things like "v1.2.3-alpha.4.56+abcdefg" This # # NOTE This must match the version_regex in build/common.sh # kube::release::parse_and_validate_ci_version() -KUBE_CI_VERSION_REGEX="^v(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)-([a-zA-Z0-9]+)\\.(0|[1-9][0-9]*)(\\.(0|[1-9][0-9]*)\\+[-0-9a-z]*)?$" -KUBE_CI_VERSION_DASHED_REGEX="^v(0|[1-9][0-9]*)-(0|[1-9][0-9]*)-(0|[1-9][0-9]*)-([a-zA-Z0-9]+)-(0|[1-9][0-9]*)(-(0|[1-9][0-9]*)\\+[-0-9a-z]*)?" +# +# TODO: KUBE_CI_VERSION_REGEX is used in hack/get-build.sh and KUBE_CI_VERSION_DASHED_REGEX is used in cluster/gce/util.sh, +# make sure to remove these vars when not used anymore +export KUBE_CI_VERSION_REGEX="^v(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)-([a-zA-Z0-9]+)\\.(0|[1-9][0-9]*)(\\.(0|[1-9][0-9]*)\\+[-0-9a-z]*)?$" +export KUBE_CI_VERSION_DASHED_REGEX="^v(0|[1-9][0-9]*)-(0|[1-9][0-9]*)-(0|[1-9][0-9]*)-([a-zA-Z0-9]+)-(0|[1-9][0-9]*)(-(0|[1-9][0-9]*)\\+[-0-9a-z]*)?" # Generate kubeconfig data for the created cluster. # Assumed vars: @@ -93,17 +99,17 @@ function create-kubeconfig() { fi local user_args=() - if [[ ! -z "${KUBE_BEARER_TOKEN:-}" ]]; then + if [[ -n "${KUBE_BEARER_TOKEN:-}" ]]; then user_args+=( "--token=${KUBE_BEARER_TOKEN}" ) - elif [[ ! -z "${KUBE_USER:-}" && ! -z "${KUBE_PASSWORD:-}" ]]; then + elif [[ -n "${KUBE_USER:-}" && -n "${KUBE_PASSWORD:-}" ]]; then user_args+=( "--username=${KUBE_USER}" "--password=${KUBE_PASSWORD}" ) fi - if [[ ! -z "${KUBE_CERT:-}" && ! -z "${KUBE_KEY:-}" ]]; then + if [[ -n "${KUBE_CERT:-}" && -n "${KUBE_KEY:-}" ]]; then user_args+=( "--client-certificate=${KUBE_CERT}" "--client-key=${KUBE_KEY}" @@ -112,7 +118,7 @@ function create-kubeconfig() { fi KUBECONFIG="${KUBECONFIG}" "${kubectl}" config set-cluster "${CONTEXT}" "${cluster_args[@]}" - if [[ -n "${user_args[@]:-}" ]]; then + if [[ -n "${user_args[*]:-}" ]]; then KUBECONFIG="${KUBECONFIG}" "${kubectl}" config set-credentials "${CONTEXT}" "${user_args[@]}" fi KUBECONFIG="${KUBECONFIG}" "${kubectl}" config set-context "${CONTEXT}" --cluster="${CONTEXT}" --user="${CONTEXT}" @@ -124,7 +130,7 @@ function create-kubeconfig() { # If we have a bearer token, also create a credential entry with basic auth # so that it is easy to discover the basic auth password for your cluster # to use in a web browser. - if [[ ! -z "${KUBE_BEARER_TOKEN:-}" && ! -z "${KUBE_USER:-}" && ! -z "${KUBE_PASSWORD:-}" ]]; then + if [[ -n "${KUBE_BEARER_TOKEN:-}" && -n "${KUBE_USER:-}" && -n "${KUBE_PASSWORD:-}" ]]; then KUBECONFIG="${KUBECONFIG}" "${kubectl}" config set-credentials "${CONTEXT}-basic-auth" "--username=${KUBE_USER}" "--password=${KUBE_PASSWORD}" fi @@ -147,7 +153,8 @@ function clear-kubeconfig() { local kubectl="${KUBE_ROOT}/cluster/kubectl.sh" # Unset the current-context before we delete it, as otherwise kubectl errors. - local cc=$("${kubectl}" config view -o jsonpath='{.current-context}') + local cc + cc=$("${kubectl}" config view -o jsonpath='{.current-context}') if [[ "${cc}" == "${CONTEXT}" ]]; then "${kubectl}" config unset current-context fi @@ -173,11 +180,13 @@ function clear-kubeconfig() { function get-kubeconfig-basicauth() { export KUBECONFIG=${KUBECONFIG:-$DEFAULT_KUBECONFIG} - local cc=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.current-context}") - if [[ ! -z "${KUBE_CONTEXT:-}" ]]; then + local cc + cc=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.current-context}") + if [[ -n "${KUBE_CONTEXT:-}" ]]; then cc="${KUBE_CONTEXT}" fi - local user=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.contexts[?(@.name == \"${cc}\")].context.user}") + local user + user=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.contexts[?(@.name == \"${cc}\")].context.user}") get-kubeconfig-user-basicauth "${user}" if [[ -z "${KUBE_USER:-}" || -z "${KUBE_PASSWORD:-}" ]]; then @@ -211,7 +220,7 @@ function get-kubeconfig-user-basicauth() { # KUBE_USER # KUBE_PASSWORD function gen-kube-basicauth() { - KUBE_USER=admin + KUBE_USER='admin' KUBE_PASSWORD=$(python -c 'import string,random; print("".join(random.SystemRandom().choice(string.ascii_letters + string.digits) for _ in range(16)))') } @@ -228,11 +237,13 @@ function gen-kube-basicauth() { function get-kubeconfig-bearertoken() { export KUBECONFIG=${KUBECONFIG:-$DEFAULT_KUBECONFIG} - local cc=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.current-context}") - if [[ ! -z "${KUBE_CONTEXT:-}" ]]; then + local cc + cc=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.current-context}") + if [[ -n "${KUBE_CONTEXT:-}" ]]; then cc="${KUBE_CONTEXT}" fi - local user=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.contexts[?(@.name == \"${cc}\")].context.user}") + local user + user=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.contexts[?(@.name == \"${cc}\")].context.user}") KUBE_BEARER_TOKEN=$("${KUBE_ROOT}/cluster/kubectl.sh" config view -o jsonpath="{.users[?(@.name == \"${user}\")].user.token}") } @@ -245,7 +256,7 @@ function gen-kube-bearertoken() { } function load-or-gen-kube-basicauth() { - if [[ ! -z "${KUBE_CONTEXT:-}" ]]; then + if [[ -n "${KUBE_CONTEXT:-}" ]]; then get-kubeconfig-basicauth fi @@ -273,11 +284,11 @@ function load-or-gen-kube-basicauth() { # # Args: # $1 version string from command line -# Vars set: +# Vars set and exported for external reference: # KUBE_VERSION function set_binary_version() { if [[ "${1}" =~ "/" ]]; then - IFS='/' read -a path <<< "${1}" + IFS='/' read -r -a path <<< "${1}" if [[ "${path[0]}" == "release" ]]; then KUBE_VERSION=$(gsutil cat "gs://kubernetes-release/${1}.txt") else @@ -286,6 +297,7 @@ function set_binary_version() { else KUBE_VERSION=${1} fi + export KUBE_VERSION } # Search for the specified tarball in the various known output locations, @@ -317,7 +329,7 @@ function find-tar() { # # Assumed vars: # KUBE_ROOT -# Vars set: +# Vars set and exported: # NODE_BINARY_TAR # SERVER_BINARY_TAR # KUBE_MANIFESTS_TAR @@ -326,15 +338,35 @@ function find-release-tars() { if [[ -z "${SERVER_BINARY_TAR}" ]]; then exit 1 fi + export SERVER_BINARY_TAR + + local find_result if [[ "${NUM_WINDOWS_NODES}" -gt "0" ]]; then - NODE_BINARY_TAR=$(find-tar kubernetes-node-windows-amd64.tar.gz) + if NODE_BINARY_TAR=$(find-tar kubernetes-node-windows-amd64.tar.gz); then + find_result=0 + else + find_result=1 + fi + export NODE_BINARY_TAR fi # This tarball is used by GCI, Ubuntu Trusty, and Container Linux. KUBE_MANIFESTS_TAR= if [[ "${MASTER_OS_DISTRIBUTION:-}" == "trusty" || "${MASTER_OS_DISTRIBUTION:-}" == "gci" || "${MASTER_OS_DISTRIBUTION:-}" == "ubuntu" ]] || \ [[ "${NODE_OS_DISTRIBUTION:-}" == "trusty" || "${NODE_OS_DISTRIBUTION:-}" == "gci" || "${NODE_OS_DISTRIBUTION:-}" == "ubuntu" || "${NODE_OS_DISTRIBUTION:-}" == "custom" ]] ; then - KUBE_MANIFESTS_TAR=$(find-tar kubernetes-manifests.tar.gz) + if KUBE_MANIFESTS_TAR=$(find-tar kubernetes-manifests.tar.gz); then + find_result=0 + else + find_result=1 + fi + export KUBE_MANIFESTS_TAR + fi + + # the function result is used in function `verify-release-tars` + if [[ $find_result == 0 ]]; then + return 0 + else + return 1 fi } @@ -344,7 +376,8 @@ function find-release-tars() { # Optional vars: # GEN_ETCD_CA_CERT (CA cert encode with base64 and ZIP compression) # GEN_ETCD_CA_KEY (CA key encode with base64) -# +# ca_cert (require when GEN_ETCD_CA_CERT and GEN_ETCD_CA_KEY is set) +# ca_key (require when GEN_ETCD_CA_CERT and GEN_ETCD_CA_KEY is set) # If GEN_ETCD_CA_CERT or GEN_ETCD_CA_KEY is not specified, it will generates certs for CA. # # Args: @@ -426,7 +459,11 @@ EOF fi if [[ -n "${GEN_ETCD_CA_CERT}" && -n "${GEN_ETCD_CA_KEY}" ]]; then + # ca_cert and ca_key are optional external vars supplied in cluster/gce/util.sh, + # so it's ok to disable shellcheck here + # shellcheck disable=SC2154 echo "${ca_cert}" | base64 --decode | gunzip > ca.pem + # shellcheck disable=SC2154 echo "${ca_key}" | base64 --decode > ca-key.pem fi @@ -443,13 +480,13 @@ EOF ;; server) echo "Generate server certificates..." - echo '{"CN":"'${member_ip}'","hosts":[""],"key":{"algo":"ecdsa","size":256}}' \ + echo '{"CN":"'"${member_ip}"'","hosts":[""],"key":{"algo":"ecdsa","size":256}}' \ | ${CFSSL_BIN} gencert -ca=ca.pem -ca-key=ca-key.pem -config=ca-config.json -profile=server -hostname="${member_ip},127.0.0.1" - \ | ${CFSSLJSON_BIN} -bare "${prefix}" ;; peer) echo "Generate peer certificates..." - echo '{"CN":"'${member_ip}'","hosts":[""],"key":{"algo":"ecdsa","size":256}}' \ + echo '{"CN":"'"${member_ip}"'","hosts":[""],"key":{"algo":"ecdsa","size":256}}' \ | ${CFSSL_BIN} gencert -ca=ca.pem -ca-key=ca-key.pem -config=ca-config.json -profile=peer -hostname="${member_ip},127.0.0.1" - \ | ${CFSSLJSON_BIN} -bare "${prefix}" ;; @@ -459,6 +496,8 @@ EOF exit 2 esac + # the popd will access `directory stack`, no `real` parameters is actually needed + # shellcheck disable=SC2119 popd } @@ -478,7 +517,7 @@ function verify-kube-binaries() { # If KUBERNETES_SKIP_CONFIRM is set to y, we'll automatically download binaries # without prompting. function verify-release-tars() { - if ! $(find-release-tars); then + if ! find-release-tars; then download-release-binaries fi } @@ -489,7 +528,7 @@ function download-release-binaries() { local resp="y" if [[ ! "${KUBERNETES_SKIP_CONFIRM:-n}" =~ ^[yY]$ ]]; then echo "Required release artifacts appear to be missing. Do you wish to download them? [Y/n]" - read resp + read -r resp fi if [[ "${resp}" =~ ^[nN]$ ]]; then echo "You must download release artifacts to continue. You can use " @@ -502,10 +541,12 @@ function download-release-binaries() { # Run pushd without stack output function pushd() { - command pushd $@ > /dev/null + command pushd "$@" > /dev/null } # Run popd without stack output +# the popd will access `directory stack`, no `real` parameters is actually needed +# shellcheck disable=SC2120 function popd() { - command popd $@ > /dev/null + command popd "$@" > /dev/null } diff --git a/hack/.shellcheck_failures b/hack/.shellcheck_failures index 82638a92176..31aa00e97fd 100644 --- a/hack/.shellcheck_failures +++ b/hack/.shellcheck_failures @@ -1,5 +1,4 @@ ./build/lib/release.sh -./cluster/common.sh ./cluster/gce/config-default.sh ./cluster/gce/config-test.sh ./cluster/gce/gci/configure-helper.sh