From 520cade49af66542d19fed82bcef363a663d5e84 Mon Sep 17 00:00:00 2001 From: Janik K <10290002+led0nk@users.noreply.github.com> Date: Tue, 23 Apr 2024 22:11:47 +0200 Subject: [PATCH] [CI] add shellcheck-workflow (#32052) **Description:** So i fixed the issues which shellcheck displayed, which were only 3 double quote issues and the `read -r` issue. After looking into the closed PR i also replaced `printf`-statements with `echo`. For the shellcheck-worfklow I used [https://github.com/ludeeus/action-shellcheck](url) I didn't find a way to check multiple directories at once, so we just use 2 steps here. I added the disabled checks provided by the intentional issue, but i'm not quite sure if we need to add `-x` here. Couldn't find anything in the documentation so far. On top of that i'm not quite sure if we should run the shellcheck in dependency of any other workflows or just on its own. Edit: I added the option `ignore_paths` so that shellcheck-action is only run in specified paths and ignores subpaths of this directory. **Link to tracking Issue:** - #17279 **Testing:** **Documentation:** --------- Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- .../workflows/scripts/add-codeowners-to-pr.sh | 14 ++--- .../scripts/check-collector-module-version.sh | 8 +-- .github/workflows/scripts/get-codeowners.sh | 4 +- .../workflows/scripts/mark-issues-as-stale.sh | 4 +- .../scripts/ping-codeowners-on-new-issue.sh | 6 +- .github/workflows/scripts/setup_e2e_tests.sh | 4 +- .github/workflows/shellcheck.yaml | 32 ++++++++++ internal/buildscripts/packaging/fpm/common.sh | 61 ++++++++++--------- .../buildscripts/packaging/fpm/deb/build.sh | 14 ++--- .../buildscripts/packaging/fpm/rpm/build.sh | 14 ++--- internal/buildscripts/packaging/fpm/test.sh | 17 +++--- 11 files changed, 107 insertions(+), 71 deletions(-) create mode 100644 .github/workflows/shellcheck.yaml diff --git a/.github/workflows/scripts/add-codeowners-to-pr.sh b/.github/workflows/scripts/add-codeowners-to-pr.sh index 5b586aaeec2a..b0b138c5d9a8 100755 --- a/.github/workflows/scripts/add-codeowners-to-pr.sh +++ b/.github/workflows/scripts/add-codeowners-to-pr.sh @@ -29,9 +29,9 @@ main () { # is only available through the API. To cut down on API calls to GitHub, # we use the latest reviews to determine which users to filter out. JSON=$(gh pr view "${PR}" --json "files,author,latestReviews" | tr -dc '[:print:]' | sed -E 's/\\[a-z]//g') - AUTHOR=$(printf "${JSON}"| jq -r '.author.login') - FILES=$(printf "${JSON}"| jq -r '.files[].path') - REVIEW_LOGINS=$(printf "${JSON}"| jq -r '.latestReviews[].author.login') + AUTHOR=$(echo -n "${JSON}"| jq -r '.author.login') + FILES=$(echo -n "${JSON}"| jq -r '.files[].path') + REVIEW_LOGINS=$(echo -n "${JSON}"| jq -r '.latestReviews[].author.login') COMPONENTS=$(bash "${CUR_DIRECTORY}/get-components.sh") REVIEWERS="" LABELS="" @@ -59,14 +59,14 @@ main () { # won't be checked against the remaining components in the components # list. This provides a meaningful speedup in practice. for FILE in ${FILES}; do - MATCH=$(echo "${FILE}" | grep -E "^${COMPONENT}" || true) + MATCH=$(echo -n "${FILE}" | grep -E "^${COMPONENT}" || true) if [[ -z "${MATCH}" ]]; then continue fi # If we match a file with a component we don't need to process the file again. - FILES=$(printf "${FILES}" | grep -v "${FILE}") + FILES=$(echo -n "${FILES}" | grep -v "${FILE}") if [[ -v PROCESSED_COMPONENTS["${COMPONENT}"] ]]; then continue @@ -87,11 +87,11 @@ main () { if [[ -n "${REVIEWERS}" ]]; then REVIEWERS+="," fi - REVIEWERS+=$(echo "${OWNER}" | sed -E 's/@(.+)/"\1"/') + REVIEWERS+=$(echo -n "${OWNER}" | sed -E 's/@(.+)/"\1"/') done # Convert the CODEOWNERS entry to a label - COMPONENT_NAME=$(echo "${COMPONENT}" | sed -E 's%^(.+)/(.+)\1%\1/\2%') + COMPONENT_NAME=$(echo -n "${COMPONENT}" | sed -E 's%^(.+)/(.+)\1%\1/\2%') if (( "${#COMPONENT_NAME}" > 50 )); then echo "'${COMPONENT_NAME}' exceeds GitHub's 50-character limit on labels, skipping adding label" diff --git a/.github/workflows/scripts/check-collector-module-version.sh b/.github/workflows/scripts/check-collector-module-version.sh index 8a00271161d4..1dc98d1e8d09 100755 --- a/.github/workflows/scripts/check-collector-module-version.sh +++ b/.github/workflows/scripts/check-collector-module-version.sh @@ -20,8 +20,8 @@ get_collector_version() { main_mod_file="$2" if grep -q "$collector_module" "$main_mod_file"; then - grep "$collector_module" "$main_mod_file" | (read mod version rest; - echo $version) + grep "$collector_module" "$main_mod_file" | (read -r mod version rest; + echo "$version") else echo "Error: failed to retrieve the \"$collector_module\" version from \"$main_mod_file\"." exit 1 @@ -53,7 +53,7 @@ BETA_MODULE="go.opentelemetry.io/collector" # only and does not return string which contains this string as a substring. BETA_MOD_VERSION=$(get_collector_version "$BETA_MODULE " "$MAIN_MOD_FILE") check_collector_versions_correct "$BETA_MODULE" "$BETA_MOD_VERSION" -for mod in ${beta_modules[@]}; do +for mod in "${beta_modules[@]}"; do check_collector_versions_correct "$mod" "$BETA_MOD_VERSION" done @@ -61,7 +61,7 @@ done STABLE_MODULE="go.opentelemetry.io/collector/pdata" STABLE_MOD_VERSION=$(get_collector_version "$STABLE_MODULE" "$MAIN_MOD_FILE") check_collector_versions_correct "$STABLE_MODULE" "$STABLE_MOD_VERSION" -for mod in ${stable_modules[@]}; do +for mod in "${stable_modules[@]}"; do check_collector_versions_correct "$mod" "$STABLE_MOD_VERSION" done diff --git a/.github/workflows/scripts/get-codeowners.sh b/.github/workflows/scripts/get-codeowners.sh index 8d84b99e7787..06348a516c2a 100755 --- a/.github/workflows/scripts/get-codeowners.sh +++ b/.github/workflows/scripts/get-codeowners.sh @@ -20,9 +20,9 @@ get_codeowners() { # ${1}: Insert first argument given to this function # [\/]\?: Match 0 or 1 instances of a forward slash # \s: Match any whitespace character - echo "$((grep -m 1 "^${1}[\/]\?\s" .github/CODEOWNERS || true) | \ +(grep -m 1 "^${1}[\/]\?\s" .github/CODEOWNERS || true) | \ sed 's/ */ /g' | \ - cut -f3- -d ' ')" + cut -f3- -d ' ' } if [[ -z "${COMPONENT:-}" ]]; then diff --git a/.github/workflows/scripts/mark-issues-as-stale.sh b/.github/workflows/scripts/mark-issues-as-stale.sh index 69226355dea9..d3f996212564 100755 --- a/.github/workflows/scripts/mark-issues-as-stale.sh +++ b/.github/workflows/scripts/mark-issues-as-stale.sh @@ -60,10 +60,10 @@ for ISSUE in ${ISSUES}; do gh issue comment "${ISSUE}" -b "${STALE_MESSAGE}" else - printf "Pinging code owners for issue #${ISSUE}:\n${OWNER_MENTIONS}" + echo -e "Pinging code owners for issue #${ISSUE}:\n${OWNER_MENTIONS}" # The GitHub CLI only offers multiline strings through file input. - printf "${STALE_MESSAGE}\n\nPinging code owners:\n${OWNER_MENTIONS}\nSee [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself." \ + echo -e "${STALE_MESSAGE}\n\nPinging code owners:\n${OWNER_MENTIONS}\nSee [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself." \ | gh issue comment "${ISSUE}" -F - fi diff --git a/.github/workflows/scripts/ping-codeowners-on-new-issue.sh b/.github/workflows/scripts/ping-codeowners-on-new-issue.sh index 02c5c0b22397..e14de82abd8b 100755 --- a/.github/workflows/scripts/ping-codeowners-on-new-issue.sh +++ b/.github/workflows/scripts/ping-codeowners-on-new-issue.sh @@ -93,11 +93,11 @@ if [[ -n "${PING_LINES}" ]]; then # workflow, so we have to ping code owners here. # 2. The GitHub CLI only offers multiline strings through file input, # so we provide the comment through stdin. - # 3. The PING_LINES variable must be directly put into the printf string + # 3. The PING_LINES variable must be directly put into the echo string # to get the newlines to render correctly, using string formatting # causes the newlines to be interpreted literally. - printf "Pinging code owners:\n${PING_LINES}" - printf "Pinging code owners:\n${PING_LINES}\n%s" "${LABELS_COMMENT}" \ + echo -e "Pinging code owners:\n${PING_LINES}" + echo -e "Pinging code owners:\n${PING_LINES}\n%s" "${LABELS_COMMENT}" \ | gh issue comment "${ISSUE}" -F - else echo "No code owners were found to ping" diff --git a/.github/workflows/scripts/setup_e2e_tests.sh b/.github/workflows/scripts/setup_e2e_tests.sh index 52e4577d6f56..cd1d077b946f 100755 --- a/.github/workflows/scripts/setup_e2e_tests.sh +++ b/.github/workflows/scripts/setup_e2e_tests.sh @@ -4,7 +4,7 @@ # SPDX-License-Identifier: Apache-2.0 TESTS="$(make -s -C testbed list-tests | xargs echo|sed 's/ /|/g')" -TESTS=(${TESTS//|/ }) +TESTS=("${TESTS//|/ }") MATRIX="{\"include\":[" curr="" for i in "${!TESTS[@]}"; do @@ -20,4 +20,4 @@ else fi done MATRIX+=",{\"test\":\"$curr\"}]}" -echo "loadtest_matrix=$MATRIX" >> $GITHUB_OUTPUT +echo "loadtest_matrix=$MATRIX" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/shellcheck.yaml b/.github/workflows/shellcheck.yaml new file mode 100644 index 000000000000..ddcf071147b2 --- /dev/null +++ b/.github/workflows/shellcheck.yaml @@ -0,0 +1,32 @@ +name: shellcheck +on: + push: + branches: + - main + pull_request: +permissions: {} + +jobs: + shellcheck: + name: shellcheck + runs-on: ubuntu-latest + env: + VERSION: v0.10.0 + steps: + - uses: actions/checkout@v4 + - name: shellcheck workflow-scripts + uses: ludeeus/action-shellcheck@2.0.0 + env: + SHELLCHECK_OPTS: -x -e SC2059 -e SC2086 + with: + scandir: ".github/workflows/scripts" + severity: warning + version: ${{ env.VERSION }} + - name: shellcheck buildscripts + uses: ludeeus/action-shellcheck@2.0.0 + env: + SHELLCHECK_OPTS: -x -e SC2059 -e SC2086 + with: + scandir: "internal/buildscripts" + severity: warning + version: ${{ env.VERSION }} diff --git a/internal/buildscripts/packaging/fpm/common.sh b/internal/buildscripts/packaging/fpm/common.sh index 3ec39d83d90c..40d90e362f6b 100644 --- a/internal/buildscripts/packaging/fpm/common.sh +++ b/internal/buildscripts/packaging/fpm/common.sh @@ -3,49 +3,52 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 -FPM_DIR="$( cd "$( dirname ${BASH_SOURCE[0]} )" && pwd )" - -PKG_NAME="otel-contrib-collector" -PKG_VENDOR="OpenTelemetry Community" -PKG_MAINTAINER="OpenTelemetry Community " -PKG_DESCRIPTION="OpenTelemetry Contrib Collector" -PKG_LICENSE="Apache 2.0" -PKG_URL="https://github.com/open-telemetry/opentelemetry-collector-contrib" -PKG_USER="otel" -PKG_GROUP="otel" - -SERVICE_NAME="otel-contrib-collector" -PROCESS_NAME="otelcontribcol" - -CONFIG_PATH="$REPO_DIR/examples/demo/otel-collector-config.yaml" -SERVICE_PATH="$FPM_DIR/$SERVICE_NAME.service" -ENVFILE_PATH="$FPM_DIR/$SERVICE_NAME.conf" -PREINSTALL_PATH="$FPM_DIR/preinstall.sh" -POSTINSTALL_PATH="$FPM_DIR/postinstall.sh" -PREUNINSTALL_PATH="$FPM_DIR/preuninstall.sh" +FPM_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +export FPM_DIR + +export PKG_NAME="otel-contrib-collector" +export PKG_VENDOR="OpenTelemetry Community" +export PKG_MAINTAINER="OpenTelemetry Community " +export PKG_DESCRIPTION="OpenTelemetry Contrib Collector" +export PKG_LICENSE="Apache 2.0" +export PKG_URL="https://github.com/open-telemetry/opentelemetry-collector-contrib" +export PKG_USER="otel" +export PKG_GROUP="otel" + +export SERVICE_NAME="otel-contrib-collector" +export PROCESS_NAME="otelcontribcol" + +export CONFIG_PATH="$REPO_DIR/examples/demo/otel-collector-config.yaml" +export SERVICE_PATH="$FPM_DIR/$SERVICE_NAME.service" +export ENVFILE_PATH="$FPM_DIR/$SERVICE_NAME.conf" +export PREINSTALL_PATH="$FPM_DIR/preinstall.sh" +export POSTINSTALL_PATH="$FPM_DIR/postinstall.sh" +export PREUNINSTALL_PATH="$FPM_DIR/preuninstall.sh" docker_cp() { local container="$1" local src="$2" local dest="$3" - local dest_dir="$( dirname "$dest" )" + local dest_dir + dest_dir="$( dirname "$dest" )" echo "Copying $src to $container:$dest ..." - podman exec $container mkdir -p "$dest_dir" - podman cp "$src" $container:"$dest" + podman exec "$container" mkdir -p "$dest_dir" + podman cp "$src" "$container":"$dest" } install_pkg() { local container="$1" local pkg_path="$2" - local pkg_base=$( basename "$pkg_path" ) + local pkg_base + pkg_base=$( basename "$pkg_path" ) echo "Installing $pkg_base ..." - docker_cp $container "$pkg_path" /tmp/$pkg_base + docker_cp "$container" "$pkg_path" /tmp/"$pkg_base" if [[ "${pkg_base##*.}" = "deb" ]]; then - podman exec $container dpkg -i /tmp/$pkg_base + podman exec "$container" dpkg -i /tmp/"$pkg_base" else - podman exec $container rpm -ivh /tmp/$pkg_base + podman exec "$container" rpm -ivh /tmp/"$pkg_base" fi } @@ -56,8 +59,8 @@ uninstall_pkg() { echo "Uninstalling $pkg_name ..." if [[ "$pkg_type" = "deb" ]]; then - podman exec $container dpkg -r $pkg_name + podman exec "$container" dpkg -r "$pkg_name" else - podman exec $container rpm -e $pkg_name + podman exec "$container" rpm -e "$pkg_name" fi } diff --git a/internal/buildscripts/packaging/fpm/deb/build.sh b/internal/buildscripts/packaging/fpm/deb/build.sh index 36ff775dc84d..b07fff55ea2e 100755 --- a/internal/buildscripts/packaging/fpm/deb/build.sh +++ b/internal/buildscripts/packaging/fpm/deb/build.sh @@ -5,14 +5,14 @@ set -euxo pipefail -SCRIPT_DIR="$( cd "$( dirname ${BASH_SOURCE[0]} )" && pwd )" +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" REPO_DIR="$( cd "$SCRIPT_DIR/../../../../../" && pwd )" VERSION="${1:-}" ARCH="${2:-"amd64"}" OUTPUT_DIR="${3:-"$REPO_DIR/dist/"}" OTELCONTRIBCOL_PATH="$REPO_DIR/bin/otelcontribcol_linux_$ARCH" -. $SCRIPT_DIR/../common.sh +. "$SCRIPT_DIR"/../common.sh if [[ -z "$VERSION" ]]; then latest_tag="$( git describe --abbrev=0 --match v[0-9]* )" @@ -21,7 +21,7 @@ fi mkdir -p "$OUTPUT_DIR" -fpm -s dir -t deb -n $PKG_NAME -v ${VERSION#v} -f -p "$OUTPUT_DIR" \ +fpm -s dir -t deb -n "$PKG_NAME" -v "${VERSION#v}" -f -p "$OUTPUT_DIR" \ --vendor "$PKG_VENDOR" \ --maintainer "$PKG_MAINTAINER" \ --description "$PKG_DESCRIPTION" \ @@ -34,7 +34,7 @@ fpm -s dir -t deb -n $PKG_NAME -v ${VERSION#v} -f -p "$OUTPUT_DIR" \ --before-install "$PREINSTALL_PATH" \ --after-install "$POSTINSTALL_PATH" \ --pre-uninstall "$PREUNINSTALL_PATH" \ - $SERVICE_PATH=/lib/systemd/system/$SERVICE_NAME.service \ - $OTELCONTRIBCOL_PATH=/usr/bin/otelcontribcol \ - $CONFIG_PATH=/etc/otel-contrib-collector/config.yaml \ - $ENVFILE_PATH=/etc/otel-contrib-collector/otel-contrib-collector.conf \ No newline at end of file + "$SERVICE_PATH"=/lib/systemd/system/"$SERVICE_NAME".service \ + "$OTELCONTRIBCOL_PATH"=/usr/bin/otelcontribcol \ + "$CONFIG_PATH"=/etc/otel-contrib-collector/config.yaml \ + "$ENVFILE_PATH"=/etc/otel-contrib-collector/otel-contrib-collector.conf diff --git a/internal/buildscripts/packaging/fpm/rpm/build.sh b/internal/buildscripts/packaging/fpm/rpm/build.sh index 0c49d8f065c2..4b2d072c62cb 100755 --- a/internal/buildscripts/packaging/fpm/rpm/build.sh +++ b/internal/buildscripts/packaging/fpm/rpm/build.sh @@ -5,14 +5,14 @@ set -euxo pipefail -SCRIPT_DIR="$( cd "$( dirname ${BASH_SOURCE[0]} )" && pwd )" +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" REPO_DIR="$( cd "$SCRIPT_DIR/../../../../../" && pwd )" VERSION="${1:-}" ARCH="${2:-"amd64"}" OUTPUT_DIR="${3:-"$REPO_DIR/dist/"}" OTELCONTRIBCOL_PATH="$REPO_DIR/bin/otelcontribcol_linux_$ARCH" -. $SCRIPT_DIR/../common.sh +. "$SCRIPT_DIR"/../common.sh if [[ -z "$VERSION" ]]; then latest_tag="$( git describe --abbrev=0 --match v[0-9]* )" @@ -27,7 +27,7 @@ fi mkdir -p "$OUTPUT_DIR" -fpm -s dir -t rpm -n $PKG_NAME -v ${VERSION#v} -f -p "$OUTPUT_DIR" \ +fpm -s dir -t rpm -n "$PKG_NAME" -v "${VERSION#v}" -f -p "$OUTPUT_DIR" \ --vendor "$PKG_VENDOR" \ --maintainer "$PKG_MAINTAINER" \ --description "$PKG_DESCRIPTION" \ @@ -40,7 +40,7 @@ fpm -s dir -t rpm -n $PKG_NAME -v ${VERSION#v} -f -p "$OUTPUT_DIR" \ --before-install "$PREINSTALL_PATH" \ --after-install "$POSTINSTALL_PATH" \ --pre-uninstall "$PREUNINSTALL_PATH" \ - $SERVICE_PATH=/lib/systemd/system/$SERVICE_NAME.service \ - $OTELCONTRIBCOL_PATH=/usr/bin/otelcontribcol \ - $CONFIG_PATH=/etc/otel-contrib-collector/config.yaml \ - $ENVFILE_PATH=/etc/otel-contrib-collector/otel-contrib-collector.conf + "$SERVICE_PATH"=/lib/systemd/system/"$SERVICE_NAME".service \ + "$OTELCONTRIBCOL_PATH"=/usr/bin/otelcontribcol \ + "$CONFIG_PATH"=/etc/otel-contrib-collector/config.yaml \ + "$ENVFILE_PATH"=/etc/otel-contrib-collector/otel-contrib-collector.conf diff --git a/internal/buildscripts/packaging/fpm/test.sh b/internal/buildscripts/packaging/fpm/test.sh index 9baf04cb0903..e52b3407a4c2 100755 --- a/internal/buildscripts/packaging/fpm/test.sh +++ b/internal/buildscripts/packaging/fpm/test.sh @@ -5,11 +5,12 @@ set -euov pipefail -SCRIPT_DIR="$( cd "$( dirname ${BASH_SOURCE[0]} )" && pwd )" +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" REPO_DIR="$( cd "$SCRIPT_DIR/../../../../" && pwd )" +export REPO_DIR PKG_PATH="${1:-}" -. $SCRIPT_DIR/common.sh +. "$SCRIPT_DIR"/common.sh if [[ -z "$PKG_PATH" ]]; then echo "usage: ${BASH_SOURCE[0]} DEB_OR_RPM_PATH" >&2 @@ -32,10 +33,10 @@ image_name="otelcontribcol-$pkg_type-test" container_name="$image_name" container_exec="podman exec $container_name" -trap "podman rm -fv $container_name >/dev/null 2>&1 || true" EXIT +trap 'podman rm -fv $container_name >/dev/null 2>&1 || true' EXIT -podman build -t $image_name -f "$SCRIPT_DIR/$pkg_type/Dockerfile.test" "$SCRIPT_DIR" -podman rm -fv $container_name >/dev/null 2>&1 || true +podman build -t "$image_name" -f "$SCRIPT_DIR/$pkg_type/Dockerfile.test" "$SCRIPT_DIR" +podman rm -fv "$container_name" >/dev/null 2>&1 || true # test install CRUN_VER='1.14.4' @@ -52,8 +53,8 @@ crun = [ EOF echo -podman run --name $container_name -d $image_name -install_pkg $container_name "$PKG_PATH" +podman run --name "$container_name" -d "$image_name" +install_pkg "$container_name" "$PKG_PATH" # ensure service has started and still running after 5 seconds sleep 5 @@ -65,7 +66,7 @@ $container_exec pgrep -a -u otel $PROCESS_NAME # test uninstall echo -uninstall_pkg $container_name $pkg_type +uninstall_pkg "$container_name" "$pkg_type" echo "Checking $SERVICE_NAME service status after uninstall ..." if $container_exec systemctl --no-pager status $SERVICE_NAME; then