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.

Sign up for GitHub

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

Jump to bottom

Add option to set SNI hostname via the Host header for RequestsWrapper #5833

Merged
merged 16 commits into from
Apr 3, 2020
Merged

Add option to set SNI hostname via the Host header for RequestsWrapper #5833

merged 16 commits into from
Apr 3, 2020

Conversation

ian28223
Copy link
Contributor

@ian28223 ian28223 commented Feb 24, 2020 •
edited
Loading

What does this PR do?

Adds tls_use_host_header parameter to the RequestsWrapper.
If set to true, allows the Host header will be used to match against SSL Certificate CN or SAN.

Motivation

  • http-checks-auto-detect-host-header-and-configure-asserthostname
  • vault-ssl-error-hostname-does-not-match-pod-ip-address-being-returned
  • tls-support-for-consul-on-k8s
  • TLS Support for consul on k8s #4999

Additional Notes

  • conf.yaml.example updated for integrations that asked for it (vault, http_check, consul) but can essentially work with others as well.

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

@ian28223 ian28223 requested review from a team as code owners February 24, 2020 03:21
@codecov
Copy link

codecov bot commented Feb 24, 2020 •
edited
Loading

Codecov Report

Merging #5833 into master will increase coverage by 3.74%.
The diff coverage is n/a.

Impacted Files Coverage Δ
tokumx/tests/test_check.py
oracle/datadog_checks/oracle/queries.py
riak/tests/common.py
tokumx/tests/common.py
mesos_slave/datadog_checks/mesos_slave/__init__.py
rethinkdb/tests/conftest.py
http_check/tests/test_unit_config.py
rethinkdb/datadog_checks/rethinkdb/__init__.py
mesos_master/tests/test_check.py
mapr/datadog_checks/mapr/__about__.py
... and 635 more

sarina-dd
sarina-dd previously approved these changes Feb 24, 2020
View reviewed changes
Copy link
Contributor

@sarina-dd sarina-dd left a comment

Choose a reason for hiding this comment

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

approved for docs

@stale
Copy link

stale bot commented Mar 26, 2020

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Note that this will not be automatically closed, but the notification will remind us to investigate why there's been inactivity.

@stale stale bot added the stale label Mar 26, 2020
ofek
ofek requested changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

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

Nice! WDYTA implicitly having it enable persist_connections?

datadog_checks_base/datadog_checks/base/data/agent_requirements.in Outdated Show resolved Hide resolved
datadog_checks_base/datadog_checks/base/utils/http.py Outdated Show resolved Hide resolved
datadog_checks_base/datadog_checks/base/utils/http.py Outdated Show resolved Hide resolved
datadog_checks_dev/datadog_checks/dev/tooling/templates/configuration/instances/http.yaml Outdated Show resolved Hide resolved
datadog_checks_dev/tests/tooling/configuration/test_load.py Outdated Show resolved Hide resolved
@stale stale bot removed the stale label Mar 26, 2020
@ofek ofek changed the title Adds HostHeaderSSLAdapter Add option to set SNI hostname via the Host header for RequestsWrapper Mar 26, 2020
@ian28223 ian28223 requested a review from a team as a code owner March 26, 2020 07:48
@ian28223
Copy link
Contributor Author

Nice! WDYTA implicitly having it enable persist_connections?

Good idea. I've added that in

@ian28223 ian28223 requested a review from ofek March 27, 2020 09:04
@ian28223 ian28223 force-pushed the ian.bucad/Add_HostHeaderSSLAdapter branch from c510622 to e362a6d Compare April 3, 2020 05:10
ofek
ofek previously approved these changes Apr 3, 2020
View reviewed changes
@ofek ofek merged commit 14643dd into master Apr 3, 2020
@ofek ofek deleted the ian.bucad/Add_HostHeaderSSLAdapter branch April 3, 2020 15:32
@florimondmanca florimondmanca mentioned this pull request Apr 3, 2020
Merged
4 tasks
@therve therve mentioned this pull request Apr 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ofek ofek ofek left review comments

@sarina-dd sarina-dd sarina-dd left review comments

Assignees
No one assigned
Labels
dependencies dev/tooling integration/activemq_xml integration/apache integration/consul integration/datadog_checks_base integration/datadog_checks_dev integration/fluentd integration/http_check integration/kong integration/vault
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ian28223 @ofek @sarina-dd