Make vpn-select modular

Submitted by Robert Langhammer on Aug. 6, 2020, 11:33 p.m.

Details

Message ID 20200806233329.4413-1-rlanghammer@web.de
State Superseded
Headers show

Commit Message

Robert Langhammer Aug. 6, 2020, 11:33 p.m. 
vpn-select is an old relic and did not reflect the opportunities of our hoodfile.
This rewrite makes vpn-select modular to easely add new vpn-protocols.

The stuff dependent on the vpn-protocol is outsourced to files in /etc/vpn-select.d and comes in with the respective vpn package.

vpn-stop is removed to use the protocol independent start/stop mechanism of vpn-select.

Signed-off-by: Robert Langhammer <rlanghammer@web.de>
---
 .../fff-fastd/files/etc/vpn-select.d/fastd    | 35 +++++++++
 .../fff-hoods/files/usr/sbin/configurehood    |  2 +-
 .../fff-vpn-select/files/usr/sbin/vpn-select  | 75 +++++++------------
 .../fff-vpn-select/files/usr/sbin/vpn-stop    |  5 --
 4 files changed, 62 insertions(+), 55 deletions(-)
 create mode 100644 src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
 delete mode 100755 src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop

--
2.20.1

Patch hide | download patch | download mbox 

diff --git a/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
new file mode 100644
index 0000000..bd73761
--- /dev/null
+++ b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
@@ -0,0 +1,35 @@ 
+protocol=fastd
+
+fastd_clear() {
+	rm /tmp/fastd_fff_peers/*
+}
+
+fastd_addpeer() {
+	[ -d /tmp/fastd_fff_peers ] || mkdir /tmp/fastd_fff_peers
+
+	# write fastd-config
+	json_get_var servername name
+	filename="/etc/fastd/fff/peers/$servername"
+	echo "#name \"${servername}\";" > "$filename"
+	json_get_var key key
+	echo "key \"${key}\";" >> "$filename"
+	json_get_var address address
+	json_get_var port port
+	echo "remote \"${address}\" port ${port};" >> "$filename"
+	echo "" >> "$filename"
+	echo "float yes;" >> "$filename"
+}
+
+fastd_start_stop() {
+	/etc/init.d/fastd reload # does nothing if fastd was not running
+
+	# fastd start/stop for various situations
+	# this is needed for first start and if fastd comes up or disappears in hoodfile
+	pidfile="/tmp/run/fastd.fff.pid"
+	if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
+		([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) || /etc/init.d/fastd start
+	else
+		([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) && /etc/init.d/fastd stop
+	fi
+}
+
diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
index 3b92cbc..c84a8cc 100755
--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
@@ -207,7 +207,7 @@  if [ -s "$hoodfiletmp" ]; then
 	if hasInternet ; then
 		/usr/sbin/vpn-select "$hoodfiletmp"
 	else
-		/usr/sbin/vpn-stop
+		/usr/sbin/vpn-select stop-VPN
 	fi

 	# now we load the prefix from the hoodfile and set this to br-mesh
diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
index 30883f5..8f48f9a 100755
--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
+++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
@@ -1,65 +1,42 @@ 
 #!/bin/sh

 # Usage: vpn-select <path-to-hood-file>
+# To add a new protocol, put a file with three functions to /etc/vpn-select.d/ .
+# The file must start with protocol=name. It is most important to use the same name here and in hoodfile.
+# The old config can be cleared in function ${protocol}_clear(). It is called once per installed protocol.
+# The function ${protocol}_addpeer() is called for every peer in hoodfile.
+# The function ${protocol}_start_stop() is called once per installed protocol.

 . /usr/share/libubox/jshn.sh

 hoodfile="$1"

-make_config() {
-	# remove old config
-	rm /tmp/fastd_fff_peers/*
+# source functions
+for file in /etc/vpn-select.d/*; do
+	. "$file"
+	supported_protocols="$supported_protocols $protocol"
+done

-	# prepare
-	Index=1
+# clear old config
+for protocol in $supported_protocols; do
+	"${protocol}_clear"
+done
+
+# load hoodfile and add peers
+if [ -s "$hoodfile" ] ; then
 	json_load "$(cat "$hoodfile")"
 	json_select vpn
-
-	# get fastd peers
-	while json_select "$Index" > /dev/null
-	do
+	index=1
+	while json_select "$index" > /dev/null ; do
 		json_get_var protocol protocol
-		if [ "$protocol" = "fastd" ]; then
-			# set up fastd
-			json_get_var servername name
-			filename="/etc/fastd/fff/peers/$servername"
-			echo "#name \"${servername}\";" > "$filename"
-			json_get_var key key
-			echo "key \"${key}\";" >> "$filename"
-			json_get_var address address
-			json_get_var port port
-			echo "remote \"${address}\" port ${port};" >> "$filename"
-			echo "" >> "$filename"
-			echo "float yes;" >> "$filename"
-		fi
+		"${protocol}_addpeer" || echo "protocol $protocol unknown"
 		json_select ".." # back to vpn
-		Index=$(( Index + 1 ))
+		index=$(( index + 1 ))
 	done
-	json_select ".." # back to root
-}
+fi

-# Only do something if file is there and not empty; otherwise exit 1
-if [ -s "$hoodfile" ]; then
-	if [ ! -d /tmp/fastd_fff_peers ]; then
-		# first run after reboot
-		mkdir /tmp/fastd_fff_peers
-		make_config
-		# start fastd only if there are some peers
-		[ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] && /etc/init.d/fastd start
-	else
-		make_config
-		/etc/init.d/fastd reload
+# start/restart/stop vpnservices
+for protocol in $supported_protocols; do
+	"${protocol}_start_stop"
+done

-		# fastd start/stop for various situations
-		pidfile="/tmp/run/fastd.fff.pid"
-		if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
-			([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) || /etc/init.d/fastd start
-		else
-			([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) && /etc/init.d/fastd stop
-		fi
-	fi
-	exit 0
-else
-	echo "vpn-select: Hood file not found or empty!"
-	exit 1
-fi
diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
deleted file mode 100755
index 03a160b..0000000
--- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
+++ /dev/null
@@ -1,5 +0,0 @@ 
-#!/bin/sh
-
-rm /tmp/fastd_fff_peers/*
-/etc/init.d/fastd stop
-

Comments

Adrian Schmutzler Aug. 7, 2020, 11:55 p.m. 
Hallo,

gefällt mir generell gut, ein paar Kommentare unten.

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
> Of Robert Langhammer
> Sent: Freitag, 7. August 2020 01:33
> To: franken-dev@freifunk.net
> Subject: [PATCH] Make vpn-select modular
> 
> vpn-select is an old relic and did not reflect the opportunities of our hoodfile.
> This rewrite makes vpn-select modular to easely add new vpn-protocols.
> 
> The stuff dependent on the vpn-protocol is outsourced to files in /etc/vpn-
> select.d and comes in with the respective vpn package.
> 
> vpn-stop is removed to use the protocol independent start/stop mechanism
> of vpn-select.
> 
> Signed-off-by: Robert Langhammer <rlanghammer@web.de>
> ---
>  .../fff-fastd/files/etc/vpn-select.d/fastd    | 35 +++++++++
>  .../fff-hoods/files/usr/sbin/configurehood    |  2 +-
>  .../fff-vpn-select/files/usr/sbin/vpn-select  | 75 +++++++------------
>  .../fff-vpn-select/files/usr/sbin/vpn-stop    |  5 --
>  4 files changed, 62 insertions(+), 55 deletions(-)  create mode 100644
> src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
>  delete mode 100755 src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
> 
> diff --git a/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
> b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
> new file mode 100644
> index 0000000..bd73761
> --- /dev/null
> +++ b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
> @@ -0,0 +1,35 @@
> +protocol=fastd
> +
> +fastd_clear() {
> +	rm /tmp/fastd_fff_peers/*
> +}
> +
> +fastd_addpeer() {
> +	[ -d /tmp/fastd_fff_peers ] || mkdir /tmp/fastd_fff_peers
> +
> +	# write fastd-config
> +	json_get_var servername name
> +	filename="/etc/fastd/fff/peers/$servername"
> +	echo "#name \"${servername}\";" > "$filename"
> +	json_get_var key key
> +	echo "key \"${key}\";" >> "$filename"
> +	json_get_var address address
> +	json_get_var port port
> +	echo "remote \"${address}\" port ${port};" >> "$filename"
> +	echo "" >> "$filename"
> +	echo "float yes;" >> "$filename"
> +}
> +
> +fastd_start_stop() {

Den Namen finde ich nicht optimal, mir fällt allerdings auch nichts ein, was jetzt substantiell besser wäre.
Ich frage mich zudem, ob ein vergleichbares Konzept für andere Dienste auch funktioniert (also ohne Trennung von start und stop).

> +	/etc/init.d/fastd reload # does nothing if fastd was not running
> +
> +	# fastd start/stop for various situations
> +	# this is needed for first start and if fastd comes up or disappears in
> hoodfile
> +	pidfile="/tmp/run/fastd.fff.pid"
> +	if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
> +		([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) ||
> /etc/init.d/fastd start
> +	else
> +		([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) &&
> /etc/init.d/fastd stop
> +	fi

Hier bin ich mir nicht ganz sicher, ob das wirklich den vpn-stop immer ersetzt. Wenn ja, wäre das eine nette Code-Reduktion :-)

> +}
> +
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 3b92cbc..c84a8cc 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -207,7 +207,7 @@ if [ -s "$hoodfiletmp" ]; then
>  	if hasInternet ; then
>  		/usr/sbin/vpn-select "$hoodfiletmp"
>  	else
> -		/usr/sbin/vpn-stop
> +		/usr/sbin/vpn-select stop-VPN

Das finde ich nicht schön. Der erste Parameter ist ein Pfad.
Allerdings könnte man hier einfach den Parameter weglassen und würde effektiv zum gewünschten Ergebnis kommen.

>  	fi
> 
>  	# now we load the prefix from the hoodfile and set this to br-mesh
> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> index 30883f5..8f48f9a 100755
> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> @@ -1,65 +1,42 @@
>  #!/bin/sh
> 
>  # Usage: vpn-select <path-to-hood-file>
> +# To add a new protocol, put a file with three functions to /etc/vpn-
> select.d/ .
> +# The file must start with protocol=name. It is most important to use the
> same name here and in hoodfile.
> +# The old config can be cleared in function ${protocol}_clear(). It is called
> once per installed protocol.
> +# The function ${protocol}_addpeer() is called for every peer in hoodfile.
> +# The function ${protocol}_start_stop() is called once per installed protocol.
> 
>  . /usr/share/libubox/jshn.sh
> 
>  hoodfile="$1"
> 
> -make_config() {
> -	# remove old config
> -	rm /tmp/fastd_fff_peers/*
> +# source functions
> +for file in /etc/vpn-select.d/*; do
> +	. "$file"

Hier sollte man vorher prüfen, ob der Ordner leer ist. Ist zwar unwahrscheinlich, aber ich möchte gerne ein
". /etc/vpn-select.d/*" vermeiden.

> +	supported_protocols="$supported_protocols $protocol"
> +done
> 
> -	# prepare
> -	Index=1
> +# clear old config
> +for protocol in $supported_protocols; do
> +	"${protocol}_clear"
> +done
> +
> +# load hoodfile and add peers
> +if [ -s "$hoodfile" ] ; then

Hier sollte man prüfen, ob $hoodfile überhaupt einen String enthält. Prinzipiell sind hier zwei Ansätze möglich:

1. Man prüft [ -n $hoodfile] gleich am Anfang und beendet das Skript, wenn false.
2. Man prüft hier [ -n $hoofile ] && [ -s $hoodfile ], und erschlägt damit das vpn-stop Szenario gleich mit. Wird keine Argument übergeben (kein Hoodfile), würde also hier gelöscht und start-stop gemacht, aber keine Peers erzeugt. Problem gelöst.

Ich würde aber in jedem Fall den Check mit -n ergänzen, da ich nicht weiß, was für spannende Sachen mit [ -s "" ] passieren können.

>  	json_load "$(cat "$hoodfile")"
>  	json_select vpn
> -
> -	# get fastd peers
> -	while json_select "$Index" > /dev/null
> -	do
> +	index=1
> +	while json_select "$index" > /dev/null ; do

Wenn wir den index nicht direkt brauchen (wie früher bei l2tp), dann geht das eleganter (habe ich hier https://github.com/openwrt/openwrt/blob/master/package/base-files/files/lib/upgrade/fwtool.sh#L61 gefunden):

json_select vpn
json_get_keys vpn_keys
for k in $vpn_keys; do
    json_select $k
    json_get_var protocol protocol
    ....
    json_select ..
done

Damit werden wir den Index los und iterieren nur über die tatsächlichen Elemente des arrays (habs gerade auch lokal getestet).

Grüße

Adrian

>  		json_get_var protocol protocol
> -		if [ "$protocol" = "fastd" ]; then
> -			# set up fastd
> -			json_get_var servername name
> -			filename="/etc/fastd/fff/peers/$servername"
> -			echo "#name \"${servername}\";" > "$filename"
> -			json_get_var key key
> -			echo "key \"${key}\";" >> "$filename"
> -			json_get_var address address
> -			json_get_var port port
> -			echo "remote \"${address}\" port ${port};" >>
> "$filename"
> -			echo "" >> "$filename"
> -			echo "float yes;" >> "$filename"
> -		fi
> +		"${protocol}_addpeer" || echo "protocol $protocol
> unknown"
>  		json_select ".." # back to vpn
> -		Index=$(( Index + 1 ))
> +		index=$(( index + 1 ))
>  	done
> -	json_select ".." # back to root
> -}
> +fi
> 
> -# Only do something if file is there and not empty; otherwise exit 1 -if [ -s
> "$hoodfile" ]; then
> -	if [ ! -d /tmp/fastd_fff_peers ]; then
> -		# first run after reboot
> -		mkdir /tmp/fastd_fff_peers
> -		make_config
> -		# start fastd only if there are some peers
> -		[ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] &&
> /etc/init.d/fastd start
> -	else
> -		make_config
> -		/etc/init.d/fastd reload
> +# start/restart/stop vpnservices
> +for protocol in $supported_protocols; do
> +	"${protocol}_start_stop"
> +done
> 
> -		# fastd start/stop for various situations
> -		pidfile="/tmp/run/fastd.fff.pid"
> -		if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
> -			([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) ||
> /etc/init.d/fastd start
> -		else
> -			([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) &&
> /etc/init.d/fastd stop
> -		fi
> -	fi
> -	exit 0
> -else
> -	echo "vpn-select: Hood file not found or empty!"
> -	exit 1
> -fi
> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
> deleted file mode 100755
> index 03a160b..0000000
> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -#!/bin/sh
> -
> -rm /tmp/fastd_fff_peers/*
> -/etc/init.d/fastd stop
> -
> --
> 2.20.1
Robert Langhammer Aug. 8, 2020, 11:42 a.m. 
Hallo Adrian,

s. inline

Am 08.08.20 um 01:55 schrieb Adrian Schmutzler:
> Hallo,
>
> gefällt mir generell gut, ein paar Kommentare unten.
>
>> -----Original Message-----
>> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf
>> Of Robert Langhammer
>> Sent: Freitag, 7. August 2020 01:33
>> To: franken-dev@freifunk.net
>> Subject: [PATCH] Make vpn-select modular
>>
>> vpn-select is an old relic and did not reflect the opportunities of our hoodfile.
>> This rewrite makes vpn-select modular to easely add new vpn-protocols.
>>
>> The stuff dependent on the vpn-protocol is outsourced to files in /etc/vpn-
>> select.d and comes in with the respective vpn package.
>>
>> vpn-stop is removed to use the protocol independent start/stop mechanism
>> of vpn-select.
>>
>> Signed-off-by: Robert Langhammer <rlanghammer@web.de>
>> ---
>>  .../fff-fastd/files/etc/vpn-select.d/fastd    | 35 +++++++++
>>  .../fff-hoods/files/usr/sbin/configurehood    |  2 +-
>>  .../fff-vpn-select/files/usr/sbin/vpn-select  | 75 +++++++------------
>>  .../fff-vpn-select/files/usr/sbin/vpn-stop    |  5 --
>>  4 files changed, 62 insertions(+), 55 deletions(-)  create mode 100644
>> src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
>>  delete mode 100755 src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
>>
>> diff --git a/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
>> b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
>> new file mode 100644
>> index 0000000..bd73761
>> --- /dev/null
>> +++ b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
>> @@ -0,0 +1,35 @@
>> +protocol=fastd
>> +
>> +fastd_clear() {
>> +	rm /tmp/fastd_fff_peers/*
>> +}
>> +
>> +fastd_addpeer() {
>> +	[ -d /tmp/fastd_fff_peers ] || mkdir /tmp/fastd_fff_peers
>> +
>> +	# write fastd-config
>> +	json_get_var servername name
>> +	filename="/etc/fastd/fff/peers/$servername"
>> +	echo "#name \"${servername}\";" > "$filename"
>> +	json_get_var key key
>> +	echo "key \"${key}\";" >> "$filename"
>> +	json_get_var address address
>> +	json_get_var port port
>> +	echo "remote \"${address}\" port ${port};" >> "$filename"
>> +	echo "" >> "$filename"
>> +	echo "float yes;" >> "$filename"
>> +}
>> +
>> +fastd_start_stop() {
> Den Namen finde ich nicht optimal, mir fällt allerdings auch nichts ein, was jetzt substantiell besser wäre.

start-stop beschreibt das was hier geschieht sehr gut. Oben wird
gelöscht. Dann wird evtl. eine neue Konfig geschrieben und je nach dem,
wird hier gestartet, gestoppt oder es bleibt beim Alten. Jeder, der
etwas Unix im Blut hat, assoziiert damit sysV-init und hat damit gleich
das richtige Bild von dem, was hier passiert.

> Ich frage mich zudem, ob ein vergleichbares Konzept für andere Dienste auch funktioniert (also ohne Trennung von start und stop).
Ja, tut es. Siehe nächsten Patch mit vxlan. Da übernimmt es der netifd
für uns. Hier bei fastd müssen wir das hier unten selbst machen. Das
kann der netifd nicht.
>
>> +	/etc/init.d/fastd reload # does nothing if fastd was not running
>> +
>> +	# fastd start/stop for various situations
>> +	# this is needed for first start and if fastd comes up or disappears in
>> hoodfile
>> +	pidfile="/tmp/run/fastd.fff.pid"
>> +	if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
>> +		([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) ||
>> /etc/init.d/fastd start
>> +	else
>> +		([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) &&
>> /etc/init.d/fastd stop
>> +	fi
> Hier bin ich mir nicht ganz sicher, ob das wirklich den vpn-stop immer ersetzt. Wenn ja, wäre das eine nette Code-Reduktion :-)

Es betrifft hier nur den fastd. Für andere Protokolle muss man es
natürlich anders lösen (s. vxlan).

Wenn keine peers da sind wird der fastd immer gestoppt. Darum wird immer
proto_clear gemacht. Egal ob gültiges hoodfile oder nicht. So
funktioniert auch vpn-select stop-VPN -> kein gültiges hoodfile -> alle
vpn aus. Dieses pidfile/proc-testen ist doch nur drin, damit
/etc/init.d/fastd stop nicht gerufen wird, wenn fastd nicht an ist.

>
>> +}
>> +
>> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> index 3b92cbc..c84a8cc 100755
>> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
>> @@ -207,7 +207,7 @@ if [ -s "$hoodfiletmp" ]; then
>>  	if hasInternet ; then
>>  		/usr/sbin/vpn-select "$hoodfiletmp"
>>  	else
>> -		/usr/sbin/vpn-stop
>> +		/usr/sbin/vpn-select stop-VPN
> Das finde ich nicht schön. Der erste Parameter ist ein Pfad.
> Allerdings könnte man hier einfach den Parameter weglassen und würde effektiv zum gewünschten Ergebnis kommen.
Weglassen ist sehr unschön. vpn-select erwartet immer $1 (Usage:
vpn-select <path-to-hood-file>). Das ist nicht optional.  vpn-select
stop-VPN ist natürlich ein ungültiger Pfad, hier aber sehr sprechend für
das was passiert. Darauf reagiert dann vpns-select und legt keine peers
an -> vpn-stop (s. oben)
>
>>  	fi
>>
>>  	# now we load the prefix from the hoodfile and set this to br-mesh
>> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> index 30883f5..8f48f9a 100755
>> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
>> @@ -1,65 +1,42 @@
>>  #!/bin/sh
>>
>>  # Usage: vpn-select <path-to-hood-file>
>> +# To add a new protocol, put a file with three functions to /etc/vpn-
>> select.d/ .
>> +# The file must start with protocol=name. It is most important to use the
>> same name here and in hoodfile.
>> +# The old config can be cleared in function ${protocol}_clear(). It is called
>> once per installed protocol.
>> +# The function ${protocol}_addpeer() is called for every peer in hoodfile.
>> +# The function ${protocol}_start_stop() is called once per installed protocol.
>>
>>  . /usr/share/libubox/jshn.sh
>>
>>  hoodfile="$1"
>>
>> -make_config() {
>> -	# remove old config
>> -	rm /tmp/fastd_fff_peers/*
>> +# source functions
>> +for file in /etc/vpn-select.d/*; do
>> +	. "$file"
> Hier sollte man vorher prüfen, ob der Ordner leer ist. Ist zwar unwahrscheinlich, aber ich möchte gerne ein
> ". /etc/vpn-select.d/*" vermeiden.
Oh ja, sehr gut! Übersehe ich irgendwie immer wieder mal. Es expandiert
nicht und dann bleibt das Alte stehen.
>
>> +	supported_protocols="$supported_protocols $protocol"
>> +done
>>
>> -	# prepare
>> -	Index=1
>> +# clear old config
>> +for protocol in $supported_protocols; do
>> +	"${protocol}_clear"
>> +done
>> +
>> +# load hoodfile and add peers
>> +if [ -s "$hoodfile" ] ; then
> Hier sollte man prüfen, ob $hoodfile überhaupt einen String enthält. Prinzipiell sind hier zwei Ansätze möglich:
>
> 1. Man prüft [ -n $hoodfile] gleich am Anfang und beendet das Skript, wenn false. 
Geht nicht, sonst funktioniert der stop nicht mehr für den Fall dass $1
leer.
> 2. Man prüft hier [ -n $hoofile ] && [ -s $hoodfile ], und erschlägt damit das vpn-stop Szenario gleich mit. Wird keine Argument übergeben (kein Hoodfile), würde also hier gelöscht und start-stop gemacht, aber keine Peers erzeugt. Problem gelöst.
Wie oben schon beschrieben ist das mit einem ungültigen hoodfile-path
bereits gelöst. Darum [ -s $hoodfile ] 
>
> Ich würde aber in jedem Fall den Check mit -n ergänzen, da ich nicht weiß, was für spannende Sachen mit [ -s "" ] passieren können.
[ -s "" ] liefert errorlevel 1 . Genau das was wir brauchen. Aber ein
zusätzliches -n tut nicht sehr weh.
>
>>  	json_load "$(cat "$hoodfile")"
>>  	json_select vpn
>> -
>> -	# get fastd peers
>> -	while json_select "$Index" > /dev/null
>> -	do
>> +	index=1
>> +	while json_select "$index" > /dev/null ; do
> Wenn wir den index nicht direkt brauchen (wie früher bei l2tp), dann geht das eleganter (habe ich hier https://github.com/openwrt/openwrt/blob/master/package/base-files/files/lib/upgrade/fwtool.sh#L61 gefunden):
>
> json_select vpn
> json_get_keys vpn_keys
> for k in $vpn_keys; do
>     json_select $k
>     json_get_var protocol protocol
>     ....
>     json_select ..
> done
>
> Damit werden wir den Index los und iterieren nur über die tatsächlichen Elemente des arrays (habs gerade auch lokal getestet
oh, sehr schön. Das kannte ich noch nicht. So machen wir das.
>
> Grüße
>
> Adrian
>
>>  		json_get_var protocol protocol
>> -		if [ "$protocol" = "fastd" ]; then
>> -			# set up fastd
>> -			json_get_var servername name
>> -			filename="/etc/fastd/fff/peers/$servername"
>> -			echo "#name \"${servername}\";" > "$filename"
>> -			json_get_var key key
>> -			echo "key \"${key}\";" >> "$filename"
>> -			json_get_var address address
>> -			json_get_var port port
>> -			echo "remote \"${address}\" port ${port};" >>
>> "$filename"
>> -			echo "" >> "$filename"
>> -			echo "float yes;" >> "$filename"
>> -		fi
>> +		"${protocol}_addpeer" || echo "protocol $protocol
>> unknown"
>>  		json_select ".." # back to vpn
>> -		Index=$(( Index + 1 ))
>> +		index=$(( index + 1 ))
>>  	done
>> -	json_select ".." # back to root
>> -}
>> +fi
>>
>> -# Only do something if file is there and not empty; otherwise exit 1 -if [ -s
>> "$hoodfile" ]; then
>> -	if [ ! -d /tmp/fastd_fff_peers ]; then
>> -		# first run after reboot
>> -		mkdir /tmp/fastd_fff_peers
>> -		make_config
>> -		# start fastd only if there are some peers
>> -		[ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] &&
>> /etc/init.d/fastd start
>> -	else
>> -		make_config
>> -		/etc/init.d/fastd reload
>> +# start/restart/stop vpnservices
>> +for protocol in $supported_protocols; do
>> +	"${protocol}_start_stop"
>> +done
>>
>> -		# fastd start/stop for various situations
>> -		pidfile="/tmp/run/fastd.fff.pid"
>> -		if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
>> -			([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) ||
>> /etc/init.d/fastd start
>> -		else
>> -			([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) &&
>> /etc/init.d/fastd stop
>> -		fi
>> -	fi
>> -	exit 0
>> -else
>> -	echo "vpn-select: Hood file not found or empty!"
>> -	exit 1
>> -fi
>> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
>> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
>> deleted file mode 100755
>> index 03a160b..0000000
>> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
>> +++ /dev/null
>> @@ -1,5 +0,0 @@
>> -#!/bin/sh
>> -
>> -rm /tmp/fastd_fff_peers/*
>> -/etc/init.d/fastd stop
>> -
>> --
>> 2.20.1
Adrian Schmutzler Aug. 8, 2020, 7:09 p.m. 
Hallo,

>>>      else 
>>> -            /usr/sbin/vpn-stop 
>>> +            /usr/sbin/vpn-select stop-VPN 
>> Das finde ich nicht schön. Der erste Parameter ist ein Pfad. 
>> Allerdings könnte man hier einfach den Parameter weglassen und würde effektiv zum gewünschten Ergebnis kommen. 
> Weglassen ist sehr unschön. vpn-select erwartet immer $1 (Usage: 
> vpn-select <path-to-hood-file>). Das ist nicht optional.  vpn-select 
> stop-VPN ist natürlich ein ungültiger Pfad, hier aber sehr sprechend für 
> das was passiert. Darauf reagiert dann vpns-select und legt keine peers 
> an -> vpn-stop (s. oben) 

Naja, wenn du hier mit der "Usage" argumentierst, ist ein falsches Argument jetzt auch nicht richtiger als gar keins.
Funktionieren würde beides. Aber das mit dem stop-VPN stört mich tatsächlich sehr.

Man könnte höchstens überlegen, hier noch einen weiteren Parameter einzuführen, der vorgibt, was vpn-select tun soll, wenn man es ganz ordentlich will.

Grüße

Adrian