Write alpine service scripts directly#2117
Conversation
|
Please rebase and address the lint failures |
| [ -f "/etc/init.d/salt-$fname" ] || __fetch_url "/etc/init.d/salt-$fname" "$script_url" | ||
| local script_path="/etc/init.d/salt-$fname" | ||
| if ! [ -f "$script_path" ]; then | ||
| cat <<_eof > "$script_path" |
There was a problem hiding this comment.
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"| 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" |
There was a problem hiding this comment.
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.
| need localmount | ||
| use net | ||
| after bootmisc | ||
| } |
There was a problem hiding this comment.
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.
2 participants
What does this PR do?
Writes alpine service files for salt daemons directly, rather than trying to download them from the salt repo (which deprecated them a while ago). This mirrors how it's done for gentoo.
What issues does this PR fix or reference?
Alpine install is broken. In the
install_alpine_linux_post()phase, it tries to install OpenRC scripts from<salt-repo>/pkg/alpine/..., but they're not there anymore. They were moved toold/when they got deprecated 3.5 years ago.