Skip to content

Write alpine service scripts directly #2117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
heewa wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Write alpine service scripts directly #2117

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions bootstrap-salt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5492,15 +5492,22 @@ install_alpine_linux_post() {
[ $fname = "syndic" ] && [ "$_INSTALL_SYNDIC" -eq $BS_FALSE ] && continue

if [ -f /sbin/rc-update ]; then
script_url="${_SALTSTACK_REPO_URL%.git}/raw/master/pkg/alpine/salt-$fname"
[ -f "/etc/init.d/salt-$fname" ] || __fetch_url "/etc/init.d/salt-$fname" "$script_url"
local script_path="/etc/init.d/salt-$fname"

@twangboy twangboy Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While script_path is correctly defined as a local variable, fname is used throughout this block without being explicitly declared local at the top of the install_alpine_linux_post() function. If it isn't already, we should ensure fname is scoped locally to avoid leaking or mutating global state.

if ! [ -f "$script_path" ]; then
cat <<_eof > "$script_path"

@twangboy twangboy Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the heredoc delimiter _eof is unquoted, the current shell will evaluate and expand all the variables at the time the bootstrap script runs, instead of writing the literal variable strings into the target file.

For example, line 5499 (command="/usr/bin/salt-${fname}") will literally write command="/usr/bin/salt-minion" into the init file. While that works fine for execution, lines like 5500 (command_args="--daemon") are okay, but lines 5501 and 5502 will expand things prematurely based on the bootstrap execution environment rather than leaving them dynamic for OpenRC.

To pass the text completely literally and allow OpenRC to handle its own variable definitions cleanly, we should quote the delimiter.

Suggested Change:

cat << '_eof' > "$script_path"

#!/sbin/openrc-run
command="/usr/bin/salt-${fname}"
command_args="--daemon"
pidfile="/var/run/salt-${fname}.pid"
name="Salt ${fname} daemon"

# shellcheck disable=SC2181
if [ $? -eq 0 ]; then
chmod +x "/etc/init.d/salt-$fname"
else
echoerror "Failed to get OpenRC init script for $OS_NAME from $script_url."
return 1
depend() {
need localmount
use net
after bootmisc
}

@twangboy twangboy Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the heredoc delimiter is quoted, these lines will write nicely to the file. However, standard ShellCheck guidelines for OpenRC scripts recommend ensuring variables inside the definitions are safe. It looks clean, but just double-check that the indentation within the depend() block (lines 5504-5508) matches the standard 4-space tab or 2-space layout preferred by Alpine's upstream openrc styles.

_eof
chmod +x "$script_path"
fi

# Skip salt-api since the service should be opt-in and not necessarily started on boot
Expand Down
Loading