First patch series for the roadmap item "Get rid of CONFIG

public inbox for development@lists.ipfire.org
 help / color / mirror / Atom feed

* First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config"
@ 2024-05-20  9:05 Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 01/21] test: Add bash lib for colors Jonatan Schlag
                   ` (20 more replies)
  0 siblings, 21 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

Hi list,

This is the first patch series for the roadmap item: https://www.ipfire.org/docs/roadmap/get-rid-of-configtype-in-network-config . This patch series introduces a bash testing library and a function readhash, which we will use in every bash script which needs to read our config files. As this patch set is already massive, I thought it might be better to discuss the function first before I introduce it in every script. Otherwise, we would talk about 50 patches and not 21. This amount would be somehow not reviewable. If this patch series get merged, fine. If you want to wait for all the patches also fine, but as told, this will get a huge patch set.

Patches in this series are:

[PATCH 01/21] test: Add bash lib for colors
[PATCH 02/21] tests: Add bash lib
[PATCH 03/21] tests/lib.sh: Add function
[PATCH 04/21] tests/lib.sh: Add check if variable exists to
[PATCH 05/21] tests/lib.sh: Add logging functions
[PATCH 06/21] tests/lib.sh: adjust to pytest logging style
[PATCH 07/21] tests/lib.sh: Fix check if array is defined
[PATCH 08/21] test_that_key_in_arry_has_value: Check if the key is
[PATCH 09/21] tests/lib.sh: Use namref to access an array
[PATCH 10/21] test_that_key_in_arry_has_value: Check if a key in an
[PATCH 11/21] tests: Add function to test the ouput of a bash
[PATCH 12/21] initscript functions: add readhash
[PATCH 13/21] initscript fkt: ignore blank lines in readhash
[PATCH 14/21] test: Add functions test_that_array_is_defined
[PATCH 15/21] tests: Add functions test_that_array_doesnt_have_key
[PATCH 16/21] initscripts fkt: Ignore comments in readhash
[PATCH 17/21] initscripts fkt: ignore invalid keys in readhash
[PATCH 18/21] initscripts fkt: readhash should only parse lines with
[PATCH 19/21] initscripts fkt: Check for invalid values in readhash
[PATCH 20/21] initscripts fkt: keep readhash compatible with older
[PATCH 21/21] initscripts fkt: Fix shebang

Greetings Jonatan


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 01/21] test: Add bash lib for colors
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 02/21] tests: Add bash lib Jonatan Schlag
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

This is borrowed from here: https://git.ipfire.org/?p=network.git;a=blob;f=src/functions/functions.colors;h=0bd6f97177c366f1d1ee4553043ae719430acdb2;hb=refs/heads/master

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib_color.sh | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 tests/lib_color.sh

diff --git a/tests/lib_color.sh b/tests/lib_color.sh
new file mode 100644
index 000000000..6ca1c9864
--- /dev/null
+++ b/tests/lib_color.sh
@@ -0,0 +1,43 @@
+#!/usr/bin/bash
+
+# Regular colours
+CLR_BLACK_R='\e[0;30m'
+CLR_RED_R='\e[0;31m'
+CLR_GREEN_R='\e[0;32m'
+CLR_YELLOW_R='\e[0;33m'
+CLR_BLUE_R='\e[0;34m'
+CLR_PURPLE_R='\e[0;35m'
+CLR_CYAN_R='\e[0;36m'
+CLR_WHITE_R='\e[0;37m'
+
+# Bold colours
+CLR_BLACK_B='\e[1;30m'
+CLR_RED_B='\e[1;31m'
+CLR_GREEN_B='\e[1;32m'
+CLR_YELLOW_B='\e[1;33m'
+CLR_BLUE_B='\e[1;34m'
+CLR_PURPLE_B='\e[1;35m'
+CLR_CYAN_B='\e[1;36m'
+CLR_WHITE_B='\e[1;37m'
+
+# Background colors
+CLR_BLACK_BG='\e[40m'
+CLR_RED_BG='\e[41m'
+CLR_GREEN_BG='\e[42m'
+CLR_YELLOW_BG='\e[43m'
+CLR_BLUE_BG='\e[44m'
+CLR_PURPLE_BG='\e[45m'
+CLR_CYAN_BG='\e[46m'
+CLR_WHITE_BG='\e[47m'
+
+# Font decoration
+FONT_RESET="\e[0m"
+FONT_BOLD="\e[1m"
+FONT_UNDERLINED="\e[4m"
+FONT_BLINKING="\e[5m"
+FONT_INVERTED="\e[7m"
+
+# Reset everything
+CLR_RESET="${FONT_RESET}"
+
+
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 02/21] tests: Add bash lib
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 01/21] test: Add bash lib for colors Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 03/21] tests/lib.sh: Add function test_that_key_in_arry_has_value Jonatan Schlag
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

This allows use to write test with less effort as we can reuse functions

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 tests/lib.sh

diff --git a/tests/lib.sh b/tests/lib.sh
new file mode 100644
index 000000000..7749d5158
--- /dev/null
+++ b/tests/lib.sh
@@ -0,0 +1,20 @@
+#!/usr/bin/bash
+
+LIB_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
+
+. ${LIB_DIR}/lib_color.sh
+
+test_that() {
+
+	if ! "$@" ; then
+		echo -e "${CLR_RED_BG} Test failed: ${*} ${CLR_RESET}"
+		return 1
+	else
+		echo -e "${CLR_GREEN_BG} Test succeded: ${*} ${CLR_RESET}"
+		return 0
+	fi
+}
+
+var_has_value() {
+	[[ "${!1}" == "${2}" ]]
+}
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 03/21] tests/lib.sh: Add function test_that_key_in_arry_has_value
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 01/21] test: Add bash lib for colors Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 02/21] tests: Add bash lib Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 04/21] tests/lib.sh: Add check if variable exists to test_that_key_in_arry_has_value Jonatan Schlag
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

I think the name says it all.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/lib.sh b/tests/lib.sh
index 7749d5158..c51257e79 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -18,3 +18,17 @@ test_that() {
 var_has_value() {
 	[[ "${!1}" == "${2}" ]]
 }
+
+test_that_key_in_arry_has_value() {
+	local array="${!1}"
+	local key="${2}"
+	local value="${3}"
+
+	if [[ "${array[${key}]}" == "${value}" ]] ; then
+		echo -e "${CLR_GREEN_BG}Test succeded: The array '${1}' contains the value '${value}' under the key '${key}' ${CLR_RESET}"
+		return 0
+	else
+		echo -e "${CLR_RED_BG}Test failed: The array '${1}' contains the value '${array[${key}]}' under the key '${key} and not '${value}' ${CLR_RESET}"
+		return 1
+	fi
+}
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 04/21] tests/lib.sh: Add check if variable exists to test_that_key_in_arry_has_value
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (2 preceding siblings ...)
  2024-05-20  9:05 ` [PATCH 03/21] tests/lib.sh: Add function test_that_key_in_arry_has_value Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 05/21] tests/lib.sh: Add logging functions Jonatan Schlag
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/lib.sh b/tests/lib.sh
index c51257e79..373b7c3a0 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -24,6 +24,11 @@ test_that_key_in_arry_has_value() {
 	local key="${2}"
 	local value="${3}"
 
+	if [[ ! -v "${array}" ]]; then
+		echo -e "${CLR_RED_BG}Test failed: The array '${1}' does not exists. The variable is not set.${CLR_RESET}'"
+		return 1
+	fi
+
 	if [[ "${array[${key}]}" == "${value}" ]] ; then
 		echo -e "${CLR_GREEN_BG}Test succeded: The array '${1}' contains the value '${value}' under the key '${key}' ${CLR_RESET}"
 		return 0
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 05/21] tests/lib.sh: Add logging functions
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (3 preceding siblings ...)
  2024-05-20  9:05 ` [PATCH 04/21] tests/lib.sh: Add check if variable exists to test_that_key_in_arry_has_value Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 06/21] tests/lib.sh: adjust to pytest logging style Jonatan Schlag
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1810 bytes --]

So we can change the style of our log messages better.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tests/lib.sh b/tests/lib.sh
index 373b7c3a0..6483c41c3 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -4,13 +4,21 @@ LIB_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
 
 . ${LIB_DIR}/lib_color.sh
 
+log_test_failed(){
+	echo -e "${CLR_RED_BG}Test failed: ${*}${CLR_RESET}'"
+}
+
+log_test_succeded(){
+	echo -e "${CLR_GREEN_BG}Test succeded: ${*}${CLR_RESET}"
+}
+
 test_that() {
 
 	if ! "$@" ; then
-		echo -e "${CLR_RED_BG} Test failed: ${*} ${CLR_RESET}"
+		log_test_failed "${*}"
 		return 1
 	else
-		echo -e "${CLR_GREEN_BG} Test succeded: ${*} ${CLR_RESET}"
+		log_test_succeded "${*}"
 		return 0
 	fi
 }
@@ -25,15 +33,15 @@ test_that_key_in_arry_has_value() {
 	local value="${3}"
 
 	if [[ ! -v "${array}" ]]; then
-		echo -e "${CLR_RED_BG}Test failed: The array '${1}' does not exists. The variable is not set.${CLR_RESET}'"
+		log_test_failed "The array '${1}' does not exists. The variable is not set."
 		return 1
 	fi
 
 	if [[ "${array[${key}]}" == "${value}" ]] ; then
-		echo -e "${CLR_GREEN_BG}Test succeded: The array '${1}' contains the value '${value}' under the key '${key}' ${CLR_RESET}"
+		log_test_succeded "The array '${1}' contains the value '${value}' under the key '${key}'"
 		return 0
 	else
-		echo -e "${CLR_RED_BG}Test failed: The array '${1}' contains the value '${array[${key}]}' under the key '${key} and not '${value}' ${CLR_RESET}"
+		log_test_failed "The array '${1}' contains the value '${array[${key}]}' under the key '${key} and not '${value}'"
 		return 1
 	fi
 }
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 06/21] tests/lib.sh: adjust to pytest logging style
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (4 preceding siblings ...)
  2024-05-20  9:05 ` [PATCH 05/21] tests/lib.sh: Add logging functions Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 07/21] tests/lib.sh: Fix check if array is defined Jonatan Schlag
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

