From 21431bb4b0adb5fbe8234b36a84ee89b30dcd773 Mon Sep 17 00:00:00 2001 From: Mike Lang Date: Fri, 9 Dec 2016 05:34:25 -0800 Subject: [PATCH] scope launch script: Fix shellcheck linter warnings By far the majority of these were variables which were not quoted. While, yes, right now we can guarentee most of these variables will never contain spaces, this could someday change and applying quoting as a universal rule prevents future mistakes. The ARGS="$@" -> "$*" change is purely stylistic and mainly is used to indicate the intent that we actually wanted to concatenate all the args by spaces, not keep them seperated as "$@" would in many situations, but not this one. Several warnings remain, in places where we intentionally want to split a variable on whitespace, or otherwise do what shellcheck is warning us against. Of note is shellcheck warning SC2166, which says to prefer [ foo ] || [ bar ] over [ foo -o bar ] as the -a and -o flags have differing behaviour on some systems. I've opted to keep these for now, since the version check test command would need to be replaced by a LOT of subshells to achieve the same effect, which feels dirtier. --- scope | 56 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/scope b/scope index 4fdb07786..9bb0ddbed 100755 --- a/scope +++ b/scope @@ -2,7 +2,7 @@ set -eu -ARGS="$@" +ARGS="$*" usage() { echo "Usage:" @@ -18,11 +18,11 @@ SCRIPT_VERSION="(unreleased version)" if [ "$SCRIPT_VERSION" = "(unreleased version)" ] ; then IMAGE_VERSION=latest else - IMAGE_VERSION=$SCRIPT_VERSION + IMAGE_VERSION="$SCRIPT_VERSION" fi IMAGE_VERSION=${VERSION:-$IMAGE_VERSION} SCOPE_IMAGE_NAME=weaveworks/scope -SCOPE_IMAGE=$SCOPE_IMAGE_NAME:$IMAGE_VERSION +SCOPE_IMAGE="$SCOPE_IMAGE_NAME:$IMAGE_VERSION" SCOPE_CONTAINER_NAME=weavescope SCOPE_APP_CONTAINER_NAME=weavescope-app IP_REGEXP="[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}" @@ -98,7 +98,7 @@ check_docker_for_mac() { # Check that a container named $1 with image $2 is not running check_not_running() { - case $(docker inspect --format='{{.State.Running}} {{.Config.Image}}' $1 2>/dev/null) in + case $(docker inspect --format='{{.State.Running}} {{.Config.Image}}' "$1" 2>/dev/null) in "true $2") echo "$1 is already running." >&2 exit 1 @@ -108,10 +108,10 @@ check_not_running() { exit 1 ;; "false $2") - docker rm $1 >/dev/null + docker rm "$1" >/dev/null ;; "false $2:"*) - docker rm $1 >/dev/null + docker rm "$1" >/dev/null ;; true*) echo "Found another running container named '$1'. Aborting." >&2 @@ -135,29 +135,29 @@ create_plugins_dir() { # sockets do not cross VM boundaries. We need this directory to exits on the VM. docker run --rm --entrypoint=/bin/sh \ -v /var/run:/var/run \ - $SCOPE_IMAGE -c "mkdir -p /var/run/scope/plugins" + "$SCOPE_IMAGE" -c "mkdir -p /var/run/scope/plugins" } launch_command() { - echo docker run --privileged -d --name=$SCOPE_CONTAINER_NAME --net=host --pid=host \ + echo docker run --privileged -d --name="$SCOPE_CONTAINER_NAME" --net=host --pid=host \ -v /var/run/docker.sock:/var/run/docker.sock \ -v /var/run/scope/plugins:/var/run/scope/plugins \ -e CHECKPOINT_DISABLE \ - $WEAVESCOPE_DOCKER_ARGS $SCOPE_IMAGE --probe.docker=true + $WEAVESCOPE_DOCKER_ARGS "$SCOPE_IMAGE" --probe.docker=true } launch_docker4mac_app_command() { - echo docker run -d --name=$SCOPE_APP_CONTAINER_NAME \ + echo docker run -d --name="$SCOPE_APP_CONTAINER_NAME" \ -e CHECKPOINT_DISABLE \ -p 0.0.0.0:4040:4040 \ - $WEAVESCOPE_DOCKER_ARGS $SCOPE_IMAGE --no-probe + $WEAVESCOPE_DOCKER_ARGS "$SCOPE_IMAGE" --no-probe } launch() { - check_not_running $SCOPE_CONTAINER_NAME $SCOPE_IMAGE_NAME - docker rm -f $SCOPE_CONTAINER_NAME >/dev/null 2>&1 || true + check_not_running "$SCOPE_CONTAINER_NAME" "$SCOPE_IMAGE_NAME" + docker rm -f "$SCOPE_CONTAINER_NAME" >/dev/null 2>&1 || true CONTAINER=$($(launch_command) "$@") - echo $CONTAINER + echo "$CONTAINER" } print_app_endpoints() { @@ -173,12 +173,12 @@ check_docker_version case "$COMMAND" in command) # TODO: properly escape/quote the output of "$@" - echo $(launch_command) "$@" + echo "$(launch_command)" "$@" ;; version) docker run --rm -e CHECKPOINT_DISABLE --entrypoint=/home/weave/scope \ - $WEAVESCOPE_DOCKER_ARGS $SCOPE_IMAGE --mode=version + $WEAVESCOPE_DOCKER_ARGS "$SCOPE_IMAGE" --mode=version ;; help) @@ -190,7 +190,7 @@ scope help - Print this scope launch - Launch Scope EOF docker run --rm -e CHECKPOINT_DISABLE --entrypoint=/home/weave/scope \ - $WEAVESCOPE_DOCKER_ARGS $SCOPE_IMAGE -h + $WEAVESCOPE_DOCKER_ARGS "$SCOPE_IMAGE" -h cat >&2 </dev/null 2>&1 || true + check_not_running "$SCOPE_APP_CONTAINER_NAME" "$SCOPE_IMAGE_NAME" + check_not_running "$SCOPE_CONTAINER_NAME" "$SCOPE_IMAGE_NAME" + docker rm -f "$SCOPE_APP_CONTAINER_NAME" >/dev/null 2>&1 || true CONTAINER=$($(launch_docker4mac_app_command) "$@") - echo $CONTAINER + echo "$CONTAINER" app_ip=$(docker inspect -f '{{.NetworkSettings.IPAddress}}' "${CONTAINER}") - docker rm -f $SCOPE_CONTAINER_NAME >/dev/null 2>&1 || true + docker rm -f "$SCOPE_CONTAINER_NAME" >/dev/null 2>&1 || true CONTAINER=$($(launch_command --no-app "$@" "${app_ip}:4040")) print_app_endpoints "localhost" exit @@ -232,7 +232,7 @@ EOF launch "$@" if ! check_probe_only ; then - IP_ADDRS=$(docker run --rm --net=host --entrypoint /bin/sh $SCOPE_IMAGE -c "$IP_ADDR_CMD") + IP_ADDRS=$(docker run --rm --net=host --entrypoint /bin/sh "$SCOPE_IMAGE" -c "$IP_ADDR_CMD") print_app_endpoints $IP_ADDRS fi @@ -240,12 +240,12 @@ EOF stop) [ $# -eq 0 ] || usage - if docker inspect $SCOPE_CONTAINER_NAME >/dev/null 2>&1 ; then - docker stop $SCOPE_CONTAINER_NAME >/dev/null + if docker inspect "$SCOPE_CONTAINER_NAME" >/dev/null 2>&1 ; then + docker stop "$SCOPE_CONTAINER_NAME" >/dev/null fi if check_docker_for_mac ; then - if docker inspect $SCOPE_APP_CONTAINER_NAME >/dev/null 2>&1 ; then - docker stop $SCOPE_APP_CONTAINER_NAME >/dev/null + if docker inspect "$SCOPE_APP_CONTAINER_NAME" >/dev/null 2>&1 ; then + docker stop "$SCOPE_APP_CONTAINER_NAME" >/dev/null fi fi ;;