From 0add4bb6f0472e0346fe1d44ec338c204b468626 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 3 Aug 2026 10:32:54 -0600 Subject: [PATCH 1/4] fix(bootstrap): rewrite salt.sources URIs to honor -R custom repo URL on Debian/Ubuntu The Debian/Ubuntu repo functions downloaded a salt.sources template that hardcodes packages.broadcom.com/artifactory in its URIs line. Only the GPG key fetch used $_REPO_URL, so -R had no effect on which apt repo packages actually came from. Add a sed rewrite of the URIs line to $_REPO_URL, mirroring how RHEL/SUSE/Photon already parameterize their repo files. Fixes #2123 --- bootstrap-salt.sh | 4 ++ tests/integration/test_installation.py | 67 ++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/bootstrap-salt.sh b/bootstrap-salt.sh index df4666a13..71fd3c551 100755 --- a/bootstrap-salt.sh +++ b/bootstrap-salt.sh @@ -3021,6 +3021,7 @@ __install_saltstack_ubuntu_repository() { # SaltStack's stable Ubuntu repository: __fetch_url "/etc/apt/sources.list.d/salt.sources" "https://github.com/saltstack/salt-install-guide/releases/latest/download/salt.sources" [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#salt-archive-keyring\.pgp#salt-archive-keyring.gpg#" /etc/apt/sources.list.d/salt.sources + [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#packages\.broadcom\.com/artifactory#${_REPO_URL}#" /etc/apt/sources.list.d/salt.sources __apt_key_fetch "${HTTP_VAL}://${_REPO_URL}/api/security/keypair/SaltProjectKey/public" || return 1 __wait_for_apt apt-get update || return 1 @@ -3074,6 +3075,7 @@ __install_saltstack_ubuntu_onedir_repository() { # SaltStack's stable Ubuntu repository: __fetch_url "/etc/apt/sources.list.d/salt.sources" "https://github.com/saltstack/salt-install-guide/releases/latest/download/salt.sources" [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#salt-archive-keyring\.pgp#salt-archive-keyring.gpg#" /etc/apt/sources.list.d/salt.sources + [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#packages\.broadcom\.com/artifactory#${_REPO_URL}#" /etc/apt/sources.list.d/salt.sources __apt_key_fetch "${HTTP_VAL}://${_REPO_URL}/api/security/keypair/SaltProjectKey/public" || return 1 __wait_for_apt apt-get update || return 1 @@ -3526,6 +3528,7 @@ __install_saltstack_debian_repository() { __fetch_url "/etc/apt/sources.list.d/salt.sources" "https://github.com/saltstack/salt-install-guide/releases/latest/download/salt.sources" [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#salt-archive-keyring\.pgp#salt-archive-keyring.gpg#" /etc/apt/sources.list.d/salt.sources + [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#packages\.broadcom\.com/artifactory#${_REPO_URL}#" /etc/apt/sources.list.d/salt.sources __apt_key_fetch "${HTTP_VAL}://${_REPO_URL}/api/security/keypair/SaltProjectKey/public" || return 1 __wait_for_apt apt-get update || return 1 @@ -3572,6 +3575,7 @@ __install_saltstack_debian_onedir_repository() { __fetch_url "/etc/apt/sources.list.d/salt.sources" "https://github.com/saltstack/salt-install-guide/releases/latest/download/salt.sources" [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#salt-archive-keyring\.pgp#salt-archive-keyring.gpg#" /etc/apt/sources.list.d/salt.sources + [ -f /etc/apt/sources.list.d/salt.sources ] && sed -i "s#packages\.broadcom\.com/artifactory#${_REPO_URL}#" /etc/apt/sources.list.d/salt.sources __apt_key_fetch "${HTTP_VAL}://${_REPO_URL}/api/security/keypair/SaltProjectKey/public" || return 1 __wait_for_apt apt-get update || return 1 diff --git a/tests/integration/test_installation.py b/tests/integration/test_installation.py index c78765716..ad3eca2c2 100644 --- a/tests/integration/test_installation.py +++ b/tests/integration/test_installation.py @@ -2,6 +2,7 @@ import logging import os import platform +import re import shutil import subprocess @@ -120,3 +121,69 @@ def test_apt_keyring_is_trusted(): assert result.returncode == 0, result.stderr assert "NO_PUBKEY" not in result.stderr, result.stderr assert "unsupported filetype" not in result.stderr, result.stderr + + +DEBIAN_REPO_FUNCTIONS = [ + "__install_saltstack_ubuntu_repository", + "__install_saltstack_ubuntu_onedir_repository", + "__install_saltstack_debian_repository", + "__install_saltstack_debian_onedir_repository", +] + +SAMPLE_SALT_SOURCES = """\ +X-Repolib-Name: Salt Project +Types: deb +URIs: https://packages.broadcom.com/artifactory/saltproject-deb +Signed-By: /etc/apt/keyrings/salt-archive-keyring.pgp +Suites: stable +Components: main +""" + + +def test_debian_repo_functions_rewrite_custom_repo_url(tmp_path): + """ + Regression test for https://github.com/saltstack/salt-bootstrap/issues/2123 + The -R/_CUSTOM_REPO_URL option must rewrite the "URIs:" line in + salt.sources for Debian/Ubuntu, not just the GPG key fetch URL. + """ + if shutil.which("bash") is None or shutil.which("sed") is None: + pytest.skip("bash/sed not available") + + bootstrap_script = os.path.join( + os.path.dirname(__file__), "..", "..", "bootstrap-salt.sh" + ) + if not os.path.exists(bootstrap_script): + pytest.skip("bootstrap-salt.sh not found (not running from a repo checkout)") + + with open(bootstrap_script) as fp: + script = fp.read() + + for func_name in DEBIAN_REPO_FUNCTIONS: + match = re.search( + rf"^{re.escape(func_name)}\(\) {{(.*?)^}}", script, re.M | re.S + ) + assert match, f"could not find {func_name}() in bootstrap-salt.sh" + + sed_exprs = re.findall( + r'sed -i "([^"]+)" /etc/apt/sources\.list\.d/salt\.sources', + match.group(1), + ) + assert sed_exprs, f"{func_name} has no salt.sources sed post-processing" + + sources_file = tmp_path / f"{func_name}.sources" + sources_file.write_text(SAMPLE_SALT_SOURCES) + + env = dict(os.environ, _REPO_URL="repo.example.com/myrepo", HTTP_VAL="https") + for expr in sed_exprs: + subprocess.run( + ["bash", "-c", f'sed -i "{expr}" "$1"', "--", str(sources_file)], + env=env, + check=True, + ) + + result = sources_file.read_text() + assert "packages.broadcom.com" not in result, ( + f"{func_name}: salt.sources still references packages.broadcom.com " + f"after applying its sed commands:\n{result}" + ) + assert "repo.example.com/myrepo" in result From a60f71b54105110c9c1063f773d19042d1b687c7 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 3 Aug 2026 12:22:07 -0600 Subject: [PATCH 2/4] test: skip custom-repo-url regression test where bash/sed isn't GNU The new test_debian_repo_functions_rewrite_custom_repo_url replays the script's sed commands via "bash -c", which broke on non-GNU targets: BSD sed on macOS parses "-i" differently, and on Windows runners "bash" resolves to the WSL launcher stub with no distro installed. Detect GNU sed through the same bash -c path the test uses and skip otherwise, instead of failing on environments the fix doesn't target. --- tests/integration/test_installation.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_installation.py b/tests/integration/test_installation.py index ad3eca2c2..214fc66db 100644 --- a/tests/integration/test_installation.py +++ b/tests/integration/test_installation.py @@ -140,14 +140,32 @@ def test_apt_keyring_is_trusted(): """ +def _bash_has_gnu_sed(): + # Check through "bash -c", the exact invocation the test below uses, since + # e.g. on GitHub's Windows runners plain "sed" on the host PATH is Git + # Bash's GNU sed, but "bash" on the host PATH resolves to the WSL launcher + # stub instead - a different, often broken, resolution path. + try: + result = subprocess.run( + ["bash", "-c", "sed --version"], capture_output=True, text=True + ) + except FileNotFoundError: + return False + return "GNU sed" in result.stdout + + def test_debian_repo_functions_rewrite_custom_repo_url(tmp_path): """ Regression test for https://github.com/saltstack/salt-bootstrap/issues/2123 The -R/_CUSTOM_REPO_URL option must rewrite the "URIs:" line in salt.sources for Debian/Ubuntu, not just the GPG key fetch URL. """ - if shutil.which("bash") is None or shutil.which("sed") is None: - pytest.skip("bash/sed not available") + if not _bash_has_gnu_sed(): + # bootstrap-salt.sh's Debian/Ubuntu sed -i syntax targets GNU sed, + # which is what those distros actually ship. BSD sed (e.g. on macOS) + # parses "-i" differently, and isn't representative of the real + # target either way. + pytest.skip("bash with GNU sed not available") bootstrap_script = os.path.join( os.path.dirname(__file__), "..", "..", "bootstrap-salt.sh" From 3a21ced8ec6b30f4720875ddcf6cfddaa33e1df6 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 3 Aug 2026 14:14:44 -0600 Subject: [PATCH 3/4] ci(test-linux): wait for container to finish booting before running pip/bootstrap systemd's default tmpfiles.d rule wipes /tmp during early boot (systemd-tmpfiles-setup.service). docker exec was running pip install immediately after docker start, racing that wipe and intermittently deleting pip's /tmp/pip-*-tracker-* scratch dirs mid-install, causing "No such file or directory" failures unrelated to the actual bootstrap logic. Poll systemctl is-system-running until the container settles before running any exec commands. --- .github/workflows/test-linux.yml | 20 ++++++++++++++++++++ tests/integration/test_installation.py | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml index 2f5465a6e..1fa930004 100644 --- a/.github/workflows/test-linux.yml +++ b/.github/workflows/test-linux.yml @@ -63,6 +63,26 @@ jobs: run: | /usr/bin/docker start ${{ github.run_id }}_salt-test + - name: "Wait for container ${{ inputs.container-slug }} to finish booting" + run: | + # systemd's default tmpfiles.d rules wipe /tmp during early boot + # (systemd-tmpfiles-setup.service). Running pip/bootstrap before + # that finishes races the wipe and fails with spurious + # "No such file or directory" errors on /tmp/pip-*-tracker-*. + for i in $(seq 1 90); do + state=$(docker exec ${{ github.run_id }}_salt-test systemctl is-system-running 2>/dev/null || true) + echo "boot state at ${i}s: '$state'" + case "$state" in + running|degraded) exit 0 ;; + esac + sleep 1 + done + echo "::error::Container did not finish booting within 90 seconds (last state: '$state')" + docker exec ${{ github.run_id }}_salt-test systemctl list-jobs || true + docker exec ${{ github.run_id }}_salt-test systemctl --failed || true + docker exec ${{ github.run_id }}_salt-test journalctl -b --no-pager || true + exit 1 + - name: "Install Python Dependencies with pip breakage in container ${{ inputs.container-slug }}" if: ${{ ( inputs.distro-slug == 'debian-12' ) || ( inputs.distro-slug == 'debian-13' ) || ( inputs.distro-slug == 'ubuntu-2404' ) || ( inputs.distro-slug == 'ubuntu-2604' ) }} run: | diff --git a/tests/integration/test_installation.py b/tests/integration/test_installation.py index 214fc66db..ea519beaf 100644 --- a/tests/integration/test_installation.py +++ b/tests/integration/test_installation.py @@ -147,7 +147,10 @@ def _bash_has_gnu_sed(): # stub instead - a different, often broken, resolution path. try: result = subprocess.run( - ["bash", "-c", "sed --version"], capture_output=True, text=True + ["bash", "-c", "sed --version"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, ) except FileNotFoundError: return False From 3425633b0bf552152bcd6a8b77a9da6c742d50b3 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 3 Aug 2026 15:02:12 -0600 Subject: [PATCH 4/4] ci(test-linux): boot test containers into rescue.target correctly "--systemd --unit rescue.target" was silently broken: --systemd isn't a real systemd flag, and space-separated --unit rescue.target doesn't parse as --unit=rescue.target. Containers were actually booting their full default target (multi-user/graphical, network-online, getty, etc.) instead of the intended minimal rescue mode. On Debian 11 that full boot pulls in NetworkManager-wait-online.service, which times out after its fixed 90s TimeoutStartSec since the container's network never satisfies NetworkManager's online check - exactly matching the 83-89s "starting" stalls seen in CI. Fix the flag to --unit=rescue.target so containers boot minimally for every distro, accept "maintenance" (its correct terminal state) in the wait condition, and drop the timeout back to 30s now that real convergence is ~2s. --- .github/workflows/test-linux.yml | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml index 1fa930004..f26cc1218 100644 --- a/.github/workflows/test-linux.yml +++ b/.github/workflows/test-linux.yml @@ -57,7 +57,7 @@ jobs: - name: "Create container ${{ inputs.container-slug }}" run: | - /usr/bin/docker create --name ${{ github.run_id }}_salt-test --workdir /_w/ --privileged -e "HOME=/github/home" -e GITHUB_ACTIONS=true -e CI=true -e $GITHUB_ENV -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work":"/__w" -v "/home/runner/work/_temp":"/__w/_temp" -v "/home/runner/work/_actions":"/__w/_actions" -v "/opt/hostedtoolcache":"/__t" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/salt-bootstrap/salt-bootstrap":"/_w/btstrap" --entrypoint "/usr/lib/systemd/systemd" ghcr.io/saltstack/salt-ci-containers/testing:${{ inputs.container-slug }} --systemd --unit rescue.target + /usr/bin/docker create --name ${{ github.run_id }}_salt-test --workdir /_w/ --privileged -e "HOME=/github/home" -e GITHUB_ACTIONS=true -e CI=true -e $GITHUB_ENV -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work":"/__w" -v "/home/runner/work/_temp":"/__w/_temp" -v "/home/runner/work/_actions":"/__w/_actions" -v "/opt/hostedtoolcache":"/__t" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/salt-bootstrap/salt-bootstrap":"/_w/btstrap" --entrypoint "/usr/lib/systemd/systemd" ghcr.io/saltstack/salt-ci-containers/testing:${{ inputs.container-slug }} --unit=rescue.target - name: "Start container ${{ inputs.container-slug }}" run: | @@ -69,15 +69,20 @@ jobs: # (systemd-tmpfiles-setup.service). Running pip/bootstrap before # that finishes races the wipe and fails with spurious # "No such file or directory" errors on /tmp/pip-*-tracker-*. - for i in $(seq 1 90); do + # + # The container boots into rescue.target (--unit=rescue.target + # above), so "is-system-running" settles on "maintenance", not + # "running"/"degraded" - those are kept as a fallback in case a + # given image's rescue.target ever pulls in extra units. + for i in $(seq 1 30); do state=$(docker exec ${{ github.run_id }}_salt-test systemctl is-system-running 2>/dev/null || true) echo "boot state at ${i}s: '$state'" case "$state" in - running|degraded) exit 0 ;; + maintenance|running|degraded) exit 0 ;; esac sleep 1 done - echo "::error::Container did not finish booting within 90 seconds (last state: '$state')" + echo "::error::Container did not finish booting within 30 seconds (last state: '$state')" docker exec ${{ github.run_id }}_salt-test systemctl list-jobs || true docker exec ${{ github.run_id }}_salt-test systemctl --failed || true docker exec ${{ github.run_id }}_salt-test journalctl -b --no-pager || true