Black on white is still the best to read. So we only style FAILED or
PASSED in green or red.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/lib.sh b/tests/lib.sh
index 6483c41c3..079755410 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -5,11 +5,11 @@ LIB_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
 . ${LIB_DIR}/lib_color.sh
 
 log_test_failed(){
-	echo -e "${CLR_RED_BG}Test failed: ${*}${CLR_RESET}'"
+	echo -e "${CLR_RED_R}FAILED:${CLR_RESET} ${*}"
 }
 
 log_test_succeded(){
-	echo -e "${CLR_GREEN_BG}Test succeded: ${*}${CLR_RESET}"
+	echo -e "${CLR_GREEN_R}PASSED:${CLR_RESET} ${*}"
 }
 
 test_that() {
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 07/21] tests/lib.sh: Fix check if array is defined
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (5 preceding siblings ...)
  2024-05-20  9:05 ` [PATCH 06/21] tests/lib.sh: adjust to pytest logging style Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 08/21] test_that_key_in_arry_has_value: Check if the key " Jonatan Schlag
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

-v does a bad job here. We need to use declare here.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/lib.sh b/tests/lib.sh
index 079755410..0f4de8e43 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -29,10 +29,11 @@ var_has_value() {
 
 test_that_key_in_arry_has_value() {
 	local array="${!1}"
+	local arrayname="${1}"
 	local key="${2}"
 	local value="${3}"
 
-	if [[ ! -v "${array}" ]]; then
+	if [[ ! "$(declare  -p "${arrayname}")" =~ "declare -a" && ! "$(declare  -p "${arrayname}")" =~ "declare -A" ]]; then
 		log_test_failed "The array '${1}' does not exists. The variable is not set."
 		return 1
 	fi
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 08/21] test_that_key_in_arry_has_value: Check if the key is defined
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (6 preceding siblings ...)
  2024-05-20  9:05 ` [PATCH 07/21] tests/lib.sh: Fix check if array is defined Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:05 ` [PATCH 09/21] tests/lib.sh: Use namref to access an array Jonatan Schlag
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/lib.sh b/tests/lib.sh
index 0f4de8e43..006862da6 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -38,6 +38,11 @@ test_that_key_in_arry_has_value() {
 		return 1
 	fi
 
+	if [[ ! -v "${array[${key}]}" ]]; then
+		log_test_failed "The array does not contain the key '${key}', valid keys are: ${!array[*]}"
+		return 1
+	fi
+
 	if [[ "${array[${key}]}" == "${value}" ]] ; then
 		log_test_succeded "The array '${1}' contains the value '${value}' under the key '${key}'"
 		return 0
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 09/21] tests/lib.sh: Use namref to access an array
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (7 preceding siblings ...)
  2024-05-20  9:05 ` [PATCH 08/21] test_that_key_in_arry_has_value: Check if the key " Jonatan Schlag
