A ipv4-static config with the same IPv4 address twice is senseless. A new function zone_config_check_same_setting is introduced. The function provides an easy way to check if a config of the given hook has the same value for a given key. We can now check inside hook_new if an ipv4-static or ipv6-static config with the same value exist and break with an error.
Fixes: #11418
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org --- src/functions/functions.zone | 33 +++++++++++++++++++++++++++++++++ src/hooks/configs/ipv4-static | 5 +++++ src/hooks/configs/ipv6-static | 5 +++++ 3 files changed, 43 insertions(+)
diff --git a/src/functions/functions.zone b/src/functions/functions.zone index d121225..768bb9f 100644 --- a/src/functions/functions.zone +++ b/src/functions/functions.zone @@ -1024,6 +1024,39 @@ zone_config_get_new_id() { done }
+zone_config_check_same_setting() { + # This functions checks if a config hook + # with the same setting is already configured for this zone. + # Returns True when yes and False when no. + + assert [ $# -eq 4 ] + + local zone=${1} + local hook=${2} + local key=${3} + local value=${4} + + # 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 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}" + # Check if the value of the config and the passed value are eqal + if [[ "${value}" == "${!key}" ]]; then + return ${EXIT_TRUE} + fi + done + + return ${EXIT_FALSE} +} + zone_config_get_hook() { assert [ $# -eq 2 ]
diff --git a/src/hooks/configs/ipv4-static b/src/hooks/configs/ipv4-static index c395200..9454282 100644 --- a/src/hooks/configs/ipv4-static +++ b/src/hooks/configs/ipv4-static @@ -102,6 +102,11 @@ hook_new() { exit ${EXIT_CONF_ERROR} fi
+ if zone_config_check_same_setting "${zone}" "ipv4-static" "ADDRESS" "${ADDRESS}"; then + error "An ipv4-static config with the same IPv4 address is already configured" + exit ${EXIT_CONF_ERROR} + fi + if ! isset GATEWAY && zone_is_nonlocal "${zone}"; then warning "You did not configure a gateway for a non-local zone" fi diff --git a/src/hooks/configs/ipv6-static b/src/hooks/configs/ipv6-static index f43ef7e..b0f6537 100644 --- a/src/hooks/configs/ipv6-static +++ b/src/hooks/configs/ipv6-static @@ -59,6 +59,11 @@ hook_new() { GATEWAY=$(ipv6_format "${GATEWAY}") fi
+ if zone_config_check_same_setting "${zone}" "ipv6-static" "ADDRESS" "${ADDRESS}"; then + error "An ipv6-static config with the same IPv6 address is already configured" + exit ${EXIT_CONF_ERROR} + fi + zone_config_settings_write "${zone}" "${HOOK}"
exit ${EXIT_OK}
Hi,
I think I might have an other idea, but I am not sure if it does not have many flaws. So here it is as RFC:
If we would implement a function called hook_hash() that returns something that is unique to that hook. If a second instance of a hook gets the same hash, we have a conflict.
That would allow us to check for conflicts across multiple variables, since this approach is hook-agnostic. That would make this very fast. As a default we could just check for all SETTINGS variables. For ipv4- static for example, we would return a string like "ipv4-static- ${ADDRESS}".
A downside is, that we kind of don't know where the conflict was which could lead to some senseless error messages. But we might be able to catch that in the hook that has just been called.
What do you think of that approach?
Best, -Michael
On Tue, 2017-07-11 at 15:41 +0200, Jonatan Schlag wrote:
A ipv4-static config with the same IPv4 address twice is senseless. A new function zone_config_check_same_setting is introduced. The function provides an easy way to check if a config of the given hook has the same value for a given key. We can now check inside hook_new if an ipv4-static or ipv6-static config with the same value exist and break with an error.
Fixes: #11418
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org
src/functions/functions.zone | 33 +++++++++++++++++++++++++++++++++ src/hooks/configs/ipv4-static | 5 +++++ src/hooks/configs/ipv6-static | 5 +++++ 3 files changed, 43 insertions(+)
diff --git a/src/functions/functions.zone b/src/functions/functions.zone index d121225..768bb9f 100644 --- a/src/functions/functions.zone +++ b/src/functions/functions.zone @@ -1024,6 +1024,39 @@ zone_config_get_new_id() { done } +zone_config_check_same_setting() {
- # This functions checks if a config hook
- # with the same setting is already configured for this zone.
- # Returns True when yes and False when no.
- assert [ $# -eq 4 ]
- local zone=${1}
- local hook=${2}
- local key=${3}
- local value=${4}
- # 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 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}"
# Check if the value of the config and the passed
value are eqal
if [[ "${value}" == "${!key}" ]]; then
return ${EXIT_TRUE}
fi
- done
- return ${EXIT_FALSE}
+}
zone_config_get_hook() { assert [ $# -eq 2 ] diff --git a/src/hooks/configs/ipv4-static b/src/hooks/configs/ipv4- static index c395200..9454282 100644 --- a/src/hooks/configs/ipv4-static +++ b/src/hooks/configs/ipv4-static @@ -102,6 +102,11 @@ hook_new() { exit ${EXIT_CONF_ERROR} fi
- if zone_config_check_same_setting "${zone}" "ipv4-static"
"ADDRESS" "${ADDRESS}"; then
error "An ipv4-static config with the same IPv4
address is already configured"
exit ${EXIT_CONF_ERROR}
- fi
if ! isset GATEWAY && zone_is_nonlocal "${zone}"; then warning "You did not configure a gateway for a non- local zone" fi diff --git a/src/hooks/configs/ipv6-static b/src/hooks/configs/ipv6- static index f43ef7e..b0f6537 100644 --- a/src/hooks/configs/ipv6-static +++ b/src/hooks/configs/ipv6-static @@ -59,6 +59,11 @@ hook_new() { GATEWAY=$(ipv6_format "${GATEWAY}") fi
- if zone_config_check_same_setting "${zone}" "ipv6-static"
"ADDRESS" "${ADDRESS}"; then
error "An ipv6-static config with the same IPv6
address is already configured"
exit ${EXIT_CONF_ERROR}
- fi
zone_config_settings_write "${zone}" "${HOOK}" exit ${EXIT_OK}
Hi,
I think this is another way with some problems which I want to describe shortly:
1. We would not get good error messages as you mentioned earlier, so we could just say "A conflict occured" In my current implementation, we could check against multiple values, by calling the function multiples times for different values. If a conflict occurs we can say exactly where it is.
2. Also with this implementation, we are able to check against multiple values as stated above.
3. How could we check against another hook?
So I would say we go with this patch and if we need this hash function we will implement it when we need it.
Regards Jonatan
Am Do, 13. Jul, 2017 um 12:09 schrieb Michael Tremer michael.tremer@ipfire.org:
Hi,
I think I might have an other idea, but I am not sure if it does not have many flaws. So here it is as RFC:
If we would implement a function called hook_hash() that returns something that is unique to that hook. If a second instance of a hook gets the same hash, we have a conflict.
That would allow us to check for conflicts across multiple variables, since this approach is hook-agnostic. That would make this very fast. As a default we could just check for all SETTINGS variables. For ipv4- static for example, we would return a string like "ipv4-static- ${ADDRESS}".
A downside is, that we kind of don't know where the conflict was which could lead to some senseless error messages. But we might be able to catch that in the hook that has just been called.
What do you think of that approach?
Best, -Michael
On Tue, 2017-07-11 at 15:41 +0200, Jonatan Schlag wrote:
A ipv4-static config with the same IPv4 address twice is senseless. A new function zone_config_check_same_setting is introduced. The function provides an easy way to check if a config of the given hook has the same value for a given key. We can now check inside hook_new if an ipv4-static or ipv6-static config with the same value exist and break with an error.
Fixes: #11418
Signed-off-by: Jonatan Schlag jonatan.schlag@ipfire.org
src/functions/functions.zone | 33 +++++++++++++++++++++++++++++++++ src/hooks/configs/ipv4-static | 5 +++++ src/hooks/configs/ipv6-static | 5 +++++ 3 files changed, 43 insertions(+)
diff --git a/src/functions/functions.zone b/src/functions/functions.zone index d121225..768bb9f 100644 --- a/src/functions/functions.zone +++ b/src/functions/functions.zone @@ -1024,6 +1024,39 @@ zone_config_get_new_id() { done }
+zone_config_check_same_setting() {
- # This functions checks if a config hook
- # with the same setting is already configured for this zone.
- # Returns True when yes and False when no.
- assert [ $# -eq 4 ]
- local zone=${1}
- local hook=${2}
- local key=${3}
- local value=${4}
- # 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 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}"
# Check if the value of the config and the passed
value are eqal
if [[ "${value}" == "${!key}" ]]; then
return ${EXIT_TRUE}
fi
- done
- return ${EXIT_FALSE}
+}
zone_config_get_hook() { assert [ $# -eq 2 ]
diff --git a/src/hooks/configs/ipv4-static b/src/hooks/configs/ipv4- static index c395200..9454282 100644 --- a/src/hooks/configs/ipv4-static +++ b/src/hooks/configs/ipv4-static @@ -102,6 +102,11 @@ hook_new() { exit ${EXIT_CONF_ERROR} fi
- if zone_config_check_same_setting "${zone}" "ipv4-static"
"ADDRESS" "${ADDRESS}"; then
error "An ipv4-static config with the same IPv4
address is already configured"
exit ${EXIT_CONF_ERROR}
- fi
- if ! isset GATEWAY && zone_is_nonlocal "${zone}"; then warning "You did not configure a gateway for a non-
local zone" fi diff --git a/src/hooks/configs/ipv6-static b/src/hooks/configs/ipv6- static index f43ef7e..b0f6537 100644 --- a/src/hooks/configs/ipv6-static +++ b/src/hooks/configs/ipv6-static @@ -59,6 +59,11 @@ hook_new() { GATEWAY=$(ipv6_format "${GATEWAY}") fi
- if zone_config_check_same_setting "${zone}" "ipv6-static"
"ADDRESS" "${ADDRESS}"; then
error "An ipv6-static config with the same IPv6
address is already configured"
exit ${EXIT_CONF_ERROR}
fi
zone_config_settings_write "${zone}" "${HOOK}"
exit ${EXIT_OK}