From 30cdd522309e7fb05ff963dbf300b8ea03db93e4 Mon Sep 17 00:00:00 2001 From: Andrew Matthews Date: Thu, 22 Aug 2024 17:25:44 -0400 Subject: [PATCH] [CICD-583] Split FLAGS variable into an array (#32) * Split FLAGS variable into an array Rsync flags sometimes include whitespace in the flag's value. For example, --filter=':- .gitignore' includes whitespace between the filter rule and the file name. The single quotes surrounding the flag's value are meant to prevent splitting. Unfortunately, when bash expands the $FLAGS variable as part of the main rsync command it escapes the single quotes and then splits on the whitespace between :- and .gitignore. This was causing rsync to see the filter flag as two separate arguments, --filter=:- and .gitignore. Simply double-quoting $FLAGS string doesn't help because it prevents splitting between flags when multiple are provided. To fix this, we needed to split the FLAGS variable into an array while respecting single quotes. This pre-split array can be passed to rsync with double-quotes to prevent further splitting. Using the --filter example from before, our new strategy will insert --filter=':- .gitignore' as a single item in the FLAGS array which is then passed to rsync as '--filter=:- .gitignore'. * Add scripts to test FLAGS parsing and quoting strategies --- .changeset/nervous-bobcats-fry.md | 5 ++ Dockerfile | 1 + Makefile | 3 ++ README.md | 13 +++++- entrypoint.sh | 19 +++++--- functions.sh | 27 +++++++++++ tests/common.sh | 5 ++ tests/test_flag_formats.sh | 55 ++++++++++++++++++++++ tests/test_functions.sh | 78 +++++++++++++++++++++++++++++++ 9 files changed, 198 insertions(+), 8 deletions(-) create mode 100644 .changeset/nervous-bobcats-fry.md create mode 100644 functions.sh create mode 100644 tests/common.sh create mode 100755 tests/test_flag_formats.sh create mode 100755 tests/test_functions.sh diff --git a/.changeset/nervous-bobcats-fry.md b/.changeset/nervous-bobcats-fry.md new file mode 100644 index 0000000..5aac22c --- /dev/null +++ b/.changeset/nervous-bobcats-fry.md @@ -0,0 +1,5 @@ +--- +"@wpengine/site-deploy": patch +--- + +Fixes a bug that caused certain flags in the FLAGS option to be incorrectly parsed by rsync diff --git a/Dockerfile b/Dockerfile index 8bb9440..d2939ed 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,6 +7,7 @@ RUN apk update \ php \ && rm -rf /var/cache/apk/* # Add entrypoint and excludes +ADD functions.sh /functions.sh ADD entrypoint.sh /entrypoint.sh ADD exclude.txt /exclude.txt ENTRYPOINT ["/entrypoint.sh"] diff --git a/Makefile b/Makefile index 4265efb..6206910 100644 --- a/Makefile +++ b/Makefile @@ -21,3 +21,6 @@ version: build docker image tag $(IMAGE) $(IMAGE_NAME):v$(MAJOR_VERSION) && \ docker image tag $(IMAGE) $(IMAGE_NAME):v$(MAJOR_VERSION).$(MINOR_VERSION) && \ docker image tag $(IMAGE) $(IMAGE_NAME):v$(MAJOR_VERSION).$(MINOR_VERSION).$(PATCH_VERSION) + +test: + ./tests/test_functions.sh diff --git a/README.md b/README.md index 96e581f..3b600e1 100644 --- a/README.md +++ b/README.md @@ -20,13 +20,22 @@ You can use this image to deploy a site from your local machine. 2. Change directories into the root of the local site you'd like to deploy. 3. Create a `.env` file with the following variables, changing their values as needed. +> [!WARNING] +> Since `docker run` does not strip double-quotes from variables in the .env file, we don't use them +> to wrap entire variable values. Instead, we must use single or double-quotes around flag values that +> contain whitespace to prevent splitting (`--filter=':= .gitignore'`). The `=` sign between the flag +> and its value is also required (`--filter=':= .gitignore'` rather than `--filter ':= .gitignore'`). + ```sh -WPE_ENV=yourinstall # The target WP Engine install name. +# Required. The target WP Engine install name. +WPE_ENV=yourinstall +# Optional. Default values shown. REMOTE_PATH= SRC_PATH=. -PHP_LINT=TRUE +PHP_LINT=FALSE CACHE_CLEAR=TRUE SCRIPT= +FLAGS=-azvr --inplace --exclude='.*' ``` 3. Set an environment variable with your private SSH key, replacing the key file name with your own. diff --git a/entrypoint.sh b/entrypoint.sh index e1579db..90edf79 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -2,13 +2,19 @@ set -e +# Get the directory of the current script +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# Source the functions.sh file relative to the current script's location +source "${SCRIPT_DIR}/functions.sh" + validate() { # mandatory params : WPE_SSHG_KEY_PRIVATE="${WPE_SSHG_KEY_PRIVATE:?'WPE_SSHG_KEY_PRIVATE variable missing from Repo or Workspace variables.'}" # optional params : REMOTE_PATH="${REMOTE_PATH:=""}" : SRC_PATH="${SRC_PATH:="."}" - : FLAGS="${FLAGS:="-azvr --inplace --exclude=".*""}" + : FLAGS="${FLAGS:="-azvr --inplace --exclude='.*'"}" : PHP_LINT="${PHP_LINT:="FALSE"}" : CACHE_CLEAR="${CACHE_CLEAR:="TRUE"}" : SCRIPT="${SCRIPT:=""}" @@ -33,8 +39,8 @@ setup_env() { else CICD_VENDOR="wpe_cicd" fi - echo "Deploying your code to:" - echo "${WPE_ENV_NAME}" + parse_flags "$FLAGS" + print_deployment_info WPE_SSH_HOST="${WPE_ENV_NAME}.ssh.wpengine.net" DIR_PATH="${REMOTE_PATH}" @@ -105,8 +111,9 @@ sync_files() { ssh -nNf -v -i "${WPE_SSHG_KEY_PRIVATE_PATH}" -o StrictHostKeyChecking=no -o ControlMaster=yes -o ControlPath="$SSH_PATH/ctl/%C" "$WPE_FULL_HOST" echo "!!! MULTIPLEX SSH CONNECTION ESTABLISHED !!!" - # shellcheck disable=SC2086 - rsync --rsh="ssh -v -p 22 -i ${WPE_SSHG_KEY_PRIVATE_PATH} -o StrictHostKeyChecking=no -o 'ControlPath=$SSH_PATH/ctl/%C'" ${FLAGS} --exclude-from='/exclude.txt' --chmod=D775,F664 "${SRC_PATH}" "${WPE_DESTINATION}" + set -x + rsync --rsh="ssh -v -p 22 -i ${WPE_SSHG_KEY_PRIVATE_PATH} -o StrictHostKeyChecking=no -o 'ControlPath=$SSH_PATH/ctl/%C'" "${FLAGS_ARRAY[@]}" --exclude-from='/exclude.txt' --chmod=D775,F664 "${SRC_PATH}" "${WPE_DESTINATION}" + set +x if [[ -n ${SCRIPT} || -n ${CACHE_CLEAR} ]]; then @@ -139,4 +146,4 @@ setup_env setup_ssh_dir check_lint check_cache -sync_files \ No newline at end of file +sync_files diff --git a/functions.sh b/functions.sh new file mode 100644 index 0000000..c2fa82c --- /dev/null +++ b/functions.sh @@ -0,0 +1,27 @@ +#!/bin/bash + +set -e + +# Function to parse FLAGS into an array +# +# Bash doesn't respect quotes when splitting the FLAGS string on whitespace +# which can lead to incorrect splitting on arguments like --filter=':- .gitignore'. +# +# xargs does respect quotes, so we use that here to convert the string into a null-delimited +# sequence of arguments. We then read that sequence into an array by splitting on the null character. +# +# https://superuser.com/questions/1529226/get-bash-to-respect-quotes-when-word-splitting-subshell-output +parse_flags() { + local flags="$1" + FLAGS_ARRAY=() + while IFS= read -r -d '' flag; do FLAGS_ARRAY+=("$flag"); done < <(echo "$flags" | xargs printf '%s\0') +} + +print_deployment_info() { + echo "Deploying your code to:" + echo -e "\t${WPE_ENV_NAME}" + echo -e "with the following ${#FLAGS_ARRAY[@]} rsync argument(s):" + for flag in "${FLAGS_ARRAY[@]}"; do + echo -e "\t$flag" + done +} diff --git a/tests/common.sh b/tests/common.sh new file mode 100644 index 0000000..154e436 --- /dev/null +++ b/tests/common.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +GREEN='\033[0;32m' +RED='\033[0;31m' +NC='\033[0m' # No Color diff --git a/tests/test_flag_formats.sh b/tests/test_flag_formats.sh new file mode 100755 index 0000000..ce3f120 --- /dev/null +++ b/tests/test_flag_formats.sh @@ -0,0 +1,55 @@ +#!/bin/bash + +# Demonstrates the different ways to pass flags to rsync. +# +# When determining the correct way to pass flags to rsync, it is helpful to +# understand how the shell interprets quotes and whitespace. This script runs +# in debug mode to show how the shell interprets the flags for each test case. +# +# This is a stand-alone script. It is only meant for demonstration and does not +# directly test code in this project. + +# Get the directory of the current script +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +source "${SCRIPT_DIR}/common.sh" + +set -x + +FLAGS="-avzr --filter=':- .gitignore' --exclude='.*'" +FLAGS_ARRAY=("-avzr" "--filter=:- .gitignore" "--exclude='.*'") + +test_flags_no_quotes() { + rsync $FLAGS --dry-run "$SCRIPT_DIR"/data . > /dev/null + if [ $? -eq 0 ]; then + echo -e "${GREEN}test_flags_no_quotes: Success${NC}" + else + echo -e "${RED}test_flags_no_quotes: Failure${NC}" + fi +} + +test_flags_double_quotes() { + rsync "$FLAGS" --dry-run "$SCRIPT_DIR"/data . > /dev/null + if [ $? -eq 0 ]; then + echo -e "${GREEN}test_flags_double_quotes: Success${NC}" + else + echo -e "${RED}test_flags_double_quotes: Failure${NC}" + fi +} + +test_flags_array() { + rsync "${FLAGS_ARRAY[@]}" --dry-run "$SCRIPT_DIR"/data . > /dev/null + if [ $? -eq 0 ]; then + echo -e "${GREEN}test_flags_array: Success${NC}" + else + echo -e "${RED}test_flags_array: Failure${NC}" + fi +} + +main() { + test_flags_no_quotes + test_flags_double_quotes + test_flags_array +} + +main diff --git a/tests/test_functions.sh b/tests/test_functions.sh new file mode 100755 index 0000000..3f1c10c --- /dev/null +++ b/tests/test_functions.sh @@ -0,0 +1,78 @@ +#!/bin/bash + +# Get the directory of the current script +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +source "${SCRIPT_DIR}/common.sh" +source "${SCRIPT_DIR}/../functions.sh" + +# First argument represents the value of the FLAGS variable. +# The rest of the arguments represent the expected values of the FLAGS_ARRAY. +# +# usage: test_parse_flags +test_parse_flags() { + local test_case=$1 + shift + local expected_args=("$@") + + parse_flags "$test_case" + + local actual_count="${#FLAGS_ARRAY[@]}" + local expected_count="${#expected_args[@]}" + + if [[ "$actual_count" -ne "$expected_count" ]]; then + echo -e "${RED}Test failed for FLAGS='$test_case': expected $expected_count arguments, got $actual_count." + echo -e "\tActual arguments: ${FLAGS_ARRAY[*]}${NC}" + return + fi + + for i in "${!expected_args[@]}"; do + if [[ "${FLAGS_ARRAY[$i]}" != "${expected_args[$i]}" ]]; then + echo -e "${RED}Test failed for FLAGS='$test_case': expected '${expected_args[$i]}', got '${FLAGS_ARRAY[$i]}'.${NC}" + return + fi + done + + echo -e "${GREEN}Test passed for FLAGS=\"$test_case\".${NC}" +} + +# Test cases +test_parse_flags \ + "-azvr --inplace --exclude='.*'" \ + "-azvr" \ + "--inplace" \ + "--exclude=.*" + +test_parse_flags \ + '-azvr --inplace --exclude=".*"' \ + "-azvr" \ + "--inplace" \ + "--exclude=.*" + +test_parse_flags \ + "-azvr --filter=':- .gitignore' --exclude='.*'" \ + "-azvr" \ + "--filter=:- .gitignore" \ + "--exclude=.*" + +test_parse_flags \ + "-avzr --delete --filter='P /wp-uploads/**'" \ + "-avzr" \ + "--delete" \ + "--filter=P /wp-uploads/**" + +test_parse_flags \ + "-avzr --delete --exclude='\$dollar'" \ + "-avzr" \ + "--delete" \ + "--exclude=\$dollar" + +test_parse_flags \ + "-avzr --exclude='\`back-ticks\`'" \ + "-avzr" \ + "--exclude=\`back-ticks\`" + +test_parse_flags \ + "-avzr --exclude='path\\with\\backslash'" \ + "-avzr" \ + "--exclude=path\\with\\backslash"