Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace InsecureRequestWarning with standard logs #7512

Merged
merged 4 commits into from
Sep 10, 2020
Merged

Replace InsecureRequestWarning with standard logs #7512

merged 4 commits into from
Sep 10, 2020

Conversation

ofek
Copy link
Contributor

@ofek ofek commented Sep 4, 2020

Motivation

Make the warning granular per request without modifying global interpreter state

Additional Notes

Based on https://urllib3.readthedocs.io/en/latest/user-guide.html#certificate-verification, you set the SSL verification method via the constant's name:

>>> import certifi
>>> import urllib3
>>> http = urllib3.PoolManager(
...     cert_reqs='CERT_REQUIRED',
...     ca_certs=certifi.where())

@ofek ofek requested a review from a team as a code owner September 8, 2020 16:30
@ghost ghost added the integration/kubelet label Sep 8, 2020
if level == logging.WARNING and message == expected_message:
break
else:
raise AssertionError('Expected WARNING log with message `{}`'.format(expected_message))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit)

Looks like we are repeating a lot of logic like this in test.

Maybe we should have some kind of utils to avoid duplication.

        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))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, I'll add one in a separate PR after this is merged

kubelet/datadog_checks/kubelet/kubelet.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants