Skip to content

Commit

Permalink
Replace InsecureRequestWarning with standard logs
Browse files Browse the repository at this point in the history
  • Loading branch information
ofek committed Sep 4, 2020
1 parent fbdf7d1 commit 797d010
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 29 deletions.
5 changes: 5 additions & 0 deletions datadog_checks_base/datadog_checks/base/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
# Licensed under a 3-clause BSD style license (see LICENSE)
import logging
import sys
import warnings
from typing import Callable

from six import PY2, text_type
from urllib3.exceptions import InsecureRequestWarning

from .utils.common import to_native_string

Expand Down Expand Up @@ -155,6 +157,9 @@ def init_logging():
rootLogger.addHandler(AgentLogHandler())
rootLogger.setLevel(_get_py_loglevel(datadog_agent.get_config('log_level')))

# We log instead of emit warnings for unintentionally insecure HTTPS requests
warnings.simplefilter('ignore', InsecureRequestWarning)

# `requests` (used in a lot of checks) imports `urllib3`, which logs a bunch of stuff at the info level
# Therefore, pre emptively increase the default level of that logger to `WARN`
urllib_logger = logging.getLogger("requests.packages.urllib3")
Expand Down
12 changes: 3 additions & 9 deletions datadog_checks_base/datadog_checks/base/utils/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,11 @@
from requests_toolbelt.adapters import host_header_ssl
from six import PY2, iteritems, string_types
from six.moves.urllib.parse import urlparse
from urllib3.exceptions import InsecureRequestWarning

from ..config import is_affirmative
from ..errors import ConfigurationError
from .common import ensure_bytes, ensure_unicode
from .headers import get_default_headers, update_headers
from .warnings_util import disable_warnings_ctx

try:
from contextlib import ExitStack
Expand Down Expand Up @@ -272,8 +270,6 @@ def __init__(self, instance, init_config, remapper=None, logger=None):

# Context managers that should wrap all requests
self.request_hooks = []
if self.ignore_tls_warning:
self.request_hooks.append(self.handle_tls_warning)

if config['kerberos_keytab']:
self.request_hooks.append(lambda: handle_kerberos_keytab(config['kerberos_keytab']))
Expand Down Expand Up @@ -311,6 +307,9 @@ def _request(self, method, url, options):

new_options = self.populate_options(options)

if not self.ignore_tls_warning and not new_options['verify']:
self.logger.warning(u'An unverified HTTPS request is being made to %s', url)

extra_headers = options.pop('extra_headers', None)
if extra_headers is not None:
new_options['headers'] = new_options['headers'].copy()
Expand All @@ -336,11 +335,6 @@ def populate_options(self, options):

return options

@contextmanager
def handle_tls_warning(self):
with disable_warnings_ctx(InsecureRequestWarning, disable=True):
yield

@property
def session(self):
if self._session is None:
Expand Down
76 changes: 56 additions & 20 deletions datadog_checks_base/tests/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -831,78 +831,114 @@ def test_instance_and_init_flag(self):

assert http.ignore_tls_warning is False

def test_default_no_ignore(self):
def test_default_no_ignore(self, caplog):
instance = {}
init_config = {}
http = RequestsWrapper(instance, init_config)

with pytest.warns(InsecureRequestWarning):
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

def test_ignore(self):
expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, level, message in caplog.record_tuples:
if level == logging.WARNING and message == expected_message:
break
else:
raise AssertionError('Expected WARNING log with message `{}`'.format(expected_message))

def test_ignore(self, caplog):
instance = {'tls_ignore_warning': True}
init_config = {}
http = RequestsWrapper(instance, init_config)

with pytest.warns(None) as record:
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)
expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, _, message in caplog.record_tuples:
assert message != expected_message

def test_default_no_ignore_session(self):
def test_default_no_ignore_session(self, caplog):
instance = {'persist_connections': True}
init_config = {}
http = RequestsWrapper(instance, init_config)

with pytest.warns(InsecureRequestWarning):
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

def test_ignore_session(self):
expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, level, message in caplog.record_tuples:
if level == logging.WARNING and message == expected_message:
break
else:
raise AssertionError('Expected WARNING log with message `{}`'.format(expected_message))

def test_ignore_session(self, caplog):
instance = {'tls_ignore_warning': True, 'persist_connections': True}
init_config = {}
http = RequestsWrapper(instance, init_config)

with pytest.warns(None) as record:
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)
expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, _, message in caplog.record_tuples:
assert message != expected_message

def test_init_ignore(self):
def test_init_ignore(self, caplog):
instance = {}
init_config = {'tls_ignore_warning': True}
http = RequestsWrapper(instance, init_config)

with pytest.warns(None) as record:
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)
expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, _, message in caplog.record_tuples:
assert message != expected_message

def test_default_init_no_ignore(self):
def test_default_init_no_ignore(self, caplog):
instance = {}
init_config = {'tls_ignore_warning': False}
http = RequestsWrapper(instance, init_config)

with pytest.warns(InsecureRequestWarning):
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

def test_instance_ignore(self):
expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, level, message in caplog.record_tuples:
if level == logging.WARNING and message == expected_message:
break
else:
raise AssertionError('Expected WARNING log with message `{}`'.format(expected_message))

def test_instance_ignore(self, caplog):
instance = {'tls_ignore_warning': True}
init_config = {'tls_ignore_warning': False}
http = RequestsWrapper(instance, init_config)

with pytest.warns(None) as record:
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)
expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, _, message in caplog.record_tuples:
assert message != expected_message

def test_instance_no_ignore(self):
def test_instance_no_ignore(self, caplog):
instance = {'tls_ignore_warning': False}
init_config = {'tls_ignore_warning': True}
http = RequestsWrapper(instance, init_config)

with pytest.warns(InsecureRequestWarning):
with caplog.at_level(logging.DEBUG), mock.patch('requests.get'):
http.get('https://www.google.com', verify=False)

expected_message = 'An unverified HTTPS request is being made to https://www.google.com'
for _, level, message in caplog.record_tuples:
if level == logging.WARNING and message == expected_message:
break
else:
raise AssertionError('Expected WARNING log with message `{}`'.format(expected_message))


class TestSession:
def test_default_none(self):
Expand Down

0 comments on commit 797d010

Please sign in to comment.