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 <dfdpublic@wildtechgarden.ca>

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
 }

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
 	}

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

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
 }
 

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=""
+	local runas ret
 
-	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
-	}
+	# 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=$?
 
-	# shellcheck disable=2034
-	RUNAS="$nut_value_via_foreach"
-	return 0
+		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