From b99ca3cbdf7c761db05598e31e9788e5e4e81cc6 Mon Sep 17 00:00:00 2001
From: Dennis Camera <dennis.camera@ssrq-sds-fds.ch>
Date: Sat, 24 Oct 2020 16:01:02 +0200
Subject: [PATCH] [type/__uci_section] Split up --option and --list

---
 cdist/conf/type/__uci_section/man.rst         | 19 +++++++-
 cdist/conf/type/__uci_section/manifest        | 48 ++++++++++++-------
 .../__uci_section/parameter/optional_multiple |  1 +
 3 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/cdist/conf/type/__uci_section/man.rst b/cdist/conf/type/__uci_section/man.rst
index 86729316..b8829433 100644
--- a/cdist/conf/type/__uci_section/man.rst
+++ b/cdist/conf/type/__uci_section/man.rst
@@ -28,15 +28,21 @@ None.
 
 OPTIONAL PARAMETERS
 -------------------
+list
+    An option that is part of a list and should be present in the section (as
+    part of a list).  Lists with multiple options can be expressed by using the
+    same ``<option>`` repeatedly.
+
+    The value to this parameter is a ``<option>=<value>`` string.
 match
     Allows to find a section to "replace" through one of its parameters.
+
     The value to this parameter is a ``<option>=<value>`` string.
 option
     An option that should be present in the section.
     This parameter can be used multiple times to specify multiple options.
-    The value to this parameter is a ``<option>=<value>`` string.
 
-    Lists can be expressed by repeatedly using the same key.
+    The value to this parameter is a ``<option>=<value>`` string.
 state
     ``present`` or ``absent``, defaults to ``present``.
 transaction
@@ -62,6 +68,15 @@ EXAMPLES
         --option PasswordAuth=off \
         --option RootPasswordAuth=off
 
+    # Define a firewall zone comprised of lan and wlan networks
+    __uci_section firewall.internal --type firewall.zone --transaction fw \
+        --option name='internal' \
+        --list network='lan' \
+        --list network='wlan' \
+        --option input='ACCEPT' \
+        --option output='ACCEPT' \
+        --option forward='ACCEPT'
+
     # Block SSH access from the guest network
     __uci_section firewall.block_ssh_from_guest --type firewall.rule \
         --transaction fwrules \
diff --git a/cdist/conf/type/__uci_section/manifest b/cdist/conf/type/__uci_section/manifest
index b5fa4a3b..a7865315 100755
--- a/cdist/conf/type/__uci_section/manifest
+++ b/cdist/conf/type/__uci_section/manifest
@@ -18,7 +18,14 @@
 # along with cdist. If not, see <http://www.gnu.org/licenses/>.
 #
 
-grep_line() { echo "$2" | grep -qxF "$1"; }
+grep_line() { { shift; printf '%s\n' "$@"; } | grep -qxF "$1"; }
+prefix_lines() {
+	while test $# -gt 0
+	do
+		echo "$2" | awk -v prefix="$1" '$0 { printf "%s %s\n", prefix, $0 }'
+		shift; shift
+	done
+}
 uci_validate_name() {
 	# like util.c uci_validate_name()
 	test -n "$*" && test -z "$(printf %s "$*" | tr -d '[:alnum:]_' | tr -c '' .)"
@@ -115,14 +122,25 @@ in
 		fi
 
 		# Check options for syntax errors
-		validate_options "${__object:?}/parameter/object" \
+		validate_options "${__object:?}/parameter/list" "${__object:?}/parameter/object" \
 		| print_errors 'Found erroneous options in arguments:'
 
 		# Collect option names
+		if test -f "${__object:?}/parameter/list"
+		then
+			listnames_should=$(
+				sed -e 's/=.*$//' "${__object:?}/parameter/list" | sort -u)
+		fi
+
 		if test -f "${__object:?}/parameter/option"
 		then
-			optnames_should=$(
-				sed -e 's/=.*$//' "${__object:?}/parameter/option" | sort -u)
+			optnames_should=$(sed -e 's/=.*$//' "${__object:?}/parameter/option" | sort)
+
+			echo "${optnames_should}" \
+			| uniq -d \
+			| print_errors \
+				  'Found duplicate --options:' \
+				  "$(printf '\nUse --list for lists, instead.')"
 		fi
 
 		# Make sure the section itself is present
@@ -130,29 +148,27 @@ in
 			--value "${sect_type}"
 		export require=__uci/"${section}"
 
-		# Delete options not in "should"
+		# Delete options/lists not in "should"
 		sed -e 's/=.*$//;s/^.*\.//' "${__object:?}/explorer/options" \
 		| while read -r optname
 		  do
-			  if ! grep_line "${optname}" "${optnames_should}"
-			  then
-				  __uci "${section}.${optname}" --state absent \
-					  --transaction "${transaction_name}" </dev/null
-			  fi
+			  grep_line "${optname}" "${listnames_should}" "${optnames_should}" || {
+				  __uci "${section}.${optname}" --state absent --transaction "${transaction_name}"
+			  } </dev/null
 		  done
 
 		# Set "should" options
-		echo "${optnames_should}" \
-		| while read -r optname
+		prefix_lines option "${optnames_should}" list "${listnames_should}" \
+		| while read -r _type _optname
 		  do
-			  test -n "${optname}" || continue  # ignore empty lines
+			  test -n "${_type}${_optname}" || continue  # ignore empty lines
 
-			  grep "^${optname}=" <"${__object:?}/parameter/option" \
+			  grep "^${_optname}=" <"${__object:?}/parameter/${_type}" \
 			  | sed -e 's/^.*=//' \
 			  | unquote_lines \
 			  | append_values \
-				    __uci "${section}.${optname}" --state present \
-					    --transaction "${transaction_name}"
+				    __uci "${section}.${_optname}" --state present \
+					    --type "${_type}" --transaction "${transaction_name}"
 		  done
 		;;
 	(absent)
diff --git a/cdist/conf/type/__uci_section/parameter/optional_multiple b/cdist/conf/type/__uci_section/parameter/optional_multiple
index 01925a15..98af35e6 100644
--- a/cdist/conf/type/__uci_section/parameter/optional_multiple
+++ b/cdist/conf/type/__uci_section/parameter/optional_multiple
@@ -1 +1,2 @@
+list
 option