[CICD-761] Generate dynamic excludes for rsync (part deux) #42
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolves a warning about unused variables in the common file.
Also moves common.sh to new utils dir.
Outputs the list to stdout, which is called with process substitution in the initial rsync command. Also disables debug mode for the block calling the generate function. This prevents unwanted debug output (specific to the excludes list generation) from being shown.
🦋 Changeset detectedLatest commit: 63be915 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
9643529 to
bdd52b4
Compare
mike-day
commented
Dec 19, 2024
mike-day
commented
Dec 19, 2024
Simplifies the logic for determining what files were copied to workdir by globbing for certain dirs instead of parsing proc/self/mountinfo. This is also better for other mounted dir structures (such as when a GHA runner mounts the image); we are simply checking what is in the current dir instead of inferring the structure based off mounted source path.
mike-day
commented
Jan 2, 2025
The goal of the excludes list is to prevent users from accidentally breaking their site by overwriting or deleting important files on the server. While there are many files that could be deemed important, we explicily want to exclude a handful of files from the site root, some from wp-content, and some from wp-content/mu-plugins. In order to determine if a deploy is at risk of overwriting or deleting an important file on the server, we just need to look at the REMOTE_PATH. If REMOTE_PATH is unset, we know the destination is the site root. If REMOTE_PATH ends in wp-content, we know the destination is wp-content. If REMOTE_PATH ends in wp-content/mu-plugins, we know the destination is wp-content/mu-plugins. All three of these cases carry the risk of overwritting or deleting important files. In order to prevent this, we need to make the exclude rules relative to the REMOTE_PATH. SRC_PATH no longer matters because rsync assumes it to be identical to REMOTE_PATH for the sake of determining transfer/deletion deltas.
Tests how different REMOTE_PATHS affect the generated exclude rule output.
0f40e85 to
63be915
Compare
Thanks for the help with bringing this one home, @apmatthews! |
|
Congrats on getting this all done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA Ticket
CICD-761
Bug description
Customer reports that mu-plugins are being deleted when deploying with the GitHub Action. The plugins that are mentioned as being deleted are already part of our global excludes list in wpengine/site-deploy.
Reported in: wpengine/github-action-wpe-site-deploy#91
Background
This PR is another implementation idea after having worked through several other options for the ticket.
In #37, we tried forcing files to adhere to a specific structure expected by the remote target (making a relative remote), but felt the approach ended up being a bit risky with re-writing directory structure on each sync.
In #39, we tried removing relative path prepending in the exclude.txt file, but the approach was deemed a risk for affecting users that deploy to the root directory of a site (the paths in exclude.txt were treated like globs, meaning custom files could be unintentionally excluded).
In #40, we tried dynamically generating the paths used for exclusion, but the solution still relies on relative pathing. This can lead to certain paths (like
cache/*) being excluded unintentionally.What Are We Doing Here
Like #40, this approach creates an exclusion lists for rsync on the fly, also relying on the
REMOTE_PATH. The main differences are with the testing approach, which was refined in this PR.