* [PATCH 2/5] zone: zone_config_settings_write never generate an id
2017-08-12 10:22 [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit Jonatan Schlag
@ 2017-08-12 10:22 ` Jonatan Schlag
2017-08-12 10:22 ` [PATCH 3/5] header-config: pass id to hook_parse_cmdline Jonatan Schlag
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jonatan Schlag @ 2017-08-12 10:22 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
We now generate the new id inside the hook_new function.
This is required because the hook_parse_cmdline
function take now the id of the config as te first argument.
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/functions/functions.zone | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/functions/functions.zone b/src/functions/functions.zone
index 941314d..34cc983 100644
--- a/src/functions/functions.zone
+++ b/src/functions/functions.zone
@@ -1410,16 +1410,13 @@ zone_config_settings_read() {
}
zone_config_settings_write() {
- assert [ $# -ge 2 ]
+ assert [ $# -eq 3 ]
local zone="${1}"
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 isinteger id
local args
if function_exists "hook_check_config_settings"; then
--
2.6.3
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/5] header-config: pass id to hook_parse_cmdline
2017-08-12 10:22 [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit Jonatan Schlag
2017-08-12 10:22 ` [PATCH 2/5] zone: zone_config_settings_write never generate an id Jonatan Schlag
@ 2017-08-12 10:22 ` Jonatan Schlag
2017-08-12 10:22 ` [PATCH 4/5] config hooks: hook_parse_cmdline take id as an argument Jonatan Schlag
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jonatan Schlag @ 2017-08-12 10:22 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 504 bytes --]
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
| 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--git a/src/header-config b/src/header-config
index 6341a22..815d417 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
--
2.6.3
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/5] config hooks: hook_parse_cmdline take id as an argument
2017-08-12 10:22 [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit Jonatan Schlag
2017-08-12 10:22 ` [PATCH 2/5] zone: zone_config_settings_write never generate an id Jonatan Schlag
2017-08-12 10:22 ` [PATCH 3/5] header-config: pass id to hook_parse_cmdline Jonatan Schlag
@ 2017-08-12 10:22 ` Jonatan Schlag
2017-08-12 10:22 ` [PATCH 5/5] config hook: generate new id inside hook_new Jonatan Schlag
2017-08-13 12:10 ` [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit Michael Tremer
4 siblings, 0 replies; 6+ messages in thread
From: Jonatan Schlag @ 2017-08-12 10:22 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 3412 bytes --]
All hook_parse_cmdline take now the id as the first argument. This is required because
zone_config_check_same_setting needs the id to ignore the config we edit in the moment.
All hook_parse_cmdline functions are changed to keep the generic hook_edit function working.
Another option was to write an own hook_edit function for the ipv4-static and ipv6-static hook.
But this solution is not easier then to keep things more generic and equal. Makes also maintaining easier.
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/hooks/configs/dhcp | 4 ++++
src/hooks/configs/ipv4-static | 7 ++++++-
src/hooks/configs/ipv6-auto | 5 +++++
src/hooks/configs/ipv6-static | 7 ++++++-
src/hooks/configs/pppoe-server | 5 +++++
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp
index 7f6780b..f1c43f3 100644
--- a/src/hooks/configs/dhcp
+++ b/src/hooks/configs/dhcp
@@ -35,6 +35,10 @@ hook_check_config_settings() {
}
hook_parse_cmdline() {
+ local id=${1}
+ shift
+
+ assert isinteger id
while [ $# -gt 0 ]; do
case "${1}" in
--enable-ipv6)
diff --git a/src/hooks/configs/ipv4-static b/src/hooks/configs/ipv4-static
index ef74991..a6b010f 100644
--- a/src/hooks/configs/ipv4-static
+++ b/src/hooks/configs/ipv4-static
@@ -36,6 +36,11 @@ hook_check_config_settings() {
}
hook_parse_cmdline() {
+ local id=${1}
+ shift
+
+ assert isinteger id
+
local arg
while read -r arg; do
@@ -99,7 +104,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
diff --git a/src/hooks/configs/ipv6-auto b/src/hooks/configs/ipv6-auto
index 375e585..e89af28 100644
--- a/src/hooks/configs/ipv6-auto
+++ b/src/hooks/configs/ipv6-auto
@@ -31,6 +31,11 @@ hook_check_config_settings() {
}
hook_parse_cmdline() {
+ local id=${1}
+ shift
+
+ assert isinteger id
+
local arg
while read arg; do
diff --git a/src/hooks/configs/ipv6-static b/src/hooks/configs/ipv6-static
index c41401c..bb75240 100644
--- a/src/hooks/configs/ipv6-static
+++ b/src/hooks/configs/ipv6-static
@@ -34,6 +34,11 @@ hook_check_config_settings() {
}
hook_parse_cmdline() {
+ local id=${1}
+ shift
+
+ assert isinteger id
+
while [ $# -gt 0 ]; do
case "${1}" in
--address=*)
@@ -49,7 +54,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
diff --git a/src/hooks/configs/pppoe-server b/src/hooks/configs/pppoe-server
index b4d2538..5861e70 100644
--- a/src/hooks/configs/pppoe-server
+++ b/src/hooks/configs/pppoe-server
@@ -49,6 +49,11 @@ hook_check_config_settings() {
}
hook_parse_cmdline() {
+ local id=${1}
+ shift
+
+ assert isinteger id
+
while [ $# -gt 0 ]; do
case "${1}" in
--dns-server=*)
--
2.6.3
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 5/5] config hook: generate new id inside hook_new
2017-08-12 10:22 [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit Jonatan Schlag
` (2 preceding siblings ...)
2017-08-12 10:22 ` [PATCH 4/5] config hooks: hook_parse_cmdline take id as an argument Jonatan Schlag
@ 2017-08-12 10:22 ` Jonatan Schlag
2017-08-13 12:10 ` [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit Michael Tremer
4 siblings, 0 replies; 6+ messages in thread
From: Jonatan Schlag @ 2017-08-12 10:22 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 3746 bytes --]
The hook_parse_cmdline function takes as first argument the id so
we need to generate the id inside hook_new
Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
---
src/hooks/configs/dhcp | 9 +++++++--
src/hooks/configs/ipv4-static | 9 +++++++--
src/hooks/configs/ipv6-auto | 9 +++++++--
src/hooks/configs/ipv6-static | 9 +++++++--
src/hooks/configs/pppoe-server | 9 +++++++--
5 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/src/hooks/configs/dhcp b/src/hooks/configs/dhcp
index f1c43f3..f2a3e56 100644
--- a/src/hooks/configs/dhcp
+++ b/src/hooks/configs/dhcp
@@ -76,12 +76,17 @@ hook_new() {
return ${EXIT_ERROR}
fi
- if ! hook_parse_cmdline $@; then
+ # Get a new id
+ local id=$(zone_config_get_new_id ${zone})
+ assert isinteger id
+ 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 a6b010f..233fb10 100644
--- a/src/hooks/configs/ipv4-static
+++ b/src/hooks/configs/ipv4-static
@@ -120,12 +120,17 @@ hook_new() {
assert zone_exists "${zone}"
- if ! hook_parse_cmdline $@; then
+ # Get a new id
+ local id=$(zone_config_get_new_id ${zone})
+ assert isinteger id
+ 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 e89af28..7c82d45 100644
--- a/src/hooks/configs/ipv6-auto
+++ b/src/hooks/configs/ipv6-auto
@@ -62,12 +62,17 @@ hook_new() {
return ${EXIT_ERROR}
fi
- if ! hook_parse_cmdline $@; then
+ # Get a new id
+ local id=$(zone_config_get_new_id ${zone})
+ assert isinteger id
+ 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 bb75240..313adc0 100644
--- a/src/hooks/configs/ipv6-static
+++ b/src/hooks/configs/ipv6-static
@@ -71,12 +71,17 @@ hook_new() {
local zone=${1}
shift
- if ! hook_parse_cmdline $@; then
+ # Get a new id
+ local id=$(zone_config_get_new_id ${zone})
+ assert isinteger id
+ 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 5861e70..64d700b 100644
--- a/src/hooks/configs/pppoe-server
+++ b/src/hooks/configs/pppoe-server
@@ -110,12 +110,17 @@ hook_new() {
return ${EXIT_ERROR}
fi
- if ! hook_parse_cmdline $@; then
+ # Get a new id
+ local id=$(zone_config_get_new_id ${zone})
+ assert isinteger id
+ 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}
}
--
2.6.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit
2017-08-12 10:22 [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit Jonatan Schlag
` (3 preceding siblings ...)
2017-08-12 10:22 ` [PATCH 5/5] config hook: generate new id inside hook_new Jonatan Schlag
@ 2017-08-13 12:10 ` Michael Tremer
4 siblings, 0 replies; 6+ messages in thread
From: Michael Tremer @ 2017-08-13 12:10 UTC (permalink / raw)
To: network
[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]
Hi,
this patchset looks fine, but I think the solution is a little bit too
complicated.
Could we not let hook_parse_cmdline() remain untouched and just let it parse all
inputs? We need to parse all arguments anyways before we can make a decision.
We could then check for any conflicts later in hook_edit().
That looks easier to me since:
a) the format of hook_parse_cmdline() would be the same throughout all zones,
ports and configs
b) we do not need to carry around the ID variable. It will be empty any ways
when we are creating a new configuration and then become difficult to handle.
Your thoughts?
Best,
-Michael
On Sat, 2017-08-12 at 12:22 +0200, Jonatan Schlag wrote:
> We now check if we get the config, which we edit in this moment and when we
> continue.
>
> Fixes: #11451
>
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/functions/functions.zone | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/src/functions/functions.zone b/src/functions/functions.zone
> index 1eb492f..941314d 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -1155,12 +1155,13 @@ 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}
> @@ -1171,6 +1172,12 @@ zone_config_check_same_setting() {
> if [[ $(zone_config_get_hook "${zone}" "${config}") !=
> ${hook} ]]; then
> continue
> fi
> +
> + # Check if we get the config we want to create or edit (we
> must ignore this config on edit)
> + if [[ "${hook}.${id}" = "${config}" ]]; then
> + continue
> + fi
> +
> # Get the value of the key for a given function
> zone_config_settings_read "${zone}" "${config}" \
> --ignore-superfluous-settings "${key}"
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread