Skip to content

Commit 84ca0c3

Browse files
authored
style(shell): clear ShellCheck warnings in 7.0.4 openemr.sh and ssl.sh (#655)
## Summary Slice 4 of the openemr-devops ShellCheck cleanup (follow-up to #649, alongside #652 / #653). Resolves all 19 warnings in `docker/openemr/7.0.4/openemr.sh` and both warnings in `docker/openemr/7.0.4/ssl.sh`. ## Changes - **New `docker/openemr/7.0.4/env.stub`** declares the 14 environment variables both scripts read from the Docker runtime (set by docker-compose `environment:` or `-e` flags): `K8S`, `SWARM_MODE`, `MANUAL_SETUP`, `REDIS_SERVER`, `PHPREDIS_BUILD`, `PHP_VERSION_ABBR`, `REDIS_USERNAME`, `REDIS_PASSWORD`, `REDIS_X509`, `REDIS_TLS`, `XDEBUG_IDE_KEY`, `XDEBUG_ON`, `DOMAIN`, `OPERATOR`. Each is declared with `: "${VAR:=}"` so the stub is a no-op when sourced and never overrides real env values. - **`# shellcheck source=docker/openemr/7.0.4/env.stub` directive** added to both scripts, followed by `. /root/env.stub 2>/dev/null || true`. The directive path is repo-root-relative (matching how CI invokes shellcheck and matching the pattern from #653); the runtime guard keeps the script working inside the container image, where the stub is not shipped. Clears all SC2154. - **`openemr.sh` line 80 (SC2188 + SC3020):** replace `{ > FILE ; } &> /dev/null` with `: > FILE 2>/dev/null`. POSIX-compliant no-op write with redirected stderr; same intent (try to create the leader marker, fall back to `AUTHORITY=no` on failure). - **`openemr.sh` line 256 (SC3037):** replace `echo -n` (POSIX-undefined) with `printf '%s'`. - **`openemr.sh` line 287 (SC2312):** hoist `nproc --all` into a `jobs` variable so `make -j "${jobs}"` no longer masks nproc's exit status. - **`openemr.sh` noclobber block:** expand the "atomically test for leadership" comment into a TOCTOU-vs-`O_CREAT|O_EXCL` explanation per review. - **`openemr.sh` SC2310 ×3** at the retry predicates `while swarm_wait` (line 99) and `while ! auto_setup` (line 241): resolved with targeted `# shellcheck disable=SC2310` plus a comment explaining the intent. The predicates' failure is the loop-termination signal, not a script-fatal condition, so the `set -e` suppression inside the `while` / `!` contexts is the intended behaviour — a structural rewrite would invert the retry-loop semantics. ## Verification ``` $ shellcheck --check-sourced --external-sources \ docker/openemr/7.0.4/openemr.sh \ docker/openemr/7.0.4/ssl.sh ``` is clean (exit 0). ## Test plan - [x] `bash -n` clean on both scripts - [x] `shellcheck --check-sourced --external-sources` clean on both scripts - [ ] CI ShellCheck workflow passes on this PR
1 parent 494e47a commit 84ca0c3

3 files changed

Lines changed: 67 additions & 4 deletions

File tree

docker/openemr/7.0.4/env.stub

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# shellcheck shell=sh
2+
# Stub file for ShellCheck SC2154 (vars referenced but not assigned).
3+
# These variables come from the Docker container environment, set by
4+
# docker-compose's `environment:` block or `-e` flags on `docker run`.
5+
# This file exists solely to tell ShellCheck the names are intentional;
6+
# it is not sourced at runtime.
7+
8+
# openemr.sh
9+
: "${K8S:=}"
10+
: "${SWARM_MODE:=}"
11+
: "${MANUAL_SETUP:=}"
12+
: "${REDIS_SERVER:=}"
13+
: "${PHPREDIS_BUILD:=}"
14+
: "${PHP_VERSION_ABBR:=}"
15+
: "${REDIS_USERNAME:=}"
16+
: "${REDIS_PASSWORD:=}"
17+
: "${REDIS_X509:=}"
18+
: "${REDIS_TLS:=}"
19+
: "${XDEBUG_IDE_KEY:=}"
20+
: "${XDEBUG_ON:=}"
21+
22+
# ssl.sh
23+
: "${DOMAIN:=}"
24+
: "${OPERATOR:=}"

docker/openemr/7.0.4/openemr.sh

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,18 @@ set -e
1212
# shellcheck source=SCRIPTDIR/utilities/devtoolsLibrary.source
1313
. /root/devtoolsLibrary.source
1414

15+
# Environment variables used by this script are supplied by the Docker runtime
16+
# (docker-compose `environment:` or `-e` flags). The env.stub file declares
17+
# them for ShellCheck's benefit using `: "${VAR:=}"` assignments that leave
18+
# real runtime values untouched. The stub is not shipped into the container
19+
# image; the `if false` keeps the `.` statically visible to ShellCheck's
20+
# source-follower without ever running it — BusyBox ash treats `.` as a
21+
# special builtin and exits the shell on file-not-found even with `|| true`.
22+
if false; then
23+
# shellcheck source=docker/openemr/7.0.4/env.stub
24+
. /root/env.stub
25+
fi
26+
1527
swarm_wait() {
1628
if [ ! -f /var/www/localhost/htdocs/openemr/sites/docker-completed ]; then
1729
# true
@@ -75,13 +87,23 @@ elif [ "${K8S}" = "worker" ]; then
7587
fi
7688

7789
if [ "${SWARM_MODE}" = "yes" ]; then
78-
# atomically test for leadership
90+
# Atomically test for leadership. Multiple swarm containers race through this block,
91+
# and exactly one must become the leader. A `[ -f file ]` check followed by a create
92+
# has a TOCTOU race — every racer sees the file missing, every racer creates it, every
93+
# racer declares itself leader. Instead, `set -o noclobber` makes the redirect below
94+
# use `open(O_CREAT|O_EXCL)`: the kernel serializes the creation, exactly one redirect
95+
# succeeds, and every other container's redirect fails with EEXIST and falls into the
96+
# `|| AUTHORITY=no` branch. One winner by construction.
7997
set -o noclobber
80-
{ > /var/www/localhost/htdocs/openemr/sites/docker-leader ; } &> /dev/null || AUTHORITY=no
98+
: > /var/www/localhost/htdocs/openemr/sites/docker-leader 2>/dev/null || AUTHORITY=no
8199
set +o noclobber
82100

83101
if [ "${AUTHORITY}" = "no" ] &&
84102
[ ! -f /var/www/localhost/htdocs/openemr/sites/docker-completed ]; then
103+
# swarm_wait is a retry predicate: its failure is the loop termination
104+
# signal, not a script-fatal condition. Disabling set -e in the while
105+
# condition is intentional.
106+
# shellcheck disable=SC2310
85107
while swarm_wait; do
86108
echo "Waiting for the docker-leader to finish configuration before proceeding."
87109
sleep 10;
@@ -224,6 +246,10 @@ if [ "${AUTHORITY}" = "yes" ]; then
224246
[ "${MANUAL_SETUP}" != "yes" ]; then
225247

226248
echo "Running quick setup!"
249+
# auto_setup is a retry predicate: the loop drives it to success and
250+
# the ! inverts its exit for the exit condition. Disabling set -e in
251+
# both the ! and while contexts is intentional.
252+
# shellcheck disable=SC2310
227253
while ! auto_setup; do
228254
echo "Couldn't set up. Any of these reasons could be what's wrong:"
229255
echo " - You didn't spin up a MySQL container or connect your OpenEMR container to a mysql instance"
@@ -253,7 +279,7 @@ if
253279
fi
254280
c=$(( c + 1 ))
255281
done
256-
echo -n "${DOCKER_VERSION_ROOT}" > /var/www/localhost/htdocs/openemr/sites/default/docker-version
282+
printf '%s' "${DOCKER_VERSION_ROOT}" > /var/www/localhost/htdocs/openemr/sites/default/docker-version
257283
echo "Completed upgrade"
258284
fi
259285
fi
@@ -284,7 +310,8 @@ if [ "${REDIS_SERVER}" != "" ] &&
284310
phpize83
285311
# note for php 8.3, needed to change from './configure --enable-redis-igbinary' to:
286312
./configure --with-php-config=/usr/bin/php-config83 --enable-redis-igbinary
287-
make -j "$(nproc --all)"
313+
jobs=$(nproc --all)
314+
make -j "${jobs}"
288315
make install
289316
echo "extension=redis" > "/etc/php${PHP_VERSION_ABBR}/conf.d/20_redis.ini"
290317
rm -fr /tmpredis/phpredis

docker/openemr/7.0.4/ssl.sh

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@
88
#
99
set -e
1010

11+
# Environment variables used by this script are supplied by the Docker runtime
12+
# (docker-compose `environment:` or `-e` flags). The env.stub file declares
13+
# them for ShellCheck's benefit using `: "${VAR:=}"` assignments that leave
14+
# real runtime values untouched. The stub is not shipped into the container
15+
# image; the `if false` keeps the `.` statically visible to ShellCheck's
16+
# source-follower without ever running it — BusyBox ash treats `.` as a
17+
# special builtin and exits the shell on file-not-found even with `|| true`.
18+
if false; then
19+
# shellcheck source=docker/openemr/7.0.4/env.stub
20+
. /root/env.stub
21+
fi
22+
1123
if ! [ -f /etc/ssl/private/selfsigned.key.pem ]; then
1224
openssl req -x509 -newkey rsa:4096 \
1325
-keyout /etc/ssl/private/selfsigned.key.pem \

0 commit comments

Comments
 (0)