From: Daniel F. Dickinson Date: Mon, 6 Jul 2026 22:23:52 +0000 (-0400) Subject: nut: simplify/improve consistency for RUNAS and STATEPATH X-Git-Url: http://git.99rst.org/?a=commitdiff_plain;h=0ebfb5b3e00c413ad001198d7d2f01675757f7ff;p=openwrt-packages.git nut: simplify/improve consistency for RUNAS and STATEPATH We claimed we only supported a single upsd section named 'upsd' and a single uspmon section named 'upsmon', but when determining RUNAS and STATEPATH we were using overly-complex functions as we were attempting to support multiple upsd and upsmon sections. Simplify the logic by adhering to our stated support. While were are at it we make RUNAS handling more consistent between nut-server (upsd) and nut-monitor (upsmon). We no longer try to limp along if RUNAS is not set by find_runas; to do this properly would require over-complicated logic when setting file and directory permissions and ownership. Signed-off-by: Daniel F. Dickinson --- diff --git a/net/nut/files/nut-common.sh.functions b/net/nut/files/nut-common.sh.functions index a10ff1174..c077c27f5 100644 --- a/net/nut/files/nut-common.sh.functions +++ b/net/nut/files/nut-common.sh.functions @@ -175,19 +175,32 @@ log_error() { log_msg "$reason" "$current_script" "$syslog_id" "error" } -note_section_of_type() { - have_expected_section="true" +note_section_match() { + local section="$1" + local wanted="$2" + + if [ -n "$section" ] && [ "$section" = "$wanted" ]; then + have_expected_section="true" + fi } -have_section_of_type() { +have_section_named() { local section_type="$1" + local section_name="$2" + local ret # 'pseudo-global' to capture result from config_foreach local have_expected_section="false" - config_foreach note_section_of_type "$section_type" + config_foreach note_section_match "$section_type" "$section_name" + ret=$? if [ "$have_expected_section" = "true" ]; then return 0 fi - return 1 + + if [ "$ret" = "0" ]; then + return 1 + else + return $ret + fi } diff --git a/net/nut/files/nut-monitor.init b/net/nut/files/nut-monitor.init index 6de2ba18a..7fa9dccbf 100644 --- a/net/nut/files/nut-monitor.init +++ b/net/nut/files/nut-monitor.init @@ -80,11 +80,11 @@ service_preconditions() { exit 1 } - if have_section_of_type "upsmon"; then + if have_section_named "upsmon" "upsmon"; then # Find the RUNAS user for upsmon - if ! find_runas "upsmon" "nut_monitor" "nutmon" || [ -z "$RUNAS" ]; then + RUNAS="$(find_runas "upsmon" "nut_monitor" "nutmon")" || { log_error_exit "Failed to determine RUNAS user" "nut-monitor" "nut-monitor" - fi + } else # If we do not have a 'upsmon' section in nut_monitor, use defaults log_msg "No upsmon section so no RUNAS user, falling back to nutmon" "nut-monitor" "nut-monitor" "notice" @@ -104,18 +104,16 @@ service_preconditions() { start_monitor_instance() { procd_open_instance upsmon procd_set_param respawn - procd_set_param stderr 0 # stderr is just a dup of stdout + syslog + procd_set_param stderr 0 # stderr is just a dup of stdout + logger (with -s) procd_set_param stdout 1 procd_set_param env NUT_QUIET_INIT_UPSNOTIFY=true procd_set_param reload_signal HUP procd_set_param command /usr/sbin/upsmon procd_append_param command -FF - if [ -n "$RUNAS" ]; then - # upsmon needs root for some actions (like - # shutting down the system, but forks - # and drops root for most of its operation) - procd_append_param command -u "$RUNAS" - fi + # upsmon needs root for some actions (like + # shutting down the system), but forks + # and drops root for most of its operation + procd_append_param command -u "$RUNAS" procd_close_instance } @@ -211,8 +209,8 @@ service_triggers() { config_get interface_reload_delay upsmon interface_reload_delay $DEFAULT_PROCD_INTERFACE_RELOAD_DELAY - # We do not pass an instance name as the nut-monitor initscript does not need - # an additional instance argument + # We do not pass a variadic instance name as the nut-monitor initscript does + # not need an additional instance name argument, only the initscript name interface_triggers "add_trigger" "upsmon" "$interface_reload_delay" "nut-monitor" || { log_error_exit "Failed to add interface triggers" nut-monitor nut-monitor } diff --git a/net/nut/files/nut-server-config.sh.functions b/net/nut/files/nut-server-config.sh.functions index 36693a55b..f5533b15b 100644 --- a/net/nut/files/nut-server-config.sh.functions +++ b/net/nut/files/nut-server-config.sh.functions @@ -140,8 +140,8 @@ build_server_config() { # are named the same config_foreach nut_user_add user "$USERS_C.new" config_foreach listen_address listen_address "$UPSD_C.new" - if have_section_of_type "upsd"; then - config_foreach srv_config upsd "$UPSD_C.new" + if have_section_named "upsd" "upsd"; then + srv_config upsd "$UPSD_C.new" else # If config 'nut_server' does not have a 'upsd' section, use a default # configuration diff --git a/net/nut/files/nut-server.init b/net/nut/files/nut-server.init index 1a593f7e5..e317e46a3 100644 --- a/net/nut/files/nut-server.init +++ b/net/nut/files/nut-server.init @@ -101,15 +101,13 @@ start_ups_driver() { procd_open_instance "$ups" procd_set_param respawn - procd_set_param stderr 0 # stderr is just a dup of stdout + syslog + procd_set_param stderr 0 # stderr is just a dup of stdout + logger (with -s) procd_set_param stdout 1 procd_set_param env NUT_QUIET_INIT_UPSNOTIFY=true procd_set_param env NUT_STATEPATH="${STATEPATH}" procd_set_param command "/usr/libexec/nut/${driver}" procd_append_param command -FF -a "$ups" - if [ -n "$RUNAS" ]; then - procd_append_param command -u "$RUNAS" - fi + procd_append_param command -u "$RUNAS" procd_close_instance # If upsd is running, reload it to pick up new driver @@ -142,16 +140,13 @@ common_preconditions() { exit 1 } - if have_section_of_type "upsd"; then - # Only find statepath and runas once per service start or reload - # defines STATEPATH - find_statepath "upsd" "nut_server" || { + # Only find statepath and runas once per service start or reload + if have_section_named "upsd" "upsd"; then + STATEPATH="$(find_statepath "upsd" "nut_server")" || { log_error_exit "Failed to determine STATEPATH" "nut-server" "nut-server" } - # sets RUNAS - find_runas "upsd" "nut_server" || { - log_error "Failed to determine RUNAS" "nut-server" "nut-server" - return 1 + RUNAS="$(find_runas "upsd" "nut_server")" || { + log_error_exit "Failed to determine RUNAS" "nut-server" "nut-server" } else # If there is no 'upsd' section in the nut_server config file, use @@ -317,15 +312,13 @@ start_server_instance() { procd_open_instance upsd procd_set_param respawn - procd_set_param stderr 0 # stderr is just a dup of stdout + syslog + procd_set_param stderr 0 # stderr is just a dup of stdout + logger (with -s) procd_set_param stdout 1 procd_set_param env NUT_QUIET_INIT_UPSNOTIFY=true procd_set_param env NUT_STATEPATH="$STATEPATH" procd_set_param command /usr/sbin/upsd procd_append_param command -FF - if [ -n "$RUNAS" ]; then - procd_append_param command -u "$RUNAS" - fi + procd_append_param command -u "$RUNAS" procd_close_instance } diff --git a/net/nut/files/nut-service.sh.functions b/net/nut/files/nut-service.sh.functions index 7ca0da4f1..b750c3bcf 100644 --- a/net/nut/files/nut-service.sh.functions +++ b/net/nut/files/nut-service.sh.functions @@ -27,7 +27,9 @@ NUT_BASE_STATEPATH="/var/run/nut" # STATEPATH and RUNAS values are not valid until after find_statepath # and/or find_runas +# shellcheck disable=SC2034 STATEPATH="" +# shellcheck disable=SC2034 RUNAS="" # Default delay between interface change and interface trigger reload action @@ -147,96 +149,66 @@ signal_instance() { return 0 } -config_foreach_get() { - local section="$1" - local option="$2" - local value - - # nut_found_value is a 'pseudo-global' variable so that the value can be - # passed back to the calling function (that is the variable is defined in - # that function's context, and modification in functions called from that - # function will update the variable there). - config_get value "$section" "$option" - if [ -n "$value" ]; then - nut_found_value="$value" - nut_found_value_count=$((nut_found_value_count + 1)) - fi - return 0 -} - -find_foreach_value() { - local section_type="$1" - local package="$2" - local option="$3" - local default="$4" - local nut_found_value="" - local nut_found_value_count=0 - - # 'pseudo-global' variable so that the value can be passed back to this function (that is the - # variable is defined in this function's context, and modification in functions called from - # this function will update variable here). - nut_value_via_foreach="" - - config_foreach config_foreach_get "$section_type" "$option" || { - log_error "config_foreach for config_foreach_get for '$package'.'$section_type' failed." nut-service.sh nut-service - # nut_found_value and nut_found_value_count are indeterminate on config_foreach failure - nut_found_value="" - nut_found_value_count=0 - return 1 - } - - if [ "$nut_found_value_count" -gt 1 ]; then - # Informational log messages, not error - logger -t nut-common "Found more than one '$option' setting in '$package' for '$section_type'." - logger -t nut-common "Using last found '$option': nut_found_value='$nut_found_value'" - fi - - if [ -z "$nut_found_value" ]; then - nut_found_value="$default" - fi - - nut_value_via_foreach="$nut_found_value" - return 0 -} - # Store path for NUT working data in the global variable STATEPATH # shellcheck disable=SC2329 find_statepath() { - local section_type="$1" + local section="$1" local package="$2" - # Callers should not need to see the variable below outside this function - # Conversely, functions called by this function are able to see and set - # the variable below, which acts as a 'pseudo-global' to called functions. - local nut_value_via_foreach="" + local ret statepath - find_foreach_value "$section_type" "$package" statepath "$NUT_BASE_STATEPATH" || return 1 + # Subshell so as not to interfere with main config_load + ( + config_load "$package" || { + log_config_load_error "$package" "nut-service.sh" "nut-service" + exit 1 + } + config_get statepath "$section" statepath "$NUT_BASE_STATEPATH" + ret=$? - # shellcheck disable=2034 - STATEPATH="$nut_value_via_foreach" - return 0 + if [ -n "$statepath" ]; then + printf "%s" "$statepath" + elif [ "$ret" != "0" ]; then + exit $ret + else + exit 1 + fi + ) || return $? } # Store user under which to run daemon processes # shellcheck disable=SC2329 find_runas() { - local section_type="$1" + local section="$1" local package="$2" local fallback_runas="$3" - # Callers should not need to see the variable below outside this function - # Conversely, functions called by this function are able to see and set - # the variable below, which acts as a 'pseudo-global' to called functions. - local nut_value_via_foreach="" - - find_foreach_value "$section_type" "$package" runas "${fallback_runas:-nut}" || return 1 - [ -n "$nut_value_via_foreach" ] || return 1 - [ -n "$(id -un "$nut_value_via_foreach")" ] || { - log_error "User '$nut_value_via_foreach' specified for RUNAS does not exist." nut-service.sh nut-service - return 1 - } + local runas ret - # shellcheck disable=2034 - RUNAS="$nut_value_via_foreach" - return 0 + # Subshell so as not to interfere with main config_load + ( + config_load "$package" || { + log_config_load_error "$package" "nut-service.sh" "nut-service" + exit 1 + } + config_get runas "$section" runas "${fallback_runas:-nut}" + ret=$? + + if [ -z "$runas" ]; then + if [ "$ret" != "0" ]; then + exit $ret + else + exit 1 + fi + fi + + if [ -n "$(id -un "$runas")" ]; then + printf "%s" "$runas" + else + # We omit the output on stderr to avoid potential mixing with the intended printf output on stdout, + # when called via command substitution, while maintaining log output to syslog + log_error "User '$runas' specified for 'runas' does not exist." nut-service.sh nut-service 2>/dev/null + exit 1 + fi + ) || return $? } # Detect if service is running under procd, with no instances