@ 2024-05-20  9:05 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 10/21] test_that_key_in_arry_has_value: Check if a key in an array exists Jonatan Schlag
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:05 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

! does not work here. So the solution here is:
https://unix.stackexchange.com/questions/60584/how-to-use-a-variable-as-part-of-an-array-name/60585#60585

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/lib.sh b/tests/lib.sh
index 006862da6..ad7c12cb0 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -28,7 +28,7 @@ var_has_value() {
 }
 
 test_that_key_in_arry_has_value() {
-	local array="${!1}"
+	local -n array="${1}"
 	local arrayname="${1}"
 	local key="${2}"
 	local value="${3}"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 10/21] test_that_key_in_arry_has_value: Check if a key in an array exists
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (8 preceding siblings ...)
  2024-05-20  9:05 ` [PATCH 09/21] tests/lib.sh: Use namref to access an array Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 11/21] tests: Add function to test the ouput of a bash function Jonatan Schlag
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/lib.sh b/tests/lib.sh
index ad7c12cb0..81bc1439c 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -38,7 +38,8 @@ test_that_key_in_arry_has_value() {
 		return 1
 	fi
 
-	if [[ ! -v "${array[${key}]}" ]]; then
+
+	if [[ "${array["${key}"]+_}" == ""  ]]; then
 		log_test_failed "The array does not contain the key '${key}', valid keys are: ${!array[*]}"
 		return 1
 	fi
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 11/21] tests: Add function to test the ouput of a bash function
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (9 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 10/21] test_that_key_in_arry_has_value: Check if a key in an array exists Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 12/21] initscript functions: add readhash Jonatan Schlag
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/tests/lib.sh b/tests/lib.sh
index 81bc1439c..9ce0201b1 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -52,3 +52,32 @@ test_that_key_in_arry_has_value() {
 		return 1
 	fi
 }
+
+test_that_output_is(){
+	local reference_output_file="${1}"
+	local file_descriptor="${2}"
+	shift
+	shift
+
+	local command="$@"
+
+	local temp="$(mktemp)"
+
+	case "${file_descriptor}" in
+		"stdout"|"1")
+			$command 1> "${temp}" 2>/dev/null
+			;;
+		"stderr"|"2")
+			$command 2> "${temp}" 1>/dev/null
+			;;
+	esac
+
+	if diff -u "${temp}" "${reference_output_file}" &> /dev/null; then
+		log_test_succeded "The output of command '${command}' on file descriptor '${file_descriptor}' is equal to the reference output."
+	else
+		log_test_failed "The output of command '${command}' on file descriptor '${file_descriptor}' is unequal to the reference output."
+		diff -u --color "${reference_output_file}" "${temp}"
+	fi
+
+	rm "${temp}"
+}
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 12/21] initscript functions: add readhash
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (10 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 11/21] tests: Add function to test the ouput of a bash function Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 13/21] initscript fkt: ignore blank lines in readhash Jonatan Schlag
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 2817 bytes --]

To avoid the usage of eval and to store the config in an key value
array, we introduce an new function. The tests only check if we
read the correct value to the correct variable.

One comment on the implementation as this has created some headache:

>From https://www.gnu.org/software/bash/manual/bash.html#Bourne-Shell-Builtins

	"When used in a function, declare makes each name local, as with the local command, unless the -g option is used."

So we need to use -g here

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions               | 15 +++++++++++++++
 tests/src/initscripts/system/functions/data/1  | 17 +++++++++++++++++
 tests/src/initscripts/system/functions/test.sh | 16 ++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 tests/src/initscripts/system/functions/data/1
 create mode 100755 tests/src/initscripts/system/functions/test.sh

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index b610143ab..44ce999d3 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -891,3 +891,18 @@ volume_fs_type() {
 
 	stat -f --format="%T" ${1}
 }
+
+readhash() {
+	local array="${1}"
+	local file="${2}"
+
+	declare -A -g "${array}"
+
+	local line
+	while read -r line; do
+		local key="${line%=*}"
+		local val="${line#*=}"
+
+		printf -v "${array}[${key}]" "%s" "${val}"
+	done < "${file}"
+}
diff --git a/tests/src/initscripts/system/functions/data/1 b/tests/src/initscripts/system/functions/data/1
new file mode 100644
index 000000000..8aca9422b
--- /dev/null
+++ b/tests/src/initscripts/system/functions/data/1
@@ -0,0 +1,17 @@
+CONFIG_TYPE=3
+GREEN_DEV=green0
+GREEN_MACADDR=00:c0:08:8a:a0:47
+GREEN_DRIVER=r8175
+RED_DEV=red0
+RED_MACADDR=00:c0:08:8a:a0:56
+RED_DRIVER=r8283
+BLUE_DEV='blue0 net0'
+BLUE_MACADDR=bc:30:7d:58:6b:e3
+BLUE_DRIVER=rt2800
+RED_DHCP_HOSTNAME=ipfire
+RED_DHCP_FORCE_MTU=
+RED_ADDRESS=0.0.0.0
+RED_NETMASK=0.0.0.0
+RED_TYPE=PPPOE
+RED_NETADDRESS=0.0.0.0
+
diff --git a/tests/src/initscripts/system/functions/test.sh b/tests/src/initscripts/system/functions/test.sh
new file mode 100755
index 000000000..ec502e199
--- /dev/null
+++ b/tests/src/initscripts/system/functions/test.sh
@@ -0,0 +1,16 @@
+#!/usr/bin/bash
+
+SCRIPT_PATH="$(dirname "$(readlink -f "$0")")"
+
+ROOT="$(readlink -f "${SCRIPT_PATH}/../../../../..")"
+
+. ${ROOT}/tests/lib.sh
+
+. ${ROOT}/src/initscripts/system/functions
+
+# read the date in
+readhash "CONFIG" "${SCRIPT_PATH}/data/1"
+
+# test if we read the correct data
+test_that_key_in_arry_has_value "CONFIG" "RED_DHCP_HOSTNAME" "ipfire"
+test_that_key_in_arry_has_value "CONFIG" "BLUE_MACADDR" "bc:30:7d:58:6b:e3"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 13/21] initscript fkt: ignore blank lines in readhash
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (11 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 12/21] initscript functions: add readhash Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 14/21] test: Add functions test_that_array_is_defined Jonatan Schlag
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions                            | 6 ++++++
 tests/src/initscripts/system/functions/data/1_output_stderr | 0
 tests/src/initscripts/system/functions/data/1_output_stdout | 0
 tests/src/initscripts/system/functions/test.sh              | 3 +++
 4 files changed, 9 insertions(+)
 create mode 100644 tests/src/initscripts/system/functions/data/1_output_stderr
 create mode 100644 tests/src/initscripts/system/functions/data/1_output_stdout

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index 44ce999d3..3f01be9e0 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -900,6 +900,12 @@ readhash() {
 
 	local line
 	while read -r line; do
+
+		# Skip Blank Lines
+		if [[ ${line} =~ ^[[:space:]]*$ ]]; then
+			continue
+		fi
+
 		local key="${line%=*}"
 		local val="${line#*=}"
 
diff --git a/tests/src/initscripts/system/functions/data/1_output_stderr b/tests/src/initscripts/system/functions/data/1_output_stderr
new file mode 100644
index 000000000..e69de29bb
diff --git a/tests/src/initscripts/system/functions/data/1_output_stdout b/tests/src/initscripts/system/functions/data/1_output_stdout
new file mode 100644
index 000000000..e69de29bb
diff --git a/tests/src/initscripts/system/functions/test.sh b/tests/src/initscripts/system/functions/test.sh
index ec502e199..8d644b8cd 100755
--- a/tests/src/initscripts/system/functions/test.sh
+++ b/tests/src/initscripts/system/functions/test.sh
@@ -14,3 +14,6 @@ readhash "CONFIG" "${SCRIPT_PATH}/data/1"
 # test if we read the correct data
 test_that_key_in_arry_has_value "CONFIG" "RED_DHCP_HOSTNAME" "ipfire"
 test_that_key_in_arry_has_value "CONFIG" "BLUE_MACADDR" "bc:30:7d:58:6b:e3"
+
+test_that_output_is "${SCRIPT_PATH}/data/1_output_stdout" "1" readhash "CONFIG" "${SCRIPT_PATH}/data/1"
+test_that_output_is "${SCRIPT_PATH}/data/1_output_stderr" "2" readhash "CONFIG" "${SCRIPT_PATH}/data/1"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 14/21] test: Add functions test_that_array_is_defined
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (12 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 13/21] initscript fkt: ignore blank lines in readhash Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 15/21] tests: Add functions test_that_array_doesnt_have_key Jonatan Schlag
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

we need this check in multiple places so it makes sense to move this to
a separate function.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/lib.sh b/tests/lib.sh
index 9ce0201b1..29f4e3b71 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -27,17 +27,25 @@ var_has_value() {
 	[[ "${!1}" == "${2}" ]]
 }
 
-test_that_key_in_arry_has_value() {
-	local -n array="${1}"
+test_that_array_is_defined() {
 	local arrayname="${1}"
-	local key="${2}"
-	local value="${3}"
 
 	if [[ ! "$(declare  -p "${arrayname}")" =~ "declare -a" && ! "$(declare  -p "${arrayname}")" =~ "declare -A" ]]; then
 		log_test_failed "The array '${1}' does not exists. The variable is not set."
 		return 1
+	else
+		log_test_succeded "The array '${1}' is defined."
+		return 0
 	fi
+}
+
+test_that_key_in_arry_has_value() {
+	local -n array="${1}"
+	local arrayname="${1}"
+	local key="${2}"
+	local value="${3}"
 
+	test_that_array_is_defined "${arrayname}"  || return 1
 
 	if [[ "${array["${key}"]+_}" == ""  ]]; then
 		log_test_failed "The array does not contain the key '${key}', valid keys are: ${!array[*]}"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 15/21] tests: Add functions test_that_array_doesnt_have_key
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (13 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 14/21] test: Add functions test_that_array_is_defined Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 16/21] initscripts fkt: Ignore comments in readhash Jonatan Schlag
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

Apparently we can set way more keys then I expected. So we need a
function to check that we do not set certain key. Some keys need to be
skipped.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 tests/lib.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/lib.sh b/tests/lib.sh
index 29f4e3b71..1eab92cf7 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -61,6 +61,24 @@ test_that_key_in_arry_has_value() {
 	fi
 }
 
+test_that_array_doesnt_have_key() {
+	local -n array="${1}"
+	local arrayname="${1}"
+	local key="${2}"
+
+	test_that_array_is_defined "${arrayname}"  || return 1
+
+	if [[ "${array["${key}"]+_}" == "_"  ]]; then
+		log_test_failed "The array '${arrayname}' does contain the key '${key}'."
+		return 1
+	else
+		log_test_succeded "The array '${arrayname}' does not contain the key '${key}'"
+		return 0
+	fi
+
+}
+
+
 test_that_output_is(){
 	local reference_output_file="${1}"
 	local file_descriptor="${2}"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 16/21] initscripts fkt: Ignore comments in readhash
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (14 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 15/21] tests: Add functions test_that_array_doesnt_have_key Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 17/21] initscripts fkt: ignore invalid keys " Jonatan Schlag
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]

As '#Another Comment' is a valid key we test this change by checking if
the comments do not end up as keys in our array.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions               | 5 +++++
 tests/src/initscripts/system/functions/data/1  | 3 +++
 tests/src/initscripts/system/functions/test.sh | 6 ++++++
 3 files changed, 14 insertions(+)

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index 3f01be9e0..6107463fc 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -906,6 +906,11 @@ readhash() {
 			continue
 		fi
 
+		# Skip Comments
+		if [[ ${line} =~ ^#.*$ ]]; then
+			continue
+		fi
+
 		local key="${line%=*}"
 		local val="${line#*=}"
 
diff --git a/tests/src/initscripts/system/functions/data/1 b/tests/src/initscripts/system/functions/data/1
index 8aca9422b..c75620b6b 100644
--- a/tests/src/initscripts/system/functions/data/1
+++ b/tests/src/initscripts/system/functions/data/1
@@ -5,6 +5,7 @@ GREEN_DRIVER=r8175
 RED_DEV=red0
 RED_MACADDR=00:c0:08:8a:a0:56
 RED_DRIVER=r8283
+# Another Comment
 BLUE_DEV='blue0 net0'
 BLUE_MACADDR=bc:30:7d:58:6b:e3
 BLUE_DRIVER=rt2800
@@ -15,3 +16,5 @@ RED_NETMASK=0.0.0.0
 RED_TYPE=PPPOE
 RED_NETADDRESS=0.0.0.0
 
+# Comment for testing  
+    # Comment for testing Comments with spaces before
diff --git a/tests/src/initscripts/system/functions/test.sh b/tests/src/initscripts/system/functions/test.sh
index 8d644b8cd..f1348a9e8 100755
--- a/tests/src/initscripts/system/functions/test.sh
+++ b/tests/src/initscripts/system/functions/test.sh
@@ -15,5 +15,11 @@ readhash "CONFIG" "${SCRIPT_PATH}/data/1"
 test_that_key_in_arry_has_value "CONFIG" "RED_DHCP_HOSTNAME" "ipfire"
 test_that_key_in_arry_has_value "CONFIG" "BLUE_MACADDR" "bc:30:7d:58:6b:e3"
 
+# Test that comments are skipped
+# apparently the way we read the file strips the whitespace, so the key does not contain any whitespace either
+test_that_array_doesnt_have_key "CONFIG" "# Another Comment"
+test_that_array_doesnt_have_key "CONFIG" "# Comment for testing"
+test_that_array_doesnt_have_key "CONFIG" "# Comment for testing Comments with spaces before"
+
 test_that_output_is "${SCRIPT_PATH}/data/1_output_stdout" "1" readhash "CONFIG" "${SCRIPT_PATH}/data/1"
 test_that_output_is "${SCRIPT_PATH}/data/1_output_stderr" "2" readhash "CONFIG" "${SCRIPT_PATH}/data/1"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 17/21] initscripts fkt: ignore invalid keys in readhash
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (15 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 16/21] initscripts fkt: Ignore comments in readhash Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 18/21] initscripts fkt: readhash should only parse lines with a = Jonatan Schlag
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 3564 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions              |  6 ++++++
 tests/src/initscripts/system/functions/data/2 | 20 +++++++++++++++++++
 .../system/functions/data/2_output_stderr     |  4 ++++
 .../system/functions/data/2_output_stdout     |  0
 .../src/initscripts/system/functions/test.sh  | 11 ++++++++++
 5 files changed, 41 insertions(+)
 create mode 100644 tests/src/initscripts/system/functions/data/2
 create mode 100644 tests/src/initscripts/system/functions/data/2_output_stderr
 create mode 100644 tests/src/initscripts/system/functions/data/2_output_stdout

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index 6107463fc..bbcfab95d 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -914,6 +914,12 @@ readhash() {
 		local key="${line%=*}"
 		local val="${line#*=}"
 
+		# Skip lines with an invalid key
+		if ! [[ ${key} =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then
+			echo "Invalid key '${key}'" >&2
+			continue
+		fi
+
 		printf -v "${array}[${key}]" "%s" "${val}"
 	done < "${file}"
 }
diff --git a/tests/src/initscripts/system/functions/data/2 b/tests/src/initscripts/system/functions/data/2
new file mode 100644
index 000000000..3e1a7028b
--- /dev/null
+++ b/tests/src/initscripts/system/functions/data/2
@@ -0,0 +1,20 @@
+CONFIG_TYPE=3
+GREEN_DEV=green0
+GREEN_MACADDR=00:c0:08:8a:a0:47
+GREEN_DRIVER=r8175
+-RED_DEV=red0
+RE??D_MACADDR=00:c0:08:8a:a0:56
+RED&&_DRIVER=r8283
+# Another Comment
+0BLUE_DEV='blue0 net0'
+BLUE_MACADDR=bc:30:7d:58:6b:e3
+BLUE_DRIVER=rt2800
+RED_DHCP_HOSTNAME=ipfire
+RED_DHCP_FORCE_MTU=
+RED_ADDRESS=0.0.0.0
+RED_NETMASK=0.0.0.0
+RED_TYPE=PPPOE
+RED_NETADDRESS=0.0.0.0
+
+# Comment for testing  
+    # Comment for testing Comments with spaces before
diff --git a/tests/src/initscripts/system/functions/data/2_output_stderr b/tests/src/initscripts/system/functions/data/2_output_stderr
new file mode 100644
index 000000000..dfcf2154b
--- /dev/null
+++ b/tests/src/initscripts/system/functions/data/2_output_stderr
@@ -0,0 +1,4 @@
+Invalid key '-RED_DEV'
+Invalid key 'RE??D_MACADDR'
+Invalid key 'RED&&_DRIVER'
+Invalid key '0BLUE_DEV'
diff --git a/tests/src/initscripts/system/functions/data/2_output_stdout b/tests/src/initscripts/system/functions/data/2_output_stdout
new file mode 100644
index 000000000..e69de29bb
diff --git a/tests/src/initscripts/system/functions/test.sh b/tests/src/initscripts/system/functions/test.sh
index f1348a9e8..9981cc732 100755
--- a/tests/src/initscripts/system/functions/test.sh
+++ b/tests/src/initscripts/system/functions/test.sh
@@ -23,3 +23,14 @@ test_that_array_doesnt_have_key "CONFIG" "# Comment for testing Comments with sp
 
 test_that_output_is "${SCRIPT_PATH}/data/1_output_stdout" "1" readhash "CONFIG" "${SCRIPT_PATH}/data/1"
 test_that_output_is "${SCRIPT_PATH}/data/1_output_stderr" "2" readhash "CONFIG" "${SCRIPT_PATH}/data/1"
+
+# Check with invalid Lines (values and keys)
+
+readhash "CONFIG2" "${SCRIPT_PATH}/data/2" &> /dev/null
+
+# test if we read the correct data
+test_that_key_in_arry_has_value "CONFIG2" "RED_DHCP_HOSTNAME" "ipfire"
+test_that_key_in_arry_has_value "CONFIG2" "BLUE_MACADDR" "bc:30:7d:58:6b:e3"
+
+test_that_output_is "${SCRIPT_PATH}/data/2_output_stdout" "1" readhash "CONFIG2" "${SCRIPT_PATH}/data/2"
+test_that_output_is "${SCRIPT_PATH}/data/2_output_stderr" "2" readhash "CONFIG2" "${SCRIPT_PATH}/data/2"
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 18/21] initscripts fkt: readhash should only parse lines with a =
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (16 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 17/21] initscripts fkt: ignore invalid keys " Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 19/21] initscripts fkt: Check for invalid values in readhash Jonatan Schlag
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]

A line without a = is clearly invalid.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions                            | 6 ++++++
 tests/src/initscripts/system/functions/data/2               | 1 +
 tests/src/initscripts/system/functions/data/2_output_stderr | 1 +
 3 files changed, 8 insertions(+)

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index bbcfab95d..38a1316a8 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -911,6 +911,12 @@ readhash() {
 			continue
 		fi
 
+		# Skip lines without a =
+		if ! [[ ${line} =~ [^=]*=[^=]*$ ]]; then
+			echo "Invalid line '${line}'" >&2
+			continue
+		fi
+
 		local key="${line%=*}"
 		local val="${line#*=}"
 
diff --git a/tests/src/initscripts/system/functions/data/2 b/tests/src/initscripts/system/functions/data/2
index 3e1a7028b..d1c4d327d 100644
--- a/tests/src/initscripts/system/functions/data/2
+++ b/tests/src/initscripts/system/functions/data/2
@@ -15,6 +15,7 @@ RED_ADDRESS=0.0.0.0
 RED_NETMASK=0.0.0.0
 RED_TYPE=PPPOE
 RED_NETADDRESS=0.0.0.0
+Line_without_a_equal_sign_is_also_invalid
 
 # Comment for testing  
     # Comment for testing Comments with spaces before
diff --git a/tests/src/initscripts/system/functions/data/2_output_stderr b/tests/src/initscripts/system/functions/data/2_output_stderr
index dfcf2154b..cccd19af6 100644
--- a/tests/src/initscripts/system/functions/data/2_output_stderr
+++ b/tests/src/initscripts/system/functions/data/2_output_stderr
@@ -2,3 +2,4 @@ Invalid key '-RED_DEV'
 Invalid key 'RE??D_MACADDR'
 Invalid key 'RED&&_DRIVER'
 Invalid key '0BLUE_DEV'
+Invalid line 'Line_without_a_equal_sign_is_also_invalid'
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 19/21] initscripts fkt: Check for invalid values in readhash
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (17 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 18/21] initscripts fkt: readhash should only parse lines with a = Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 20/21] initscripts fkt: keep readhash compatible with older implementation Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 21/21] initscripts fkt: Fix shebang Jonatan Schlag
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 2035 bytes --]

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions                          | 6 ++++++
 tests/src/initscripts/system/functions/data/2             | 8 ++++----
 .../src/initscripts/system/functions/data/2_output_stderr | 4 ++++
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index 38a1316a8..c9c4c0675 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -926,6 +926,12 @@ readhash() {
 			continue
 		fi
 
+		# Skip lines with invalid values
+		if ! [[ ${val} =~ ^[\'][\ A-Za-z0-9=/,.:%_@#+-]*[\']$ ]] && ! [[ ${val} =~ ^[A-Za-z0-9=/,.:%_@#+-]*$ ]]; then
+			echo "Invalid value '${val}' for key '${key}'" >&2
+			continue
+		fi
+
 		printf -v "${array}[${key}]" "%s" "${val}"
 	done < "${file}"
 }
diff --git a/tests/src/initscripts/system/functions/data/2 b/tests/src/initscripts/system/functions/data/2
index d1c4d327d..760294c30 100644
--- a/tests/src/initscripts/system/functions/data/2
+++ b/tests/src/initscripts/system/functions/data/2
@@ -1,7 +1,7 @@
-CONFIG_TYPE=3
-GREEN_DEV=green0
-GREEN_MACADDR=00:c0:08:8a:a0:47
-GREEN_DRIVER=r8175
+CONFIG_TYPE=?3
+GREEN_DEV=gree!n0
+GREEN_MACADDR=00:c0:08:8a  :a0:47
+GREEN_DRIVER="r8175"
 -RED_DEV=red0
 RE??D_MACADDR=00:c0:08:8a:a0:56
 RED&&_DRIVER=r8283
diff --git a/tests/src/initscripts/system/functions/data/2_output_stderr b/tests/src/initscripts/system/functions/data/2_output_stderr
index cccd19af6..82f035e26 100644
--- a/tests/src/initscripts/system/functions/data/2_output_stderr
+++ b/tests/src/initscripts/system/functions/data/2_output_stderr
@@ -1,3 +1,7 @@
+Invalid value '?3' for key 'CONFIG_TYPE'
+Invalid value 'gree!n0' for key 'GREEN_DEV'
+Invalid value '00:c0:08:8a  :a0:47' for key 'GREEN_MACADDR'
+Invalid value '"r8175"' for key 'GREEN_DRIVER'
 Invalid key '-RED_DEV'
 Invalid key 'RE??D_MACADDR'
 Invalid key 'RED&&_DRIVER'
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 20/21] initscripts fkt: keep readhash compatible with older implementation
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (18 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 19/21] initscripts fkt: Check for invalid values in readhash Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  2024-05-20  9:06 ` [PATCH 21/21] initscripts fkt: Fix shebang Jonatan Schlag
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

