From 9209953c620792cf77984504ac7d04b0cfb5a2e5 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 13 Feb 2018 16:00:56 +0200 Subject: [PATCH 1/9] hack/lib/golang.sh: split strings into arrays safely. --- hack/lib/golang.sh | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh index 1c234d2524e..4528ec0323d 100755 --- a/hack/lib/golang.sh +++ b/hack/lib/golang.sh @@ -37,7 +37,8 @@ kube::golang::server_targets() { echo "${targets[@]}" } -readonly KUBE_SERVER_TARGETS=($(kube::golang::server_targets)) +IFS=" " read -ra KUBE_SERVER_TARGETS <<< "$(kube::golang::server_targets)" +readonly KUBE_SERVER_TARGETS readonly KUBE_SERVER_BINARIES=("${KUBE_SERVER_TARGETS[@]##*/}") # The set of server targets that we are only building for Kubernetes nodes @@ -51,15 +52,20 @@ kube::golang::node_targets() { echo "${targets[@]}" } -readonly KUBE_NODE_TARGETS=($(kube::golang::node_targets)) +IFS=" " read -ra KUBE_NODE_TARGETS <<< "$(kube::golang::node_targets)" +readonly KUBE_NODE_TARGETS readonly KUBE_NODE_BINARIES=("${KUBE_NODE_TARGETS[@]##*/}") readonly KUBE_NODE_BINARIES_WIN=("${KUBE_NODE_BINARIES[@]/%/.exe}") if [[ -n "${KUBE_BUILD_PLATFORMS:-}" ]]; then - readonly KUBE_SERVER_PLATFORMS=(${KUBE_BUILD_PLATFORMS}) - readonly KUBE_NODE_PLATFORMS=(${KUBE_BUILD_PLATFORMS}) - readonly KUBE_TEST_PLATFORMS=(${KUBE_BUILD_PLATFORMS}) - readonly KUBE_CLIENT_PLATFORMS=(${KUBE_BUILD_PLATFORMS}) + IFS=" " read -ra KUBE_SERVER_PLATFORMS <<< "$KUBE_BUILD_PLATFORMS" + IFS=" " read -ra KUBE_NODE_PLATFORMS <<< "$KUBE_BUILD_PLATFORMS" + IFS=" " read -ra KUBE_TEST_PLATFORMS <<< "$KUBE_BUILD_PLATFORMS" + IFS=" " read -ra KUBE_CLIENT_PLATFORMS <<< "$KUBE_BUILD_PLATFORMS" + readonly KUBE_SERVER_PLATFORMS + readonly KUBE_NODE_PLATFORMS + readonly KUBE_TEST_PLATFORMS + readonly KUBE_CLIENT_PLATFORMS elif [[ "${KUBE_FASTBUILD:-}" == "true" ]]; then readonly KUBE_SERVER_PLATFORMS=(linux/amd64) readonly KUBE_NODE_PLATFORMS=(linux/amd64) @@ -146,7 +152,8 @@ kube::golang::test_targets() { ) echo "${targets[@]}" } -readonly KUBE_TEST_TARGETS=($(kube::golang::test_targets)) +IFS=" " read -ra KUBE_TEST_TARGETS <<< "$(kube::golang::test_targets)" +readonly KUBE_TEST_TARGETS readonly KUBE_TEST_BINARIES=("${KUBE_TEST_TARGETS[@]##*/}") readonly KUBE_TEST_BINARIES_WIN=("${KUBE_TEST_BINARIES[@]/%/.exe}") # If you update this list, please also update build/BUILD. @@ -177,7 +184,8 @@ kube::golang::server_test_targets() { echo "${targets[@]}" } -readonly KUBE_TEST_SERVER_TARGETS=($(kube::golang::server_test_targets)) +IFS=" " read -ra KUBE_TEST_SERVER_TARGETS <<< "$(kube::golang::server_test_targets)" +readonly KUBE_TEST_SERVER_TARGETS readonly KUBE_TEST_SERVER_BINARIES=("${KUBE_TEST_SERVER_TARGETS[@]##*/}") readonly KUBE_TEST_SERVER_PLATFORMS=("${KUBE_SERVER_PLATFORMS[@]}") @@ -316,7 +324,7 @@ EOF fi local go_version - go_version=($(go version)) + IFS=" " read -ra go_version <<< "$(go version)" local minimum_go_version minimum_go_version=go1.9.1 if [[ "${minimum_go_version}" != $(echo -e "${minimum_go_version}\n${go_version[2]}" | sort -s -t. -k 1,1 -k 2,2n -k 3,3n | head -n1) && "${go_version[2]}" != "devel" ]]; then @@ -640,7 +648,8 @@ kube::golang::build_binaries() { targets=("${KUBE_ALL_TARGETS[@]}") fi - local -a platforms=(${KUBE_BUILD_PLATFORMS:-}) + local -a platforms + IFS=" " read -ra platforms <<< "${KUBE_BUILD_PLATFORMS:-}" if [[ ${#platforms[@]} -eq 0 ]]; then platforms=("${host_platform}") fi From c0c888fdc1a4e6ac5bf663332ea0a40b013eef59 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 13 Feb 2018 16:01:44 +0200 Subject: [PATCH 2/9] hack/lib/golang.sh: do not split on array items. --- hack/lib/golang.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh index 4528ec0323d..e38fdfd17b5 100755 --- a/hack/lib/golang.sh +++ b/hack/lib/golang.sh @@ -675,7 +675,7 @@ kube::golang::build_binaries() { kube::golang::build_kube_toolchain kube::log::status "Generating bindata:" "${KUBE_BINDATAS[@]}" - for bindata in ${KUBE_BINDATAS[@]}; do + for bindata in "${KUBE_BINDATAS[@]}"; do # Only try to generate bindata if the file exists, since in some cases # one-off builds of individual directories may exclude some files. if [[ -f "${KUBE_ROOT}/${bindata}" ]]; then From 173f8e2e4af6b5e0b36ebd4609dc90aec544ac89 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 13 Feb 2018 16:31:39 +0200 Subject: [PATCH 3/9] hack/lib/golang.sh: use double quotes. It's admittedly extremely unlikely that the host platform name would contain special characters, but still use double quotes to pattern matching. Consider this script: #!/bin/bash bar="foobar" foo="foo*" [[ $bar == $foo ]] && echo "first true" [[ "$bar" == "$foo" ]] && echo "second true" We get the output: first true The plan is to move from first case to the second case to prevent pattern match where there shouldn't be any. --- hack/lib/golang.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/lib/golang.sh b/hack/lib/golang.sh index e38fdfd17b5..5f56a81500d 100755 --- a/hack/lib/golang.sh +++ b/hack/lib/golang.sh @@ -401,7 +401,7 @@ kube::golang::place_bins() { # The substitution on platform_src below will replace all slashes with # underscores. It'll transform darwin/amd64 -> darwin_amd64. local platform_src="/${platform//\//_}" - if [[ $platform == $host_platform ]]; then + if [[ "$platform" == "$host_platform" ]]; then platform_src="" rm -f "${THIS_PLATFORM_BIN}" ln -s "${KUBE_OUTPUT_BINPATH}/${platform}" "${THIS_PLATFORM_BIN}" @@ -465,7 +465,7 @@ kube::golang::output_filename_for_binary() { local binary=$1 local platform=$2 local output_path="${KUBE_GOPATH}/bin" - if [[ $platform != $host_platform ]]; then + if [[ "$platform" != "$host_platform" ]]; then output_path="${output_path}/${platform//\//_}" fi local bin=$(basename "${binary}") From 73b5f1b78c657b5358e8f1f6933ca147c6ab9bc9 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 13 Feb 2018 16:35:54 +0200 Subject: [PATCH 4/9] hack/lib/init.sh: prevent splitting in 'dirname' result. You can test this change like this: $ mkdir -p '/tmp/foo bar/x' $ cd $(dirname "/tmp/foo bar/x") bash: cd: too many arguments $ cd "$(dirname "/tmp/foo bar/x")" $ pwd /tmp/foo bar --- hack/lib/init.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/lib/init.sh b/hack/lib/init.sh index 5876d4234e3..bd0a5e3fbfc 100755 --- a/hack/lib/init.sh +++ b/hack/lib/init.sh @@ -131,7 +131,7 @@ function kube::readlinkdashf { cd "$1" pwd -P else - cd $(dirname "$1") + cd "$(dirname "$1")" local f f=$(basename "$1") if [[ -L "$f" ]]; then From 39f4763afe41233f1c506ba1b52b76e3c105fa3c Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Wed, 14 Feb 2018 11:10:16 +0200 Subject: [PATCH 5/9] hack/lib/util.sh: add double quotes. Also move one test to use bash [[ insted of [. --- hack/lib/util.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hack/lib/util.sh b/hack/lib/util.sh index 1a96acce65b..35764f68995 100755 --- a/hack/lib/util.sh +++ b/hack/lib/util.sh @@ -30,13 +30,13 @@ kube::util::wait_for_url() { } local i - for i in $(seq 1 $times); do + for i in $(seq 1 "$times"); do local out - if out=$(curl --max-time 1 -gkfs $url 2>/dev/null); then + if out=$(curl --max-time 1 -gkfs "$url" 2>/dev/null); then kube::log::status "On try ${i}, ${prefix}: ${out}" return 0 fi - sleep ${wait} + sleep "${wait}" done kube::log::error "Timed out waiting for ${prefix} to answer at ${url}; tried ${times} waiting ${wait} between each" return 1 @@ -209,7 +209,7 @@ kube::util::gen-docs() { # Puts a placeholder for every generated doc. This makes the link checker work. kube::util::set-placeholder-gen-docs() { local list_file="${KUBE_ROOT}/docs/.generated_docs" - if [ -e ${list_file} ]; then + if [[ -e "${list_file}" ]]; then # remove all of the old docs; we don't want to check them in. while read file; do if [[ "${list_file}" != "${KUBE_ROOT}/${file}" ]]; then From 4a82a011a0132052dfefb5b6a2d383a5dc8ae234 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Wed, 14 Feb 2018 13:18:28 +0200 Subject: [PATCH 6/9] hack/lib/util.sh: improve staging api finding. Do not bother with pushd/popd construct -- there is no need to go back to original directory in a subshell. Use "find --exec" instead of "find | xargs" to handle special file names better. --- hack/lib/util.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hack/lib/util.sh b/hack/lib/util.sh index 35764f68995..cc7f8945a3f 100755 --- a/hack/lib/util.sh +++ b/hack/lib/util.sh @@ -244,11 +244,9 @@ kube::util::remove-gen-docs() { kube::util::group-version-to-pkg-path() { staging_apis=( $( - pushd ${KUBE_ROOT}/staging/src/k8s.io/api > /dev/null - find . -name types.go | xargs -n1 dirname | sed "s|\./||g" | sort - popd > /dev/null - ) - ) + cd "${KUBE_ROOT}/staging/src/k8s.io/api" && + find . -name types.go -exec dirname {} \; | sed "s|\./||g" | sort + )) local group_version="$1" From f03730682c28f3ca8ace8d93c685fd240a08ab81 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Wed, 14 Feb 2018 13:43:44 +0200 Subject: [PATCH 7/9] hack/lib/util.sh: do not iterate over ls output. Use globs to make the code more resilient for directory names with special characters or whitespace. --- hack/lib/util.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hack/lib/util.sh b/hack/lib/util.sh index cc7f8945a3f..f65a7d00140 100755 --- a/hack/lib/util.sh +++ b/hack/lib/util.sh @@ -459,7 +459,12 @@ kube::util::ensure_godep_version() { kube::util::ensure_no_staging_repos_in_gopath() { kube::util::ensure_single_dir_gopath local error=0 - for repo in $(ls ${KUBE_ROOT}/staging/src/k8s.io); do + for repo_file in "${KUBE_ROOT}"/staging/src/k8s.io/*; do + if [[ ! -d "$repo_file" ]]; then + # not a directory or there were no files + continue; + fi + repo="$(basename "$repo_file")" if [ -e "${GOPATH}/src/k8s.io/${repo}" ]; then echo "k8s.io/${repo} exists in GOPATH. Remove before running godep-save.sh." 1>&2 error=1 From afc459758e1500885705695e25c24dbe6887cb25 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 20 Feb 2018 12:25:27 +0200 Subject: [PATCH 8/9] hack/lib/util.sh: remove shadowed case statements. The topmost case statements shadow those coming after them. Since they are never run, remove them. --- hack/lib/util.sh | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hack/lib/util.sh b/hack/lib/util.sh index f65a7d00140..d88787be51d 100755 --- a/hack/lib/util.sh +++ b/hack/lib/util.sh @@ -272,15 +272,9 @@ kube::util::group-version-to-pkg-path() { meta/v1) echo "vendor/k8s.io/apimachinery/pkg/apis/meta/v1" ;; - meta/v1) - echo "../vendor/k8s.io/apimachinery/pkg/apis/meta/v1" - ;; meta/v1beta1) echo "vendor/k8s.io/apimachinery/pkg/apis/meta/v1beta1" ;; - meta/v1beta1) - echo "../vendor/k8s.io/apimachinery/pkg/apis/meta/v1beta1" - ;; unversioned) echo "pkg/api/unversioned" ;; From 1f41f32d3eecbd2822e00f7662cf7848f91c0984 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 20 Feb 2018 14:29:22 +0200 Subject: [PATCH 9/9] hack/lib/protoc.sh: don't split find-binary output. --- hack/lib/protoc.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/lib/protoc.sh b/hack/lib/protoc.sh index 980684cbd8a..1adcea4b944 100644 --- a/hack/lib/protoc.sh +++ b/hack/lib/protoc.sh @@ -57,7 +57,7 @@ function kube::protoc::check_protoc() { # $1: Full path to the directory where the api.proto file is function kube::protoc::protoc() { local package=${1} - gogopath=$(dirname $(kube::util::find-binary "protoc-gen-gogo")) + gogopath=$(dirname "$(kube::util::find-binary "protoc-gen-gogo")") PATH="${gogopath}:${PATH}" protoc \ --proto_path="${package}" \