Skip to content

Commit

Permalink
hack: add verify-shellcheck target to validate bash scripts in the re…
Browse files Browse the repository at this point in the history
…pository

* add a bash script to run shellcheck on all the scripts in the
  repository.
* add `verify-shellcheck` make target to run the shellcheck script.
* fix shellcheck warnings in existing bash scripts.

Signed-off-by: Deepesh Pathak <[email protected]>
  • Loading branch information
fristonio committed Jun 21, 2021
1 parent 9cb9556 commit 3f8b9f1
Show file tree
Hide file tree
Showing 23 changed files with 262 additions and 91 deletions.
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ clean-release: ## Remove the release folder
rm -rf $(RELEASE_DIR)

.PHONY: verify
verify: verify-boilerplate verify-modules verify-gen
verify: verify-boilerplate verify-modules verify-gen verify-shellcheck

.PHONY: verify-boilerplate
verify-boilerplate:
Expand All @@ -585,3 +585,7 @@ verify-gen: generate
@if !(git diff --quiet HEAD); then \
git diff; echo "generated files are out of date, run make generate"; exit 1; \
fi

.PHONY: verify-shellcheck
verify-shellcheck:
./hack/verify-shellcheck.sh
17 changes: 11 additions & 6 deletions hack/create-dev-cluster.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,13 @@ export AZURE_VNET_NAME=${CLUSTER_NAME}-vnet
# Azure settings.
export AZURE_LOCATION="${AZURE_LOCATION:-southcentralus}"
export AZURE_RESOURCE_GROUP=${CLUSTER_NAME}
export AZURE_SUBSCRIPTION_ID_B64="$(echo -n "$AZURE_SUBSCRIPTION_ID" | base64 | tr -d '\n')"
export AZURE_TENANT_ID_B64="$(echo -n "$AZURE_TENANT_ID" | base64 | tr -d '\n')"
export AZURE_CLIENT_ID_B64="$(echo -n "$AZURE_CLIENT_ID" | base64 | tr -d '\n')"
export AZURE_CLIENT_SECRET_B64="$(echo -n "$AZURE_CLIENT_SECRET" | base64 | tr -d '\n')"

AZURE_SUBSCRIPTION_ID_B64="$(echo -n "$AZURE_SUBSCRIPTION_ID" | base64 | tr -d '\n')"
AZURE_TENANT_ID_B64="$(echo -n "$AZURE_TENANT_ID" | base64 | tr -d '\n')"
AZURE_CLIENT_ID_B64="$(echo -n "$AZURE_CLIENT_ID" | base64 | tr -d '\n')"
AZURE_CLIENT_SECRET_B64="$(echo -n "$AZURE_CLIENT_SECRET" | base64 | tr -d '\n')"

export AZURE_SUBSCRIPTION_ID_B64 AZURE_TENANT_ID_B64 AZURE_CLIENT_ID_B64 AZURE_CLIENT_SECRET_B64

# Machine settings.
export CONTROL_PLANE_MACHINE_COUNT=${CONTROL_PLANE_MACHINE_COUNT:-3}
Expand All @@ -49,13 +52,15 @@ export CLUSTER_TEMPLATE="${CLUSTER_TEMPLATE:-cluster-template.yaml}"

# Generate SSH key.
SSH_KEY_FILE=${SSH_KEY_FILE:-""}
if ! [ -n "$SSH_KEY_FILE" ]; then
if [ -z "$SSH_KEY_FILE" ]; then
SSH_KEY_FILE=.sshkey
rm -f "${SSH_KEY_FILE}" 2>/dev/null
ssh-keygen -t rsa -b 2048 -f "${SSH_KEY_FILE}" -N '' 1>/dev/null
echo "Machine SSH key generated in ${SSH_KEY_FILE}"
fi
export AZURE_SSH_PUBLIC_KEY_B64=$(cat "${SSH_KEY_FILE}.pub" | base64 | tr -d '\r\n')

AZURE_SSH_PUBLIC_KEY_B64=$(base64 "${SSH_KEY_FILE}.pub" | tr -d '\r\n')
export AZURE_SSH_PUBLIC_KEY_B64

