Skip to content

Commit fcad25c

Browse files
mcattamoredhatmcattamoredhat
committed
fix: avoid unbound _brew_* vars in parse_brew_url callers
`parse_brew_url()` set `_brew_ver`/`_brew_rel`/`_brew_arch` as side effects, but every caller captured it via `url=$(parse_brew_url ...)`. Command substitution runs the function in a subshell, so those assignments never reached the calling scope — under `set -u`, the first reference to `_brew_arch` in install_client()/install_server() (test/ci, test/rpm, test/bootc) crashed with "unbound variable" whenever SERVER_RPM_URL or CLIENT_RPM_URL was actually set. Fix: have parse_brew_url() set _brew_url directly instead of echoing it, and call it as a plain statement (no command substitution) so all four variables land in the caller's scope. No behavior change for any path that doesn't set SERVER_RPM_URL / CLIENT_RPM_URL: PACKIT_COPR_RPMS-driven Packit/Testing Farm runs and the GitHub Actions test/ci matrix never touch parse_brew_url() at all (they short-circuit on PACKIT_COPR_RPMS or don't set these vars), so this only affects the manual brew-build testing path introduced in daf355b. Assisted-by: Claude (claude-sonnet-5) Signed-off-by: Mario Cattamo <mcattamo@redhat.com>
1 parent a34fce9 commit fcad25c

3 files changed

Lines changed: 21 additions & 24 deletions

File tree

‎test/bootc/utils.sh‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,13 @@ EOF
7373
if [ -n "${CLIENT_RPM_URL:-}" ]; then
7474
# Install go-fdo-client from a specific brew build base path.
7575
# CLIENT_RPM_URL should point to the version/release directory of the package in brew.
76-
local url
77-
url=$(parse_brew_url "${CLIENT_RPM_URL}")
76+
parse_brew_url "${CLIENT_RPM_URL}"
7877
tee Containerfile >/dev/null <<EOF
7978
FROM ${base_image_url}
8079
# --nogpgcheck and sslverify=false are intentional: internal brew servers
8180
# use self-signed certificates and builds may not be GPG-signed.
8281
RUN dnf install -y --nogpgcheck --setopt=sslverify=false \
83-
"${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm"
82+
"${_brew_url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm"
8483
EOF
8584
elif [ -n "${COMPOSE_BASE_URL:-}" ]; then
8685
# Install go-fdo-client from a compose repository.
@@ -184,15 +183,14 @@ install_server() {
184183
elif [ -n "${SERVER_RPM_URL:-}" ]; then
185184
# Install from a specific brew build base path.
186185
# SERVER_RPM_URL should point to the version/release directory of the package in brew.
187-
local url
188-
url=$(parse_brew_url "${SERVER_RPM_URL}")
186+
parse_brew_url "${SERVER_RPM_URL}"
189187
# --nogpgcheck and sslverify=false are intentional: internal brew servers
190188
# use self-signed certificates and builds may not be GPG-signed.
191189
sudo dnf install -y --nogpgcheck --setopt=sslverify=false \
192-
"${url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \
193-
"${url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \
194-
"${url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \
195-
"${url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm"
190+
"${_brew_url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \
191+
"${_brew_url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \
192+
"${_brew_url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \
193+
"${_brew_url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm"
196194
elif [ -n "${COMPOSE_BASE_URL:-}" ]; then
197195
install_from_compose ${go_fdo_server_rpms}
198196
else

‎test/ci/utils.sh‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -398,26 +398,27 @@ stop_services() {
398398

399399
# Parse a brew build base URL into its components.
400400
# The URL must point to the version/release directory of the package in brew.
401-
# Sets _brew_ver, _brew_rel, _brew_arch and returns the trailing-slash-stripped
402-
# URL on stdout so callers can capture it with: local url; url=$(parse_brew_url …)
401+
# Sets _brew_url, _brew_ver, _brew_rel and _brew_arch. Must be called directly
402+
# (not via command substitution) so the assignments aren't lost in a subshell:
403+
# parse_brew_url "${SOME_RPM_URL}"
404+
# echo "${_brew_url} ${_brew_ver} ${_brew_rel} ${_brew_arch}"
403405
parse_brew_url() {
404406
local url="${1%/}" # strip any trailing slash to prevent empty basename
407+
_brew_url="${url}"
405408
_brew_ver=$(basename "$(dirname "${url}")")
406409
_brew_rel=$(basename "${url}")
407410
_brew_arch=$(uname -m)
408-
echo "${url}"
409411
}
410412

411413
install_client() {
412414
if [ -n "${CLIENT_RPM_URL:-}" ]; then
413415
# Install from a specific brew build base path.
414416
# CLIENT_RPM_URL should point to the version/release directory of the package in brew.
415-
local url
416-
url=$(parse_brew_url "${CLIENT_RPM_URL}")
417+
parse_brew_url "${CLIENT_RPM_URL}"
417418
# --nogpgcheck and sslverify=false are intentional: internal brew servers
418419
# use self-signed certificates and builds may not be GPG-signed.
419420
sudo dnf install -y --nogpgcheck --setopt=sslverify=false \
420-
"${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm"
421+
"${_brew_url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm"
421422
else
422423
go install github.com/fido-device-onboard/go-fdo-client@main
423424
fi

‎test/rpm/utils.sh‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -276,12 +276,11 @@ install_client() {
276276
elif [ -n "${CLIENT_RPM_URL:-}" ]; then
277277
# Install from a specific brew build base path.
278278
# CLIENT_RPM_URL should point to the version/release directory of the package in brew.
279-
local url
280-
url=$(parse_brew_url "${CLIENT_RPM_URL}")
279+
parse_brew_url "${CLIENT_RPM_URL}"
281280
# --nogpgcheck and sslverify=false are intentional: internal brew servers
282281
# use self-signed certificates and builds may not be GPG-signed.
283282
sudo dnf install -y --nogpgcheck --setopt=sslverify=false \
284-
"${url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm"
283+
"${_brew_url}/${_brew_arch}/go-fdo-client-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm"
285284
elif [ -n "${COMPOSE_BASE_URL:-}" ]; then
286285
install_from_compose ${go_fdo_client_rpms}
287286
else
@@ -308,15 +307,14 @@ install_server() {
308307
elif [ -n "${SERVER_RPM_URL:-}" ]; then
309308
# Install from a specific brew build base path.
310309
# SERVER_RPM_URL should point to the version/release directory of the package in brew.
311-
local url
312-
url=$(parse_brew_url "${SERVER_RPM_URL}")
310+
parse_brew_url "${SERVER_RPM_URL}"
313311
# --nogpgcheck and sslverify=false are intentional: internal brew servers
314312
# use self-signed certificates and builds may not be GPG-signed.
315313
sudo dnf install -y --nogpgcheck --setopt=sslverify=false \
316-
"${url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \
317-
"${url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \
318-
"${url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \
319-
"${url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm"
314+
"${_brew_url}/${_brew_arch}/go-fdo-server-${_brew_ver}-${_brew_rel}.${_brew_arch}.rpm" \
315+
"${_brew_url}/noarch/go-fdo-server-manufacturer-${_brew_ver}-${_brew_rel}.noarch.rpm" \
316+
"${_brew_url}/noarch/go-fdo-server-owner-${_brew_ver}-${_brew_rel}.noarch.rpm" \
317+
"${_brew_url}/noarch/go-fdo-server-rendezvous-${_brew_ver}-${_brew_rel}.noarch.rpm"
320318
elif [ -n "${COMPOSE_BASE_URL:-}" ]; then
321319
install_from_compose ${go_fdo_server_rpms}
322320
else

0 commit comments

Comments
 (0)