Fix zone_config_check_same_setting

Message ID 1517165962-12420-1-git-send-email-jonatan.schlag@ipfire.org
State New
Headers
Series Fix zone_config_check_same_setting |

Commit Message

Jonatan Schlag Jan. 29, 2018, 5:59 a.m. UTC
  Every time we edited a config zone_config_check_same_setting
returns that a identical config was found but this config was the config
we want to edit. So we now generate the id inside hook_new and pass the
id always to hook_parse_cmdline and to zone_config_check_same_setting.

So we cann skip this config.

Fixes: #11451

Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
---
 src/functions/functions.zone   | 18 +++++++++++-------
 src/header-config              |  2 +-
 src/hooks/configs/dhcp         | 10 ++++++++--
 src/hooks/configs/ipv4-static  | 12 +++++++++---
 src/hooks/configs/ipv6-auto    | 10 ++++++++--
 src/hooks/configs/ipv6-static  | 12 +++++++++---
 src/hooks/configs/pppoe-server | 10 ++++++++--
 7 files changed, 54 insertions(+), 20 deletions(-)
  

Comments

Michael Tremer Feb. 6, 2018, 11:58 a.m. UTC | #1
Can you explain this again. I do not really understand what problem you are
trying to solve here.

-Michael

On Sun, 2018-01-28 at 18:59 +0000, Jonatan Schlag wrote:
> Every time we edited a config zone_config_check_same_setting
> returns that a identical config was found but this config was the config
> we want to edit. So we now generate the id inside hook_new and pass the
> id always to hook_parse_cmdline and to zone_config_check_same_setting.
> 
> So we cann skip this config.
> 
> Fixes: #11451
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag@ipfire.org>
> ---
>  src/functions/functions.zone   | 18 +++++++++++-------
>  src/header-config              |  2 +-
>  src/hooks/configs/dhcp         | 10 ++++++++--
>  src/hooks/configs/ipv4-static  | 12 +++++++++---
>  src/hooks/configs/ipv6-auto    | 10 ++++++++--
>  src/hooks/configs/ipv6-static  | 12 +++++++++---
>  src/hooks/configs/pppoe-server | 10 ++++++++--
>  7 files changed, 54 insertions(+), 20 deletions(-)
> 
> diff --git a/src/functions/functions.zone b/src/functions/functions.zone
> index 2d3d2c7..0d767cf 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -1098,22 +1098,29 @@ zone_config_check_same_setting() {
>  	# with the same setting is already configured for this zone.
>  	# Returns True when yes and False when no.
>  
> -	assert [ $# -eq 4 ]
> +	assert [ $# -eq 5 ]
>  
>  	local zone=${1}
>  	local hook=${2}
> -	local key=${3}
> -	local value=${4}
> +	local id=${3}
> +	local key=${4}
> +	local value=${5}
>  
>  	# The key should be local for this function
>  	local ${key}
>  	local config
>  
>  	for config in $(zone_configs_list ${zone}); do
> +		# Check if the config is eqal with the config we want to
> edit, when continue
> +		if [[ "${config}" = "${hook}.${id}" ]]; then
> +			continue
> +		fi
> +
>  		# Check if the config is from the given hook, when not
> continue
>  		if  [[ $(zone_config_get_hook "${zone}" "${config}") !=
> ${hook} ]]; then
>  			continue
>  		fi
> +
>  		# Get the value of the key for a given function
>  		zone_config_settings_read "${zone}" "${config}" \
>                  --ignore-superfluous-settings "${key}"
> @@ -1346,10 +1353,7 @@ zone_config_settings_write() {
>  	local hook="${2}"
>  	local id=${3}
>  
> -	if ! isset id; then
> -		id=$(zone_config_get_new_id ${zone})
> -		log DEBUG "ID for the config is: ${id}"
> -	fi
> +	assert isset id
>  
>  	local args
>  	if function_exists "hook_check_config_settings"; then
> diff --git a/src/header-config b/src/header-config
> index ec85a70..10b0279 100644
> --- a/src/header-config
> +++ b/src/header-config
> @@ -55,7 +55,7 @@ hook_edit() {
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	if ! hook_parse_cmdline "$@"; then
> +	if ! hook_parse_cmdline "${id}" "$@"; then
>  		# Return an error if the parsing of the cmd line fails
>  		return ${EXIT_ERROR}
>  	fi
> diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp
> index f22507c..40c6b48 100644
> --- a/src/hooks/configs/dhcp
> +++ b/src/hooks/configs/dhcp
> @@ -35,6 +35,9 @@ hook_check_config_settings() {
>  }
>  
>  hook_parse_cmdline() {
> +	local id="${1}"
> +	shift
> +
>  	while [ $# -gt 0 ]; do
>  		case "${1}" in
>  			--enable-ipv6)
> @@ -72,12 +75,15 @@ hook_new() {
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	if ! hook_parse_cmdline "$@"; then
> +	id=$(zone_config_get_new_id ${zone})
> +	log DEBUG "ID for the config is: ${id}"
> +
> +	if ! hook_parse_cmdline "${id}" "$@"; then
>  		# Return an error if the parsing of the cmd line fails
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	zone_config_settings_write "${zone}" "${HOOK}"
> +	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
>  
>  	exit ${EXIT_OK}
>  }
> diff --git a/src/hooks/configs/ipv4-static b/src/hooks/configs/ipv4-static
> index 7aea0b9..0c16322 100644
> --- a/src/hooks/configs/ipv4-static
> +++ b/src/hooks/configs/ipv4-static
> @@ -36,6 +36,9 @@ hook_check_config_settings() {
>  }
>  
>  hook_parse_cmdline() {
> +	local id="${1}"
> +	shift
> +
>  	local arg
>  
>  	while read -r arg; do
> @@ -99,7 +102,7 @@ hook_parse_cmdline() {
>  		exit ${EXIT_CONF_ERROR}
>  	fi
>  
> -	if zone_config_check_same_setting "${zone}" "ipv4-static" "ADDRESS"
> "${ADDRESS}"; then
> +	if zone_config_check_same_setting "${zone}" "ipv4-static" "${id}"
> "ADDRESS" "${ADDRESS}"; then
>  		error "An ipv4-static config with the same IPv4 address is
> already configured"
>  		exit ${EXIT_CONF_ERROR}
>  	fi
> @@ -115,12 +118,15 @@ hook_new() {
>  
>  	assert zone_exists "${zone}"
>  
> -	if ! hook_parse_cmdline "$@"; then
> +	id=$(zone_config_get_new_id ${zone})
> +	log DEBUG "ID for the config is: ${id}"
> +
> +	if ! hook_parse_cmdline "${id}" "$@"; then
>  		# Return an error if the parsing of the cmd line fails
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	zone_config_settings_write "${zone}" "${HOOK}"
> +	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
>  
>  	exit ${EXIT_OK}
>  }
> diff --git a/src/hooks/configs/ipv6-auto b/src/hooks/configs/ipv6-auto
> index bf1003d..b6301b0 100644
> --- a/src/hooks/configs/ipv6-auto
> +++ b/src/hooks/configs/ipv6-auto
> @@ -31,6 +31,9 @@ hook_check_config_settings() {
>  }
>  
>  hook_parse_cmdline() {
> +	local id="${1}"
> +	shift
> +
>  	local arg
>  
>  	while read arg; do
> @@ -57,12 +60,15 @@ hook_new() {
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	if ! hook_parse_cmdline "$@"; then
> +	id=$(zone_config_get_new_id ${zone})
> +	log DEBUG "ID for the config is: ${id}"
> +
> +	if ! hook_parse_cmdline "${id}" "$@"; then
>  		# Return an error if the parsing of the cmd line fails
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	zone_config_settings_write "${zone}" "${HOOK}"
> +	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
>  
>  	exit ${EXIT_OK}
>  }
> diff --git a/src/hooks/configs/ipv6-static b/src/hooks/configs/ipv6-static
> index 273c201..8f94819 100644
> --- a/src/hooks/configs/ipv6-static
> +++ b/src/hooks/configs/ipv6-static
> @@ -34,6 +34,9 @@ hook_check_config_settings() {
>  }
>  
>  hook_parse_cmdline() {
> +	local id="${1}"
> +	shift
> +
>  	while [ $# -gt 0 ]; do
>  		case "${1}" in
>  			--address=*)
> @@ -49,7 +52,7 @@ hook_parse_cmdline() {
>  		shift
>  	done
>  
> -	if zone_config_check_same_setting "${zone}" "ipv6-static" "ADDRESS"
> "${ADDRESS}"; then
> +	if zone_config_check_same_setting "${zone}" "ipv6-static" "${id}"
> "ADDRESS" "${ADDRESS}"; then
>  		error "An ipv6-static config with the same IPv6 address is
> already configured"
>  		exit ${EXIT_CONF_ERROR}
>  	fi
> @@ -66,12 +69,15 @@ hook_new() {
>  	local zone=${1}
>  	shift
>  
> -	if ! hook_parse_cmdline "$@"; then
> +	id=$(zone_config_get_new_id ${zone})
> +	log DEBUG "ID for the config is: ${id}"
> +
> +	if ! hook_parse_cmdline "${id}" "$@"; then
>  		# Return an error if the parsing of the cmd line fails
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	zone_config_settings_write "${zone}" "${HOOK}"
> +	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
>  
>  	exit ${EXIT_OK}
>  }
> diff --git a/src/hooks/configs/pppoe-server b/src/hooks/configs/pppoe-server
> index 7021be2..35de2eb 100644
> --- a/src/hooks/configs/pppoe-server
> +++ b/src/hooks/configs/pppoe-server
> @@ -49,6 +49,9 @@ hook_check_config_settings() {
>  }
>  
>  hook_parse_cmdline() {
> +	local id="${1}"
> +	shift
> +
>  	while [ $# -gt 0 ]; do
>  		case "${1}" in
>  			--dns-server=*)
> @@ -105,12 +108,15 @@ hook_new() {
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	if ! hook_parse_cmdline "$@"; then
> +	id=$(zone_config_get_new_id ${zone})
> +	log DEBUG "ID for the config is: ${id}"
> +
> +	if ! hook_parse_cmdline "${id}" "$@"; then
>  		# Return an error if the parsing of the cmd line fails
>  		return ${EXIT_ERROR}
>  	fi
>  
> -	zone_config_settings_write "${zone}" "${HOOK}"
> +	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
>  
>  	exit ${EXIT_OK}
>  }
  

Patch

diff --git a/src/functions/functions.zone b/src/functions/functions.zone
index 2d3d2c7..0d767cf 100644
--- a/src/functions/functions.zone
+++ b/src/functions/functions.zone
@@ -1098,22 +1098,29 @@  zone_config_check_same_setting() {
 	# with the same setting is already configured for this zone.
 	# Returns True when yes and False when no.
 
-	assert [ $# -eq 4 ]
+	assert [ $# -eq 5 ]
 
 	local zone=${1}
 	local hook=${2}
-	local key=${3}
-	local value=${4}
+	local id=${3}
+	local key=${4}
+	local value=${5}
 
 	# The key should be local for this function
 	local ${key}
 	local config
 
 	for config in $(zone_configs_list ${zone}); do
+		# Check if the config is eqal with the config we want to edit, when continue
+		if [[ "${config}" = "${hook}.${id}" ]]; then
+			continue
+		fi
+
 		# Check if the config is from the given hook, when not continue
 		if  [[ $(zone_config_get_hook "${zone}" "${config}") != ${hook} ]]; then
 			continue
 		fi
+
 		# Get the value of the key for a given function
 		zone_config_settings_read "${zone}" "${config}" \
                 --ignore-superfluous-settings "${key}"
@@ -1346,10 +1353,7 @@  zone_config_settings_write() {
 	local hook="${2}"
 	local id=${3}
 
-	if ! isset id; then
-		id=$(zone_config_get_new_id ${zone})
-		log DEBUG "ID for the config is: ${id}"
-	fi
+	assert isset id
 
 	local args
 	if function_exists "hook_check_config_settings"; then
diff --git a/src/header-config b/src/header-config
index ec85a70..10b0279 100644
--- a/src/header-config
+++ b/src/header-config
@@ -55,7 +55,7 @@  hook_edit() {
 		return ${EXIT_ERROR}
 	fi
 
-	if ! hook_parse_cmdline "$@"; then
+	if ! hook_parse_cmdline "${id}" "$@"; then
 		# Return an error if the parsing of the cmd line fails
 		return ${EXIT_ERROR}
 	fi
diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp
index f22507c..40c6b48 100644
--- a/src/hooks/configs/dhcp
+++ b/src/hooks/configs/dhcp
@@ -35,6 +35,9 @@  hook_check_config_settings() {
 }
 
 hook_parse_cmdline() {
+	local id="${1}"
+	shift
+
 	while [ $# -gt 0 ]; do
 		case "${1}" in
 			--enable-ipv6)
@@ -72,12 +75,15 @@  hook_new() {
 		return ${EXIT_ERROR}
 	fi
 
-	if ! hook_parse_cmdline "$@"; then
+	id=$(zone_config_get_new_id ${zone})
+	log DEBUG "ID for the config is: ${id}"
+
+	if ! hook_parse_cmdline "${id}" "$@"; then
 		# Return an error if the parsing of the cmd line fails
 		return ${EXIT_ERROR}
 	fi
 
-	zone_config_settings_write "${zone}" "${HOOK}"
+	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
 
 	exit ${EXIT_OK}
 }
diff --git a/src/hooks/configs/ipv4-static b/src/hooks/configs/ipv4-static
index 7aea0b9..0c16322 100644
--- a/src/hooks/configs/ipv4-static
+++ b/src/hooks/configs/ipv4-static
@@ -36,6 +36,9 @@  hook_check_config_settings() {
 }
 
 hook_parse_cmdline() {
+	local id="${1}"
+	shift
+
 	local arg
 
 	while read -r arg; do
@@ -99,7 +102,7 @@  hook_parse_cmdline() {
 		exit ${EXIT_CONF_ERROR}
 	fi
 
-	if zone_config_check_same_setting "${zone}" "ipv4-static" "ADDRESS" "${ADDRESS}"; then
+	if zone_config_check_same_setting "${zone}" "ipv4-static" "${id}" "ADDRESS" "${ADDRESS}"; then
 		error "An ipv4-static config with the same IPv4 address is already configured"
 		exit ${EXIT_CONF_ERROR}
 	fi
@@ -115,12 +118,15 @@  hook_new() {
 
 	assert zone_exists "${zone}"
 
-	if ! hook_parse_cmdline "$@"; then
+	id=$(zone_config_get_new_id ${zone})
+	log DEBUG "ID for the config is: ${id}"
+
+	if ! hook_parse_cmdline "${id}" "$@"; then
 		# Return an error if the parsing of the cmd line fails
 		return ${EXIT_ERROR}
 	fi
 
-	zone_config_settings_write "${zone}" "${HOOK}"
+	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
 
 	exit ${EXIT_OK}
 }
diff --git a/src/hooks/configs/ipv6-auto b/src/hooks/configs/ipv6-auto
index bf1003d..b6301b0 100644
--- a/src/hooks/configs/ipv6-auto
+++ b/src/hooks/configs/ipv6-auto
@@ -31,6 +31,9 @@  hook_check_config_settings() {
 }
 
 hook_parse_cmdline() {
+	local id="${1}"
+	shift
+
 	local arg
 
 	while read arg; do
@@ -57,12 +60,15 @@  hook_new() {
 		return ${EXIT_ERROR}
 	fi
 
-	if ! hook_parse_cmdline "$@"; then
+	id=$(zone_config_get_new_id ${zone})
+	log DEBUG "ID for the config is: ${id}"
+
+	if ! hook_parse_cmdline "${id}" "$@"; then
 		# Return an error if the parsing of the cmd line fails
 		return ${EXIT_ERROR}
 	fi
 
-	zone_config_settings_write "${zone}" "${HOOK}"
+	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
 
 	exit ${EXIT_OK}
 }
diff --git a/src/hooks/configs/ipv6-static b/src/hooks/configs/ipv6-static
index 273c201..8f94819 100644
--- a/src/hooks/configs/ipv6-static
+++ b/src/hooks/configs/ipv6-static
@@ -34,6 +34,9 @@  hook_check_config_settings() {
 }
 
 hook_parse_cmdline() {
+	local id="${1}"
+	shift
+
 	while [ $# -gt 0 ]; do
 		case "${1}" in
 			--address=*)
@@ -49,7 +52,7 @@  hook_parse_cmdline() {
 		shift
 	done
 
-	if zone_config_check_same_setting "${zone}" "ipv6-static" "ADDRESS" "${ADDRESS}"; then
+	if zone_config_check_same_setting "${zone}" "ipv6-static" "${id}" "ADDRESS" "${ADDRESS}"; then
 		error "An ipv6-static config with the same IPv6 address is already configured"
 		exit ${EXIT_CONF_ERROR}
 	fi
@@ -66,12 +69,15 @@  hook_new() {
 	local zone=${1}
 	shift
 
-	if ! hook_parse_cmdline "$@"; then
+	id=$(zone_config_get_new_id ${zone})
+	log DEBUG "ID for the config is: ${id}"
+
+	if ! hook_parse_cmdline "${id}" "$@"; then
 		# Return an error if the parsing of the cmd line fails
 		return ${EXIT_ERROR}
 	fi
 
-	zone_config_settings_write "${zone}" "${HOOK}"
+	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
 
 	exit ${EXIT_OK}
 }
diff --git a/src/hooks/configs/pppoe-server b/src/hooks/configs/pppoe-server
index 7021be2..35de2eb 100644
--- a/src/hooks/configs/pppoe-server
+++ b/src/hooks/configs/pppoe-server
@@ -49,6 +49,9 @@  hook_check_config_settings() {
 }
 
 hook_parse_cmdline() {
+	local id="${1}"
+	shift
+
 	while [ $# -gt 0 ]; do
 		case "${1}" in
 			--dns-server=*)
@@ -105,12 +108,15 @@  hook_new() {
 		return ${EXIT_ERROR}
 	fi
 
-	if ! hook_parse_cmdline "$@"; then
+	id=$(zone_config_get_new_id ${zone})
+	log DEBUG "ID for the config is: ${id}"
+
+	if ! hook_parse_cmdline "${id}" "$@"; then
 		# Return an error if the parsing of the cmd line fails
 		return ${EXIT_ERROR}
 	fi
 
-	zone_config_settings_write "${zone}" "${HOOK}"
+	zone_config_settings_write "${zone}" "${HOOK}" "${id}"
 
 	exit ${EXIT_OK}
 }