From abc4ae877289913eb14a3461d46c6488aea7b9dd Mon Sep 17 00:00:00 2001 From: Ben Schwartz Date: Thu, 11 Feb 2021 15:15:57 -0500 Subject: [PATCH] Substantive shell script cleanups Running "shellcheck", and attempting to comply with the shell script style guide, required hundreds of trivial changes, and a small number of nontrivial ones. For ease of review, this PR includes only the nontrivial changes. --- src/metrics_server/test_integration_action.sh | 4 +-- .../electron_app/build_action.sh | 3 -- .../install_scripts/do_install_server.sh | 10 +++--- .../install_scripts/install_server.sh | 32 +++++++++++-------- .../scripts/fill_packaging_opts.sh | 4 ++- .../web_app/build_install_script_action.sh | 2 +- src/shadowbox/scripts/update_mmdb.sh | 2 +- 7 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/metrics_server/test_integration_action.sh b/src/metrics_server/test_integration_action.sh index 03f6507d..78b51c49 100755 --- a/src/metrics_server/test_integration_action.sh +++ b/src/metrics_server/test_integration_action.sh @@ -85,14 +85,14 @@ EOF echo "Connections request:" cat $CONNECTIONS_REQUEST -curl -X POST -H "Content-Type: application/json" -d @$CONNECTIONS_REQUEST $METRICS_URL/connections && echo +curl -X POST -H "Content-Type: application/json" -d @$CONNECTIONS_REQUEST $METRICS_URL/$CONNECTIONS_PATH && echo sleep 5 bq --project_id $BIGQUERY_PROJECT --format json query --nouse_legacy_sql 'SELECT serverId, userId, bytesTransferred, countries FROM `'"$BIGQUERY_DATASET.$CONNECTIONS_TABLE"'` WHERE serverId = "'"$SERVER_ID"'" ORDER BY bytesTransferred DESC LIMIT 2' > $CONNECTIONS_RESPONSE diff $CONNECTIONS_RESPONSE $CONNECTIONS_EXPECTED_RESPONSE echo "Features request:" cat $FEATURES_REQUEST -curl -X POST -H "Content-Type: application/json" -d @$FEATURES_REQUEST $METRICS_URL/features && echo +curl -X POST -H "Content-Type: application/json" -d @$FEATURES_REQUEST $METRICS_URL/$FEATURES_PATH && echo sleep 5 bq --project_id $BIGQUERY_PROJECT --format json query --nouse_legacy_sql 'SELECT serverId, serverVersion, dataLimit FROM `'"$BIGQUERY_DATASET.$FEATURES_TABLE"'` WHERE serverId = "'"$SERVER_ID"'" ORDER BY timestamp DESC LIMIT 1' > $FEATURES_RESPONSE diff $FEATURES_RESPONSE $FEATURES_EXPECTED_RESPONSE diff --git a/src/server_manager/electron_app/build_action.sh b/src/server_manager/electron_app/build_action.sh index b10ec52d..fd661829 100755 --- a/src/server_manager/electron_app/build_action.sh +++ b/src/server_manager/electron_app/build_action.sh @@ -19,8 +19,6 @@ readonly OUT_DIR=$BUILD_DIR/server_manager/electron_app rm -rf $OUT_DIR -readonly NODE_MODULES_BIN_DIR=$ROOT_DIR/src/server_manager/node_modules/.bin - # Build the Web App. do_action server_manager/web_app/build @@ -30,7 +28,6 @@ do_action server_manager/web_app/build tsc -p src/server_manager/electron_app/tsconfig.json --outDir build/server_manager/electron_app/js # Assemble everything together. -readonly MODULE_DIR=$(dirname $0) readonly STATIC_DIR=$OUT_DIR/static mkdir -p $STATIC_DIR mkdir -p $STATIC_DIR/server_manager diff --git a/src/server_manager/install_scripts/do_install_server.sh b/src/server_manager/install_scripts/do_install_server.sh index 9254d77d..f63ec3fd 100755 --- a/src/server_manager/install_scripts/do_install_server.sh +++ b/src/server_manager/install_scripts/do_install_server.sh @@ -1,3 +1,5 @@ +#!/bin/bash +# # Copyright 2018 The Outline Authors # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -34,11 +36,11 @@ export SHADOWBOX_DIR="${SHADOWBOX_DIR:-${HOME:-/root}/shadowbox}" mkdir -p $SHADOWBOX_DIR # Save output for debugging -exec 2>&1 >$SHADOWBOX_DIR/install-shadowbox-output +exec >$SHADOWBOX_DIR/install-shadowbox-output 2>&1 # Initialize sentry log file. export SENTRY_LOG_FILE="$SHADOWBOX_DIR/sentry-log-file.txt" -> $SENTRY_LOG_FILE +true > $SENTRY_LOG_FILE function log_for_sentry() { echo [$(date "+%Y-%m-%d@%H:%M:%S")] "do_install_server.sh" "$@" >>$SENTRY_LOG_FILE } @@ -147,13 +149,13 @@ export SB_PUBLIC_IP=$(cloud::public_ip) log_for_sentry "Initializing ACCESS_CONFIG" export ACCESS_CONFIG="$SHADOWBOX_DIR/access.txt" -> $ACCESS_CONFIG +true > $ACCESS_CONFIG # Set trap which publishes an error tag and sentry report only if there is an error. function finish { INSTALL_SERVER_EXIT_CODE=$? log_for_sentry "In EXIT trap, exit code $INSTALL_SERVER_EXIT_CODE" - if [[ -z $(grep apiUrl $ACCESS_CONFIG) ]] || [[ -z $(grep certSha256 $ACCESS_CONFIG) ]]; then + if ! ( grep -q apiUrl $ACCESS_CONFIG && grep -q certSha256 $ACCESS_CONFIG ); then echo "INSTALL_SCRIPT_FAILED: $INSTALL_SERVER_EXIT_CODE" | cloud::add_kv_tag "install-error" # Post error report to sentry. post_sentry_report diff --git a/src/server_manager/install_scripts/install_server.sh b/src/server_manager/install_scripts/install_server.sh index a7476120..3fc424c7 100755 --- a/src/server_manager/install_scripts/install_server.sh +++ b/src/server_manager/install_scripts/install_server.sh @@ -1,3 +1,5 @@ +#!/bin/bash +# # Copyright 2018 The Outline Authors # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -65,18 +67,18 @@ function log_command() { function log_error() { local -r ERROR_TEXT="\033[0;31m" # red local -r NO_COLOR="\033[0m" - printf "${ERROR_TEXT}${1}${NO_COLOR}\n" + echo -e "${ERROR_TEXT}$1${NO_COLOR}" echo "${1}" >> ${FULL_LOG} } # Pretty prints text to stdout, and also writes to sentry log file if set. function log_start_step() { log_for_sentry "$@" - str="> $@" + str="> $*" lineLength=47 echo -n "$str" - numDots=$(expr $lineLength - ${#str} - 1) - if [[ $numDots > 0 ]]; then + numDots=$(( lineLength - ${#str} - 1 )) + if (( numDots > 0 )); then echo -n " " for i in $(seq 1 "$numDots"); do echo -n .; done fi @@ -102,7 +104,7 @@ function confirm() { echo -n "> $1 [Y/n] " local RESPONSE read RESPONSE - RESPONSE=$(echo "$RESPONSE" | tr '[A-Z]' '[a-z]') + RESPONSE=$(echo "$RESPONSE" | tr '[:upper:]' '[:lower:]') if [[ -z "$RESPONSE" ]] || [[ "$RESPONSE" = "y" ]] || [[ "$RESPONSE" = "yes" ]]; then return 0 fi @@ -176,7 +178,7 @@ function handle_docker_container_conflict() { local readonly CONTAINER_NAME=$1 local readonly EXIT_ON_NEGATIVE_USER_RESPONSE=$2 local PROMPT="The container name \"$CONTAINER_NAME\" is already in use by another container. This may happen when running this script multiple times." - if $EXIT_ON_NEGATIVE_USER_RESPONSE; then + if [[ "${EXIT_ON_NEGATIVE_USER_RESPONSE}" == 'true' ]]; then PROMPT="$PROMPT We will attempt to remove the existing container and restart it. Would you like to proceed?" else PROMPT="$PROMPT Would you like to replace this container? If you answer no, we will proceed with the remainder of the installation." @@ -221,8 +223,8 @@ function get_random_port { function create_persisted_state_dir() { readonly STATE_DIR="$SHADOWBOX_DIR/persisted-state" - mkdir -p --mode=770 "${STATE_DIR}" - chmod g+s "${STATE_DIR}" + mkdir -p "${STATE_DIR}" + chmod ug+rwx,g+s,o-rwx "${STATE_DIR}" } # Generate a secret key for access to the Management API and store it in a tag. @@ -390,8 +392,8 @@ install_shadowbox() { log_for_sentry "Creating Outline directory" export SHADOWBOX_DIR="${SHADOWBOX_DIR:-/opt/outline}" - mkdir -p --mode=770 $SHADOWBOX_DIR - chmod u+s $SHADOWBOX_DIR + mkdir -p $SHADOWBOX_DIR + chmod u+s,ug+rwx,o-rwx $SHADOWBOX_DIR log_for_sentry "Setting API port" API_PORT="${FLAGS_API_PORT}" @@ -412,11 +414,13 @@ install_shadowbox() { exit 1 fi - # If $ACCESS_CONFIG already exists, copy it to backup then clear it. - # Note we can't do "mv" here as do_install_server.sh may already be tailing - # this file. + # If $ACCESS_CONFIG is already populated, make a backup before clearing it. log_for_sentry "Initializing ACCESS_CONFIG" - [[ -f $ACCESS_CONFIG ]] && cp $ACCESS_CONFIG $ACCESS_CONFIG.bak && > $ACCESS_CONFIG + if [[ -s $ACCESS_CONFIG ]]; then + # Note we can't do "mv" here as do_install_server.sh may already be tailing + # this file. + cp $ACCESS_CONFIG $ACCESS_CONFIG.bak && true > $ACCESS_CONFIG + fi # Make a directory for persistent state run_step "Creating persistent state dir" create_persisted_state_dir diff --git a/src/server_manager/scripts/fill_packaging_opts.sh b/src/server_manager/scripts/fill_packaging_opts.sh index 62ba8ec1..c7f4cd20 100755 --- a/src/server_manager/scripts/fill_packaging_opts.sh +++ b/src/server_manager/scripts/fill_packaging_opts.sh @@ -22,7 +22,9 @@ # Note that you MUST use "source" in order to run the script in the same process as the calling # script, allowing fill_packaging_opts.sh to fill variables for the caller. -readonly YARN_COMMAND="yarn do $(echo "${1}" | sed 's:.*\(src/server_manager/electron_app/.*\)_action.sh:\1:')" +readonly ELECTRON_PATH='server_manager/electron_app/' +readonly RELATIVE="${1#*/src/${ELECTRON_PATH}}" +readonly YARN_COMMAND="yarn do ${ELECTRON_PATH}${RELATIVE%_action.sh}" shift function usage () { diff --git a/src/server_manager/web_app/build_install_script_action.sh b/src/server_manager/web_app/build_install_script_action.sh index babd4e31..b2a401e8 100755 --- a/src/server_manager/web_app/build_install_script_action.sh +++ b/src/server_manager/web_app/build_install_script_action.sh @@ -23,7 +23,7 @@ rm -rf $OUT_DIR mkdir -p $OUT_DIR pushd $ROOT_DIR/src/server_manager/install_scripts > /dev/null -tar --create --gzip -f $OUT_DIR/scripts.tgz *.sh +tar --create --gzip -f $OUT_DIR/scripts.tgz ./*.sh # Node.js on Cygwin doesn't like absolute Unix-style paths. # So, we use a relative path as input. diff --git a/src/shadowbox/scripts/update_mmdb.sh b/src/shadowbox/scripts/update_mmdb.sh index acbfb9e7..43e92e1a 100755 --- a/src/shadowbox/scripts/update_mmdb.sh +++ b/src/shadowbox/scripts/update_mmdb.sh @@ -15,7 +15,7 @@ for monthdelta in $(seq 10); do newdate=$(date --date="-$monthdelta months" +%Y-%m) ADDRESS="https://download.db-ip.com/free/dbip-country-lite-${newdate}.mmdb.gz" curl --fail --silent "${ADDRESS}" -o "$TMPDIR/$FILENAME.gz" > /dev/null && break - if [[ $monthdelta -eq 10 ]]; then + if [ $monthdelta -eq 10 ]; then # A weird exit code on purpose -- we should catch this long before it triggers exit 2 fi