From 1b9a2e4a3657fd938c037f71af7c8b2d78411c72 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Thu, 9 Feb 2017 15:01:06 -0500 Subject: [PATCH 1/2] correctly set the Authorization header for non-async log handling see: #5276 --- awx/main/tests/unit/utils/test_handlers.py | 87 ++++++++++++++++++---- awx/main/utils/handlers.py | 21 ++++-- 2 files changed, 88 insertions(+), 20 deletions(-) diff --git a/awx/main/tests/unit/utils/test_handlers.py b/awx/main/tests/unit/utils/test_handlers.py index 9c4d3f95d5..5a30cdb316 100644 --- a/awx/main/tests/unit/utils/test_handlers.py +++ b/awx/main/tests/unit/utils/test_handlers.py @@ -1,14 +1,29 @@ +import base64 import json import logging from django.conf import LazySettings import pytest import requests +from requests_futures.sessions import FuturesSession from awx.main.utils.handlers import BaseHTTPSHandler as HTTPSHandler, PARAM_NAMES from awx.main.utils.formatters import LogstashFormatter +@pytest.fixture() +def dummy_log_record(): + return logging.LogRecord( + 'awx', # logger name + 20, # loglevel INFO + './awx/some/module.py', # pathname + 100, # lineno + 'User joe logged in', # msg + tuple(), # args, + None # exc_info + ) + + @pytest.fixture() def ok200_adapter(): class OK200Adapter(requests.adapters.HTTPAdapter): @@ -25,6 +40,14 @@ def ok200_adapter(): return OK200Adapter() +@pytest.mark.parametrize('async, implementation', [ + (True, FuturesSession), + (True, requests.Session) +]) +def test_https_logging_handler_requests_implementation(async, implementation): + handler = HTTPSHandler(async=async) + assert isinstance(handler.session, implementation) + @pytest.mark.parametrize('param', PARAM_NAMES.keys()) def test_https_logging_handler_defaults(param): @@ -89,23 +112,21 @@ def test_https_logging_handler_skip_log(params, logger_name, expected): assert handler.skip_log(logger_name) is expected -@pytest.mark.parametrize('message_type', ['logstash', 'splunk']) -def test_https_logging_handler_emit(ok200_adapter, message_type): +@pytest.mark.parametrize('message_type, async', [ + ('logstash', False), + ('logstash', True), + ('splunk', False), + ('splunk', True), +]) +def test_https_logging_handler_emit(ok200_adapter, dummy_log_record, + message_type, async): handler = HTTPSHandler(host='127.0.0.1', enabled_flag=True, message_type=message_type, - enabled_loggers=['awx', 'activity_stream', 'job_events', 'system_tracking']) + enabled_loggers=['awx', 'activity_stream', 'job_events', 'system_tracking'], + async=async) handler.setFormatter(LogstashFormatter()) handler.session.mount('http://', ok200_adapter) - record = logging.LogRecord( - 'awx', # logger name - 20, # loglevel INFO - './awx/some/module.py', # pathname - 100, # lineno - 'User joe logged in', # msg - tuple(), # args, - None # exc_info - ) - async_futures = handler.emit(record) + async_futures = handler.emit(dummy_log_record) [future.result() for future in async_futures] assert len(ok200_adapter.requests) == 1 @@ -114,15 +135,55 @@ def test_https_logging_handler_emit(ok200_adapter, message_type): assert request.method == 'POST' body = json.loads(request.body) + if message_type == 'logstash': + # A username + password weren't used, so this header should be missing + assert 'Authorization' not in request.headers + if message_type == 'splunk': # splunk messages are nested under the 'event' key body = body['event'] + assert request.headers['Authorization'] == 'Splunk None' assert body['level'] == 'INFO' assert body['logger_name'] == 'awx' assert body['message'] == 'User joe logged in' +@pytest.mark.parametrize('async', (True, False)) +def test_https_logging_handler_emit_logstash_with_creds(ok200_adapter, + dummy_log_record, async): + handler = HTTPSHandler(host='127.0.0.1', enabled_flag=True, + username='user', password='pass', + message_type='logstash', + enabled_loggers=['awx', 'activity_stream', 'job_events', 'system_tracking'], + async=async) + handler.setFormatter(LogstashFormatter()) + handler.session.mount('http://', ok200_adapter) + async_futures = handler.emit(dummy_log_record) + [future.result() for future in async_futures] + + assert len(ok200_adapter.requests) == 1 + request = ok200_adapter.requests[0] + assert request.headers['Authorization'] == 'Basic %s' % base64.b64encode("user:pass") + + +@pytest.mark.parametrize('async', (True, False)) +def test_https_logging_handler_emit_splunk_with_creds(ok200_adapter, + dummy_log_record, async): + handler = HTTPSHandler(host='127.0.0.1', enabled_flag=True, + password='pass', message_type='splunk', + enabled_loggers=['awx', 'activity_stream', 'job_events', 'system_tracking'], + async=async) + handler.setFormatter(LogstashFormatter()) + handler.session.mount('http://', ok200_adapter) + async_futures = handler.emit(dummy_log_record) + [future.result() for future in async_futures] + + assert len(ok200_adapter.requests) == 1 + request = ok200_adapter.requests[0] + assert request.headers['Authorization'] == 'Splunk pass' + + def test_https_logging_handler_emit_one_record_per_fact(ok200_adapter): handler = HTTPSHandler(host='127.0.0.1', enabled_flag=True, message_type='logstash', indv_facts=True, diff --git a/awx/main/utils/handlers.py b/awx/main/utils/handlers.py index c6c0b5defa..b53a1e83eb 100644 --- a/awx/main/utils/handlers.py +++ b/awx/main/utils/handlers.py @@ -52,7 +52,10 @@ class BaseHTTPSHandler(logging.Handler): self.async = kwargs.get('async', True) for fd in PARAM_NAMES: setattr(self, fd, kwargs.get(fd, None)) - self.session = FuturesSession() + if self.async: + self.session = FuturesSession() + else: + self.session = requests.Session() self.add_auth_information() @classmethod @@ -126,7 +129,6 @@ class BaseHTTPSHandler(logging.Handler): return [] try: payload = self.format(record) - host = self.get_http_host() # Special action for System Tracking, queue up multiple log messages if self.indv_facts: @@ -139,21 +141,26 @@ class BaseHTTPSHandler(logging.Handler): for key in facts_dict: fact_payload = copy(payload_data) fact_payload.update(facts_dict[key]) - async_futures.append( - self.session.post(host, **self.get_post_kwargs(fact_payload)) - ) + if self.async: + async_futures.append(self._send(fact_payload)) + else: + self._send(fact_payload) return async_futures if self.async: - return [self.session.post(host, **self.get_post_kwargs(payload))] + return [self._send(payload)] - requests.post(host, auth=requests.auth.HTTPBasicAuth(self.username, self.password), **self.get_post_kwargs(payload)) + self._send(payload) return [] except (KeyboardInterrupt, SystemExit): raise except: self.handleError(record) + def _send(self, payload): + return self.session.post(self.get_http_host(), + **self.get_post_kwargs(payload)) + class HTTPSHandler(object): From cee0b29fef58efc7c46ccaf185563ff9e259889e Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Thu, 9 Feb 2017 15:16:28 -0500 Subject: [PATCH 2/2] clarify a logging handler docstring --- awx/main/utils/handlers.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/awx/main/utils/handlers.py b/awx/main/utils/handlers.py index b53a1e83eb..b7b851a5c5 100644 --- a/awx/main/utils/handlers.py +++ b/awx/main/utils/handlers.py @@ -118,9 +118,13 @@ class BaseHTTPSHandler(logging.Handler): def emit(self, record): """ - Emit a log record. When ``self.async`` is True, returns a list of + Emit a log record. Returns a list of zero or more ``concurrent.futures.Future`` objects. + When ``self.async`` is True, the list will contain one + Future object for each HTTP request made. When ``self.async`` is + False, the list will be empty. + See: https://docs.python.org/3/library/concurrent.futures.html#future-objects http://pythonhosted.org/futures/