diff --git a/.github/actions/test-setup/action.yml b/.github/actions/test-setup/action.yml index c24e388..7708096 100644 --- a/.github/actions/test-setup/action.yml +++ b/.github/actions/test-setup/action.yml @@ -76,7 +76,11 @@ runs: echo baseline > e mkdir -p language/en echo subdir-config > language/en/system.site.yml - echo dotfile-config > .htaccess + # A legacy config repo that still carries .htaccess copies. They belong + # to the site repo, so the action must never import them; the content + # differs from the site repo's so the tests can tell them apart. + echo stale-htaccess > .htaccess + echo stale-htaccess > language/en/.htaccess git add c e language .htaccess git rm d git commit -m "Updated" @@ -91,6 +95,10 @@ runs: git rm -rf . mkdir -p a z config/sync find ../remote -mindepth 1 -maxdepth 1 ! -name .git -exec cp -a {} config/sync \; + # The site repo owns the .htaccess files: overwrite the copies that came + # from the config repo so tests can prove the site's version survives. + echo site-htaccess > config/sync/.htaccess + echo site-htaccess > config/sync/language/en/.htaccess touch {a,z}/.gitkeep b git add . git commit -m "Add mock Drupal site" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 2be892d..ae12fbe 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,14 +20,17 @@ on: branches: - main - develop - schedule: - # * is a special character in YAML so you have to quote this string - - cron: '0 0 1 * *' workflow_dispatch: +# These tests share two real GitHub repos, and the branches they use are named +# after the job alone, so two runs of this workflow cannot safely overlap. The +# group deliberately omits github.ref: a pull_request run and the push run for +# the same merge have different refs and would otherwise never serialise +# against each other. cancel-in-progress would kill a run mid-test, and a +# cancelled job's teardown doesn't reliably finish deleting its branches. concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true + group: ${{ github.workflow }} + cancel-in-progress: false jobs: test-initial-setup: @@ -140,11 +143,15 @@ jobs: git commit -m "Prod config change before mirror branch exists" git push origin HEAD:${{ env.test_config_repo_branch }} - # Leave an obsolete config subdirectory in live to prove subdir removal. + # Leave an obsolete config subdirectory in live to prove subdir + # removal. Its .htaccess proves the restore step's dirname guard: the + # directory is gone from the export, so its .htaccess must not + # resurrect it. cd ../site git checkout ${{ env.test_site_repo_live_branch }} mkdir -p config/sync/obsolete_collection echo stale > config/sync/obsolete_collection/legacy.yml + echo site-htaccess > config/sync/obsolete_collection/.htaccess git add config/sync git commit -m "Leave an obsolete config subdirectory in live" git push origin HEAD @@ -198,8 +205,14 @@ jobs: echo "**TEST FAILURE:** config/sync/language/en/system.site.yml should be 'subdir-config' (config subdirectory not copied)." >> "$GITHUB_STEP_SUMMARY" exit=1 fi - if [[ "$(git show "$ref:config/sync/.htaccess")" != "dotfile-config" ]]; then - echo "**TEST FAILURE:** config/sync/.htaccess should be 'dotfile-config' (dotfile not copied)." >> "$GITHUB_STEP_SUMMARY" + # The .htaccess belongs to the site repo: the config repo's stale copy + # must not overwrite it, and it must not be deleted. + if [[ "$(git show "$ref:config/sync/.htaccess")" != "site-htaccess" ]]; then + echo "**TEST FAILURE:** config/sync/.htaccess should be 'site-htaccess' (site repo's dotfile lost or overwritten by the config repo's)." >> "$GITHUB_STEP_SUMMARY" + exit=1 + fi + if [[ "$(git show "$ref:config/sync/language/en/.htaccess")" != "site-htaccess" ]]; then + echo "**TEST FAILURE:** config/sync/language/en/.htaccess should be 'site-htaccess' (nested dotfile lost or overwritten by the config repo's)." >> "$GITHUB_STEP_SUMMARY" exit=1 fi if grep -q '^config/sync/obsolete_collection/' <<<"$tree"; then @@ -275,6 +288,9 @@ jobs: find config/sync -mindepth 1 -delete mv "$tmp_dir"/* config/sync echo baseline > config/sync/e + # The site repo owns the .htaccess; the config repo's stale copy must + # never replace it. + echo site-htaccess > config/sync/.htaccess git add config/sync git commit --amend -m "Update" git push --force origin HEAD @@ -343,8 +359,16 @@ jobs: exit=1 fi - if [[ ! -f config/sync/.htaccess ]]; then - echo "**TEST FAILURE:** config/sync/.htaccess should be present after merging the config PR (dotfile lost)." >> "$GITHUB_STEP_SUMMARY" + if [[ "$(cat config/sync/.htaccess)" != "site-htaccess" ]]; then + echo "**TEST FAILURE:** config/sync/.htaccess should be 'site-htaccess' after merging the config PR (site repo's dotfile lost or overwritten by the config repo's)." >> "$GITHUB_STEP_SUMMARY" + exit=1 + fi + + # The config repo carries a stale language/en/.htaccess; it must not + # be imported into a config collection directory that has only just + # appeared in the site repo. + if [[ -f config/sync/language/en/.htaccess ]]; then + echo "**TEST FAILURE:** config/sync/language/en/.htaccess should be absent (the config repo's copy was imported)." >> "$GITHUB_STEP_SUMMARY" exit=1 fi @@ -414,6 +438,9 @@ jobs: find config/sync -mindepth 1 -delete mv "$tmp_dir"/* config/sync echo baseline > config/sync/e + # The site repo owns the .htaccess; the config repo's stale copy must + # never replace it. + echo site-htaccess > config/sync/.htaccess git add config/sync # A plain commit (not --amend): the live branch tip is the orphan root # commit that staging also descends from. Amending it would create a @@ -499,6 +526,12 @@ jobs: echo "**TEST FAILURE:** config/sync/c should carry the prod value 'modified'." >> "$GITHUB_STEP_SUMMARY" exit=1 fi + # The PR branch was recreated with `git branch -f` from live, so the + # restored .htaccess comes from live's index. + if [[ "$(cat config/sync/.htaccess)" != "site-htaccess" ]]; then + echo "**TEST FAILURE:** config/sync/.htaccess should be 'site-htaccess' in the recreated PR branch (site repo's dotfile lost or overwritten by the config repo's)." >> "$GITHUB_STEP_SUMMARY" + exit=1 + fi exit $exit @@ -545,11 +578,25 @@ jobs: # Ensure the site repo config branch is behind the config repo. git remote add remote-config ../remote git fetch remote-config - git checkout -b ${{ env.test_site_repo_config_branch }} remote-config/${{ env.test_config_repo_branch }}^ + # Pin the mirror branch's start point before the config repo gains + # another commit below, so it stays the initial mock config. + mirror_base=$(git rev-parse remote-config/${{ env.test_config_repo_branch }}^) + + # Drop the .htaccess files from the config repo, leaving its HEAD with + # none. The site repo owns them, so its copies must survive an export + # that no longer carries any. (This is the regression this job guards.) + cd ../remote + git rm --quiet .htaccess language/en/.htaccess + git commit -m "Drop the .htaccess files from the config repo" + git push origin HEAD:${{ env.test_config_repo_branch }} + cd ../site + git fetch remote-config + + git checkout -b ${{ env.test_site_repo_config_branch }} "$mirror_base" tmp_dir=$(mktemp -d) cp * "$tmp_dir" git push origin HEAD - + # Set up an existing PR. git checkout ${{ env.test_site_repo_pr_branch_base }} find config/sync -mindepth 1 -delete @@ -557,9 +604,10 @@ jobs: git add config/sync git commit --amend -m "Initial commmit" git push origin HEAD --force - + git checkout -b ${{ env.test_site_repo_pr_branch }} cp -r "$tmp_dir"/* config/sync + echo site-htaccess > config/sync/.htaccess git add config/sync git commit -m "Update" git push origin HEAD @@ -626,8 +674,10 @@ jobs: echo "**TEST FAILURE:** config/sync/language/en/system.site.yml should be 'subdir-config' (config subdirectory not copied on update)." >> "$GITHUB_STEP_SUMMARY" exit=1 fi - if [[ "$(git show "origin/${{ env.test_site_repo_pr_branch }}:config/sync/.htaccess")" != "dotfile-config" ]]; then - echo "**TEST FAILURE:** config/sync/.htaccess should be 'dotfile-config' (dotfile not copied on update)." >> "$GITHUB_STEP_SUMMARY" + # The config repo HEAD carries no .htaccess at all; the site repo's own + # must survive the update rather than being deleted. + if [[ "$(git show "origin/${{ env.test_site_repo_pr_branch }}:config/sync/.htaccess")" != "site-htaccess" ]]; then + echo "**TEST FAILURE:** config/sync/.htaccess should be 'site-htaccess' (the site repo's dotfile was deleted by an export that carries none)." >> "$GITHUB_STEP_SUMMARY" exit=1 fi diff --git a/README.md b/README.md index 33e3750..95587ef 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,9 @@ and creating a PR with the changes. # Run from the project root. config_dir=config/sync config_repo_dir="$(mktemp -d)" - cp "$config_dir"/* "$config_repo_dir" + find "$config_dir" -mindepth 1 -maxdepth 1 -exec cp -a {} "$config_repo_dir" \; + # The .htaccess belongs to the site repo, so keep it out of the config repo. + find "$config_repo_dir" -name .htaccess -delete pushd "$config_repo_dir" git init git add . @@ -66,6 +68,11 @@ The action is responsible for keeping the site repo's config branch up-to-date with the config repo, and opening a PR when the latest config doesn't match what's in the site repo's staging branch. +`config/sync/.htaccess` (and any in a config collection subdirectory) is owned +by the site repo: the action never deletes it or overwrites it from the config +export. Drupal ships an updated one via a core update, so the site repo's copy +is the current one and any copy carried in the config repo may be stale. + ```yaml uses: FabbDev/update-config@main with: diff --git a/action.yml b/action.yml index 6d32632..7024e58 100644 --- a/action.yml +++ b/action.yml @@ -197,13 +197,27 @@ runs: cd config/sync # Remove all existing config: files, dotfiles, and the subdirectories # that config collections/config_split produce without tripping `set -e` - # when config/sync is empty. + # when config/sync is empty. The site repo's .htaccess files are + # restored below. find . -mindepth 1 -delete # Copy in the new config, recursing into subdirectories and skipping the # remote checkout's own .git. `find` is a no-op (not an error) if the # config repo is empty. find "$GITHUB_WORKSPACE/remote" -mindepth 1 -maxdepth 1 ! -name .git \ -exec cp -a {} . \; + # Drupal writes .htaccess files into the config directory to block web + # access to it. They belong to the site repo, not the config export: an + # updated .htaccess lands in the site repo first, and any copy carried + # in the config repo may be stale. Discard whatever the export brought + # and restore the site repo's own, skipping any whose directory the + # current export no longer produces. `git add` hasn't run yet, so the + # index still holds the PR branch's versions. + find . -name .htaccess -delete + while IFS= read -r -d '' htaccess; do + if [[ -d "$(dirname "$htaccess")" ]]; then + git checkout -- "$htaccess" + fi + done < <(git ls-files -z -- '*.htaccess') git add . if [[ -n "$(git status --porcelain)" ]]; then git config user.name '${{ inputs.committer_name }}' diff --git a/scripts/check-and-push-config.sh b/scripts/check-and-push-config.sh index aee217d..2d28f22 100755 --- a/scripts/check-and-push-config.sh +++ b/scripts/check-and-push-config.sh @@ -27,11 +27,16 @@ temp_dir=${CONFIG_REPO_TEMP_DIR-/tmp/config_change_track} mkdir -p "$temp_dir" pushd "$temp_dir" if [[ -d .git ]]; then - # Shouldn't really be necessary, but just in case. - git fetch - git reset --hard origin/$config_repo_branch + # The URL may have changed since this checkout was created (typically a + # rotated access token embedded in the URL) so always repoint origin at the + # current value rather than reusing the stored one. + git remote set-url origin "$CONFIG_REPO_URL" + git fetch origin + # -f -B copes with $CONFIG_REPO_URL potentially pointing at a different repo + # (unrelated history) or a different branch. + git checkout -f -B "$config_repo_branch" "origin/$config_repo_branch" else - git clone --branch $config_repo_branch "$CONFIG_REPO_URL" . + git clone --branch "$config_repo_branch" "$CONFIG_REPO_URL" . fi time=$(date '+%s') # Note if using config_split, this will only work with 2.x and collection @@ -40,13 +45,22 @@ time=$(date '+%s') # (An alternative solution would be to modify $settings['config_sync_director'] # just for this command.) drush config:export --destination="$temp_dir" --yes -git add . +# Drupal writes an .htaccess into the config directory to block web access to +# it. It belongs to the site repo, not the config export, so keep it out of the +# config repo (and drop it if an earlier run committed one). +git rm --cached --quiet --ignore-unmatch -- '*.htaccess' +git add --all -- . ':(exclude)*.htaccess' git config user.name "${UPDATE_CONFIG_GIT_NAME:-R2D2}" git config user.email "${UPDATE_CONFIG_GIT_EMAIL:-config-update@example.com}" -# Allow for the possibility that there are no changes. -if [[ -n "$(git status --porcelain)" ]]; then +# Allow for the possibility that there are no changes. The .htaccess is +# untracked and not ignored, so it always shows up in `git status`; only staged +# changes tell us whether there's anything to commit. +if ! git diff --cached --quiet; then git commit -m "${UPDATE_CONFIG_GIT_MESSAGE:-Export config from Prod}" - git push + # Explicit remote and refspec: a bare push honours the host user's + # push.default / remote.pushDefault, which can silently push to the wrong + # remote and still exit 0. + git push origin "HEAD:$config_repo_branch" fi drush config-change-track:set-last-export --time $time diff --git a/tests/check-and-push-config.bats b/tests/check-and-push-config.bats index 57823ed..ac70960 100644 --- a/tests/check-and-push-config.bats +++ b/tests/check-and-push-config.bats @@ -39,9 +39,14 @@ case "$subcommand" in case "$arg" in --destination=*) dest="${arg#--destination=}" ;; esac done if [[ -n "${EXPORT_FILES_DIR:-}" && -n "$dest" ]]; then - for f in "$EXPORT_FILES_DIR"/*; do - [[ -f "$f" ]] && cp "$f" "$dest/" - done + # -type f so dotfile fixtures are copied too (a plain glob would miss + # them). + find "$EXPORT_FILES_DIR" -mindepth 1 -maxdepth 1 -type f \ + -exec cp {} "$dest/" \; + fi + # Drupal always writes an .htaccess into the config directory. + if [[ -n "$dest" ]]; then + printf 'deny from all\n' > "$dest/.htaccess" fi ;; esac @@ -129,6 +134,75 @@ _remote_head() { git -C "$1" rev-parse "${2:-HEAD}"; } grep -q "set-last-export --time" "$DRUSH_CALL_LOG" } +# Existing checkout, but CONFIG_REPO_URL now points elsewhere (e.g. rotated token) + +@test "uses the current CONFIG_REPO_URL when it changed after the checkout was created" { + export NEEDS_EXPORT=1 + + local original_bare="$CONFIG_REPO_URL" + + # Pre-clone from the original bare repo so the script enters the + # fetch+reset branch with a stored origin URL that is about to go stale. + git clone "$original_bare" "$CONFIG_REPO_TEMP_DIR" --quiet 2>/dev/null + local original_head; original_head=$(_remote_head "$original_bare") + + # Seed a second bare repo and repoint CONFIG_REPO_URL at it, simulating + # the URL (and embedded token) changing since the checkout was made. + local new_bare="$BATS_TEST_TMPDIR/new-repo.git" + git init --bare "$new_bare" + local seed="$BATS_TEST_TMPDIR/new-seed" + git clone "$new_bare" "$seed" --quiet 2>/dev/null + echo "other initial" > "$seed/other-initial.yml" + git -C "$seed" add . + git -C "$seed" commit -m "Initial commit" --quiet + git -C "$seed" push origin main --quiet + rm -rf "$seed" + + export CONFIG_REPO_URL="$new_bare" + + run "$SCRIPT" + + [ "$status" -eq 0 ] + # Exported config lands in the new repo, not the original. + git -C "$new_bare" show HEAD:config.yml + grep -q "set-last-export --time" "$DRUSH_CALL_LOG" + # Original bare repo was never touched. + [[ "$(_remote_head "$original_bare")" == "$original_head" ]] + # Checkout is left pointing at the current URL, not the stale one. + [[ "$(git -C "$CONFIG_REPO_TEMP_DIR" remote get-url origin)" == "$new_bare" ]] +} + +# Existing checkout, but CONFIG_REPO_BRANCH now points elsewhere + +@test "uses the current CONFIG_REPO_BRANCH when it changed after the checkout was created" { + export NEEDS_EXPORT=1 + + # Pre-clone on 'main' so the script enters the fetch+reset branch. + git clone "$CONFIG_REPO_URL" "$CONFIG_REPO_TEMP_DIR" --quiet 2>/dev/null + local main_head; main_head=$(_remote_head "$CONFIG_REPO_URL" "refs/heads/main") + + # Add a second branch to the same bare repo and switch to it. + local seed="$BATS_TEST_TMPDIR/branch-seed" + git clone "$CONFIG_REPO_URL" "$seed" --quiet 2>/dev/null + git -C "$seed" checkout -b other-branch --quiet + echo "other" > "$seed/other.yml" + git -C "$seed" add . + git -C "$seed" commit -m "Other branch commit" --quiet + git -C "$seed" push origin other-branch --quiet + rm -rf "$seed" + + export CONFIG_REPO_BRANCH="other-branch" + + run "$SCRIPT" + + [ "$status" -eq 0 ] + # Exported config lands on the new branch. + git -C "$CONFIG_REPO_URL" show refs/heads/other-branch:config.yml + grep -q "set-last-export --time" "$DRUSH_CALL_LOG" + # main is untouched. + [[ "$(_remote_head "$CONFIG_REPO_URL" "refs/heads/main")" == "$main_head" ]] +} + # Export produces no diff @test "skips commit and push but still calls set-last-export when export yields no diff" { @@ -146,6 +220,54 @@ _remote_head() { git -C "$1" rev-parse "${2:-HEAD}"; } grep -q "set-last-export --time" "$DRUSH_CALL_LOG" } +# .htaccess is site-repo-owned and must never reach the config repo + +@test "does not commit the .htaccess written by the export" { + export NEEDS_EXPORT=1 + + run "$SCRIPT" + + [ "$status" -eq 0 ] + # The rest of the export still landed. + git -C "$CONFIG_REPO_URL" show HEAD:config.yml + ! git -C "$CONFIG_REPO_URL" show HEAD:.htaccess +} + +@test "removes an .htaccess already tracked in the config repo" { + export NEEDS_EXPORT=1 + + # Seed the repo with a legacy tracked .htaccess. + local seed="$BATS_TEST_TMPDIR/htaccess-seed" + git clone "$CONFIG_REPO_URL" "$seed" --quiet 2>/dev/null + printf 'stale\n' > "$seed/.htaccess" + git -C "$seed" add .htaccess + git -C "$seed" commit -m "Legacy .htaccess" --quiet + git -C "$seed" push origin main --quiet + rm -rf "$seed" + + run "$SCRIPT" + + [ "$status" -eq 0 ] + git -C "$CONFIG_REPO_URL" show HEAD:config.yml + ! git -C "$CONFIG_REPO_URL" show HEAD:.htaccess +} + +@test "skips commit and push when the .htaccess is the only difference" { + export NEEDS_EXPORT=1 + # Export exactly what is already committed; the stub still writes an + # .htaccess, which must not be treated as a change to push. + echo "initial" > "$EXPORT_FILES_DIR/initial.yml" + rm -f "$EXPORT_FILES_DIR/config.yml" + + local initial_head; initial_head=$(_remote_head "$CONFIG_REPO_URL") + + run "$SCRIPT" + + [ "$status" -eq 0 ] + [[ "$(_remote_head "$CONFIG_REPO_URL")" == "$initial_head" ]] + grep -q "set-last-export --time" "$DRUSH_CALL_LOG" +} + # Commit metadata - custom values @test "uses custom git identity and message when env vars are set" {