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

[7.x][heartbeat][libbeat][metricbeat][filebeat] #15874

Merged
merged 2 commits into from
Jan 27, 2020

Conversation

andrewvc
Copy link
Contributor

Pass TLS options to forwar…d proxies (#15516)

Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( #15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes #15524

(cherry picked from commit 0e57f6b)

What does this PR do?

Why is it important?

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

…d proxies (elastic#15516)

Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( elastic#15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes elastic#15524

(cherry picked from commit 0e57f6b)
@andrewvc andrewvc added bug backport Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team v7.7.0 labels Jan 27, 2020
@andrewvc andrewvc requested a review from urso January 27, 2020 18:00
@andrewvc andrewvc requested a review from a team as a code owner January 27, 2020 18:00
@andrewvc andrewvc self-assigned this Jan 27, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (:uptime)

@@ -64,6 +64,8 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix missing output in dockerlogbeat {pull}15719[15719]
- Fix logging target settings being ignored when Beats are started via systemd or docker. {issue}12024[12024] {pull}15422[15442]
- Do not load dashboards where not available. {pull}15802[15802]
- Fix issue where TLS settings would be ignored when a forward proxy was in use. {pull}15516{15516}
- Update replicaset group to apps/v1 {pull}15854[15802]
Copy link

Choose a reason for hiding this comment

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

looks like changelog needs to be cleaned up.

@andrewvc andrewvc merged commit f8022d5 into elastic:7.x Jan 27, 2020
@andrewvc andrewvc deleted the backport_15516_7.x branch January 27, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport bug Heartbeat Team:obs-ds-hosted-services Label for the Observability Hosted Services team v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants