From 5fd5df54b487e5c81cd5851a736d69efab994fd6 Mon Sep 17 00:00:00 2001 From: Mike Reeves Date: Thu, 9 Jul 2026 13:47:50 -0400 Subject: [PATCH 01/11] Install UEK8 in so-kernel-upgrade when no UEK kernel is present The script assumed the UEK8 kernel was already installed and only switched the boot default to it. On a node running the EL9 stock kernel (RHCK 5.14) there is no kernel-uek* package at all, so `dnf update` has nothing to upgrade and UEK8 never lands -- the script just logged "nothing to do" and exited 0. When no 6.x UEK boot entry exists, install the kernel-uek metapackage (it pulls kernel-uek-core plus the module subpackages, including kernel-uek-modules-extra-netfilter) and then proceed with the grubby switch. Fail loudly if securityonionkernel is not an enabled repo, since that assignment is gated on the NIC-pin marker and the salt version match and a silent no-op there is hard to diagnose. Also point DEFAULTKERNEL at kernel-uek-core so later kernel updates stay on the UEK line rather than falling back to RHCK. Still idempotent and still never reboots. --- salt/common/tools/sbin/so-kernel-upgrade | 77 ++++++++++++++++++------ 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/salt/common/tools/sbin/so-kernel-upgrade b/salt/common/tools/sbin/so-kernel-upgrade index 46d471051..f901dad8d 100755 --- a/salt/common/tools/sbin/so-kernel-upgrade +++ b/salt/common/tools/sbin/so-kernel-upgrade @@ -5,40 +5,81 @@ # https://securityonion.net/license; you may not use this file except in compliance with the # Elastic License 2.0. # -# so-kernel-upgrade — switch the boot default to the installed UEK8 (6.x) kernel. +# so-kernel-upgrade — install the UEK8 (6.x) kernel if needed and make it the boot default. # -# Security Onion is moving off the EL9 stock kernel / UEK7 (5.x) onto UEK8 (6.x). -# Installing the kernel-uek-core package adds a UEK8 boot entry but does NOT make it the -# default: kernel-install/grubby only auto-promote a new kernel within the running -# kernel's flavor lineage, and we're crossing from a 5.x kernel to the new 6.x UEK flavor. -# So even with UPDATEDEFAULT=yes and DEFAULTKERNEL=kernel-uek-core the box keeps booting -# the old kernel. This tool finds the newest installed 6.x UEK kernel and makes it the -# GRUB default via grubby so the next boot comes up on UEK8. +# Security Onion is moving off the EL9 stock kernel (RHCK, 5.14) / UEK7 (5.15) onto UEK8 (6.x). +# Two separate things have to happen, and neither is automatic: # -# Idempotent: if the UEK8 kernel is already the default it does nothing. It only sets the -# boot default; it does NOT reboot — the admin reboots the node on their own schedule. +# 1. Install it. A node running RHCK has no kernel-uek* package at all, so 'dnf update' +# never pulls UEK8 in — there is nothing to upgrade. The kernel-uek metapackage has to +# be installed explicitly from the securityonionkernel repo. +# 2. Boot it. Installing kernel-uek-core adds a UEK8 boot entry but does NOT make it the +# default: kernel-install/grubby only auto-promote a new kernel within the running +# kernel's flavor lineage, and we're crossing from a 5.x kernel to the new 6.x UEK +# flavor. So even with UPDATEDEFAULT=yes and DEFAULTKERNEL=kernel-uek-core the box +# keeps booting the old kernel until grubby is told otherwise. +# +# This tool does both, then points DEFAULTKERNEL at kernel-uek-core so later kernel updates +# stay on the UEK line. +# +# Idempotent: an already-installed, already-default UEK8 kernel is left alone. It only sets +# the boot default; it does NOT reboot — the admin reboots the node on their own schedule. + +KERNEL_REPO="securityonionkernel" +KERNEL_PKG="kernel-uek" log() { echo "[so-kernel-upgrade] $*"; } [ "$(id -u)" -eq 0 ] || { log "must run as root"; exit 1; } command -v grubby >/dev/null 2>&1 || { log "grubby not found"; exit 1; } +command -v dnf >/dev/null 2>&1 || { log "dnf not found"; exit 1; } # Newest installed UEK8 (6.x) kernel known to the bootloader. UEK8 vmlinuz paths look like -# /boot/vmlinuz-6.12.0-203.76.7.5.el9uek.x86_64; the 5.x UEK7 and 5.14 RHCK won't match. -target="$(grubby --info=ALL 2>/dev/null \ - | sed -n 's/^kernel="\(.*\)"$/\1/p' \ - | grep -E '/vmlinuz-6\.[0-9]+.*uek' \ - | sort -V | tail -1)" +# /boot/vmlinuz-6.12.0-203.76.7.5.el9uek.x86_64; the 5.15 UEK7 and 5.14 RHCK won't match. +find_uek8() { + grubby --info=ALL 2>/dev/null \ + | sed -n 's/^kernel="\(.*\)"$/\1/p' \ + | grep -E '/vmlinuz-6\.[0-9]+.*uek' \ + | sort -V | tail -1 +} + +target="$(find_uek8)" if [ -z "$target" ]; then - log "no installed 6.x UEK (UEK8) kernel found — confirm the kernel repo is assigned and" - log "'dnf update' has installed kernel-uek-core. Nothing to do." - exit 0 + log "no UEK8 kernel installed — installing $KERNEL_PKG from $KERNEL_REPO" + + # The repo is assigned by the repo.client highstate, and only once NICs are pinned by MAC + # (/opt/so/state/nic_names_pinned) so the kernel swap can't renumber interfaces SO binds + # by name. Without the repo we'd silently pull nothing, so fail loudly instead. + if ! dnf -q repolist --enabled 2>/dev/null | awk '{print $1}' | grep -qx "$KERNEL_REPO"; then + log "ERROR: repo '$KERNEL_REPO' is not enabled on this node." + log "Run a highstate first; it is skipped until /opt/so/state/nic_names_pinned exists" + log "(run so-nic-pin) and this node's salt matches the version this release ships." + exit 1 + fi + + dnf -y install "$KERNEL_PKG" || { log "ERROR: failed to install $KERNEL_PKG"; exit 1; } + + target="$(find_uek8)" + if [ -z "$target" ]; then + log "ERROR: $KERNEL_PKG installed but no 6.x UEK boot entry appeared — check 'grubby --info=ALL'" + exit 1 + fi + log "installed UEK8 kernel: $target" +fi + +# Keep future kernel updates on the UEK line rather than falling back to RHCK. Oracle ships +# this file; only rewrite it when it's actually pointing somewhere else. +if [ -f /etc/sysconfig/kernel ] && ! grep -q '^DEFAULTKERNEL=kernel-uek-core$' /etc/sysconfig/kernel; then + log "setting DEFAULTKERNEL=kernel-uek-core in /etc/sysconfig/kernel" + sed -i 's/^DEFAULTKERNEL=.*/DEFAULTKERNEL=kernel-uek-core/' /etc/sysconfig/kernel fi current="$(grubby --default-kernel 2>/dev/null)" if [ "$current" = "$target" ]; then log "UEK8 kernel is already the boot default: $target" + [ "$(uname -r)" = "$(basename "$target" | sed 's/^vmlinuz-//')" ] \ + || log "REBOOT REQUIRED to start using it (currently running $(uname -r))." exit 0 fi From 40c02b3149c1bb4edf5d30f074c3607f8f52594c Mon Sep 17 00:00:00 2001 From: Mike Reeves Date: Thu, 9 Jul 2026 14:21:08 -0400 Subject: [PATCH 02/11] Make so-kernel-upgrade populate the kernel repo and fail loudly Three stages of the UEK8 path fail silently, and the script only handled the last one: 1. Populate. so-repo-sync runs before the highstate deploys the [securityonionkernel] section into repodownload.conf, so the first kernel-aware soup skips the kernel sync. kernelrepo_init_empty then seeds valid-but-empty repodata, leaving an enabled repo with zero packages. dnf resolves it happily and installs nothing, no error. 2. Install. `dnf install kernel-uek` on a UEK7 node sees kernel-uek 5.15 already installed, prints "Nothing to do" and exits 0 -- so the script sailed past the install and died later with a misleading grubby error. 3. Boot. Already handled: grubby only auto-promotes within the running kernel's flavor lineage, so 5.x -> 6.x UEK never promotes on its own. Add ensure_kernel_repo(), which verifies the repo is enabled (necessary because skip_if_unavailable=1 hides a broken repo) and that it can serve a 6.x kernel-uek. When it cannot, a manager runs so-repo-sync to populate /nsm/kernelrepo and re-checks; a minion cannot fix it and exits non-zero pointing the admin at the manager. Airgap managers bail, since their repo comes from the ISO rather than a sync. Install the explicit UEK8 NEVRA instead of the bare package name so the "Nothing to do" exit-0 case cannot mask a no-op, and pin the repoquery to securityonionkernel so a UEK7 kernel-uek in the main repo is never picked. Still idempotent and still never reboots. --- salt/common/tools/sbin/so-kernel-upgrade | 160 +++++++++++++++++------ 1 file changed, 119 insertions(+), 41 deletions(-) diff --git a/salt/common/tools/sbin/so-kernel-upgrade b/salt/common/tools/sbin/so-kernel-upgrade index f901dad8d..d04ca533c 100755 --- a/salt/common/tools/sbin/so-kernel-upgrade +++ b/salt/common/tools/sbin/so-kernel-upgrade @@ -5,37 +5,56 @@ # https://securityonion.net/license; you may not use this file except in compliance with the # Elastic License 2.0. # -# so-kernel-upgrade — install the UEK8 (6.x) kernel if needed and make it the boot default. +# so-kernel-upgrade — install the UEK8 (6.x) kernel and make it the boot default. # -# Security Onion is moving off the EL9 stock kernel (RHCK, 5.14) / UEK7 (5.15) onto UEK8 (6.x). -# Two separate things have to happen, and neither is automatic: +# Security Onion is moving off the EL9 stock kernel (RHCK, 5.14) and UEK7 (5.15) onto UEK8 +# (6.x). Three things have to happen, and none of them are automatic: # -# 1. Install it. A node running RHCK has no kernel-uek* package at all, so 'dnf update' -# never pulls UEK8 in — there is nothing to upgrade. The kernel-uek metapackage has to -# be installed explicitly from the securityonionkernel repo. -# 2. Boot it. Installing kernel-uek-core adds a UEK8 boot entry but does NOT make it the -# default: kernel-install/grubby only auto-promote a new kernel within the running -# kernel's flavor lineage, and we're crossing from a 5.x kernel to the new 6.x UEK -# flavor. So even with UPDATEDEFAULT=yes and DEFAULTKERNEL=kernel-uek-core the box -# keeps booting the old kernel until grubby is told otherwise. +# 1. Populate. The manager mirrors the UEK8 packages into /nsm/kernelrepo via so-repo-sync, +# and serves them to the grid over https:///kernelrepo. Until that sync runs the +# repo is valid but EMPTY -- dnf resolves it happily and installs nothing, with no error. +# 2. Install. A node on RHCK has no kernel-uek* package at all, so there is nothing for +# 'dnf update' to upgrade. A node on UEK7 does have kernel-uek installed, so +# 'dnf install kernel-uek' reports "Nothing to do" and exits 0 without installing 6.x. +# Both cases need an explicit install of the UEK8 NEVRA. +# 3. Boot it. Installing kernel-uek-core adds a UEK8 boot entry but does NOT make it the +# default: kernel-install/grubby only auto-promote within the running kernel's flavor +# lineage, and we're crossing from 5.x to the 6.x UEK flavor. So even with +# UPDATEDEFAULT=yes and DEFAULTKERNEL=kernel-uek-core the box keeps booting the old +# kernel until grubby is told otherwise. # -# This tool does both, then points DEFAULTKERNEL at kernel-uek-core so later kernel updates -# stay on the UEK line. +# Every one of those failure modes is silent by default. This tool does all three and fails +# loudly when it cannot, rather than reporting success while changing nothing. +# +# Manager vs minion: only the manager owns /nsm/kernelrepo, so only the manager can populate +# it. If the repo is empty here, a manager runs so-repo-sync itself; a minion has no way to +# fix it and exits non-zero telling the admin to sync the manager first. # # Idempotent: an already-installed, already-default UEK8 kernel is left alone. It only sets -# the boot default; it does NOT reboot — the admin reboots the node on their own schedule. +# the boot default; it does NOT reboot -- the admin reboots the node on their own schedule. + +. /usr/sbin/so-common KERNEL_REPO="securityonionkernel" KERNEL_PKG="kernel-uek" +KERNEL_REPO_DIR="/nsm/kernelrepo" +REPOSYNC_CONF="/opt/so/conf/reposync/repodownload.conf" +GLOBAL_PILLAR="/opt/so/saltstack/local/pillar/global/soc_global.sls" log() { echo "[so-kernel-upgrade] $*"; } +die() { echo "[so-kernel-upgrade] ERROR: $*" >&2; exit 1; } -[ "$(id -u)" -eq 0 ] || { log "must run as root"; exit 1; } -command -v grubby >/dev/null 2>&1 || { log "grubby not found"; exit 1; } -command -v dnf >/dev/null 2>&1 || { log "dnf not found"; exit 1; } +command -v grubby >/dev/null 2>&1 || die "grubby not found" +command -v dnf >/dev/null 2>&1 || die "dnf not found" + +ARCH="$(rpm -E '%{_arch}')" + +is_airgap() { + [ -f "$GLOBAL_PILLAR" ] && grep -q 'airgap: *[Tt]rue' "$GLOBAL_PILLAR" +} # Newest installed UEK8 (6.x) kernel known to the bootloader. UEK8 vmlinuz paths look like -# /boot/vmlinuz-6.12.0-203.76.7.5.el9uek.x86_64; the 5.15 UEK7 and 5.14 RHCK won't match. +# /boot/vmlinuz-6.12.0-204.92.4.2.el9uek.x86_64; UEK7 (5.15) and RHCK (5.14) won't match. find_uek8() { grubby --info=ALL 2>/dev/null \ | sed -n 's/^kernel="\(.*\)"$/\1/p' \ @@ -43,28 +62,86 @@ find_uek8() { | sort -V | tail -1 } +# Newest UEK8 kernel-uek NEVRA offered by the kernel repo, empty if the repo has none. +# Restricted to the kernel repo so a UEK7 kernel-uek in the main repo can't be picked up, +# and filtered to 6.x so we never "succeed" by reinstalling the 5.15 we already have. +uek8_available() { + dnf -q repoquery --disablerepo='*' --enablerepo="$KERNEL_REPO" \ + --arch="$ARCH" --latest-limit=1 \ + --qf '%{name}-%{evr}.%{arch}\n' "$KERNEL_PKG" 2>/dev/null \ + | grep -E "^${KERNEL_PKG}-6\." | tail -1 +} + +kernelrepo_rpm_count() { + find "$KERNEL_REPO_DIR" -maxdepth 1 -name '*.rpm' 2>/dev/null | wc -l +} + +# The kernel repo starts life as valid-but-empty (kernelrepo_init_empty in +# salt/manager/init.sls) and is filled by so-repo-sync. During a soup, so-repo-sync runs +# BEFORE the highstate deploys the [securityonionkernel] section into repodownload.conf, so +# the first kernel-aware soup leaves the repo empty until the next nightly sync. +sync_kernel_repo() { + if is_airgap; then + log "airgap install: $KERNEL_REPO_DIR is populated from the airgap ISO, not by so-repo-sync." + return 1 + fi + if ! grep -q "^\[${KERNEL_REPO}\]" "$REPOSYNC_CONF" 2>/dev/null; then + log "$REPOSYNC_CONF has no [${KERNEL_REPO}] section -- run a highstate to deploy it." + return 1 + fi + + log "populating $KERNEL_REPO_DIR with so-repo-sync (mirrors upstream; can take several minutes)" + su socore -c '/usr/sbin/so-repo-sync' || { log "so-repo-sync failed"; return 1; } + + dnf -q clean expire-cache >/dev/null 2>&1 + return 0 +} + +# Make the kernel repo actually able to serve a UEK8 package, or fail trying. +ensure_kernel_repo() { + # The repo is assigned by the repo.client highstate, and only once NICs are pinned by MAC + # (/opt/so/state/nic_names_pinned) so the kernel swap can't renumber interfaces SO binds + # by name. skip_if_unavailable=1 means a broken repo is silently ignored, so check first. + if ! dnf -q repolist --enabled 2>/dev/null | awk '{print $1}' | grep -qx "$KERNEL_REPO"; then + log "repo '$KERNEL_REPO' is not enabled on this node." + log "Run a highstate first; the repo is skipped until /opt/so/state/nic_names_pinned" + log "exists (run so-nic-pin) and this node's salt matches the version this release ships." + die "kernel repo unavailable" + fi + + [ -n "$(uek8_available)" ] && return 0 + + log "repo '$KERNEL_REPO' is enabled but offers no UEK8 $KERNEL_PKG package" + + if ! is_manager_node; then + log "This is a minion; it consumes the kernel repo from the manager and cannot populate it." + log "On the manager, run: su socore -c /usr/sbin/so-repo-sync" + log "then re-run this script here." + die "manager's kernel repo is empty" + fi + + log "this is a manager and $KERNEL_REPO_DIR holds $(kernelrepo_rpm_count) rpm(s)" + sync_kernel_repo || die "could not populate $KERNEL_REPO_DIR" + + [ -n "$(uek8_available)" ] \ + || die "so-repo-sync completed but $KERNEL_REPO still offers no UEK8 $KERNEL_PKG" +} + target="$(find_uek8)" if [ -z "$target" ]; then - log "no UEK8 kernel installed — installing $KERNEL_PKG from $KERNEL_REPO" + log "no UEK8 kernel installed (running $(uname -r))" - # The repo is assigned by the repo.client highstate, and only once NICs are pinned by MAC - # (/opt/so/state/nic_names_pinned) so the kernel swap can't renumber interfaces SO binds - # by name. Without the repo we'd silently pull nothing, so fail loudly instead. - if ! dnf -q repolist --enabled 2>/dev/null | awk '{print $1}' | grep -qx "$KERNEL_REPO"; then - log "ERROR: repo '$KERNEL_REPO' is not enabled on this node." - log "Run a highstate first; it is skipped until /opt/so/state/nic_names_pinned exists" - log "(run so-nic-pin) and this node's salt matches the version this release ships." - exit 1 - fi + ensure_kernel_repo + nevra="$(uek8_available)" - dnf -y install "$KERNEL_PKG" || { log "ERROR: failed to install $KERNEL_PKG"; exit 1; } + # Install the explicit NEVRA, not the bare package name. On a UEK7 node 'dnf install + # kernel-uek' sees kernel-uek-5.15 already installed, prints "Nothing to do" and exits 0. + log "installing $nevra from $KERNEL_REPO" + dnf -y install "$nevra" || die "failed to install $nevra" target="$(find_uek8)" - if [ -z "$target" ]; then - log "ERROR: $KERNEL_PKG installed but no 6.x UEK boot entry appeared — check 'grubby --info=ALL'" - exit 1 - fi + [ -n "$target" ] || die "$nevra installed but no 6.x UEK boot entry appeared -- check 'grubby --info=ALL'" log "installed UEK8 kernel: $target" fi @@ -75,24 +152,25 @@ if [ -f /etc/sysconfig/kernel ] && ! grep -q '^DEFAULTKERNEL=kernel-uek-core$' / sed -i 's/^DEFAULTKERNEL=.*/DEFAULTKERNEL=kernel-uek-core/' /etc/sysconfig/kernel fi +reboot_notice() { + [ "$(uname -r)" = "$(basename "$1" | sed 's/^vmlinuz-//')" ] \ + || log "REBOOT REQUIRED to start using the UEK8 kernel (currently running $(uname -r))." +} + current="$(grubby --default-kernel 2>/dev/null)" if [ "$current" = "$target" ]; then log "UEK8 kernel is already the boot default: $target" - [ "$(uname -r)" = "$(basename "$target" | sed 's/^vmlinuz-//')" ] \ - || log "REBOOT REQUIRED to start using it (currently running $(uname -r))." + reboot_notice "$target" exit 0 fi log "current default kernel: ${current:-unknown}" log "switching boot default to UEK8 kernel: $target" -grubby --set-default="$target" || { log "ERROR: grubby --set-default failed for $target"; exit 1; } +grubby --set-default="$target" || die "grubby --set-default failed for $target" # Verify the change actually took before claiming success. now="$(grubby --default-kernel 2>/dev/null)" -if [ "$now" != "$target" ]; then - log "ERROR: default kernel is still '${now:-unknown}' after set-default" - exit 1 -fi +[ "$now" = "$target" ] || die "default kernel is still '${now:-unknown}' after set-default" log "boot default is now $target" -log "REBOOT REQUIRED to start using the UEK8 kernel (currently running $(uname -r))." +reboot_notice "$target" From 9a71f64a358121ce0bd7bca6fd872c03d17a5ba0 Mon Sep 17 00:00:00 2001 From: Mike Reeves Date: Thu, 9 Jul 2026 15:10:13 -0400 Subject: [PATCH 03/11] Branch so-kernel-upgrade on the running kernel flavor Only the RHCK->UEK flavor cross needs grubby --set-default; a UEK7->UEK8 update stays in the kernel-uek lineage and auto-promotes on its own. Detect the running kernel and act accordingly: - UEK8: already on target, no-op. - UEK7: populate the repo and install UEK8, then verify it auto-promoted (warn with the manual grubby command if it did not) -- no grubby change. - RHCK: install UEK8 and set the boot default explicitly, as before. Also make an already-installed UEK8 skip the repo entirely so a disabled or empty kernel repo can't block flipping the default, and correct the header comment that claimed every transition needs grubby. --- salt/common/tools/sbin/so-kernel-upgrade | 153 ++++++++++++++++------- 1 file changed, 108 insertions(+), 45 deletions(-) diff --git a/salt/common/tools/sbin/so-kernel-upgrade b/salt/common/tools/sbin/so-kernel-upgrade index d04ca533c..960505aee 100755 --- a/salt/common/tools/sbin/so-kernel-upgrade +++ b/salt/common/tools/sbin/so-kernel-upgrade @@ -8,7 +8,7 @@ # so-kernel-upgrade — install the UEK8 (6.x) kernel and make it the boot default. # # Security Onion is moving off the EL9 stock kernel (RHCK, 5.14) and UEK7 (5.15) onto UEK8 -# (6.x). Three things have to happen, and none of them are automatic: +# (6.x). Three things have to happen, and the tool has to drive each one: # # 1. Populate. The manager mirrors the UEK8 packages into /nsm/kernelrepo via so-repo-sync, # and serves them to the grid over https:///kernelrepo. Until that sync runs the @@ -17,13 +17,17 @@ # 'dnf update' to upgrade. A node on UEK7 does have kernel-uek installed, so # 'dnf install kernel-uek' reports "Nothing to do" and exits 0 without installing 6.x. # Both cases need an explicit install of the UEK8 NEVRA. -# 3. Boot it. Installing kernel-uek-core adds a UEK8 boot entry but does NOT make it the -# default: kernel-install/grubby only auto-promote within the running kernel's flavor -# lineage, and we're crossing from 5.x to the 6.x UEK flavor. So even with -# UPDATEDEFAULT=yes and DEFAULTKERNEL=kernel-uek-core the box keeps booting the old -# kernel until grubby is told otherwise. +# 3. Boot it. Whether a newly installed UEK8 kernel becomes the boot default depends on the +# RUNNING kernel's flavor. kernel-install/grubby (with UPDATEDEFAULT=yes) only auto-promote +# within the running kernel's flavor lineage: +# - From UEK7 (5.x, kernel-uek) the install stays in the kernel-uek lineage and IS +# auto-promoted, so no grubby change is needed -- just make sure the repo is populated +# and install UEK8. +# - From the stock EL9 kernel (RHCK, 5.14, no UEK) it is a flavor CROSS that is NOT +# auto-promoted, so the box keeps booting RHCK until grubby is told otherwise. +# This tool inspects the running kernel and only runs 'grubby --set-default' for RHCK. # -# Every one of those failure modes is silent by default. This tool does all three and fails +# Every one of those failure modes is silent by default. This tool handles each case and fails # loudly when it cannot, rather than reporting success while changing nothing. # # Manager vs minion: only the manager owns /nsm/kernelrepo, so only the manager can populate @@ -62,6 +66,19 @@ find_uek8() { | sort -V | tail -1 } +# Classify the RUNNING kernel (uname -r) -- this, not what's installed, is what decides whether +# a UEK8 install auto-promotes to the boot default: +# uek8 6.x UEK already on the target line; nothing to do +# uek7 5.x UEK a UEK8 install stays in the kernel-uek lineage and auto-promotes (no grubby) +# rhck 5.14 EL9 crossing into the UEK flavor does NOT auto-promote (needs grubby --set-default) +running_flavor() { + case "$(uname -r)" in + 6.*uek*) echo uek8 ;; + *uek*) echo uek7 ;; + *) echo rhck ;; + esac +} + # Newest UEK8 kernel-uek NEVRA offered by the kernel repo, empty if the repo has none. # Restricted to the kernel repo so a UEK7 kernel-uek in the main repo can't be picked up, # and filtered to 6.x so we never "succeed" by reinstalling the 5.15 we already have. @@ -127,50 +144,96 @@ ensure_kernel_repo() { || die "so-repo-sync completed but $KERNEL_REPO still offers no UEK8 $KERNEL_PKG" } -target="$(find_uek8)" - -if [ -z "$target" ]; then - log "no UEK8 kernel installed (running $(uname -r))" - - ensure_kernel_repo - nevra="$(uek8_available)" - - # Install the explicit NEVRA, not the bare package name. On a UEK7 node 'dnf install - # kernel-uek' sees kernel-uek-5.15 already installed, prints "Nothing to do" and exits 0. - log "installing $nevra from $KERNEL_REPO" - dnf -y install "$nevra" || die "failed to install $nevra" - - target="$(find_uek8)" - [ -n "$target" ] || die "$nevra installed but no 6.x UEK boot entry appeared -- check 'grubby --info=ALL'" - log "installed UEK8 kernel: $target" -fi - -# Keep future kernel updates on the UEK line rather than falling back to RHCK. Oracle ships -# this file; only rewrite it when it's actually pointing somewhere else. -if [ -f /etc/sysconfig/kernel ] && ! grep -q '^DEFAULTKERNEL=kernel-uek-core$' /etc/sysconfig/kernel; then - log "setting DEFAULTKERNEL=kernel-uek-core in /etc/sysconfig/kernel" - sed -i 's/^DEFAULTKERNEL=.*/DEFAULTKERNEL=kernel-uek-core/' /etc/sysconfig/kernel -fi - reboot_notice() { [ "$(uname -r)" = "$(basename "$1" | sed 's/^vmlinuz-//')" ] \ || log "REBOOT REQUIRED to start using the UEK8 kernel (currently running $(uname -r))." } -current="$(grubby --default-kernel 2>/dev/null)" -if [ "$current" = "$target" ]; then - log "UEK8 kernel is already the boot default: $target" - reboot_notice "$target" +# Keep future kernel updates on the UEK line rather than falling back to RHCK. Oracle ships +# /etc/sysconfig/kernel; only rewrite it when it's actually pointing somewhere else. +set_default_kernel_conf() { + if [ -f /etc/sysconfig/kernel ] && ! grep -q '^DEFAULTKERNEL=kernel-uek-core$' /etc/sysconfig/kernel; then + log "setting DEFAULTKERNEL=kernel-uek-core in /etc/sysconfig/kernel" + sed -i 's/^DEFAULTKERNEL=.*/DEFAULTKERNEL=kernel-uek-core/' /etc/sysconfig/kernel + fi +} + +# Make sure a UEK8 kernel is installed, leaving its boot entry in INSTALLED_UEK8. If one is +# already present we leave the repo alone -- it may be disabled or empty and we don't need it +# just to flip the boot default. Otherwise install the explicit NEVRA, not the bare package +# name: on a UEK7 node 'dnf install kernel-uek' sees 5.15 already present, prints "Nothing to +# do" and exits 0 without installing 6.x. +ensure_uek8_installed() { + INSTALLED_UEK8="$(find_uek8)" + if [ -n "$INSTALLED_UEK8" ]; then + log "UEK8 kernel already installed: $INSTALLED_UEK8" + return 0 + fi + + ensure_kernel_repo + local nevra; nevra="$(uek8_available)" + log "installing $nevra from $KERNEL_REPO" + dnf -y install "$nevra" || die "failed to install $nevra" + + INSTALLED_UEK8="$(find_uek8)" + [ -n "$INSTALLED_UEK8" ] || die "$nevra installed but no 6.x UEK boot entry appeared -- check 'grubby --info=ALL'" + log "installed UEK8 kernel: $INSTALLED_UEK8" +} + +case "$(running_flavor)" in +uek8) + # Already on the 6.x UEK line. A plain 'dnf update' keeps this node current within the + # lineage and auto-promotes newer builds, so there is nothing for this tool to do. + log "already running a UEK8 kernel ($(uname -r)); nothing to do." exit 0 -fi + ;; -log "current default kernel: ${current:-unknown}" -log "switching boot default to UEK8 kernel: $target" -grubby --set-default="$target" || die "grubby --set-default failed for $target" +uek7) + # On a 5.x UEK kernel. Installing UEK8 stays inside the kernel-uek lineage, so dnf/grubby + # (UPDATEDEFAULT=yes) auto-promote it and we do NOT touch grubby. A node still on UEK7 + # usually means the kernel repo was empty when it last updated, so populate it and install. + log "running UEK7 kernel ($(uname -r)); the kernel repo was likely not yet populated when" + log "this node last updated. Populating it and installing UEK8 -- the update stays on the" + log "kernel-uek line, so it becomes the boot default automatically (no grubby change needed)." + set_default_kernel_conf + ensure_uek8_installed -# Verify the change actually took before claiming success. -now="$(grubby --default-kernel 2>/dev/null)" -[ "$now" = "$target" ] || die "default kernel is still '${now:-unknown}' after set-default" + now="$(grubby --default-kernel 2>/dev/null)" + if [ "$now" = "$INSTALLED_UEK8" ]; then + log "boot default auto-promoted to UEK8 kernel: $INSTALLED_UEK8" + else + log "WARNING: expected the UEK8 kernel to auto-promote but the default is still" + log "'${now:-unknown}'. Run 'grubby --set-default=$INSTALLED_UEK8' to force it." + fi + reboot_notice "$INSTALLED_UEK8" + ;; -log "boot default is now $target" -reboot_notice "$target" +rhck) + # On the stock EL9 kernel (5.14, no UEK installed). Crossing from RHCK into the UEK flavor + # does NOT auto-promote -- kernel-install/grubby only auto-promote within the running + # kernel's flavor lineage -- so after installing we must set the boot default explicitly. + log "running stock EL9 (RHCK) kernel ($(uname -r)); installing UEK8 and setting it as the" + log "boot default explicitly (a RHCK->UEK flavor change does not auto-promote)." + set_default_kernel_conf + ensure_uek8_installed + target="$INSTALLED_UEK8" + + current="$(grubby --default-kernel 2>/dev/null)" + if [ "$current" = "$target" ]; then + log "UEK8 kernel is already the boot default: $target" + reboot_notice "$target" + exit 0 + fi + + log "current default kernel: ${current:-unknown}" + log "switching boot default to UEK8 kernel: $target" + grubby --set-default="$target" || die "grubby --set-default failed for $target" + + # Verify the change actually took before claiming success. + now="$(grubby --default-kernel 2>/dev/null)" + [ "$now" = "$target" ] || die "default kernel is still '${now:-unknown}' after set-default" + + log "boot default is now $target" + reboot_notice "$target" + ;; +esac From 52885e28c5012116fad1d096caad45fa0c70856e Mon Sep 17 00:00:00 2001 From: Mike Reeves Date: Thu, 9 Jul 2026 17:02:47 -0400 Subject: [PATCH 04/11] Name the reposync-side kernel repo securityonionkernelsync The reposync section in repodownload.conf and the client repo assigned in repo/client/oracle.sls both used the bare name securityonionkernel, colliding across the two roles. Rename the reposync-side section (and its --repoid, the so-repo-sync guard, and the so-kernel-upgrade presence check) to securityonionkernelsync, mirroring the existing securityonion/securityonionsync split for the main repo. The client repo stays securityonionkernel. Also give the section its own name=Security Onion Kernel Repo repo. --- salt/common/tools/sbin/so-kernel-upgrade | 10 +++++++--- salt/manager/files/repodownload.conf | 4 ++-- salt/manager/tools/sbin/so-repo-sync | 6 +++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/salt/common/tools/sbin/so-kernel-upgrade b/salt/common/tools/sbin/so-kernel-upgrade index 960505aee..e750c1a52 100755 --- a/salt/common/tools/sbin/so-kernel-upgrade +++ b/salt/common/tools/sbin/so-kernel-upgrade @@ -39,7 +39,11 @@ . /usr/sbin/so-common +# Client-side repo id (what dnf enables on this node, from repo/client/oracle.sls) vs the +# reposync-side section in repodownload.conf that the manager mirrors from (mirrors the +# securityonion/securityonionsync split for the main repo). KERNEL_REPO="securityonionkernel" +KERNEL_REPO_SYNC="securityonionkernelsync" KERNEL_PKG="kernel-uek" KERNEL_REPO_DIR="/nsm/kernelrepo" REPOSYNC_CONF="/opt/so/conf/reposync/repodownload.conf" @@ -95,15 +99,15 @@ kernelrepo_rpm_count() { # The kernel repo starts life as valid-but-empty (kernelrepo_init_empty in # salt/manager/init.sls) and is filled by so-repo-sync. During a soup, so-repo-sync runs -# BEFORE the highstate deploys the [securityonionkernel] section into repodownload.conf, so +# BEFORE the highstate deploys the [securityonionkernelsync] section into repodownload.conf, so # the first kernel-aware soup leaves the repo empty until the next nightly sync. sync_kernel_repo() { if is_airgap; then log "airgap install: $KERNEL_REPO_DIR is populated from the airgap ISO, not by so-repo-sync." return 1 fi - if ! grep -q "^\[${KERNEL_REPO}\]" "$REPOSYNC_CONF" 2>/dev/null; then - log "$REPOSYNC_CONF has no [${KERNEL_REPO}] section -- run a highstate to deploy it." + if ! grep -q "^\[${KERNEL_REPO_SYNC}\]" "$REPOSYNC_CONF" 2>/dev/null; then + log "$REPOSYNC_CONF has no [${KERNEL_REPO_SYNC}] section -- run a highstate to deploy it." return 1 fi diff --git a/salt/manager/files/repodownload.conf b/salt/manager/files/repodownload.conf index 67ae4b121..9c9cb5109 100644 --- a/salt/manager/files/repodownload.conf +++ b/salt/manager/files/repodownload.conf @@ -11,8 +11,8 @@ name=Security Onion Repo repo mirrorlist=file:///opt/so/conf/reposync/mirror.txt enabled=1 gpgcheck=1 -[securityonionkernel] -name=Security Onion Repo repo +[securityonionkernelsync] +name=Security Onion Kernel Repo repo mirrorlist=file:///opt/so/conf/reposync/mirror-kernel.txt enabled=1 gpgcheck=1 diff --git a/salt/manager/tools/sbin/so-repo-sync b/salt/manager/tools/sbin/so-repo-sync index 6c1b9d509..d6a290c25 100755 --- a/salt/manager/tools/sbin/so-repo-sync +++ b/salt/manager/tools/sbin/so-repo-sync @@ -17,9 +17,9 @@ createrepo /nsm/repo # The kernel repo section is deployed to repodownload.conf by the manager highstate, which # runs AFTER this script during soup. On the first upgrade to a kernel-aware version the # on-disk config still predates the section, so guard on its presence to avoid dnf's -# "Unknown repo: 'securityonionkernel'" aborting the sync (set -e). The next sync after the +# "Unknown repo: 'securityonionkernelsync'" aborting the sync (set -e). The next sync after the # highstate deploys the section will pick it up. -if grep -q '^\[securityonionkernel\]' /opt/so/conf/reposync/repodownload.conf; then - dnf reposync --norepopath -g --delete -m -c /opt/so/conf/reposync/repodownload.conf --repoid=securityonionkernel --download-metadata -p /nsm/kernelrepo/ +if grep -q '^\[securityonionkernelsync\]' /opt/so/conf/reposync/repodownload.conf; then + dnf reposync --norepopath -g --delete -m -c /opt/so/conf/reposync/repodownload.conf --repoid=securityonionkernelsync --download-metadata -p /nsm/kernelrepo/ createrepo /nsm/kernelrepo fi From 517538a9a73ad07ee9b9cf4a6bc887c6976ebc57 Mon Sep 17 00:00:00 2001 From: Josh Patterson Date: Wed, 15 Jul 2026 16:28:35 -0400 Subject: [PATCH 05/11] remove comments --- salt/libvirt/dev/saltDev.sls | 67 ++++++++++++++++++++++++++++++++++ salt/reactor/push_strelka.sls | 2 +- salt/reactor/push_suricata.sls | 2 +- 3 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 salt/libvirt/dev/saltDev.sls diff --git a/salt/libvirt/dev/saltDev.sls b/salt/libvirt/dev/saltDev.sls new file mode 100644 index 000000000..016407635 --- /dev/null +++ b/salt/libvirt/dev/saltDev.sls @@ -0,0 +1,67 @@ +# This state is designed to run on a development manager running in a libvirt VM. It will map the default pillar and salt directories +# from /opt/so/saltstack/default to your local development machine as the source path. +# The VM requires a filesystem to be added. Only the source path should be changed to your development codebase +# Driver: virtio-9p +# Source path: ~/project/securityonion +# Target path: saltDev + +# If you want a directory to be RW, then kvm must have group privileges. +# ll /home/user/projects/securityonion/salt/hypervisor +# total 48 +# drwxrwxr-x 3 user kvm 4096 Feb 13 11:18 ./ +# drwxrwxr-x 64 user user 4096 Feb 13 10:32 ../ +# -rw-rw-r-- 1 user kvm 2238 Feb 12 15:06 defaults.yaml +# -rw-rw-r-- 1 user kvm 1467 Feb 12 15:06 init.sls +# -rw-rw-r-- 1 user kvm 70 Feb 13 09:37 soc_hypervisor.yaml +# drwxrwxr-x 3 user kvm 4096 Feb 12 15:06 tools/ + +# Ensure required kernel modules are configured for loading +/etc/modules-load.d/virtio-9p.conf: + file.managed: + - contents: | + 9pnet_virtio + 9pnet + 9p + - mode: 644 + - user: root + - group: root + +# Load the kernel modules immediately (in the correct order) +load_9p_modules: + cmd.run: + - names: + - modprobe 9pnet_virtio + - modprobe 9pnet + - modprobe 9p + - unless: lsmod | grep -E '9pnet_virtio|9pnet|9p' + +# Ensure mount point exists +/opt/so/saltstack/default: + file.directory: + - user: root + - group: root + - mode: 755 + - makedirs: True + +# Configure fstab entry using mount.fstab_present +# Configure fstab entry using mount.fstab_present +saltdev_fstab: + mount.fstab_present: + - name: saltDev + - fs_file: /opt/so/saltstack/default + - fs_vfstype: 9p + - fs_mntops: _netdev,trans=virtio,version=9p2000.L + - fs_freq: 0 + - fs_passno: 0 + +# Mount the filesystem if not already mounted +mount_saltdev: + mount.mounted: + - name: /opt/so/saltstack/default + - device: saltDev + - fstype: 9p + - opts: _netdev,trans=virtio,version=9p2000.L + - require: + - file: /opt/so/saltstack/default + - mount: saltdev_fstab + - cmd: load_9p_modules diff --git a/salt/reactor/push_strelka.sls b/salt/reactor/push_strelka.sls index 52e3fd3ef..31f66e0a6 100644 --- a/salt/reactor/push_strelka.sls +++ b/salt/reactor/push_strelka.sls @@ -6,7 +6,7 @@ # Writes (or updates) a push intent at /opt/so/state/push_pending/rules_strelka.json # and returns {}. The so-push-drainer schedule picks up ready intents, dedupes # across pending files, and dispatches orch.push_batch. Reactors never dispatch -# directly -- see plan /home/mreeves/.claude/plans/goofy-marinating-hummingbird.md. +# directly import fcntl import json diff --git a/salt/reactor/push_suricata.sls b/salt/reactor/push_suricata.sls index cce95fdb7..11fbd1286 100644 --- a/salt/reactor/push_suricata.sls +++ b/salt/reactor/push_suricata.sls @@ -6,7 +6,7 @@ # Writes (or updates) a push intent at /opt/so/state/push_pending/rules_suricata.json # and returns {}. The so-push-drainer schedule picks up ready intents, dedupes # across pending files, and dispatches orch.push_batch. Reactors never dispatch -# directly -- see plan /home/mreeves/.claude/plans/goofy-marinating-hummingbird.md. +# directly import fcntl import json From f9b154ccef3ade676439f16824fc348aac7b1ace Mon Sep 17 00:00:00 2001 From: Josh Patterson Date: Wed, 15 Jul 2026 16:32:29 -0400 Subject: [PATCH 06/11] remove comments --- salt/manager/tools/sbin/so-push-drainer | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/salt/manager/tools/sbin/so-push-drainer b/salt/manager/tools/sbin/so-push-drainer index fbbd952a4..8cc97a8d5 100644 --- a/salt/manager/tools/sbin/so-push-drainer +++ b/salt/manager/tools/sbin/so-push-drainer @@ -21,8 +21,7 @@ is older than debounce_seconds, this script: * deletes the contributed intent files on successful dispatch Reactor sls files (push_suricata, push_strelka, push_pillar) write intents -but never dispatch directly -- see plan -/home/mreeves/.claude/plans/goofy-marinating-hummingbird.md for the full design. +but never dispatch directly """ import fcntl From 5867b50720dc395f1fb354b781680451e83729eb Mon Sep 17 00:00:00 2001 From: reyesj2 <94730068+reyesj2@users.noreply.github.com> Date: Thu, 16 Jul 2026 12:28:23 -0500 Subject: [PATCH 07/11] include so-yaml.py in get_soup_script_hashes() so we ensure its at latest version before using it later on in soup --- salt/manager/tools/sbin/soup | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/salt/manager/tools/sbin/soup b/salt/manager/tools/sbin/soup index ec2dcaed5..a96acdf43 100755 --- a/salt/manager/tools/sbin/soup +++ b/salt/manager/tools/sbin/soup @@ -437,6 +437,8 @@ get_soup_script_hashes() { GITIMGCMN=$(md5sum $UPDATE_DIR/salt/common/tools/sbin/so-image-common | awk '{print $1}') CURRENTSOFIREWALL=$(md5sum /usr/sbin/so-firewall | awk '{print $1}') GITSOFIREWALL=$(md5sum $UPDATE_DIR/salt/manager/tools/sbin/so-firewall | awk '{print $1}') + CURRENTSOYAML=$(md5sum /usr/sbin/so-yaml.py | awk '{print $1}') + GITSOYAML=$(md5sum $UPDATE_DIR/salt/manager/tools/sbin/so-yaml.py | awk '{print $1}') } highstate() { @@ -1224,7 +1226,7 @@ upgrade_salt() { verify_latest_update_script() { get_soup_script_hashes - if [[ "$CURRENTSOUP" == "$GITSOUP" && "$CURRENTCMN" == "$GITCMN" && "$CURRENTIMGCMN" == "$GITIMGCMN" && "$CURRENTSOFIREWALL" == "$GITSOFIREWALL" ]]; then + if [[ "$CURRENTSOUP" == "$GITSOUP" && "$CURRENTCMN" == "$GITCMN" && "$CURRENTIMGCMN" == "$GITIMGCMN" && "$CURRENTSOFIREWALL" == "$GITSOFIREWALL" && "$CURRENTSOYAML" == "$GITSOYAML" ]]; then echo "This version of the soup script is up to date. Proceeding." else echo "You are not running the latest soup version. Updating soup and its components. This might take multiple runs to complete." @@ -1233,7 +1235,7 @@ verify_latest_update_script() { # Verify that soup scripts updated as expected get_soup_script_hashes - if [[ "$CURRENTSOUP" == "$GITSOUP" && "$CURRENTCMN" == "$GITCMN" && "$CURRENTIMGCMN" == "$GITIMGCMN" && "$CURRENTSOFIREWALL" == "$GITSOFIREWALL" ]]; then + if [[ "$CURRENTSOUP" == "$GITSOUP" && "$CURRENTCMN" == "$GITCMN" && "$CURRENTIMGCMN" == "$GITIMGCMN" && "$CURRENTSOFIREWALL" == "$GITSOFIREWALL" && "$CURRENTSOYAML" == "$GITSOYAML" ]]; then echo "Succesfully updated soup scripts." else echo "There was a problem updating soup scripts. Trying to rerun script update." From cc2bfc26e20751470dca8e78f740f80383d94382 Mon Sep 17 00:00:00 2001 From: Doug Burks Date: Thu, 16 Jul 2026 15:13:51 -0400 Subject: [PATCH 08/11] Fix typos in CPU affinity descriptions --- salt/suricata/soc_suricata.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/suricata/soc_suricata.yaml b/salt/suricata/soc_suricata.yaml index ce6b7d008..c0b07a0cb 100644 --- a/salt/suricata/soc_suricata.yaml +++ b/salt/suricata/soc_suricata.yaml @@ -153,12 +153,12 @@ suricata: cpu-affinity: management-cpu-set: cpu: - description: Bind management threads to a core or range of cores. This can be a sigle core, list of cores, or list of range of cores. set-cpu-affinity must be set to true for this to be used. + description: Bind management threads to a core or range of cores. This can be a single core, list of cores, or list of range of cores. set-cpu-affinity must be set to true for this to be used. forcedType: "[]string" helpLink: suricata worker-cpu-set: cpu: - description: Bind worker threads to a core or range of cores. This can be a sigle core, list of cores, or list of range of cores. set-cpu-affinity must be set to true for this to be used. + description: Bind worker threads to a core or range of cores. This can be a single core, list of cores, or list of range of cores. set-cpu-affinity must be set to true for this to be used. forcedType: "[]string" helpLink: suricata vars: From 9e7e6edae070d622a533fca34eba5cdd48849844 Mon Sep 17 00:00:00 2001 From: Josh Patterson Date: Thu, 16 Jul 2026 15:28:22 -0400 Subject: [PATCH 09/11] Add unit tests for _beacons and wire into CI Add 100%-coverage unit tests for the three custom salt beacons (postgres_pillar_beacon, rules_beacon, zeek) and add salt/_beacons to the python-test workflow's paths trigger and matrix. To pass the workflow's flake8 lint over the whole directory: - zeek.py: reindent to 4 spaces, drop trailing blank line, noqa the Salt-injected __salt__ references (F821); no logic change. - postgres_pillar_beacon.py: noqa C901 on beacon() (complexity 13 > 12). --- .github/workflows/pythontest.yml | 3 +- salt/_beacons/postgres_pillar_beacon.py | 2 +- salt/_beacons/postgres_pillar_beacon_test.py | 165 ++++++++++++++++++ salt/_beacons/rules_beacon_test.py | 172 +++++++++++++++++++ salt/_beacons/zeek.py | 37 ++-- salt/_beacons/zeek_test.py | 59 +++++++ 6 files changed, 417 insertions(+), 21 deletions(-) create mode 100644 salt/_beacons/postgres_pillar_beacon_test.py create mode 100644 salt/_beacons/rules_beacon_test.py create mode 100644 salt/_beacons/zeek_test.py diff --git a/.github/workflows/pythontest.yml b/.github/workflows/pythontest.yml index a4cc92d8d..dc95b01c3 100644 --- a/.github/workflows/pythontest.yml +++ b/.github/workflows/pythontest.yml @@ -5,6 +5,7 @@ on: paths: - "salt/sensoroni/files/analyzers/**" - "salt/manager/tools/sbin/**" + - "salt/_beacons/**" jobs: build: @@ -14,7 +15,7 @@ jobs: fail-fast: false matrix: python-version: ["3.14"] - python-code-path: ["salt/sensoroni/files/analyzers", "salt/manager/tools/sbin"] + python-code-path: ["salt/sensoroni/files/analyzers", "salt/manager/tools/sbin", "salt/_beacons"] steps: - uses: actions/checkout@v3 diff --git a/salt/_beacons/postgres_pillar_beacon.py b/salt/_beacons/postgres_pillar_beacon.py index d22eef0ea..074e9e8af 100644 --- a/salt/_beacons/postgres_pillar_beacon.py +++ b/salt/_beacons/postgres_pillar_beacon.py @@ -83,7 +83,7 @@ def _query(sql): return result.stdout -def beacon(config): +def beacon(config): # noqa: C901 retval = [] watermark = _read_watermark() diff --git a/salt/_beacons/postgres_pillar_beacon_test.py b/salt/_beacons/postgres_pillar_beacon_test.py new file mode 100644 index 000000000..377f0ff04 --- /dev/null +++ b/salt/_beacons/postgres_pillar_beacon_test.py @@ -0,0 +1,165 @@ +# Copyright Security Onion Solutions LLC and/or licensed to Security Onion Solutions LLC under one +# or more contributor license agreements. Licensed under the Elastic License 2.0 as shown at +# https://securityonion.net/license; you may not use this file except in compliance with the +# Elastic License 2.0. + +import os +import shutil +import subprocess +import tempfile +import unittest +from unittest.mock import patch + +import postgres_pillar_beacon + + +class TestPostgresPillarBeacon(unittest.TestCase): + + def setUp(self): + # Point WATERMARK_FILE at a throwaway dir so the real read/write helpers + # (and their os.makedirs/os.rename) run against actual files, then clean + # it all up in tearDown. + self.tmpdir = tempfile.mkdtemp() + self.watermark = os.path.join(self.tmpdir, 'state', 'watch.id') + patcher = patch.object(postgres_pillar_beacon, 'WATERMARK_FILE', self.watermark) + patcher.start() + self.addCleanup(patcher.stop) + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + # -- trivial contract ------------------------------------------------- + + def test_virtual_returns_true(self): + self.assertTrue(postgres_pillar_beacon.__virtual__()) + + def test_validate_returns_valid(self): + self.assertEqual(postgres_pillar_beacon.validate({}), (True, 'valid')) + + # -- _read_watermark -------------------------------------------------- + + def test_read_watermark_valid(self): + postgres_pillar_beacon._write_watermark(42) + self.assertEqual(postgres_pillar_beacon._read_watermark(), 42) + + def test_read_watermark_missing_file_returns_none(self): + # tmp watermark file was never created + self.assertIsNone(postgres_pillar_beacon._read_watermark()) + + def test_read_watermark_garbage_returns_none(self): + os.makedirs(os.path.dirname(self.watermark), exist_ok=True) + with open(self.watermark, 'w') as f: + f.write('nope') + self.assertIsNone(postgres_pillar_beacon._read_watermark()) + + # -- _write_watermark ------------------------------------------------- + + def test_write_watermark_round_trip(self): + postgres_pillar_beacon._write_watermark(7) + with open(self.watermark) as f: + self.assertEqual(f.read(), '7') + + def test_write_watermark_swallows_oserror(self): + with patch.object(postgres_pillar_beacon.os, 'makedirs', side_effect=OSError): + # Must not raise; failure is logged and the beacon retries next pass. + postgres_pillar_beacon._write_watermark(5) + self.assertFalse(os.path.exists(self.watermark)) + + # -- _query ----------------------------------------------------------- + + def test_query_success_returns_stdout_and_builds_argv(self): + completed = subprocess.CompletedProcess(args=[], returncode=0, stdout='rows', stderr='') + with patch.object(postgres_pillar_beacon.subprocess, 'run', return_value=completed) as mock_run: + result = postgres_pillar_beacon._query('SELECT 1;') + self.assertEqual(result, 'rows') + argv = mock_run.call_args[0][0] + self.assertEqual(argv[:5], ['docker', 'exec', 'so-postgres', 'psql', '-U']) + self.assertIn('SELECT 1;', argv) + self.assertFalse(mock_run.call_args[1].get('shell', False)) + + def test_query_timeout_returns_none(self): + with patch.object(postgres_pillar_beacon.subprocess, 'run', + side_effect=subprocess.TimeoutExpired(cmd='psql', timeout=30)): + self.assertIsNone(postgres_pillar_beacon._query('SELECT 1;')) + + def test_query_generic_exception_returns_none(self): + with patch.object(postgres_pillar_beacon.subprocess, 'run', side_effect=Exception('boom')): + self.assertIsNone(postgres_pillar_beacon._query('SELECT 1;')) + + def test_query_nonzero_returncode_returns_none(self): + completed = subprocess.CompletedProcess(args=[], returncode=1, stdout='', stderr='bad') + with patch.object(postgres_pillar_beacon.subprocess, 'run', return_value=completed): + self.assertIsNone(postgres_pillar_beacon._query('SELECT 1;')) + + # -- beacon: first run / seeding -------------------------------------- + + def test_beacon_seeds_when_postgres_not_ready(self): + with patch.object(postgres_pillar_beacon, '_read_watermark', return_value=None), \ + patch.object(postgres_pillar_beacon, '_query', return_value=None), \ + patch.object(postgres_pillar_beacon, '_write_watermark') as mock_write: + self.assertEqual(postgres_pillar_beacon.beacon({}), []) + mock_write.assert_not_called() + + def test_beacon_seeds_to_max_id_and_emits_nothing(self): + with patch.object(postgres_pillar_beacon, '_read_watermark', return_value=None), \ + patch.object(postgres_pillar_beacon, '_query', return_value='7\n'), \ + patch.object(postgres_pillar_beacon, '_write_watermark') as mock_write: + self.assertEqual(postgres_pillar_beacon.beacon({}), []) + mock_write.assert_called_once_with(7) + + def test_beacon_seed_unparseable_is_swallowed(self): + with patch.object(postgres_pillar_beacon, '_read_watermark', return_value=None), \ + patch.object(postgres_pillar_beacon, '_query', return_value='abc'), \ + patch.object(postgres_pillar_beacon, '_write_watermark') as mock_write: + self.assertEqual(postgres_pillar_beacon.beacon({}), []) + mock_write.assert_not_called() + + # -- beacon: steady state --------------------------------------------- + + def test_beacon_query_failure_returns_empty(self): + with patch.object(postgres_pillar_beacon, '_read_watermark', return_value=10), \ + patch.object(postgres_pillar_beacon, '_query', return_value=None), \ + patch.object(postgres_pillar_beacon, '_write_watermark') as mock_write: + self.assertEqual(postgres_pillar_beacon.beacon({}), []) + mock_write.assert_not_called() + + def test_beacon_emits_events_and_advances_watermark(self): + sep = postgres_pillar_beacon.FIELD_SEP + rows = '11%s5%snode1\n12%s6%s\n' % (sep, sep, sep, sep) + with patch.object(postgres_pillar_beacon, '_read_watermark', return_value=10), \ + patch.object(postgres_pillar_beacon, '_query', return_value=rows), \ + patch.object(postgres_pillar_beacon, '_write_watermark') as mock_write: + result = postgres_pillar_beacon.beacon({}) + self.assertEqual(result, [ + {'tag': 'audit_settings', 'id': 11, 'setting_id': '5', 'node_id': 'node1'}, + {'tag': 'audit_settings', 'id': 12, 'setting_id': '6', 'node_id': ''}, + ]) + mock_write.assert_called_once_with(12) + + def test_beacon_skips_malformed_blank_and_noninteger_rows(self): + sep = postgres_pillar_beacon.FIELD_SEP + rows = ( + '\n' # blank line -> skipped + '13%s7\n' # too few fields -> skipped + 'abc%s8%snodeX\n' # non-integer id -> skipped + '14%s9%snodeY\n' # the one good row + ) % (sep, sep, sep, sep, sep) + with patch.object(postgres_pillar_beacon, '_read_watermark', return_value=10), \ + patch.object(postgres_pillar_beacon, '_query', return_value=rows), \ + patch.object(postgres_pillar_beacon, '_write_watermark') as mock_write: + result = postgres_pillar_beacon.beacon({}) + self.assertEqual(result, [ + {'tag': 'audit_settings', 'id': 14, 'setting_id': '9', 'node_id': 'nodeY'}, + ]) + mock_write.assert_called_once_with(14) + + def test_beacon_no_new_rows_does_not_advance_watermark(self): + with patch.object(postgres_pillar_beacon, '_read_watermark', return_value=10), \ + patch.object(postgres_pillar_beacon, '_query', return_value=''), \ + patch.object(postgres_pillar_beacon, '_write_watermark') as mock_write: + self.assertEqual(postgres_pillar_beacon.beacon({}), []) + mock_write.assert_not_called() + + +if __name__ == '__main__': + unittest.main() diff --git a/salt/_beacons/rules_beacon_test.py b/salt/_beacons/rules_beacon_test.py new file mode 100644 index 000000000..adcd19181 --- /dev/null +++ b/salt/_beacons/rules_beacon_test.py @@ -0,0 +1,172 @@ +# Copyright Security Onion Solutions LLC and/or licensed to Security Onion Solutions LLC under one +# or more contributor license agreements. Licensed under the Elastic License 2.0 as shown at +# https://securityonion.net/license; you may not use this file except in compliance with the +# Elastic License 2.0. + +import hashlib +import os +import shutil +import tempfile +import unittest +from unittest.mock import patch + +import rules_beacon + + +class TestRulesBeacon(unittest.TestCase): + + def setUp(self): + # Isolate all on-disk state (watermarks and the dirs we fingerprint) in a + # throwaway tree, and point WATERMARK_DIR at it so the real read/write + # helpers run against actual files. + self.tmpdir = tempfile.mkdtemp() + self.state = os.path.join(self.tmpdir, 'state') + patcher = patch.object(rules_beacon, 'WATERMARK_DIR', self.state) + patcher.start() + self.addCleanup(patcher.stop) + + def tearDown(self): + shutil.rmtree(self.tmpdir, ignore_errors=True) + + def _make_dir(self, name, files=None): + path = os.path.join(self.tmpdir, name) + os.makedirs(path, exist_ok=True) + for fname, content in (files or {}).items(): + with open(os.path.join(path, fname), 'w') as f: + f.write(content) + return path + + # -- trivial contract ------------------------------------------------- + + def test_virtual_returns_true(self): + self.assertTrue(rules_beacon.__virtual__()) + + def test_validate_returns_valid(self): + self.assertEqual(rules_beacon.validate({}), (True, 'valid')) + + # -- _paths_from_config ----------------------------------------------- + + def test_paths_from_config_list_of_dicts(self): + config = [{'interval': 10}, {'paths': {'/a': 'suricata', '/b': 'strelka'}}] + self.assertEqual( + rules_beacon._paths_from_config(config), + {'/a': 'suricata', '/b': 'strelka'}, + ) + + def test_paths_from_config_plain_dict(self): + self.assertEqual( + rules_beacon._paths_from_config({'paths': {'/a': 'suricata'}}), + {'/a': 'suricata'}, + ) + + def test_paths_from_config_skips_non_dict_items(self): + self.assertEqual(rules_beacon._paths_from_config(['bogus', 42]), {}) + + def test_paths_from_config_paths_not_a_dict(self): + self.assertEqual(rules_beacon._paths_from_config({'paths': 'nope'}), {}) + + def test_paths_from_config_unexpected_type(self): + self.assertEqual(rules_beacon._paths_from_config('nonsense'), {}) + + # -- _excluded -------------------------------------------------------- + + def test_excluded_matches_temp_and_editor_files(self): + for pathname in ('/rules/foo.swp', '/rules/foo~', '/rules/4913', '/rules/.#foo'): + self.assertTrue(rules_beacon._excluded(pathname), pathname) + + def test_excluded_allows_real_rule_files(self): + self.assertFalse(rules_beacon._excluded('/rules/suricata.rules')) + + # -- _fingerprint ----------------------------------------------------- + + def test_fingerprint_missing_dir_is_empty_tree_digest(self): + missing = os.path.join(self.tmpdir, 'does-not-exist') + self.assertEqual(rules_beacon._fingerprint(missing), hashlib.sha1().hexdigest()) + + def test_fingerprint_changes_when_content_changes(self): + d = self._make_dir('rules', {'a.rules': 'alert'}) + before = rules_beacon._fingerprint(d) + with open(os.path.join(d, 'a.rules'), 'w') as f: + f.write('alert tcp any any -> any any') # different size + self.assertNotEqual(rules_beacon._fingerprint(d), before) + + def test_fingerprint_ignores_excluded_files(self): + d = self._make_dir('rules', {'a.rules': 'alert'}) + before = rules_beacon._fingerprint(d) + with open(os.path.join(d, 'a.rules.swp'), 'w') as f: + f.write('editor swap') + self.assertEqual(rules_beacon._fingerprint(d), before) + + def test_fingerprint_skips_unstatable_entries(self): + # A dangling symlink appears in os.walk's file list but os.stat raises + # OSError, exercising the except-continue path. + d = self._make_dir('rules', {'a.rules': 'alert'}) + good = rules_beacon._fingerprint(d) + os.symlink(os.path.join(d, 'missing-target'), os.path.join(d, 'broken.link')) + self.assertEqual(rules_beacon._fingerprint(d), good) + + # -- _read_watermark / _write_watermark ------------------------------- + + def test_watermark_round_trip(self): + rules_beacon._write_watermark('suricata', 'deadbeef') + self.assertEqual(rules_beacon._read_watermark('suricata'), 'deadbeef') + + def test_read_watermark_missing_returns_none(self): + self.assertIsNone(rules_beacon._read_watermark('suricata')) + + def test_read_watermark_empty_file_returns_none(self): + os.makedirs(self.state, exist_ok=True) + with open(rules_beacon._watermark_file('suricata'), 'w') as f: + f.write('') + self.assertIsNone(rules_beacon._read_watermark('suricata')) + + def test_write_watermark_swallows_oserror(self): + with patch.object(rules_beacon.os, 'makedirs', side_effect=OSError): + rules_beacon._write_watermark('suricata', 'deadbeef') + self.assertIsNone(rules_beacon._read_watermark('suricata')) + + # -- beacon ----------------------------------------------------------- + + def _config(self, mapping): + return [{'paths': mapping}] + + def test_beacon_seeds_first_run_and_emits_nothing(self): + with patch.object(rules_beacon, '_fingerprint', return_value='hash1'), \ + patch.object(rules_beacon, '_read_watermark', return_value=None), \ + patch.object(rules_beacon, '_write_watermark') as mock_write: + result = rules_beacon.beacon(self._config({'/rules/suricata': 'suricata'})) + self.assertEqual(result, []) + mock_write.assert_called_once_with('suricata', 'hash1') + + def test_beacon_emits_on_change(self): + with patch.object(rules_beacon, '_fingerprint', return_value='newhash'), \ + patch.object(rules_beacon, '_read_watermark', return_value='oldhash'), \ + patch.object(rules_beacon, '_write_watermark') as mock_write: + result = rules_beacon.beacon(self._config({'/rules/suricata': 'suricata'})) + self.assertEqual(result, [{'tag': 'suricata', 'path': '/rules/suricata'}]) + mock_write.assert_called_once_with('suricata', 'newhash') + + def test_beacon_no_change_emits_nothing(self): + with patch.object(rules_beacon, '_fingerprint', return_value='samehash'), \ + patch.object(rules_beacon, '_read_watermark', return_value='samehash'), \ + patch.object(rules_beacon, '_write_watermark') as mock_write: + result = rules_beacon.beacon(self._config({'/rules/suricata': 'suricata'})) + self.assertEqual(result, []) + mock_write.assert_not_called() + + def test_beacon_end_to_end_with_real_files(self): + # Exercise the full stack (real fingerprint + real watermark files) across + # two poll passes: first seeds silently, second fires after a write. + d = self._make_dir('rules', {'a.rules': 'alert'}) + config = self._config({d: 'suricata'}) + + self.assertEqual(rules_beacon.beacon(config), []) # seed pass + self.assertEqual(rules_beacon.beacon(config), []) # unchanged pass + + with open(os.path.join(d, 'b.rules'), 'w') as f: + f.write('alert tcp any any -> any any') + self.assertEqual(rules_beacon.beacon(config), [{'tag': 'suricata', 'path': d}]) + + +if __name__ == '__main__': + unittest.main() diff --git a/salt/_beacons/zeek.py b/salt/_beacons/zeek.py index 117c2b401..6c4d01b11 100644 --- a/salt/_beacons/zeek.py +++ b/salt/_beacons/zeek.py @@ -3,31 +3,30 @@ import logging def status(): - cmd = "runuser -l zeek -c '/opt/zeek/bin/zeekctl status'" - retval = __salt__['docker.run']('so-zeek', cmd) - logging.info('zeekctl_module: zeekctl.status retval: %s' % retval) + cmd = "runuser -l zeek -c '/opt/zeek/bin/zeekctl status'" + retval = __salt__['docker.run']('so-zeek', cmd) # noqa: F821 + logging.info('zeekctl_module: zeekctl.status retval: %s' % retval) - return retval + return retval def beacon(config): - retval = [] + retval = [] - is_enabled = __salt__['healthcheck.is_enabled']() - logging.info('zeek_beacon: healthcheck_is_enabled: %s' % is_enabled) + is_enabled = __salt__['healthcheck.is_enabled']() # noqa: F821 + logging.info('zeek_beacon: healthcheck_is_enabled: %s' % is_enabled) - if is_enabled: - zeekstatus = status().lower().split(' ') - logging.info('zeek_beacon: zeekctl.status: %s' % str(zeekstatus)) - if 'stopped' in zeekstatus or 'crashed' in zeekstatus or 'error' in zeekstatus or 'error:' in zeekstatus: - zeek_restart = True - else: - zeek_restart = False + if is_enabled: + zeekstatus = status().lower().split(' ') + logging.info('zeek_beacon: zeekctl.status: %s' % str(zeekstatus)) + if 'stopped' in zeekstatus or 'crashed' in zeekstatus or 'error' in zeekstatus or 'error:' in zeekstatus: + zeek_restart = True + else: + zeek_restart = False - __salt__['telegraf.send']('healthcheck zeek_restart=%s' % str(zeek_restart)) - retval.append({'zeek_restart': zeek_restart}) - logging.info('zeek_beacon: retval: %s' % str(retval)) - - return retval + __salt__['telegraf.send']('healthcheck zeek_restart=%s' % str(zeek_restart)) # noqa: F821 + retval.append({'zeek_restart': zeek_restart}) + logging.info('zeek_beacon: retval: %s' % str(retval)) + return retval diff --git a/salt/_beacons/zeek_test.py b/salt/_beacons/zeek_test.py new file mode 100644 index 000000000..db3b4236f --- /dev/null +++ b/salt/_beacons/zeek_test.py @@ -0,0 +1,59 @@ +# Copyright Security Onion Solutions LLC and/or licensed to Security Onion Solutions LLC under one +# or more contributor license agreements. Licensed under the Elastic License 2.0 as shown at +# https://securityonion.net/license; you may not use this file except in compliance with the +# Elastic License 2.0. + +import unittest +from unittest.mock import MagicMock + +import zeek + +ZEEKCTL_CMD = "runuser -l zeek -c '/opt/zeek/bin/zeekctl status'" + + +class TestZeekBeacon(unittest.TestCase): + + def setUp(self): + # zeek.py relies on the __salt__ dunder that Salt injects at load time. + # Nothing defines it under test, so we attach a dict of mock loader + # functions to the module and remove it again afterwards. + self.salt = { + 'docker.run': MagicMock(return_value='Zeek is running'), + 'healthcheck.is_enabled': MagicMock(return_value=True), + 'telegraf.send': MagicMock(), + } + zeek.__salt__ = self.salt + self.addCleanup(lambda: delattr(zeek, '__salt__')) + + # -- status ----------------------------------------------------------- + + def test_status_runs_zeekctl_and_returns_output(self): + self.salt['docker.run'].return_value = 'Zeek is running' + result = zeek.status() + self.assertEqual(result, 'Zeek is running') + self.salt['docker.run'].assert_called_once_with('so-zeek', ZEEKCTL_CMD) + + # -- beacon ----------------------------------------------------------- + + def test_beacon_disabled_returns_empty_and_skips_telegraf(self): + self.salt['healthcheck.is_enabled'].return_value = False + self.assertEqual(zeek.beacon({}), []) + self.salt['telegraf.send'].assert_not_called() + + def test_beacon_running_reports_no_restart(self): + self.salt['docker.run'].return_value = 'Zeek is running' + self.assertEqual(zeek.beacon({}), [{'zeek_restart': False}]) + self.salt['telegraf.send'].assert_called_once_with('healthcheck zeek_restart=False') + + def test_beacon_unhealthy_status_triggers_restart(self): + # Each of these status tokens should flag a restart (the or-chain in beacon). + for status_text in ('Zeek is stopped', 'Zeek crashed', 'Zeek error state', 'Zeek error:'): + with self.subTest(status=status_text): + self.salt['docker.run'].return_value = status_text + self.salt['telegraf.send'].reset_mock() + self.assertEqual(zeek.beacon({}), [{'zeek_restart': True}]) + self.salt['telegraf.send'].assert_called_once_with('healthcheck zeek_restart=True') + + +if __name__ == '__main__': + unittest.main() From 141116f5500b21d900a7db3ffcb30249e9adccd1 Mon Sep 17 00:00:00 2001 From: Josh Patterson Date: Thu, 16 Jul 2026 16:52:06 -0400 Subject: [PATCH 10/11] Make so-salt-minion-wait work without requiring a restart The wait required both a socket gate and a log gate to pass. The log gate greps the minion log for salt's one-time startup line "Minion is ready to receive requests!", which scrolls out of the log tail on a minion that has not restarted recently. On such a minion the log gate could never pass, so the script ran to its full 120s timeout and exited 1 even though the minion was healthy and connected. This also false-timed-out when salt_minion_service reported a non-restart change (e.g. an enable toggle). The log gate's only remaining purpose is closing the ~2.8s post-connect window where the master sockets are up but _post_master_init() is still loading. Gate it on the current pid's uptime: enforce the ready line only within READY_LINE_WINDOW (90s) of (re)start, and let the already restart-independent socket gate be the steady-state authority past that. The fresh-restart path is unchanged, and if uptime can't be read the strict behavior is kept. --- salt/salt/minion/init.sls | 17 ++++--- salt/salt/tools/sbin/so-salt-minion-wait | 63 ++++++++++++++++++------ 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/salt/salt/minion/init.sls b/salt/salt/minion/init.sls index 1f1ec8305..fa94ec7be 100644 --- a/salt/salt/minion/init.sls +++ b/salt/salt/minion/init.sls @@ -131,13 +131,16 @@ salt_minion_service: {% endif %} - order: last -# block until the just-restarted salt-minion daemon logs "Minion is ready to receive requests!" -# for the current instance, so follow-on jobs and the next highstate iteration do not race the -# restart. onchanges + require on salt_minion_service catches every restart trigger uniformly -# because watch mod_watch results replace the service state's running entry. wait logic lives in -# /usr/sbin/so-salt-minion-wait (deployed by salt_sbin from salt/tools/sbin/); it keys the ready -# line to the current daemon pid (resolved via systemd, not the pidfile) and corroborates with the -# master req/publish sockets. set_log_levels above enforces the log_level_logfile: info that the +# block until the salt-minion daemon is ready for the current instance, so follow-on jobs and the +# next highstate iteration do not race the restart. onchanges + require on salt_minion_service +# catches every restart trigger uniformly because watch mod_watch results replace the service +# state's running entry. wait logic lives in /usr/sbin/so-salt-minion-wait (deployed by salt_sbin +# from salt/tools/sbin/); its steady-state authority is the master req/publish sockets for the +# current daemon pid (resolved via systemd, not the pidfile), and it corroborates a just-restarted +# instance with the pid-tagged "Minion is ready to receive requests!" log line only within a short +# window of startup. Because that socket signal does not require a recent restart, the wait also +# succeeds cleanly when salt_minion_service reports a non-restart change (e.g. an enable toggle) +# rather than false-timing-out. set_log_levels above enforces the log_level_logfile: info that the # ready line depends on. salt restarts this unit with --no-block, so mod_watch returns while the old # daemon is still up; the script waits for systemd's restart job to drain before it reads MainPID. wait_for_salt_minion_ready: diff --git a/salt/salt/tools/sbin/so-salt-minion-wait b/salt/salt/tools/sbin/so-salt-minion-wait index 984f144f3..cf16ab0d1 100644 --- a/salt/salt/tools/sbin/so-salt-minion-wait +++ b/salt/salt/tools/sbin/so-salt-minion-wait @@ -5,21 +5,29 @@ # https://securityonion.net/license; you may not use this file except in compliance with the # Elastic License 2.0. -# Block until the just-restarted salt-minion daemon reaches the point where salt itself logs -# "Minion is ready to receive requests!". Invoked from the wait_for_salt_minion_ready state in -# salt/minion/init.sls after salt_minion_service fires its watch-driven restart, so follow-on jobs -# and the next highstate iteration do not race it. +# Block until the current salt-minion daemon is ready to receive requests. Invoked from the +# wait_for_salt_minion_ready state in salt/minion/init.sls after salt_minion_service fires its +# watch-driven restart, so follow-on jobs and the next highstate iteration do not race it. It is +# also correct on an already-running minion (no recent restart): the steady-state readiness signal +# is the live master sockets, so it does not depend on a restart having just happened. # -# Salt logs that line from Minion.tune_in() only after sync_connect_master() returns, which means -# the pub channel authenticated, the long-running req channel connected, and _post_master_init() -# finished loading modules and compiling pillar. Two signals reproduce that: +# Salt logs "Minion is ready to receive requests!" from Minion.tune_in() only after +# sync_connect_master() returns, which means the pub channel authenticated, the long-running req +# channel connected, and _post_master_init() finished loading modules and compiling pillar. Two +# signals reproduce that: # -# 1. Primary the pid-tagged ready line in the minion log. Salt's log_fmt_logfile embeds +# 1. Steady state the pid holds an ESTABLISHED req connection to a master on 4506 plus a second +# (publish) connection to that same master IP on another port. The publish port is +# learned from the master's auth reply and is absent from minion config, so it is +# derived from the connection rather than read from config. This is the always-on +# authority: it reflects whatever daemon is running now, restart or not. +# 2. Startup only the pid-tagged ready line in the minion log. Salt's log_fmt_logfile embeds # [%(process)d] just before the message, so this is keyed to one daemon instance. -# 2. Corroborating that same pid holds an ESTABLISHED req connection to a master on 4506 plus a -# second (publish) connection to that same master IP on another port. The publish -# port is learned from the master's auth reply and is absent from minion config, -# so it is derived from the connection rather than read from config. +# Salt logs it exactly once per start, so it exists only to close a ~2.8s window +# after the sockets come up where they are established but _post_master_init() is +# still finishing. It is therefore required only within READY_LINE_WINDOW seconds +# of (re)start (by pid uptime); past that the line has scrolled out of the log and +# the socket gate alone decides. See instance_ready(). # # The daemon pid is resolved from systemd, never from /var/run/salt-minion.pid. salt_minion() runs # the real minion in a multiprocessing child; that child writes the pidfile, owns the sockets and @@ -43,6 +51,12 @@ INITIAL_SLEEP=3 TIMEOUT=120 MASTER_PORT=4506 LOG_TAIL_LINES=10000 +# Seconds after a (re)start during which the pid-tagged ready line is still required. Past this the +# daemon is clearly beyond the ~2.8s post-connect race and the socket gate is authoritative -- the +# one-time ready line has scrolled out of the log tail on a long-running minion. Kept under TIMEOUT +# so a fresh minion that connects but never logs the line still falls back to socket-only near the +# end instead of false-timing-out. +READY_LINE_WINDOW=90 DEFAULT_LOG_FILE="/opt/so/log/salt/minion" LOG_FILE="$DEFAULT_LOG_FILE" @@ -103,6 +117,15 @@ resolve_daemon_pids() { printf '%s\n' "${children:-$mainpid}" } +# Elapsed seconds since this pid started (Linux procps etimes). Empty/non-numeric -> failure, so the +# caller can fall back to the strict (log-gate-enforced) behavior when uptime cannot be read. +pid_uptime() { + local pid=$1 secs + secs=$(ps -o etimes= -p "$pid" 2>/dev/null | tr -d ' ') + case "$secs" in ''|*[!0-9]*) return 1 ;; esac + printf '%s\n' "$secs" +} + # True iff the ready line tagged with this pid is in the current or most recently rotated log. ready_logged() { local pid=$1 f @@ -135,9 +158,19 @@ socket_ready() { } instance_ready() { - local pid=$1 - if [ "$USE_LOG_GATE" -eq 1 ] && ! ready_logged "$pid"; then - return 1 + local pid=$1 uptime + # The log gate only closes the ~2.8s window right after the master sockets come up where they are + # established but _post_master_init() is still loading modules/compiling pillar. Salt logs the + # pid-tagged ready line exactly once at startup, so on a daemon that started long ago the line has + # scrolled out of the log tail and the gate could never pass -- making the wait require a recent + # restart. Enforce it only while the daemon is young enough that the race could still be open; past + # READY_LINE_WINDOW the socket gate is authoritative. If uptime can't be read, keep the strict + # behavior (uptime=0 -> gate enforced) so the fresh-restart path never regresses. + if [ "$USE_LOG_GATE" -eq 1 ]; then + uptime=$(pid_uptime "$pid") || uptime=0 + if [ "$uptime" -lt "$READY_LINE_WINDOW" ] && ! ready_logged "$pid"; then + return 1 + fi fi if [ "$USE_SOCKET_GATE" -eq 1 ] && ! socket_ready "$pid"; then return 1 From 8095b828411d230bf964b336ca352c7d892ec8a9 Mon Sep 17 00:00:00 2001 From: Josh Patterson Date: Thu, 16 Jul 2026 16:58:21 -0400 Subject: [PATCH 11/11] soup use so-salt-minion-wait to ensure salt-minion is ready --- salt/common/tools/sbin/so-common | 36 -------------------------------- salt/manager/tools/sbin/soup | 13 ++++-------- 2 files changed, 4 insertions(+), 45 deletions(-) diff --git a/salt/common/tools/sbin/so-common b/salt/common/tools/sbin/so-common index 4e6580ae1..b5d1ab286 100755 --- a/salt/common/tools/sbin/so-common +++ b/salt/common/tools/sbin/so-common @@ -602,42 +602,6 @@ run_check_net_err() { fi } -wait_for_salt_minion() { - local minion="$1" - local max_wait="${2:-30}" - local interval="${3:-2}" - local logfile="${4:-'/dev/stdout'}" - local elapsed=0 - - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - Waiting for salt-minion '$minion' to be ready..." - - while [ $elapsed -lt $max_wait ]; do - # Check if service is running - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - Check if salt-minion service is running" - if ! systemctl is-active --quiet salt-minion; then - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - salt-minion service not running (elapsed: ${elapsed}s)" - sleep $interval - elapsed=$((elapsed + interval)) - continue - fi - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - salt-minion service is running" - - # Check if minion responds to ping - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - Check if $minion responds to ping" - if salt "$minion" test.ping --timeout=3 --out=json 2>> "$logfile" | grep -q "true"; then - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - salt-minion '$minion' is connected and ready!" - return 0 - fi - - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - Waiting... (${elapsed}s / ${max_wait}s)" - sleep $interval - elapsed=$((elapsed + interval)) - done - - echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - ERROR: salt-minion '$minion' not ready after $max_wait seconds" - return 1 -} - salt_minion_count() { local MINIONDIR="/opt/so/saltstack/local/pillar/minions" MINIONCOUNT=$(ls -la $MINIONDIR/*.sls | grep -v adv_ | wc -l) diff --git a/salt/manager/tools/sbin/soup b/salt/manager/tools/sbin/soup index 52f310e81..751621647 100755 --- a/salt/manager/tools/sbin/soup +++ b/salt/manager/tools/sbin/soup @@ -1563,18 +1563,13 @@ verify_es_version_compatibility() { } wait_for_salt_minion_with_restart() { - local minion="$1" - local max_wait="${2:-60}" - local interval="${3:-3}" - local logfile="$4" - - wait_for_salt_minion "$minion" "$max_wait" "$interval" "$logfile" + /usr/sbin/so-salt-minion-wait local result=$? if [[ $result -ne 0 ]]; then echo "$(date '+%a %d %b %Y %H:%M:%S.%6N') - salt-minion not ready, attempting restart..." systemctl_func "restart" "salt-minion" - wait_for_salt_minion "$minion" "$max_wait" "$interval" "$logfile" + /usr/sbin/so-salt-minion-wait result=$? fi @@ -2030,7 +2025,7 @@ main() { echo "" echo "Running a highstate. This could take several minutes." set +e - wait_for_salt_minion_with_restart "$MINIONID" "60" "3" "$SOUP_LOG" || fail "Salt minion was not running or ready." + wait_for_salt_minion_with_restart || fail "Salt minion was not running or ready." highstate set -e @@ -2043,7 +2038,7 @@ main() { check_saltmaster_status echo "Running a highstate to complete the Security Onion upgrade on this manager. This could take several minutes." - wait_for_salt_minion_with_restart "$MINIONID" "60" "3" "$SOUP_LOG" || fail "Salt minion was not running or ready." + wait_for_salt_minion_with_restart || fail "Salt minion was not running or ready." # Stop long-running scripts to allow potentially updated scripts to load on the next execution. if pgrep salt-relay.sh > /dev/null 2>&1; then