With the use of eval BLUE_DEV='blue0 net0' stored "blue0 net0" in the
variable BLUE_DEV not "'blue0 net0'"

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions               | 4 ++++
 tests/src/initscripts/system/functions/test.sh | 1 +
 2 files changed, 5 insertions(+)

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index c9c4c0675..6d72e4119 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -932,6 +932,10 @@ readhash() {
 			continue
 		fi
 
+		# strip leading and trailing single quotes
+		val="${val#\'}"
+		val="${val%\'}"
+
 		printf -v "${array}[${key}]" "%s" "${val}"
 	done < "${file}"
 }
diff --git a/tests/src/initscripts/system/functions/test.sh b/tests/src/initscripts/system/functions/test.sh
index 9981cc732..1a715d7d9 100755
--- a/tests/src/initscripts/system/functions/test.sh
+++ b/tests/src/initscripts/system/functions/test.sh
@@ -14,6 +14,7 @@ readhash "CONFIG" "${SCRIPT_PATH}/data/1"
 # test if we read the correct data
 test_that_key_in_arry_has_value "CONFIG" "RED_DHCP_HOSTNAME" "ipfire"
 test_that_key_in_arry_has_value "CONFIG" "BLUE_MACADDR" "bc:30:7d:58:6b:e3"
+test_that_key_in_arry_has_value "CONFIG" "BLUE_DEV" "blue0 net0"
 
 # Test that comments are skipped
 # apparently the way we read the file strips the whitespace, so the key does not contain any whitespace either
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 21/21] initscripts fkt: Fix shebang
  2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
                   ` (19 preceding siblings ...)
  2024-05-20  9:06 ` [PATCH 20/21] initscripts fkt: keep readhash compatible with older implementation Jonatan Schlag
