-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update TLS configuration options #2330
Conversation
@@ -19,7 +19,7 @@ configuration settings, you need to restart {beatname_uc} to pick up the changes | |||
* <<redis-output>> | |||
* <<file-output>> | |||
* <<console-output>> | |||
* <<configuration-output-tls>> | |||
* <<configuration-output-ssl>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note that when we change IDs for topics, we need to ask the web team to set up redirects, or our links from forum topics will be broken. That's a good reason to avoid renaming IDs unless (which is probably true in this case) you definitely want the name to change, and it's worth the extra work to get the redirect set up.
0df5e97
to
a5cffdc
Compare
|
||
===== enabled | ||
|
||
The `enabled` settings can be used to disable the ssl configuration by setting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: setting
|
||
If you follow the section on migrating the configuration, you will have TLS | ||
enabled. However, you must be aware that if the `tls` section is missing from the | ||
If you follow the section on migrating the configuration, you will have WSSL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WSSL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ups
This needs an update to the CHANGELOG.md file under breaking changes. |
# Enable SSL support. SSL is automatically enabled, if any SSL setting is set. | ||
#ssl.enabled: true | ||
|
||
# Configure SSL verification mode. If `none` is configured, all server host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/host/hosts/
|
||
|
||
@collect_lines | ||
def migrate_tls_settings(content): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
LGTM. I left a few minor comments. Would be good to get a second review as this one is quite big. |
@urso Did you run |
Yep, still builds. Only issue with docs is (as described by @dedemorton):
|
All looking good, thanks @urso for the great work. Seems like quite a few redirects are required. We should decide one way or the other so we know if we want to merge the PR as is or adjust it. |
I suggest to go with the new urls and do redirects, as ssl is probably also what people search for and want to see in the url. @urso Needs rebase. |
Yeah, not really happy about all the redirect. But agree with @ruflin about using ssl over tls. |
3e74fce
to
2f2bbe5
Compare
rename tls to SSL + adapt some fields: - `insecure` has become `verification_mode`. `insecure: false` is replaced by mode `full` and `insecure: true` by mode `none`. - renamed `certificate_key` to `key` - introduced `key_passphrase` - replaced `min_version` and `max_version` with `supported_protocols` using an array of ok versions. New versions strings: `SSLv3`, `SSLV3.0`, `TLSv1` (use TLSv1.0), `TLSv1.0`, `TLSv1.1` and `TLSv1.2` Changes: - rewrite TLS dialer and TLS configuration - add support for encrypted key files - use TLS dialer with elasticsearch output (some more manual tests with and without system certificates required) - update unit/integration tests to use new settings - update default configuration files to use new ssl settings - update documentation (docs URL did also change from `tls` to `ssl`) - update migration script
rename tls to SSL + adapt some fields:
insecure
has becomeverification_mode
.insecure: false
is replaced by modefull
andinsecure: true
by modenone
.certificate_key
tokey
key_passphrase
min_version
andmax_version
withsupported_protocols
using an array of ok versions. New versions strings:SSLv3
,SSLV3.0
,TLSv1
(use TLSv1.0),TLSv1.0
,TLSv1.1
andTLSv1.2
Changes:
tls
tossl
)