[PATCH 1/2] fff-vpn-select: make vpn-select modular

[Fabian Bläse]

Hallo Robert,

gute Sache. Folgende Anmerkungen:

On 17.06.20 23:46, Robert Langhammer wrote:
> 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..bf9c199 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,36 @@
>  #!/bin/sh
> 
>  # Usage: vpn-select <path-to-hood-file>
> +# To add a new protocol, put a file with two functions to /etc/vpn-select.d/ .
> +# The function ${protocol}_config is called for every peer in hoodfile.
> +# The second function ${protocol}_start_stop is called once per installed protocol
Hier könnte man noch erwähnen, dass sämtliche alte Config beim Sourcen entfernt werden muss.
Eigentlich gefällt mir das nicht so recht, eine ${protocol}_clear() Funktion wäre mir dafür glaube ich irgendwie lieber.

> 
>  . /usr/share/libubox/jshn.sh
> 
>  hoodfile="$1"
> -
> -make_config() {
> -	# remove old config
> -	rm /tmp/fastd_fff_peers/*
> -
> -	# prepare
> -	Index=1
> -	json_load "$(cat "$hoodfile")"
> -	json_select vpn
> -
> -	# get fastd peers
> -	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
> -		json_select ".." # back to vpn
> -		Index=$(( Index + 1 ))
> -	done
> -	json_select ".." # back to root
> -}
> -
>  # 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
> +[ -s "$hoodfile" ] || exit 1
> +
> +# source functions
> +for file in /etc/vpn-select.d/*; do
> +	. "$file"
> +done
> +
> +# load hoodfile
> +json_load "$(cat "$hoodfile")"
> +json_select vpn
> +
> +# configure vpn
> +index=1
Stimmt das, hier mit Index 1 anzufangen? Ich konnte das dem jshn.sh auf die schnelle nicht entnehmen.

> +while json_select "$index" > /dev/null ; do
> +	json_get_var protocol protocol
> +	"${protocol}_config"
Was passiert, wenn ein VPN Protokoll nicht unterstützt wird (aka keine passende Funktion gesourced wurde)?
Kann man das irgendwie abfangen und dann eine entsprechende Meldung ausgeben?
Da das ganze für jeden Peer einmal aufgerufen wird, würde ich da zu tendieren es ${protocol}_addpeer() o.ä. zu nennen.

Wenn wir neue Protokolle hinzufügen möchten, dann brauchen wir vermutlich einen Weg, Protokolle zu bevorzugen, da batman das nicht kann.
Da müssen wir uns noch was einfallen lassen, das sollte aber ein lösbares Problem sein.

Abgesehen davon gefällts mir, danke! :-)
Schlussendlich bleibt für mich die Frage, ob das ganze überhaupt ein eigenes Paket wert ist. Aber das ist eine Frage für einen anderen Patch.

Gruß
Fabian

-------------- nächster Teil --------------
Ein Dateianhang mit Binärdaten wurde abgetrennt...
Dateiname   : signature.asc
Dateityp    : application/pgp-signature
Dateigröße  : 833 bytes
Beschreibung: OpenPGP digital signature
URL         : <https://lists.freifunk.net/pipermail/franken-dev-freifunk.net/attachments/20200618/c42dae3d/attachment.sig>