commit 0f6b0b3342ab9125d0768e4f57b80414c28c7f70
Author: Thomas Woerner <twoerner@redhat.com>
Date: Fri Aug 12 16:42:43 2016 +0200
firewall.command: ALREADY_ENABLED, NOT_ENABLED, ZONE_ALREADY_SET are warnings
Refixes: RHBZ#845257
diff --git a/src/firewall/command.py b/src/firewall/command.py
index 7964fab..c907cd3 100644
--- a/src/firewall/command.py
+++ b/src/firewall/command.py
@@ -121,12 +121,18 @@ class FirewallCommand(object):
try:
action_method(*call_item)
except DBusException as msg:
- code = FirewallError.get_code(msg.get_dbus_message())
if len(option) > 1:
self.print_warning("Warning: %s" % msg.get_dbus_message())
else:
- self.print_and_exit("Error: %s" % msg.get_dbus_message(),
- code)
+ code = FirewallError.get_code(msg.get_dbus_message())
+ if code in [ errors.ALREADY_ENABLED, errors.NOT_ENABLED,
+ errors.ZONE_ALREADY_SET ]:
+ self.print_warning("Warning: %s" % \
+ msg.get_dbus_message())
+ sys.exit(0)
+ else:
+ self.print_and_exit("Error: %s" % \
+ msg.get_dbus_message(), code)
_errors += 1
except Exception as msg:
if len(option) > 1:
@@ -135,7 +141,12 @@ class FirewallCommand(object):
continue
else:
code = FirewallError.get_code(str(msg))
- self.print_and_exit("Error: %s" % msg, code)
+ if code in [ errors.ALREADY_ENABLED, errors.NOT_ENABLED,
+ errors.ZONE_ALREADY_SET ]:
+ self.print_warning("Warning: %s" % msg)
+ sys.exit(0)
+ else:
+ self.print_and_exit("Error: %s" % msg, code)
_errors += 1
self.activate_exception_handler()
commit e9e3ae8a6ac6fe121ed8a2c875bac355a2c7d102
Author: Thomas Woerner <twoerner@redhat.com>
Date: Fri Aug 12 17:12:08 2016 +0200
test-suite: Do not fail on ALREADY_ENABLED --add-destination tests
Related to 0f6b0b3
diff --git a/src/tests/firewall-cmd_test.sh b/src/tests/firewall-cmd_test.sh
index dd7c4f8..59ea81a 100755
--- a/src/tests/firewall-cmd_test.sh
+++ b/src/tests/firewall-cmd_test.sh
@@ -577,7 +577,7 @@ assert_bad "--permanent --service=${myservice} --query-destination=ipv6:fd00:dea
# test icmptype options, ipv4 and ipv6 destinations are default
assert_bad "--permanent --icmptype=${myicmp} --add-destination=ipv5"
-assert_bad "--permanent --icmptype=${myicmp} --add-destination=ipv4"
+assert_good "--permanent --icmptype=${myicmp} --add-destination=ipv4"
assert_good "--permanent --icmptype=${myicmp} --remove-destination=ipv4"
assert_good "--permanent --icmptype=${myicmp} --add-destination=ipv4"
assert_good "--permanent --icmptype=${myicmp} --query-destination=ipv4"
diff --git a/src/tests/firewall-offline-cmd_test.sh b/src/tests/firewall-offline-cmd_test.sh
index 344b825..00f9a01 100755
--- a/src/tests/firewall-offline-cmd_test.sh
+++ b/src/tests/firewall-offline-cmd_test.sh
@@ -474,7 +474,7 @@ assert_bad "--service=${myservice} --query-destination=ipv6:fd00:dead:beef:ff0::
# test icmptype options, ipv4 and ipv6 destinations are default
assert_bad "--icmptype=${myicmp} --add-destination=ipv5"
-assert_bad "--icmptype=${myicmp} --add-destination=ipv4"
+assert_good "--icmptype=${myicmp} --add-destination=ipv4"
assert_good "--icmptype=${myicmp} --remove-destination=ipv4"
assert_good "--icmptype=${myicmp} --add-destination=ipv4"
assert_good "--icmptype=${myicmp} --query-destination=ipv4"
commit 84f54680c14ab1aa149cebe65e25eef16f8ba621
Author: Thomas Woerner <twoerner@redhat.com>
Date: Thu Aug 18 16:37:20 2016 +0200
firewall.command: Do not use error code 254 for {ALREADY,NOT}_ENABLED sequences
if there is one error code, then it will be directly used. If there are two and
one is 0, then the other will be used. If there are two or more different than
254 will be used.
This is an addition for RHBZ#1366654
diff --git a/src/firewall/command.py b/src/firewall/command.py
index c907cd3..a587d56 100644
--- a/src/firewall/command.py
+++ b/src/firewall/command.py
@@ -91,19 +91,21 @@ class FirewallCommand(object):
self.fw.authorizeAll()
items = [ ]
_errors = 0
+ _error_codes = [ ]
for item in option:
if parse_method is not None:
try:
item = parse_method(item)
except Exception as msg:
+ code = FirewallError.get_code(str(msg))
if len(option) > 1:
self.print_warning("Warning: %s" % msg)
- _errors += 1
- continue
else:
- code = FirewallError.get_code(msg)
self.print_and_exit("Error: %s" % msg, code)
- _errors += 1
+ if code not in _error_codes:
+ _error_codes.append(code)
+ _errors += 1
+ continue
items.append(item)
@@ -120,37 +122,32 @@ class FirewallCommand(object):
self.deactivate_exception_handler()
try:
action_method(*call_item)
- except DBusException as msg:
- if len(option) > 1:
- self.print_warning("Warning: %s" % msg.get_dbus_message())
+ except (DBusException, Exception) as msg:
+ if isinstance(msg, DBusException):
+ msg = msg.get_dbus_message()
else:
- code = FirewallError.get_code(msg.get_dbus_message())
- if code in [ errors.ALREADY_ENABLED, errors.NOT_ENABLED,
- errors.ZONE_ALREADY_SET ]:
- self.print_warning("Warning: %s" % \
- msg.get_dbus_message())
- sys.exit(0)
- else:
- self.print_and_exit("Error: %s" % \
- msg.get_dbus_message(), code)
- _errors += 1
- except Exception as msg:
+ msg = str(msg)
+ code = FirewallError.get_code(msg)
+ if code in [ errors.ALREADY_ENABLED, errors.NOT_ENABLED,
+ errors.ZONE_ALREADY_SET ]:
+ code = 0
if len(option) > 1:
self.print_warning("Warning: %s" % msg)
- _errors += 1
- continue
+ elif code == 0:
+ self.print_warning("Warning: %s" % msg)
+ sys.exit(0)
else:
- code = FirewallError.get_code(str(msg))
- if code in [ errors.ALREADY_ENABLED, errors.NOT_ENABLED,
- errors.ZONE_ALREADY_SET ]:
- self.print_warning("Warning: %s" % msg)
- sys.exit(0)
- else:
- self.print_and_exit("Error: %s" % msg, code)
- _errors += 1
+ self.print_and_exit("Error: %s" % msg, code)
+ if code not in _error_codes:
+ _error_codes.append(code)
+ _errors += 1
self.activate_exception_handler()
if _errors == len(option) and not no_exit:
+ if len(_error_codes) == 1:
+ sys.exit(_error_codes[0])
+ elif len(_error_codes) == 2 and 0 in _error_codes:
+ sys.exit(list(set(_error_codes) - set([0]))[0])
sys.exit(errors.UNKNOWN_ERROR)
def add_sequence(self, option, action_method, query_method, parse_method,
diff --git a/src/tests/firewall-cmd_test.sh b/src/tests/firewall-cmd_test.sh
index bc06ce0..c30c759 100755
--- a/src/tests/firewall-cmd_test.sh
+++ b/src/tests/firewall-cmd_test.sh
@@ -148,6 +148,20 @@ assert_rich_bad() {
fi
}
+assert_exit_code() {
+ local args="${1}"
+ local ret="${2}"
+
+ ${path}firewall-cmd ${args} > /dev/null 2>&1
+ got=$?
+ if [[ "$got" -eq "$ret" ]]; then
+ echo "${args} ... OK"
+ else
+ ((failures++))
+ echo -e "${args} ... ${RED}${failures}. FAILED (bad exit status ${got} != ${ret})${RESTORE}"
+ fi
+}
+
if ! ${path}firewall-cmd --state --quiet; then
echo "FirewallD is not running"
exit 1
@@ -620,6 +634,14 @@ assert_good "--zone=${zone} --remove-source=${source}"
assert_good "--permanent --delete-ipset=${ipset}"
assert_good "--reload"
+# exit return value tests
+assert_exit_code "--add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udp --add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udp --add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo" 254
+assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo --add-port bar" 254
+assert_exit_code "--add-port 122/udp --add-port 122/udp" 0
+
# ... --direct ...
modprobe dummy
assert_good "--direct --passthrough ipv4 --table mangle --append POSTROUTING --out-interface dummy0 --protocol udp --destination-port 68 --jump CHECKSUM --checksum-fill"
commit f70190d20448f52556745e88d50de7c7a8a774b8
Author: Thomas Woerner <twoerner@redhat.com>
Date: Tue Aug 23 15:50:53 2016 +0200
firewall.command: Fix sequence exit code with at least one succeeded item
For sequences with errors but at least one succeeded item, the exit code should
be 0 as there has been a change. The errors will be printed as warnings.
Without any succeeded item, the exit code will depend on the error codes. If
there is exactly one error code, then this is used. If there are more than one
then UNKNOWN_ERROR (254) will be used.
This is an additional fix for 84f5468 and RHBZ#1366654
diff --git a/src/firewall/command.py b/src/firewall/command.py
index a587d56..6e26c46 100644
--- a/src/firewall/command.py
+++ b/src/firewall/command.py
@@ -135,7 +135,7 @@ class FirewallCommand(object):
self.print_warning("Warning: %s" % msg)
elif code == 0:
self.print_warning("Warning: %s" % msg)
- sys.exit(0)
+ return
else:
self.print_and_exit("Error: %s" % msg, code)
if code not in _error_codes:
@@ -143,12 +143,20 @@ class FirewallCommand(object):
_errors += 1
self.activate_exception_handler()
- if _errors == len(option) and not no_exit:
- if len(_error_codes) == 1:
+ if not no_exit:
+ if len(option) > _errors or 0 in _error_codes:
+ # There have been more options than errors or there
+ # was at least one error code 0, return.
+ return
+ elif len(_error_codes) == 1:
+ # Exactly one error code, use it.
sys.exit(_error_codes[0])
- elif len(_error_codes) == 2 and 0 in _error_codes:
- sys.exit(list(set(_error_codes) - set([0]))[0])
- sys.exit(errors.UNKNOWN_ERROR)
+ elif len(_error_codes) > 1:
+ # There is more than error, exit using
+ # UNKNOWN_ERROR. This could happen within sequences
+ # where parsing failed with different errors like
+ # INVALID_PORT and INVALID_PROTOCOL.
+ sys.exit(errors.UNKNOWN_ERROR)
def add_sequence(self, option, action_method, query_method, parse_method,
message, no_exit=False):
diff --git a/src/tests/firewall-cmd_test.sh b/src/tests/firewall-cmd_test.sh
index c30c759..f521840 100755
--- a/src/tests/firewall-cmd_test.sh
+++ b/src/tests/firewall-cmd_test.sh
@@ -635,12 +635,18 @@ assert_good "--permanent --delete-ipset=${ipset}"
assert_good "--reload"
# exit return value tests
+assert_exit_code "--remove-port 122/udp" 0
assert_exit_code "--add-port 122/udpp" 103
-assert_exit_code "--add-port 122/udp --add-port 122/udpp" 103
-assert_exit_code "--add-port 122/udp --add-port 122/udpp" 103
-assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo" 254
-assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo --add-port bar" 254
+assert_exit_code "--add-port 122/udp --add-port 122/udpp" 0
+assert_exit_code "--add-port 122/udp --add-port 122/udpp" 0
+assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo" 0
+assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo --add-port bar" 0
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp --add-port 8745897/foo" 254
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp --add-port 8745897/foo --add-port bar" 254
assert_exit_code "--add-port 122/udp --add-port 122/udp" 0
+assert_exit_code "--remove-port 122/udp" 0
# ... --direct ...
modprobe dummy
diff --git a/src/tests/firewall-offline-cmd_test.sh b/src/tests/firewall-offline-cmd_test.sh
index ebbdfad..fb15300 100755
--- a/src/tests/firewall-offline-cmd_test.sh
+++ b/src/tests/firewall-offline-cmd_test.sh
@@ -156,6 +156,20 @@ assert_rich_bad() {
fi
}
+assert_exit_code() {
+ local args="${1}"
+ local ret="${2}"
+
+ ${path}firewall-cmd ${args} > /dev/null 2>&1
+ got=$?
+ if [[ "$got" -eq "$ret" ]]; then
+ echo "${args} ... OK"
+ else
+ ((failures++))
+ echo -e "${args} ... ${RED}${failures}. FAILED (bad exit status ${got} != ${ret})${RESTORE}"
+ fi
+}
+
test_lokkit_opts() {
rm -f /etc/firewalld/zones/*
assert_good "${lokkit_opts}"
@@ -515,6 +529,20 @@ assert_good "--zone=${zone} --remove-source=${source}"
assert_good "--delete-ipset=${ipset}"
+# exit return value tests
+assert_exit_code "--remove-port 122/udp" 0
+assert_exit_code "--add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udp --add-port 122/udpp" 0
+assert_exit_code "--add-port 122/udp --add-port 122/udpp" 0
+assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo" 0
+assert_exit_code "--add-port 122/udp --add-port 122/udpp --add-port 8745897/foo --add-port bar" 0
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp" 103
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp --add-port 8745897/foo" 254
+assert_exit_code "--add-port 122/udpa --add-port 122/udpp --add-port 8745897/foo --add-port bar" 254
+assert_exit_code "--add-port 122/udp --add-port 122/udp" 0
+assert_exit_code "--remove-port 122/udp" 0
+
# ... --direct ...
assert_bad "--direct --add-passthrough ipv7 --table filter -A INPUT --in-interface dummy0 --protocol tcp --destination-port 67 --jump ACCEPT" # bad ipv
assert_good "--direct --add-passthrough ipv4 --table filter --append INPUT --in-interface dummy0 --protocol tcp --destination-port 67 --jump ACCEPT"
commit e28dac9c52d1f661e1203ad5128ced95a1bef072
Author: Thomas Woerner <twoerner@redhat.com>
Date: Tue Aug 30 16:36:36 2016 +0200
Command line tools man pages: New section about sequence options and exit codes
This has been added to the firewall-[offline-]cmd and firewallctl man pages.
diff --git a/doc/xml/firewall-cmd.xml b/doc/xml/firewall-cmd.xml
index d441198..d77115a 100644
--- a/doc/xml/firewall-cmd.xml
+++ b/doc/xml/firewall-cmd.xml
@@ -67,6 +67,12 @@
<refsect1 id="options">
<title>Options</title>
<para>
+ For sequence options, this are the options that can be specified multiple times, the exit code is 0 if there is at least one item that succeded. The <literal>ALREADY_ENABLED</literal> (11), <literal>NOT_ENABLED</literal> (12) and also <literal>ZONE_ALREADY_SET</literal> (16) errors are treated as succeeded.
+ If there are issues while parsing the items, then these are treated as warnings and will not change the result as long as there is a succeeded one.
+ Without any succeeded item, the exit code will depend on the error codes. If there is exactly one error code, then this is used. If there are more than one then <literal>UNKNOWN_ERROR</literal> (254) will be used.
+ </para>
+
+ <para>
The following options are supported:
</para>
diff --git a/doc/xml/firewall-offline-cmd.xml b/doc/xml/firewall-offline-cmd.xml
index c4e5b80..0c45921 100644
--- a/doc/xml/firewall-offline-cmd.xml
+++ b/doc/xml/firewall-offline-cmd.xml
@@ -70,6 +70,13 @@
<para>
If no options are given, configuration from <command>/etc/sysconfig/system-config-firewall</command> will be migrated.
</para>
+
+ <para>
+ For sequence options, this are the options that can be specified multiple times, the exit code is 0 if there is at least one item that succeded. The <literal>ALREADY_ENABLED</literal> (11), <literal>NOT_ENABLED</literal> (12) and also <literal>ZONE_ALREADY_SET</literal> (16) errors are treated as succeeded.
+ If there are issues while parsing the items, then these are treated as warnings and will not change the result as long as there is a succeeded one.
+ Without any succeeded item, the exit code will depend on the error codes. If there is exactly one error code, then this is used. If there are more than one then <literal>UNKNOWN_ERROR</literal> (254) will be used.
+ </para>
+
<para>
The following options are supported:
</para>
diff --git a/doc/xml/firewallctl.xml b/doc/xml/firewallctl.xml
index ff99e5d..30c51ec 100644
--- a/doc/xml/firewallctl.xml
+++ b/doc/xml/firewallctl.xml
@@ -67,6 +67,12 @@
<refsect1 id="options">
<title>Options</title>
<para>
+ For sequence options, this are the options that can be specified multiple times, the exit code is 0 if there is at least one item that succeded. The <literal>ALREADY_ENABLED</literal> (11), <literal>NOT_ENABLED</literal> (12) and also <literal>ZONE_ALREADY_SET</literal> (16) errors are treated as succeeded.
+ If there are issues while parsing the items, then these are treated as warnings and will not change the result as long as there is a succeeded one.
+ Without any succeeded item, the exit code will depend on the error codes. If there is exactly one error code, then this is used. If there are more than one then <literal>UNKNOWN_ERROR</literal> (254) will be used.
+ </para>
+
+ <para>
The following options are supported:
</para>