helmrelease_has_remediation_cycle in hack/e2e-chainsaw/_lib/remediation-guard.sh assigns its argument to a global:
helmrelease_has_remediation_cycle() {
statuses="$1"
Line 26 on main, and the only assignment in the file that is not declared local. The file is sourced rather than executed, so the function body runs in the caller's shell and this silently overwrites whatever the caller was keeping in statuses.
Nothing is broken today. The single caller in hack/e2e-chainsaw/_lib/run-kubernetes.sh reads the history into history_statuses, and statuses does not appear anywhere in that file, so the names do not currently collide.
The reason to fix it anyway is the shape of the collision rather than its present absence. run-kubernetes.sh is a 2400-line shared shell sourced by the heavy suites; a future local named statuses anywhere in it would be clobbered by a call to this guard, and the failure would be silent, delayed, and would look like a bug in whatever code read the variable afterwards rather than in the guard that overwrote it. A one-word fix now costs nothing; the same fix after a collision costs whoever has to find it.
local statuses is enough. local is not in POSIX, but the file already targets shells that provide it (the suites run under bash and the helper is exercised by hack/remediation-guard.bats), and dash supports it too.
Worth a pass over the other _lib/ helpers while touching this, since the same sourcing pattern applies to all of them.
helmrelease_has_remediation_cycleinhack/e2e-chainsaw/_lib/remediation-guard.shassigns its argument to a global:Line 26 on
main, and the only assignment in the file that is not declaredlocal. The file is sourced rather than executed, so the function body runs in the caller's shell and this silently overwrites whatever the caller was keeping instatuses.Nothing is broken today. The single caller in
hack/e2e-chainsaw/_lib/run-kubernetes.shreads the history intohistory_statuses, andstatusesdoes not appear anywhere in that file, so the names do not currently collide.The reason to fix it anyway is the shape of the collision rather than its present absence.
run-kubernetes.shis a 2400-line shared shell sourced by the heavy suites; a future local namedstatusesanywhere in it would be clobbered by a call to this guard, and the failure would be silent, delayed, and would look like a bug in whatever code read the variable afterwards rather than in the guard that overwrote it. A one-word fix now costs nothing; the same fix after a collision costs whoever has to find it.local statusesis enough.localis not in POSIX, but the file already targets shells that provide it (the suites run underbashand the helper is exercised byhack/remediation-guard.bats), anddashsupports it too.Worth a pass over the other
_lib/helpers while touching this, since the same sourcing pattern applies to all of them.