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>