From c7d56fa9c57a18473da166254c01f4abc44c7ee3 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Tue, 16 Mar 2021 16:07:12 +0000 Subject: [PATCH] py36: Fix syslog fallback to UDP With py2 the SysLogHandler would raise an error if it could not connect to the configured log_address socket. This prompted the utils.get_logger() method to fallback to using UDP. With py3 an error is not raised [1], so the fallback never happens. In that scenario, exceptions are raised when swift services are started and attempt to log. This patch modifies get_logger() to test if the log_address is a socket before instantiating a SysLogHandler and fallback to UDP if not a socket. [1] https://github.com/python/cpython/blob/master/Lib/logging/handlers.py#L833-L842 Change-Id: Ic099fdc3925bf99f8528c7aa763b651c19df1322 --- swift/common/utils.py | 14 ++++++--- test/unit/common/test_utils.py | 57 +++++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/swift/common/utils.py b/swift/common/utils.py index 022ecdd068..1813d5c0cc 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -2373,13 +2373,19 @@ def get_logger(conf, name=None, log_to_console=False, log_route=None, facility=facility) else: log_address = conf.get('log_address', '/dev/log') + handler = None try: - handler = ThreadSafeSysLogHandler(address=log_address, - facility=facility) - except socket.error as e: - # Either /dev/log isn't a UNIX socket or it does not exist at all + mode = os.stat(log_address).st_mode + if stat.S_ISSOCK(mode): + handler = ThreadSafeSysLogHandler(address=log_address, + facility=facility) + except (OSError, socket.error) as e: + # If either /dev/log isn't a UNIX socket or it does not exist at + # all then py2 would raise an error if e.errno not in [errno.ENOTSOCK, errno.ENOENT]: raise + if handler is None: + # fallback to default UDP handler = ThreadSafeSysLogHandler(facility=facility) handler.setFormatter(formatter) logger.addHandler(handler) diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index cf878f6e3b..e27b694079 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -1752,7 +1752,8 @@ class TestUtils(unittest.TestCase): self.assertEqual(sio.getvalue(), 'test1\ntest3\ntest4\ntest6\n') - def test_get_logger_sysloghandler_plumbing(self): + @with_tempdir + def test_get_logger_sysloghandler_plumbing(self, tempdir): orig_sysloghandler = utils.ThreadSafeSysLogHandler syslog_handler_args = [] @@ -1773,6 +1774,7 @@ class TestUtils(unittest.TestCase): with mock.patch.object(utils, 'ThreadSafeSysLogHandler', syslog_handler_catcher), \ mock.patch.object(socket, 'getaddrinfo', fake_getaddrinfo): + # default log_address utils.get_logger({ 'log_facility': 'LOG_LOCAL3', }, 'server', log_route='server') @@ -1783,19 +1785,51 @@ class TestUtils(unittest.TestCase): os.path.isdir('/dev/log'): # Since socket on OSX is in /var/run/syslog, there will be # a fallback to UDP. - expected_args.append( - ((), {'facility': orig_sysloghandler.LOG_LOCAL3})) + expected_args = [ + ((), {'facility': orig_sysloghandler.LOG_LOCAL3})] self.assertEqual(expected_args, syslog_handler_args) + # custom log_address - file doesn't exist: fallback to UDP + log_address = os.path.join(tempdir, 'foo') syslog_handler_args = [] utils.get_logger({ 'log_facility': 'LOG_LOCAL3', - 'log_address': '/foo/bar', + 'log_address': log_address, }, 'server', log_route='server') + expected_args = [ + ((), {'facility': orig_sysloghandler.LOG_LOCAL3})] self.assertEqual( - ((), {'address': '/foo/bar', - 'facility': orig_sysloghandler.LOG_LOCAL3}), - syslog_handler_args[0]) + expected_args, syslog_handler_args) + + # custom log_address - file exists, not a socket: fallback to UDP + with open(log_address, 'w'): + pass + syslog_handler_args = [] + utils.get_logger({ + 'log_facility': 'LOG_LOCAL3', + 'log_address': log_address, + }, 'server', log_route='server') + expected_args = [ + ((), {'facility': orig_sysloghandler.LOG_LOCAL3})] + self.assertEqual( + expected_args, syslog_handler_args) + + # custom log_address - file exists, is a socket: use it + os.unlink(log_address) + with contextlib.closing( + socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)) as sock: + sock.settimeout(5) + sock.bind(log_address) + syslog_handler_args = [] + utils.get_logger({ + 'log_facility': 'LOG_LOCAL3', + 'log_address': log_address, + }, 'server', log_route='server') + expected_args = [ + ((), {'address': log_address, + 'facility': orig_sysloghandler.LOG_LOCAL3})] + self.assertEqual( + expected_args, syslog_handler_args) # Using UDP with default port syslog_handler_args = [] @@ -1819,6 +1853,15 @@ class TestUtils(unittest.TestCase): 'facility': orig_sysloghandler.LOG_LOCAL0})], syslog_handler_args) + with mock.patch.object(utils, 'ThreadSafeSysLogHandler', + side_effect=OSError(errno.EPERM, 'oops')): + with self.assertRaises(OSError) as cm: + utils.get_logger({ + 'log_facility': 'LOG_LOCAL3', + 'log_address': 'log_address', + }, 'server', log_route='server') + self.assertEqual(errno.EPERM, cm.exception.errno) + @reset_logger_state def test_clean_logger_exception(self): # setup stream logging