@ 2024-05-20  9:06 ` Jonatan Schlag
  20 siblings, 0 replies; 24+ messages in thread
From: Jonatan Schlag @ 2024-05-20  9:06 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 899 bytes --]

We use features only available in bash. So we should state correctly
that the script should be executed in bash. As sh is a symlink to bash
this makes not differences on a ipfire system. But my linter is less
chatty with this change.

Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
 src/initscripts/system/functions | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index 6d72e4119..0775d74a1 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 ###############################################################################
 #                                                                             #
 # IPFire.org - A linux based firewall                                         #
-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

[parent not found: <01045B9E-DD44-47CA-A0DF-525E452C1D72@ipfire.org>]

* Re: [PATCH 02/21] tests: Add bash lib
       [not found] <01045B9E-DD44-47CA-A0DF-525E452C1D72@ipfire.org>
@ 2024-06-02 17:49 ` Jonatan Schlag
  2024-06-03  9:19   ` Michael Tremer
  0 siblings, 1 reply; 24+ messages in thread
From: Jonatan Schlag @ 2024-06-02 17:49 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]


Hi,


Am Freitag, dem 31.05.2024 um 10:47 +0100 schrieb Michael Tremer:
> Hello,
> 
> > On 20 May 2024, at 10:05, Jonatan Schlag
> > <jonatan.schlag(a)ipfire.org> wrote:
> > 
> > This allows use to write test with less effort as we can reuse
> > functions
> > 
> > Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> > ---
> > tests/lib.sh | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> > create mode 100644 tests/lib.sh
> > 
> > diff --git a/tests/lib.sh b/tests/lib.sh
> > new file mode 100644
> > index 000000000..7749d5158
> > --- /dev/null
> > +++ b/tests/lib.sh
> > @@ -0,0 +1,20 @@
> > +#!/usr/bin/bash
> > +
> > +LIB_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
> 
> ???
> 
> I don’t think this is very intuitive.

Ok, what am I supposed to do about it? I need the path where this file
is located, and I cannot hard-code it. Should I add a comment?

Jonatan

> 
> > +. ${LIB_DIR}/lib_color.sh
> > +
> > +test_that() {
> > +
> > + if ! "$@" ; then
> > + echo -e "${CLR_RED_BG} Test failed: ${*} ${CLR_RESET}"
> > + return 1
> > + else
> > + echo -e "${CLR_GREEN_BG} Test succeded: ${*} ${CLR_RESET}"
> > + return 0
> > + fi
> > +}
> 
> We have had so many discussions about how to name functions. “That”
> is quite generic in my opinion.
> 
> Why not “test_command” or something like that?
> 
> > +
> > +var_has_value() {
> > + [[ "${!1}" == "${2}" ]]
> > +}
> > -- 
> > 2.39.2
> > 
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 02/21] tests: Add bash lib
  2024-06-02 17:49 ` [PATCH 02/21] tests: Add bash lib Jonatan Schlag
