From 60d50f4ce8d7559757100b274cc61bfdbbcfdcff Mon Sep 17 00:00:00 2001 From: Benjamin Elder Date: Thu, 4 Mar 2021 16:32:11 -0800 Subject: [PATCH] banish .shellcheck_failures we've eliminated these. don't allow any regression. this should also be much faster now. --- hack/.shellcheck_failures | 0 hack/verify-shellcheck.sh | 125 +++++++------------------------------- 2 files changed, 21 insertions(+), 104 deletions(-) delete mode 100644 hack/.shellcheck_failures diff --git a/hack/.shellcheck_failures b/hack/.shellcheck_failures deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/hack/verify-shellcheck.sh b/hack/verify-shellcheck.sh index cef7cdc6420..654ba88047c 100755 --- a/hack/verify-shellcheck.sh +++ b/hack/verify-shellcheck.sh @@ -31,9 +31,6 @@ SHELLCHECK_VERSION="0.7.0" # upstream shellcheck latest stable image as of October 23rd, 2019 SHELLCHECK_IMAGE="koalaman/shellcheck-alpine:v0.7.0@sha256:24bbf52aae6eaa27accc9f61de32d30a1498555e6ef452966d0702ff06f38ecb" -# fixed name for the shellcheck docker container so we can reliably clean it up -SHELLCHECK_CONTAINER="k8s-shellcheck" - # disabled lints disabled=( # this lint disallows non-constant source, which we use extensively without @@ -51,21 +48,6 @@ join_by() { SHELLCHECK_DISABLED="$(join_by , "${disabled[@]}")" readonly SHELLCHECK_DISABLED -# creates the shellcheck container for later use -create_container () { - # TODO(bentheelder): this is a performance hack, we create the container with - # a sleep MAX_INT32 so that it is effectively paused. - # We then repeatedly exec to it to run each shellcheck, and later rm it when - # we're done. - # This is incredibly much faster than creating a container for each shellcheck - # call ... - docker run --name "${SHELLCHECK_CONTAINER}" -d --rm -v "${KUBE_ROOT}:${KUBE_ROOT}" -w "${KUBE_ROOT}" --entrypoint="sleep" "${SHELLCHECK_IMAGE}" 2147483647 -} -# removes the shellcheck container -remove_container () { - docker rm -f "${SHELLCHECK_CONTAINER}" &> /dev/null || true -} - # ensure we're linting the k8s source tree cd "${KUBE_ROOT}" @@ -87,16 +69,6 @@ done < <(find . -name "*.sh" \ \( -path ./third_party\* -a -not -path ./third_party/forked\* \) \ \)) -# make sure known failures are sorted -failure_file="${KUBE_ROOT}/hack/.shellcheck_failures" -kube::util::check-file-in-alphabetical-order "${failure_file}" - -# load known failure files -failing_files=() -while IFS=$'\n' read -r script; - do failing_files+=("$script"); -done < <(cat "${failure_file}") - # detect if the host machine has the required shellcheck version installed # if so, we will use that instead. HAVE_SHELLCHECK=false @@ -107,25 +79,6 @@ if which shellcheck &>/dev/null; then fi fi -# tell the user which we've selected and possibly set up the container -if ${HAVE_SHELLCHECK}; then - echo "Using host shellcheck ${SHELLCHECK_VERSION} binary." -else - echo "Using shellcheck ${SHELLCHECK_VERSION} docker image." - # remove any previous container, ensure we will attempt to cleanup on exit, - # and create the container - remove_container - kube::util::trap_add 'remove_container' EXIT - if ! output="$(create_container 2>&1)"; then - { - echo "Failed to create shellcheck container with output: " - echo "" - echo "${output}" - } >&2 - exit 1 - fi -fi - # if KUBE_JUNIT_REPORT_DIR is set, disable colorized output. # Colorized output causes malformed XML in the JUNIT report. SHELLCHECK_COLORIZED_OUTPUT="auto" @@ -145,71 +98,35 @@ SHELLCHECK_OPTIONS=( "--color=${SHELLCHECK_COLORIZED_OUTPUT}" ) -# lint each script, tracking failures -errors=() -not_failing=() -for f in "${all_shell_scripts[@]}"; do - set +o errexit - if ${HAVE_SHELLCHECK}; then - failedLint=$(shellcheck "${SHELLCHECK_OPTIONS[@]}" "${f}") - else - failedLint=$(docker exec -t ${SHELLCHECK_CONTAINER} \ - shellcheck "${SHELLCHECK_OPTIONS[@]}" "${f}") - fi - set -o errexit - kube::util::array_contains "${f}" "${failing_files[@]}" && in_failing=$? || in_failing=$? - if [[ -n "${failedLint}" ]] && [[ "${in_failing}" -ne "0" ]]; then - errors+=( "${failedLint}" ) - fi - if [[ -z "${failedLint}" ]] && [[ "${in_failing}" -eq "0" ]]; then - not_failing+=( "${f}" ) - fi -done +# tell the user which we've selected and lint all scripts +res=0 +if ${HAVE_SHELLCHECK}; then + echo "Using host shellcheck ${SHELLCHECK_VERSION} binary." + shellcheck "${SHELLCHECK_OPTIONS[@]}" "${all_shell_scripts[@]}" || res=$? +else + echo "Using shellcheck ${SHELLCHECK_VERSION} docker image." + docker run \ + --rm -v "${KUBE_ROOT}:${KUBE_ROOT}" -w "${KUBE_ROOT}" \ + "${SHELLCHECK_IMAGE}" \ + shellcheck "${SHELLCHECK_OPTIONS[@]}" "${all_shell_scripts[@]}" || res=$? +fi -# Check to be sure all the files that should pass lint are. -if [ ${#errors[@]} -eq 0 ]; then +# print a message based on the result +if [ $res -eq 0 ]; then echo 'Congratulations! All shell files are passing lint (excluding those in hack/.shellcheck_failures).' else { - echo "Errors from shellcheck:" - for err in "${errors[@]}"; do - echo "$err" - done echo echo 'Please review the above warnings. You can test via "./hack/verify-shellcheck.sh"' - echo 'If the above warnings do not make sense, you can exempt this package from shellcheck' - echo 'checking by adding it to hack/.shellcheck_failures (if your reviewer is okay with it).' + echo 'If the above warnings do not make sense, you can exempt this warning with a comment' + echo ' (if your reviewer is okay with it).' + echo 'In general please prefer to fix the error, we have already disabled specific lints' + echo ' that the project chooses to ignire.' + echo 'See: https://github.com/koalaman/shellcheck/wiki/Ignore#ignoring-one-specific-instance-in-a-file' echo } >&2 exit 1 fi -if [[ ${#not_failing[@]} -gt 0 ]]; then - { - echo "Some files in hack/.shellcheck_failures are passing shellcheck. Please remove them." - echo - for f in "${not_failing[@]}"; do - echo " $f" - done - echo - } >&2 - exit 1 -fi - -# Check that all failing_files actually still exist -gone=() -for f in "${failing_files[@]}"; do - kube::util::array_contains "$f" "${all_shell_scripts[@]}" || gone+=( "$f" ) -done - -if [[ ${#gone[@]} -gt 0 ]]; then - { - echo "Some files in hack/.shellcheck_failures do not exist anymore. Please remove them." - echo - for f in "${gone[@]}"; do - echo " $f" - done - echo - } >&2 - exit 1 -fi +# preserve the result +exit $res