echo "================ DOCKER BUILD ==============="
PULL_POLICY=IfNotPresent make modules docker-build
Expand Down
2 changes: 1 addition & 1 deletion hack/ensure-acr-login.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ cd "${REPO_ROOT}" || exit 1

if [[ "${REGISTRY:-}" =~ capzci\.azurecr\.io ]]; then
# if we are using the prow Azure Container Registry, login.
${REPO_ROOT}/hack/ensure-azcli.sh
"${REPO_ROOT}/hack/ensure-azcli.sh"
: "${AZURE_SUBSCRIPTION_ID:?Environment variable empty or not defined.}"
az account set -s "${AZURE_SUBSCRIPTION_ID}"
az acr login --name capzci
Expand Down
2 changes: 1 addition & 1 deletion hack/ensure-kind.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ verify_kind_version() {
IFS=" " read -ra kind_version <<< "$(kind version)"
if [[ "${MINIMUM_KIND_VERSION}" != $(echo -e "${MINIMUM_KIND_VERSION}\n${kind_version[1]}" | sort -s -t. -k 1,1 -k 2,2n -k 3,3n | head -n1) ]]; then
cat <<EOF
Detected kind version: ${kind_version}.
Detected kind version: ${kind_version[0]}.
Requires ${MINIMUM_KIND_VERSION} or greater.
Please install ${MINIMUM_KIND_VERSION} or later.
EOF
Expand Down
6 changes: 5 additions & 1 deletion hack/install-cert-manager.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ kubectl wait --for=condition=Available --timeout=5m -n cert-manager deployment/c
kubectl wait --for=condition=Available --timeout=5m -n cert-manager deployment/cert-manager-cainjector
kubectl wait --for=condition=Available --timeout=5m -n cert-manager deployment/cert-manager-webhook

for i in {1..6}; do (echo "$TEST_RESOURCE" | kubectl apply -f - ) && break || sleep 15; done
for _ in {1..6}; do
(echo "$TEST_RESOURCE" | kubectl apply -f -) && break
sleep 15
done

kubectl wait --for=condition=Ready --timeout=300s -n cert-manager-test certificate/selfsigned-cert
echo "$TEST_RESOURCE" | kubectl delete -f -
2 changes: 1 addition & 1 deletion hack/kustomize-sub.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ set -o nounset
set -o pipefail

root=$(dirname "${BASH_SOURCE[0]}")
$root/tools/bin/kustomize build $1 | $root/tools/bin/envsubst
"$root/tools/bin/kustomize" build "$1" | "$root/tools/bin/envsubst"
14 changes: 9 additions & 5 deletions hack/log/log-dump.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ set -o pipefail
REPO_ROOT=$(dirname "${BASH_SOURCE[0]}")/../..
cd "${REPO_ROOT}" || exit 1

# shellcheck source=../hack/ensure-kind.sh
# shellcheck source=hack/ensure-kind.sh
source "${REPO_ROOT}/hack/ensure-kind.sh"
# shellcheck source=../hack/ensure-kubectl.sh
# shellcheck source=hack/ensure-kubectl.sh
source "${REPO_ROOT}/hack/ensure-kubectl.sh"

export ARTIFACTS="${ARTIFACTS:-${PWD}/_artifacts}"
Expand All @@ -33,7 +33,8 @@ export KUBECONFIG="${KUBECONFIG:-${PWD}/kubeconfig}"

get_node_name() {
local -r pod_name="${1}"
echo "$(kubectl get pod "${pod_name}" -ojsonpath={.spec.nodeName})"
# shellcheck disable=SC1083
kubectl get pod "${pod_name}" -ojsonpath={.spec.nodeName}
}

dump_mgmt_cluster_logs() {
Expand Down Expand Up @@ -90,7 +91,8 @@ dump_workload_cluster_logs() {
kubectl apply -f "${REPO_ROOT}/hack/log/log-dump-daemonset.yaml"
kubectl wait pod -l app=log-dump-node --for=condition=Ready --timeout=5m

local -r log_dump_pods=( $(kubectl get pod -l app=log-dump-node -ojsonpath={.items[*].metadata.name}) )
local -r log_dump_pods=()
IFS=" " read -r -a log_dump_pods <<< "$(kubectl get pod -l app=log-dump-node -ojsonpath='{.items[*].metadata.name}')"
local log_dump_commands=(
"journalctl --output=short-precise -u kubelet > kubelet.log"
"journalctl --output=short-precise -u containerd > containerd.log"
Expand All @@ -108,7 +110,8 @@ dump_workload_cluster_logs() {
fi

for log_dump_pod in "${log_dump_pods[@]}"; do
local node_name="$(get_node_name "${log_dump_pod}")"
local node_name
node_name="$(get_node_name "${log_dump_pod}")"

local log_dump_dir="${ARTIFACTS}/workload-cluster/${node_name}"
mkdir -p "${log_dump_dir}"
Expand All @@ -127,6 +130,7 @@ dump_workload_cluster_logs() {

cleanup() {
kubectl delete -f "${REPO_ROOT}/hack/log/log-dump-daemonset.yaml" || true
# shellcheck source=hack/log/redact.sh
source "${REPO_ROOT}/hack/log/redact.sh"
}

Expand Down
3 changes: 2 additions & 1 deletion hack/log/redact.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ set -o pipefail

echo "================ REDACTING LOGS ================"

log_files=( $(find "${ARTIFACTS:-${PWD}/_artifacts}" -type f) )
log_files=()
while IFS='' read -r line; do log_files+=("$line"); done < <(find "${ARTIFACTS:-${PWD}/_artifacts}" -type f)
redact_vars=(
"${AZURE_CLIENT_ID:-}"
"${AZURE_CLIENT_SECRET:-}"
Expand Down
18 changes: 10 additions & 8 deletions hack/parse-prow-creds.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ parse_cred() {
# for Prow we use the provided AZURE_CREDENTIALS file.
# the file is expected to be in toml format.
if [[ -n "${AZURE_CREDENTIALS:-}" ]]; then
export AZURE_SUBSCRIPTION_ID="$(cat ${AZURE_CREDENTIALS} | parse_cred SubscriptionID)"
export AZURE_TENANT_ID="$(cat ${AZURE_CREDENTIALS} | parse_cred TenantID)"
export AZURE_CLIENT_ID="$(cat ${AZURE_CREDENTIALS} | parse_cred ClientID)"
export AZURE_CLIENT_SECRET="$(cat ${AZURE_CREDENTIALS} | parse_cred ClientSecret)"
export AZURE_MULTI_TENANCY_ID="$(cat ${AZURE_CREDENTIALS} | parse_cred MultiTenancyClientID)"
export AZURE_MULTI_TENANCY_SECRET="$(cat ${AZURE_CREDENTIALS} | parse_cred MultiTenancyClientSecret)"
export AZURE_STORAGE_ACCOUNT="$(cat ${AZURE_CREDENTIALS} | parse_cred StorageAccountName)"
export AZURE_STORAGE_KEY="$(cat ${AZURE_CREDENTIALS} | parse_cred StorageAccountKey)"
AZURE_SUBSCRIPTION_ID="$(parse_cred SubscriptionID < "${AZURE_CREDENTIALS}")"
AZURE_TENANT_ID="$(parse_cred TenantID < "${AZURE_CREDENTIALS}")"
AZURE_CLIENT_ID="$(parse_cred ClientID < "${AZURE_CREDENTIALS}")"
AZURE_CLIENT_SECRET="$(parse_cred ClientSecret < "${AZURE_CREDENTIALS}")"
AZURE_MULTI_TENANCY_ID="$(parse_cred MultiTenancyClientID < "${AZURE_CREDENTIALS}")"
AZURE_MULTI_TENANCY_SECRET="$(parse_cred MultiTenancyClientSecret < "${AZURE_CREDENTIALS}")"
AZURE_STORAGE_ACCOUNT="$(parse_cred StorageAccountName < "${AZURE_CREDENTIALS}")"
AZURE_STORAGE_KEY="$(parse_cred StorageAccountKey < "${AZURE_CREDENTIALS}")"

export AZURE_SUBSCRIPTION_ID AZURE_TENANT_ID AZURE_CLIENT_ID AZURE_CLIENT_SECRET AZURE_MULTI_TENANCY_ID AZURE_MULTI_TENANCY_SECRET AZURE_STORAGE_ACCOUNT AZURE_STORAGE_KEY
fi
7 changes: 5 additions & 2 deletions hack/print-workspace-status.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,15 @@ if GIT_VERSION=$(git describe --tags --abbrev=14 2>/dev/null); then
#
# TODO: We continue calling this "git version" because so many
# downstream consumers are expecting it there.
# shellcheck disable=SC2001
DASHES_IN_VERSION=$(echo "${GIT_VERSION}" | sed "s/[^-]//g")
if [[ "${DASHES_IN_VERSION}" == "---" ]] ; then
# We have distance to subversion (v1.1.0-subversion-1-gCommitHash)
# shellcheck disable=SC2001
GIT_VERSION=$(echo "${GIT_VERSION}" | sed "s/-\([0-9]\{1,\}\)-g\([0-9a-f]\{14\}\)$/.\1\-\2/")
elif [[ "${DASHES_IN_VERSION}" == "--" ]] ; then
# We have distance to base tag (v1.1.0-1-gCommitHash)
# shellcheck disable=SC2001
GIT_VERSION=$(echo "${GIT_VERSION}" | sed "s/-g\([0-9a-f]\{14\}\)$/-\1/")
fi
if [[ "${GIT_TREE_STATE}" == "dirty" ]]; then
Expand All @@ -66,9 +69,9 @@ if GIT_VERSION=$(git describe --tags --abbrev=14 2>/dev/null); then
fi
fi

GIT_BRANCH=$(git branch | grep \* | cut -d ' ' -f2)
GIT_BRANCH=$(git branch | grep '\*' | cut -d ' ' -f2)
GIT_RELEASE_TAG=$(git describe --abbrev=0 --tags)
GIT_RELEASE_COMMIT=$(git rev-list -n 1 ${GIT_RELEASE_TAG} | head -c 14)
GIT_RELEASE_COMMIT=$(git rev-list -n 1 "${GIT_RELEASE_TAG}" | head -c 14)

cat <<EOF
GIT_COMMIT ${GIT_COMMIT-}
Expand Down
130 changes: 130 additions & 0 deletions hack/verify-shellcheck.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
#!/usr/bin/env bash

# Copyright 2018 The Kubernetes Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# This script lints each shell script by `shellcheck`.
# Usage: `hack/verify-shellcheck.sh`.
# Taken from - https://github.com/kubernetes/kubernetes/blob/master/hack/verify-shellcheck.sh
# modified according to use.

set -o errexit
set -o nounset
set -o pipefail

CAPZ_ROOT=$( cd "$(dirname "${BASH_SOURCE[0]}")/.." &> /dev/null && pwd )

# allow overriding docker cli, which should work fine for this script
DOCKER="${DOCKER:-docker}"
SHELLCHECK_COLORIZED_OUTPUT="${SHELLCHECK_COLORIZED_OUTPUT:-auto}"

# required version for this script, if not installed on the host we will
# use the official docker image instead. keep this in sync with SHELLCHECK_IMAGE
SHELLCHECK_VERSION="0.7.2"
# upstream shellcheck latest stable image as of October 23rd, 2019
SHELLCHECK_IMAGE="docker.io/koalaman/shellcheck:v0.7.2@sha256:90680b9c98552cb5468966f6e83cd419a951d2a099454663429be796462c7549"

# disabled lints
disabled=(
# this lint disallows non-constant source, which we use extensively without
# any known bugs
1090
# this lint prefers command -v to which, they are not the same
2230
)

# comma separate for passing to shellcheck
join_by() {
local IFS="$1";
shift;
echo "$*";
}
SHELLCHECK_DISABLED="$(join_by , "${disabled[@]}")"
readonly SHELLCHECK_DISABLED

# Ensure we're linting the correct source tree
cd "${CAPZ_ROOT}"

# Find all shell scripts excluding:
# - Anything git-ignored - No need to lint untracked files.
# - ./_* - No need to lint output directories.
# - ./.git/* - Ignore anything in the git object store.
# - ./vendor* - Vendored code should be fixed upstream instead.
# - ./third_party/*, but re-include ./third_party/forked/* - only code we
# forked should be linted and fixed.
all_shell_scripts=()
while IFS=$'\n' read -r script;
do git check-ignore -q "$script" || all_shell_scripts+=("$script");
done < <(find . -name "*.sh" \
-not \( \
-path ./_\* -o \
-path ./.git\* -o \
-path ./vendor\* -o \
\( -path ./third_party\* -a -not -path ./third_party/forked\* \) \
\))

# Detect if the host machine has the required shellcheck version installed
# if so, we will use that instead.
HAVE_SHELLCHECK=false
if which shellcheck &>/dev/null; then
detected_version="$(shellcheck --version | grep 'version: .*')"
if [[ "${detected_version}" = "version: ${SHELLCHECK_VERSION}" ]]; then
HAVE_SHELLCHECK=true
fi
fi

# common arguments we'll pass to shellcheck
SHELLCHECK_OPTIONS=(
# allow following sourced files that are not specified in the command,
# we need this because we specify one file at a time in order to trivially
# detect which files are failing
"--external-sources"
# include our disabled lints
"--exclude=${SHELLCHECK_DISABLED}"
# set colorized output
"--color=${SHELLCHECK_COLORIZED_OUTPUT}"
)

# 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 "${CAPZ_ROOT}:${CAPZ_ROOT}" -w "${CAPZ_ROOT}" \
"${SHELLCHECK_IMAGE}" \
"${SHELLCHECK_OPTIONS[@]}" "${all_shell_scripts[@]}" || res=$?
fi

# print a message based on the result
if [ $res -eq 0 ]; then
echo 'Congratulations! All shell files are passing lint :-)'
else
{
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 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 ignore.'
echo 'See: https://github.com/koalaman/shellcheck/wiki/Ignore#ignoring-one-specific-instance-in-a-file'
echo
} >&2
exit 1
fi

# preserve the result
exit $res
6 changes: 4 additions & 2 deletions hack/version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ set -o nounset
set -o pipefail

version::get_version_vars() {
# shellcheck disable=SC1083
GIT_COMMIT="$(git rev-parse HEAD^{commit})"

if git_status=$(git status --porcelain 2>/dev/null) && [[ -z ${git_status} ]]; then
Expand All @@ -32,12 +33,15 @@ version::get_version_vars() {
# This translates the "git describe" to an actual semver.org
# compatible semantic version that looks something like this:
# v1.1.0-alpha.0.6+84c76d1142ea4d
# shellcheck disable=SC2001
DASHES_IN_VERSION=$(echo "${GIT_VERSION}" | sed "s/[^-]//g")
if [[ "${DASHES_IN_VERSION}" == "---" ]] ; then
# We have distance to subversion (v1.1.0-subversion-1-gCommitHash)
# shellcheck disable=SC2001
GIT_VERSION=$(echo "${GIT_VERSION}" | sed "s/-\([0-9]\{1,\}\)-g\([0-9a-f]\{14\}\)$/.\1\-\2/")
elif [[ "${DASHES_IN_VERSION}" == "--" ]] ; then
# We have distance to base tag (v1.1.0-1-gCommitHash)
# shellcheck disable=SC2001
GIT_VERSION=$(echo "${GIT_VERSION}" | sed "s/-g\([0-9a-f]\{14\}\)$/-\1/")
fi
if [[ "${GIT_TREE_STATE}" == "dirty" ]]; then
Expand All @@ -63,8 +67,6 @@ version::get_version_vars() {
exit 1
fi
fi

GIT_RELEASE_TAG=$(git describe --abbrev=0 --tags)
}

# borrowed from k8s.io/hack/lib/version.sh and modified
Expand Down
2 changes: 1 addition & 1 deletion scripts/ci-apidiff.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ cd "${REPO_ROOT}"
APIDIFF="${PWD}/hack/tools/bin/go-apidiff"
PULL_BASE_SHA=${PULL_BASE_SHA:-$(git rev-parse origin/master)}

make ${APIDIFF}
make "${APIDIFF}"
echo "*** Running go-apidiff ***"

${APIDIFF} "${PULL_BASE_SHA}" --print-compatible
Loading

0 comments on commit 3f8b9f1

Please sign in to comment.