commit 0f6b0b3342ab9125d0768e4f57b80414c28c7f70 Author: Thomas Woerner 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 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 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 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 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 @@ Options + 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 ALREADY_ENABLED (11), NOT_ENABLED (12) and also ZONE_ALREADY_SET (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 UNKNOWN_ERROR (254) will be used. + + + The following options are supported: 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 @@ If no options are given, configuration from /etc/sysconfig/system-config-firewall will be migrated. + + + 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 ALREADY_ENABLED (11), NOT_ENABLED (12) and also ZONE_ALREADY_SET (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 UNKNOWN_ERROR (254) will be used. + + The following options are supported: 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 @@ Options + 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 ALREADY_ENABLED (11), NOT_ENABLED (12) and also ZONE_ALREADY_SET (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 UNKNOWN_ERROR (254) will be used. + + + The following options are supported: