main/openvpn: fix backward compatibility of init script

This fixes problem introduced in commit 4a66978dd949d571fdd984d800b3121c3a1a297f.
When user upgrades openvpn package, but (s)he has never modified
/etc/conf.d/openvpn file, apk automatically updates it and so sets openvpn
to the client mode. I forgot to this case and wrongly assumed that existing
config is always preserved.

BTW, the previoud change was based on
https://github.com/OpenRC/openrc/blob/master/support/init.d.examples/openvpn.in.

Ref #8875 (https://bugs.alpinelinux.org/issues/8875)

3 files changed, 58 insertions, 47 deletions

diff --git a/main/openvpn/APKBUILD b/main/openvpn/APKBUILD
index de3c5442d8..79cb419cad 100644
--- a/main/openvpn/APKBUILD
+++ b/main/openvpn/APKBUILD
@@ -2,7 +2,7 @@
 # Maintainer: Natanael Copa <ncopa@alpinelinux.org>
 pkgname=openvpn
 pkgver=2.4.6
-pkgrel=1
+pkgrel=2
 pkgdesc="A robust, and highly configurable VPN (Virtual Private Network)"
 url="http://openvpn.sourceforge.net/"
 arch="all"
@@ -60,7 +60,7 @@ pam() {
 
 sha512sums="cdd70bfd03177bc6cb70d0d614e40389df00816b7097740b4cda9d7bee094d1463fdb5afeaf604c52c7b3167d1edb098a2e095e131a8b9fed0ed8b29da90cbe8  openvpn-2.4.6.tar.gz
 5a083cdf8216db5e6e4577c00ccfe8e03ca318935ec9daa2018a8a6d4d7fd2b04fe395d7b329f16108101d69a6c0b70690883fda2cb552db7abf2f8246cc561f  libressl.patch
-78324d4eb59fb91cfab8d246f52c729a3656117e715d9704c89edf628398ed3ed203b7841047754f208c5e90dec1cd2983e651efbfeb16faab5aa48e4aa166eb  openvpn.initd
-643b5ee3f23b1634c12488f6de2d42cdc5c49618d5291a25963fb1516156273b0308a46b6623905fd441753cb5ae1b08c0e29bc5543e287847bb41f73fd357f0  openvpn.confd
+5ed0fd6cc1d3b19217c76dd6d6dadee20563c2455f076236ab5f0616f06ea155cdd593d8f5f6db06fbffb5a0743dad9d8821d9c293311d5311762611b1fcec1d  openvpn.initd
+6b2353aca9df7f43044e4e37990491b4ba077e259ebe13b8f2eb43e35ca7a617c1a65c5bfb8ab05e87cf12c4444184ae064f01f9abbb3c023dbbc07ff3f9c84e  openvpn.confd
 cdb73c9a5b1eb56e9cbd29955d94297ce5a87079419cd626d6a0b6680d88cbf310735a53f794886df02030b687eaea553c7c569a8ea1282a149441add1c65760  openvpn.up
 4456880d5c2db061219ba94e4052786700efa5e685f03b0d12d75a6023e3c0fc7b5242cc3d2bd3988e42fcd99701ab13a6257b1a0943b812318d30c64843ad27  openvpn.down"
diff --git a/main/openvpn/openvpn.confd b/main/openvpn/openvpn.confd
index 4359ef8cd5..7274a079de 100644
--- a/main/openvpn/openvpn.confd
+++ b/main/openvpn/openvpn.confd
@@ -1,17 +1,21 @@
 # Configuration for /etc/init.d/openvpn{,.*}
 
-# Run in client mode. If this is a server, set to "no".
-client_mode="yes"
+# OpenVPN can run in many modes. Most people want the init script to
+# automatically detect the mode and try and apply a good default configuration
+# and setup scripts. However, there are cases where the OpenVPN configuration
+# looks like a client, but it's really a peer or something else.
+# detect_client controls this behaviour.
+#detect_client="yes"
 
 # Path of the OpenVPN configuration file to load.
 # Default is /etc/openvpn/openvpn.conf, or /etc/openvpn/<vpn>.conf if
 # the runscript is symlinked to openvpn.<vpn> or <vpn>.
 #cfgfile=
 
-# The script to run after successful TUN/TAP device open.
+# The script to run in client mode after successful TUN/TAP device open.
 #up_script="/etc/openvpn/up.sh"
 
-# The script to run after TUN/TAP device close.
+# The script to run in client mode after TUN/TAP device close.
 #down_script="/etc/openvpn/down.sh"
 
 # OpenVPN automatically creates an /etc/resolv.conf (or sends it to
diff --git a/main/openvpn/openvpn.initd b/main/openvpn/openvpn.initd
index 407bf0ca58..c57a2925e9 100644
--- a/main/openvpn/openvpn.initd
+++ b/main/openvpn/openvpn.initd
@@ -10,6 +10,7 @@ instance_name=${RC_SVCNAME#*.}
 # Upper case variables are for backward compatibility with Alpine < v3.8.
 : ${cfgdir:=${VPNDIR:-"/etc/openvpn"}}
 : ${cfgfile:="$cfgdir/$instance_name.conf"}
+: ${detect_client:="${DETECT_CLIENT:-yes}"}
 : ${up_script:="$cfgdir/up.sh"}
 : ${down_script:="$cfgdir/down.sh"}
 : ${peer_dns:=${PEER_DNS:-"yes"}}
@@ -26,35 +27,6 @@ command_args="
 required_dirs="$cfgdir"
 required_files="$cfgfile"
 
-# If client_mode is not specified (user has old config), infer it from the
-# cfgfile as in old version of this runscript. Eventually we try to fix the
-# config when checkconfig() is run.
-# This is for backward compatibility with Alpine < v3.8.
-if [ -z "$client_mode" ] && [ -f "$cfgfile" ]; then
-	yesno "${DETECT_CLIENT:-yes}" && grep -q '^\s*remote\s' "$cfgfile" \
-		&& client_mode=yes \
-		|| client_mode=no
-	client_mode_not_set=yes
-fi
-
-if yesno "$client_mode"; then
-	command_args="$command_args
-		--up-delay
-		--up-restart
-		--down-pre
-		--script-security 2
-		--up $up_script
-		--down $down_script"
-
-	required_files="$required_files $up_script $down_script"
-
-	# If env. variable IN_BACKGROUND is set, fake start and stop commands
-	# (i.e. don't run them). We do this so we can "start" ourselves from
-	# inactive (from OpenVPN's up.sh script) which then triggers other
-	# services to start which depend on us. See openrc-run(8).
-	in_background_fake="start stop"
-	start_inactive="yes"
-fi
 
 depend() {
 	need localmount net
@@ -63,6 +35,14 @@ depend() {
 }
 
 checkconfig() {
+	# Note: This is not just a check; we need to detect the mode both for
+	# "start" and "checkconfig" commands, that's why it's here.
+	if [ -z "$client_mode" ] && yesno "$detect_client"; then
+		cfgfile_has_option 'remote' \
+			&& client_mode=yes \
+			|| client_mode=no
+	fi
+
 	if [ ! -e /dev/net/tun ]; then
 		if ! modprobe tun; then
 			eerror "TUN/TAP support is not available in this kernel"
@@ -77,6 +57,10 @@ checkconfig() {
 	fi
 
 	if yesno "$client_mode"; then
+		local f; for f in "$up_script" "$down_script"; do
+			[ -r "$f" ] || { eerror "'$f' is not readable"; return 1; }
+		done
+
 		# Warn about setting scripts as we override them
 		if cfgfile_has_option "(up|down)"; then
 			ewarn "WARNING: You have defined your own up/down scripts"
@@ -93,29 +77,52 @@ checkconfig() {
 			ewarn "or DNS configuration."
 		fi
 	fi
-
-	# This is for backward compatibility with Alpine < v3.8.
-	if yesno "$client_mode_not_set"; then
-		ewarn "client_mode is not specified in /etc/conf.d/$RC_SVCNAME, fixing..."
-		echo "client_mode=$client_mode" >> /etc/conf.d/$RC_SVCNAME 2>/dev/null
-		eend $?
-	fi
 }
 
 start_pre() {
 	checkconfig || return 1
 
+	if yesno "$client_mode"; then
+		command_args="$command_args
+			--up-delay
+			--up-restart
+			--down-pre
+			--script-security 2
+			--up $up_script
+			--down $down_script"
+		start_inactive="yes"
+	else
+		# Run as openvpn unless otherwise specified.
+		cfgfile_has_option "user" || command_args="$command_args --user openvpn"
+		cfgfile_has_option "group" || command_args="$command_args --group openvpn"
+	fi
+
 	# If the config file does not specify the cd option, we do.
 	# But if we specify it, we override the config option which we do not want.
 	if cfgfile_has_option "cd"; then
 		command_args="$command_args --cd $cfgdir"
 	fi
+}
 
-	if ! yesno "$client_mode"; then
-		# Run as openvpn unless otherwise specified.
-		cfgfile_has_option "user" || command_args="$command_args --user openvpn"
-		cfgfile_has_option "group" || command_args="$command_args --group openvpn"
+start() {
+	# If we are re-called by the up.sh script, then we don't actually want
+	# to start OpenVPN. We do this so we can "start" ourselves from
+	# inactive (from the up.sh script) which then triggers other
+	# services to start which depend on us.
+	yesno "$IN_BACKGROUND" && return 0
+
+	default_start
+}
+
+stop() {
+	# If we are re-called by the down.sh script, then we don't actually
+	# want to stop OpenVPN.
+	if yesno "$IN_BACKGROUND"; then
+		mark_service_inactive "$RC_SVCNAME"
+		return 0
 	fi
+
+	default_stop
 }
 
 cfgfile_has_option() {