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

TLS config renegotiation setting (Issue #10871) #12354

Merged
merged 3 commits into from
May 31, 2019
Merged

TLS config renegotiation setting (Issue #10871) #12354

merged 3 commits into from
May 31, 2019

Conversation

konstantinoskostis
Copy link
Contributor

@konstantinoskostis konstantinoskostis commented May 29, 2019 •
edited
Loading

This Pull Request fixes the issue described at #10871

@konstantinoskostis konstantinoskostis requested a review from a team as a code owner May 29, 2019 20:34
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@elasticcla
Copy link

Hi @konstantinoskostis, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in your Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@ph
Copy link
Contributor

ph commented May 29, 2019

@konstantinoskostis Can you sign the CLA?

@ph
Copy link
Contributor

ph commented May 29, 2019

jenkins test this please

@konstantinoskostis konstantinoskostis mentioned this pull request May 29, 2019
Closed
@konstantinoskostis
Copy link
Contributor Author

@ph hello!
I have already signed the CLA ... but as i wrote in the issue
Not sure why the CLA signature is marked as erroneous. My email is correct, present in https://patch-diff.githubusercontent.com/raw/elastic/beats/pull/12354.patch and present on github!

Let me sign it one more time and see if it works.

@ph
Copy link
Contributor

ph commented May 29, 2019

@konstantinoskostis Not all email are actually valid in the PR, like this one:

From 3c67cf822dc4bd8b5e3765ad01700471267acafd Mon Sep 17 00:00:00 2001
From: konstantinos kostis <konstantinos@MacBook-Pro.local>
Date: Wed, 29 May 2019 21:49:22 +0300
Subject: [PATCH 1/3] [tlscommon] Config: Fix yaml typo

@konstantinoskostis konstantinoskostis force-pushed the tls_config_renegotiation_setting_10871 branch from 911b710 to 5592fbe Compare May 29, 2019 21:13
@konstantinoskostis
Copy link
Contributor Author

@ph sorry about that... I edited the commit to fix the author and now CLA check has gone green!

@ph
Copy link
Contributor

ph commented May 29, 2019

jenkins test this please

kvch
kvch reviewed May 30, 2019
View reviewed changes
CHANGELOG.next.asciidoc Outdated
@@ -54,6 +54,8 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d

*Affecting all Beats*

- Fix a yaml typo in libbeat/common/transport/tlscommon/config.go {issue}10871[10871], {pull}12354[12354]
Copy link
Contributor

@kvch kvch May 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I think one line should be enough to describe the change. As the Changelog is intended for users, mentioning Golang packages and structs is not appropiate. I would reword this to "Fix typo in TLS renegotiation configuration and setting the option correctly".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kvch your proposition sounds and looks better, thank you! I updated the entry accordingly!

@konstantinoskostis
[tlscommon] Config: Fix yaml typo
065fec6 
This commit fixes `yaml:"renegotation"` to `yaml:"renegotiation"`
in libbeat/common/transport/tlscommon/config.go
@konstantinoskostis
[tlscommon] TLSConfig: Pass Renegotiation setting in tls.Config
de3afee 
This commit passes in tls.Config the Renegotiation setting
which was missing. It also enhances tests regarding this
setting.

Fixes elastic#10871
@konstantinoskostis
Update CHANGELOG.next.asciidoc
cca0e88 
This commit adds two entries to CHANGELOG.next.asciidoc since it fixes
both a typo and a bug. The entries have been added since it is
instructed to do so according to
https://www.elastic.co/guide/en/beats/devguide/current/beats-contributing.html
@konstantinoskostis konstantinoskostis force-pushed the tls_config_renegotiation_setting_10871 branch from 5592fbe to cca0e88 Compare May 30, 2019 21:19
@kvch kvch added the needs_backport PR is waiting to be backported to other branches. label May 31, 2019
@kvch kvch merged commit 92ab96b into elastic:master May 31, 2019
@kvch
Copy link
Contributor

kvch commented May 31, 2019

Closes #10871

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvch kvch kvch approved these changes

Assignees
No one assigned
Labels
needs_backport PR is waiting to be backported to other branches.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@konstantinoskostis @elasticmachine @elasticcla @ph @kvch