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

Load CA certs if SSL is enabled and CA certs are not passed in the configurations #10377

Merged
merged 5 commits into from
Oct 14, 2021

Conversation

steveny91
Copy link
Contributor

What does this PR do?

A recent change in pymongo(3.11.0+) is causing the mongo check to fail if SSL is enabled and no ssl_ca_certs is passed in the config when running the container agent. When tested against Mongodb Atlas, this has been observed to throw an ssl_validation error. This change is aimed at trying to load CA certs from certifi if SSL is enabled and ssl_ca_certs is not configured in the config.

Motivation

From support ticket

Additional Notes

I'm suspecting that with the inclusion of pyopenssl in pymongo(3.11.0), the default location in which pymongo loads these CA certs are no longer the same or it's not being loaded at all.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

@steveny91 steveny91 requested a review from a team as a code owner October 7, 2021 20:01
@ghost ghost added the integration/mongo label Oct 7, 2021
@codecov
Copy link

codecov bot commented Oct 7, 2021

Codecov Report

Merging #10377 (7c12c8c) into master (427adbb) will decrease coverage by 0.06%.
The diff coverage is 50.00%.

Flag Coverage Δ
datadog_checks_dev 79.76% <ø> (-0.11%) ⬇️
mongo 94.52% <50.00%> (+0.36%) ⬆️
sqlserver 82.34% <ø> (-1.91%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@yzhan289 yzhan289 left a comment

Choose a reason for hiding this comment

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

Left a small comment regarding the logic of the conditional, but otherwise LGTM

mongo/datadog_checks/mongo/config.py Outdated Show resolved Hide resolved
Copy link
Contributor

@yzhan289 yzhan289 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@yzhan289 yzhan289 merged commit 8ed69d9 into master Oct 14, 2021
@yzhan289 yzhan289 deleted the steveny91/mongo_ssl branch October 14, 2021 17:02
@steveny91 steveny91 changed the title Load CA certs if SSL is enabled and CA certs are not passed in Load CA certs if SSL is enabled and CA certs are not passed in the configurations Oct 14, 2021
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.

4 participants