Skip to content

Commit

Permalink
Remove warning lock
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexandreYang committed Nov 25, 2019
1 parent 704fa31 commit f53af46
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ def send_request(self, endpoint, scraper_config, headers=None):
if isinstance(scraper_config['ssl_ca_cert'], string_types):
verify = scraper_config['ssl_ca_cert']
elif verify is False:
disable_warnings(InsecureRequestWarning)
disable_warnings(InsecureRequestWarning) # NOTE: disables InsecureRequestWarning *globally*

# Determine the authentication settings
username = scraper_config['username']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ def poll(self, endpoint, pFormat=PrometheusFormat.PROTOBUF, headers=None):
if isinstance(self.ssl_ca_cert, string_types):
verify = self.ssl_ca_cert
elif self.ssl_ca_cert is False:
disable_warnings(InsecureRequestWarning)
disable_warnings(InsecureRequestWarning) # NOTE: disables InsecureRequestWarning *globally*
verify = False
try:
response = requests.get(
Expand Down
22 changes: 5 additions & 17 deletions datadog_checks_base/datadog_checks/base/utils/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@
# Licensed under a 3-clause BSD style license (see LICENSE)
import logging
import os
import threading
import warnings
from contextlib import contextmanager

import requests
from six import iteritems, string_types
from six.moves.urllib.parse import urlparse
from urllib3 import disable_warnings
from urllib3.exceptions import InsecureRequestWarning

from ..config import is_affirmative
Expand Down Expand Up @@ -94,10 +93,6 @@ class RequestsWrapper(object):
'request_hooks',
)

# For modifying the warnings filter since the context
# manager that is provided changes module constants
warning_lock = threading.Lock()

def __init__(self, instance, init_config, remapper=None, logger=None):
self.logger = logger or LOGGER
default_fields = dict(STANDARD_FIELDS)
Expand Down Expand Up @@ -275,8 +270,11 @@ def __init__(self, instance, init_config, remapper=None, logger=None):
# Whether or not to log request information like method and url
self.log_requests = is_affirmative(config['log_requests'])

if self.ignore_tls_warning:
disable_warnings(InsecureRequestWarning) # NOTE: disables InsecureRequestWarning *globally*

# Context managers that should wrap all requests
self.request_hooks = [self.handle_tls_warning]
self.request_hooks = []

if config['kerberos_keytab']:
self.request_hooks.append(lambda: handle_kerberos_keytab(config['kerberos_keytab']))
Expand Down Expand Up @@ -337,16 +335,6 @@ def populate_options(self, options):

return options

@contextmanager
def handle_tls_warning(self):
with self.warning_lock:

with warnings.catch_warnings():
if self.ignore_tls_warning:
warnings.simplefilter('ignore', InsecureRequestWarning)

yield

@property
def session(self):
if self._session is None:
Expand Down
29 changes: 25 additions & 4 deletions datadog_checks_base/tests/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import logging
import os
from collections import OrderedDict
from contextlib import contextmanager

import mock
import pytest
Expand Down Expand Up @@ -594,12 +595,12 @@ def test_default_no_ignore(self):
def test_ignore(self):
instance = {'tls_ignore_warning': True}
init_config = {}
http = RequestsWrapper(instance, init_config)

with pytest.warns(None) as record:
http = RequestsWrapper(instance, init_config)
http.get('https://www.google.com', verify=False)

assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)
assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)

def test_default_no_ignore_session(self):
instance = {'persist_connections': True}
Expand All @@ -612,12 +613,32 @@ def test_default_no_ignore_session(self):
def test_ignore_session(self):
instance = {'tls_ignore_warning': True, 'persist_connections': True}
init_config = {}
http = RequestsWrapper(instance, init_config)

with pytest.warns(None) as record:
http = RequestsWrapper(instance, init_config)
http.get('https://www.google.com', verify=False)

assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)
assert all(not issubclass(warning.category, InsecureRequestWarning) for warning in record)

def test_warning_raised_only_once(self, caplog):
""" This test is here to prove that InsecureRequestWarning is only raised once """
instance = {}
init_config = {}

@contextmanager
def capture_warnings_as_log():
logging.captureWarnings(True)
yield
logging.captureWarnings(False)

with capture_warnings_as_log():
http = RequestsWrapper(instance, init_config)
http.get('https://www.example.com', verify=False)

http = RequestsWrapper(instance, init_config)
http.get('https://www.example.com', verify=False)

assert 1 == caplog.text.count("Unverified HTTPS request is being made.")


class TestSession:
Expand Down

0 comments on commit f53af46

Please sign in to comment.