[CICD-583] Split FLAGS variable into an array

[apmatthews]

JIRA Ticket

CICD-583

What Are We Doing Here

1. Converting the FLAGS variable to 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. For example:

FLAGS="-avzr --filter=':- .gitignore' --exclude='.*'"
# Using $FLAGS without quotes
rsync $FLAGS src/ dest/
# ...expands to...
rsync -avzr '--filter='\'':-' '.gitignore'\''' '--exclude='\''.*'\''' src/ dest/
# ...resulting in the error...
Unknown filter rule: `':-'

If the example above hadn't encountered the error with --filter, it still would have misinterpreted the --exclude rule and not excluded dotfiles from the deploy.

Simply double-quoting $FLAGS string doesn't help because it prevents splitting between flags when multiple are provided. For example:

FLAGS="-avzr --filter=':- .gitignore' --exclude='.*'"
# Double-quoting $FLAGS
rsync "$FLAGS" src/ dest/
# ...expands to...
rsync '-avzr --filter='\'':- .gitignore'\'' --exclude='\''.*'\''' src/ dest/
# ...resulting in the error...
rsync: -avzr --filter=':- .gitignore' --exclude='.*': unknown option

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. So we end up with:

FLAGS_ARRAY=("-avzr" "--filter=:- .gitignore" "--exclude='.*'")
# Double-quoting $FLAGS_ARRAY[@]
rsync "${FLAGS_ARRAY[@]}" src/ dest/
# ...expands to...
rsync -avzr '--filter=:- .gitignore' '--exclude=.*' src/ dest/
# No errors!

2. Enabling debug mode for the main rsync command

Bash debug mode was especially useful for determining how the FLAGS input was being transformed in all these different cases. I believe it would be just as useful for GitHub Actions and Bitbucket Pipelines users to spot issues with escaping in their FLAGS input. For that reason, I've left debug mode on for the main rsync call. Users will now see output similar to:

+ rsync '--rsh=ssh -v -p 22 -i /root/.ssh/wpe_id_rsa -o StrictHostKeyChecking=no -o '\''ControlPath=/root/.ssh/ctl/%C'\''' -avzr '--filter=:- .gitignore' --exclude-from=/exclude.txt --chmod=D775,F664 . [email protected]:sites/installname/

3. Starting a new functions.sh file

Testing these changes has been pretty difficult given that we don't have anything more granular than an end-to-end smoke test configured for this project. Fixing that is beyond the scope of this PR, but I did take the opportunity to set us up for unit testing in the future by putting the new functions I added into a separate file. This allowed me to test the function independently of a full deploy using several different test cases. In the future, we can formalize this a bit more, add a framework like (bats)[https://bats-core.readthedocs.io/en/stable/], and extract other functions from entrypoint.sh.

Testing

I've added tests for the parse_flags function. You can run those using make.

make test

Additionally, I've included a demo script that illustrates the need to parse FLAGS into an array using the examples described above. You can run that one as a stand-alone script.

./tests/test_flag_formats.sh

[changeset-bot]

🦋 Changeset detected

Latest commit: f7591e2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@wpengine/site-deploy	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[cseeman]

Thanks for running through this, look good switching this to a flags array and great job with the testing too!