Substitution safety, portability, deploy refactor, tests#6
Merged
Conversation
…add tests
Substitution safety
- New functions/_substitute.sh exposing safe_replace_token, a perl
s///ge helper that treats values as opaque strings. Replaces every
sed -i 's;__VAR__;...;g' call site that silently corrupted compose
and config files when a value contained ;, &, \, $1, or $2y$10$.
The bug was most visible with bcrypt hashes (Portainer, OpenSearch,
STACK_ADMIN), which sometimes shipped truncated or empty.
- New functions/_array_contains.sh: exact-match argument check. The
prior `[[ "${arr[@]}" =~ "${needle}" ]]` was a substring regex, so
`rod` silently validated against an environments file containing
`prod`.
- gpd.sh: the elif branches for invalid --deploy-type printed an
error and fell through to the workflow; now they exit 1.
- Drop several useless `$(echo VAR)` subshells that built a name
string already present in plain text.
Portability (Linux + macOS + Git-Bash/WSL)
- New functions/_compat.sh with gpd_http_get (curl -> wget),
gpd_bcrypt_hash (htpasswd -> python3+bcrypt), gpd_apr1_hash
(htpasswd -> openssl passwd -apr1), gpd_htpasswd_create.
- gpd.sh: assert Bash 4+ on startup and inline a gpd_realpath shim
(readlink -e -> readlink -f -> realpath -> perl Cwd::realpath) so
the script no longer depends on GNU readlink.
- functions/include.sh: BASH_SOURCE[0] instead of readlink -e $0.
- functions/_check_openssl.sh: replace grep -P (GNU-only) with perl.
- functions/_config_files.sh: replace `for u in $(seq 0 N-1)` with
`for x in "${arr[@]}"`. BSD seq counts down through -1 instead of
producing nothing, which made the empty-asset-template path call
`cp /path/to/asset/ /dest` and explode.
- Drop htpasswd and wget from the required-binary list in
_generate.sh; both stay preferred when available.
Documentation
- README rewrite: parent project layout contract, file formats, the
three substitution passes, password-token table, full flag
reference, runtime requirements (Bash 4+, GNU getopt for macOS),
platform notes.
- CHANGELOG seeded in keepachangelog format.
- functions/_usage.sh: drop -l/--docker-login and -k/--docker-logout.
They were parsed into LOGIN_DOCKER/LOGOUT_DOCKER but never read;
login/logout already happens inline in _deploy.sh.
Deploy refactor (functions/_deploy.sh, functions/_gpd.sh)
- Add run_in_target / compose_in_target to _gpd.sh, routing a command
to local docker for local* environments or via gpd ssh otherwise.
Args are shell-quoted with printf %q for the SSH path.
- Collapse every dual-branch local-vs-remote in _deploy.sh onto these
helpers (333 -> 232 lines, no logic forks).
- Replace `if ! $(cmd &>/dev/null)` with `if ! cmd >/dev/null 2>&1`.
The old form preserved exit status only because stdout happened to
be empty under &>/dev/null.
- deploy_login_to_registry: fix copy-paste bug where CI_REGISTRY_USER
was checked twice instead of CI_REGISTRY_USER + CI_REGISTRY_PASSWORD.
- deploy_logout_from_registry: skip when CI_REGISTRY=null.
- deploy_dry_run: 3 retries with exponential backoff (1/2/4 s) and a
fatal-pattern grep over stderr (access denied, manifest unknown,
unauthorized, undefined service, ...) so config errors fail fast
instead of burning every retry.
- functions/_variables_env.sh: change ${arr[@]} -> ${arr[*]} inside a
double-quoted string so shellcheck SC2145 stops complaining.
Retries on flaky-network operations
- New -r/--retries=<N> flag (default 3, validated as a positive
integer) wrapping rsync push, registry login, image pull, and the
two GeoIP downloads with a new gpd_retry helper in _compat.sh.
Backoff between attempts is exponential (1s, 2s, 4s, ...).
- gpd_silent companion suppresses stdout/stderr of noisy commands
(docker login, docker compose pull) so retry warnings stay visible
while command spam is hidden.
- functions/_gpd.sh rsync mode: switch from `exit 1` to `return 1`
on transfer failure so callers can re-invoke through gpd_retry.
Only one caller (_push.sh) and the change is internal.
- The obvious `if "$@"; then return 0; fi; RC=$?` is wrong - bash
spec: an `if` block with no executed branch exits 0, so RC always
saw 0. The implementation uses `"$@" || RC=$?` instead and the
comments call out the gotcha so the next reader doesn't reintroduce
it.
Tests + CI
- tests/bats: 46 cases covering safe_replace_token (every sed-killer
+ every perl replacement gotcha), array_contains, every _compat.sh
fallback path (with mask_command in helpers.bash to force the
fallback), gpd_retry (success first-try, inner exit-status
preservation, success on attempt N, gives-up-after-MAX, COUNT < 1
still runs once - sleep stubbed out so tests don't actually wait),
gpd_silent, and end-to-end generate against tests/fixtures/parent-
stack/. Fixture is intentionally minimal: one local environment,
one deploy-type, no nginx/opensearch.
- .github/workflows/test.yml: bats matrix on Ubuntu and macOS (with
homebrew bash + bats-core + zstd + gnu-getopt installed and
prepended to GITHUB_PATH) plus an Ubuntu shellcheck pass at
severity: error. SC2148/SC2034/SC1090/SC1091 excluded.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
safe_replace_token(perls///ge) replaces everysed -i 's;__VAR__;…;g'call site that silently corrupted compose/config files when a value contained;,&,\,$1, or$2y$10$…— bcrypt hashes were the most-affected. Addsarray_containsto fix the substring regex that let"rod"validate against"prod". Fixes thegpd.shfall-through on invalid--deploy-type._compat.shwith curl→wget, htpasswd→python3+bcrypt, htpasswd→openssl-apr1 fallbacks. Inlinegpd_realpathreplaces GNU-onlyreadlink -e. Bash 4+ asserted on startup._check_openssl.shswaps GNU-onlygrep -Pforperl._config_files.shswapsseq 0 N-1for array iteration (BSDseq 0 -1counts down)._gpd.shgetsrun_in_target/compose_in_targetthat quote args viaprintf %qand route locally or via SSH. Every dual-branch in_deploy.shcollapses onto them (333 → 232 lines). Fixes theif ! \$(cmd &>/dev/null)pattern, theCI_REGISTRY_USERchecked-twice typo, and adds exponential-backoff retry with fatal-pattern detection indeploy_dry_run.-l/--docker-loginand-k/--docker-logoutflags. Removedhtpasswdandwgetfrom required binaries.array_contains, every_compat.shfallback (withmask_commandhelper), and end-to-end generate againsttests/fixtures/parent-stack/. GitHub Actions matrix runs on Ubuntu + macOS (with brew bash + bats-core + zstd + gnu-getopt) plus a shellcheck pass atseverity: error.Test plan
bats tests/bats/locally on macOS — 40/40 greenmake gpd-generateagainst the xyxyx/cloud parent stack — passes end-to-end (substitution, all four bcrypt + apr1 password flows, GeoIP download, checksum)local*SSH target — out of scope here, follow-up before merge