@ 2024-06-03  9:19   ` Michael Tremer
  0 siblings, 0 replies; 24+ messages in thread
From: Michael Tremer @ 2024-06-03  9:19 UTC (permalink / raw)
  To: development

[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]

Hello,

> On 2 Jun 2024, at 18:49, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
> 
> 
> Hi,
> 
> 
> Am Freitag, dem 31.05.2024 um 10:47 +0100 schrieb Michael Tremer:
>> Hello,
>> 
>>> On 20 May 2024, at 10:05, Jonatan Schlag
>>> <jonatan.schlag(a)ipfire.org> wrote:
>>> 
>>> This allows use to write test with less effort as we can reuse
>>> functions
>>> 
>>> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
>>> ---
>>> tests/lib.sh | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>> create mode 100644 tests/lib.sh
>>> 
>>> diff --git a/tests/lib.sh b/tests/lib.sh
>>> new file mode 100644
>>> index 000000000..7749d5158
>>> --- /dev/null
>>> +++ b/tests/lib.sh
>>> @@ -0,0 +1,20 @@
>>> +#!/usr/bin/bash
>>> +
>>> +LIB_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
>> 
>> ???
>> 
>> I don’t think this is very intuitive.
> 
> Ok, what am I supposed to do about it? I need the path where this file
> is located, and I cannot hard-code it. Should I add a comment?

Generally speaking, I like to split things into one line per task. So that it is very easy to see what is happening when reading the code. This approach also has some more advantages. For example, you can easily put a print statement between lines to see if you are getting the output that you are expecting. That makes debugging nice and easy.

When I say generally, there are of course exceptions like very trivial things that can often be put into the same line.

Commenting is also a lot easier when you only have to comment one thing at once. You need to add way more comments, please.

-Michael

> Jonatan
> 
>> 
>>> +. ${LIB_DIR}/lib_color.sh
>>> +
>>> +test_that() {
>>> +
>>> + if ! "$@" ; then
>>> + echo -e "${CLR_RED_BG} Test failed: ${*} ${CLR_RESET}"
>>> + return 1
>>> + else
>>> + echo -e "${CLR_GREEN_BG} Test succeded: ${*} ${CLR_RESET}"
>>> + return 0
>>> + fi
>>> +}
>> 
>> We have had so many discussions about how to name functions. “That”
>> is quite generic in my opinion.
>> 
>> Why not “test_command” or something like that?
>> 
>>> +
>>> +var_has_value() {
>>> + [[ "${!1}" == "${2}" ]]
>>> +}
>>> -- 
>>> 2.39.2



^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-06-03  9:19 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-20  9:05 First patch series for the roadmap item "Get rid of CONFIG_TYPE in the network config" Jonatan Schlag
2024-05-20  9:05 ` [PATCH 01/21] test: Add bash lib for colors Jonatan Schlag
2024-05-20  9:05 ` [PATCH 02/21] tests: Add bash lib Jonatan Schlag
2024-05-20  9:05 ` [PATCH 03/21] tests/lib.sh: Add function test_that_key_in_arry_has_value Jonatan Schlag
2024-05-20  9:05 ` [PATCH 04/21] tests/lib.sh: Add check if variable exists to test_that_key_in_arry_has_value Jonatan Schlag
2024-05-20  9:05 ` [PATCH 05/21] tests/lib.sh: Add logging functions Jonatan Schlag
2024-05-20  9:05 ` [PATCH 06/21] tests/lib.sh: adjust to pytest logging style Jonatan Schlag
2024-05-20  9:05 ` [PATCH 07/21] tests/lib.sh: Fix check if array is defined Jonatan Schlag
2024-05-20  9:05 ` [PATCH 08/21] test_that_key_in_arry_has_value: Check if the key " Jonatan Schlag
2024-05-20  9:05 ` [PATCH 09/21] tests/lib.sh: Use namref to access an array Jonatan Schlag
2024-05-20  9:06 ` [PATCH 10/21] test_that_key_in_arry_has_value: Check if a key in an array exists Jonatan Schlag
2024-05-20  9:06 ` [PATCH 11/21] tests: Add function to test the ouput of a bash function Jonatan Schlag
2024-05-20  9:06 ` [PATCH 12/21] initscript functions: add readhash Jonatan Schlag
2024-05-20  9:06 ` [PATCH 13/21] initscript fkt: ignore blank lines in readhash Jonatan Schlag
2024-05-20  9:06 ` [PATCH 14/21] test: Add functions test_that_array_is_defined Jonatan Schlag
2024-05-20  9:06 ` [PATCH 15/21] tests: Add functions test_that_array_doesnt_have_key Jonatan Schlag
2024-05-20  9:06 ` [PATCH 16/21] initscripts fkt: Ignore comments in readhash Jonatan Schlag
2024-05-20  9:06 ` [PATCH 17/21] initscripts fkt: ignore invalid keys " Jonatan Schlag
2024-05-20  9:06 ` [PATCH 18/21] initscripts fkt: readhash should only parse lines with a = Jonatan Schlag
2024-05-20  9:06 ` [PATCH 19/21] initscripts fkt: Check for invalid values in readhash Jonatan Schlag
2024-05-20  9:06 ` [PATCH 20/21] initscripts fkt: keep readhash compatible with older implementation Jonatan Schlag
2024-05-20  9:06 ` [PATCH 21/21] initscripts fkt: Fix shebang Jonatan Schlag
     [not found] <01045B9E-DD44-47CA-A0DF-525E452C1D72@ipfire.org>
2024-06-02 17:49 ` [PATCH 02/21] tests: Add bash lib Jonatan Schlag
2024-06-03  9:19   ` Michael Tremer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox