-
Notifications
You must be signed in to change notification settings - Fork 86
Standardize and add SSL settings #168
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
Standardize and add SSL settings #168
Conversation
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.
Left some comments, that are quite similar to the observations done for output and input.
…tandardize_ssl_settings
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.
Left a nitpick style note
Co-authored-by: Andrea Selva <[email protected]>
…ttings into a new section
Thank you again for reviewing it, @andsel. I've addressed your suggestions plus the suggestion #1 and suggestion #2 made on the output plugin. |
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.
LGTM
Just update the
deprecated[8.8.0, Replaced by
to refer to the plugin's version instead of Logstash one.
|
||
ssl_key = params["ssl_key"] | ||
if ssl_certificate | ||
raise LogStash::ConfigurationError, 'Using an "ssl_certificate" requires an "ssl_key"' unless ssl_key |
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.
replace an
by a
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.
Thanks for reviewing, @kaisecheng. Although it looks wrong, I think the article "an" is correct here, we pronounce the "s" as "ess" (vowel sound).
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.
I'm gonna move on with this PR and merge it! If that's a mistake I can submit another PR to fix it! Thanks!
Co-authored-by: kaisecheng <[email protected]>
How can I tell what version of Elastic stack this is in? |
Did the trick, thank you! |
I am just running v3.15.3 and I am seeing an error saying " Unknown setting 'ssl_certificate_verification' for elasticsearch" when using this filter |
Hi @tommycahir! The |
What this PR does?
Added the following SSL settings:
ssl_enabled
: Enable/disable the SSL settings. If not provided, the value is inferred from the hosts schemessl_certificate
: OpenSSL-style X.509 certificate file to authenticate the clientssl_key
: OpenSSL-style RSA private key that corresponds to thessl_certificate
ssl_truststore_path
: he JKS truststore to validate the server's certificatessl_truststore_type
: The format of the truststore filessl_truststore_password
: The truststore passwordssl_keystore_path
: The keystore used to present a certificate to the serverssl_keystore_type
: The format of the keystore filessl_keystore_password
: The keystore passwordssl_cipher_suites
: The list of cipher suites to usessl_supported_protocols
: Supported protocols with versionsssl_verification_mode
: Defines how to verify the certificates presented by another party in the TLS connectionReviewed and deprecated SSL settings to comply with Logstash's naming convention
ssl
in favor ofssl_enabled
ca_file
in favor ofssl_certificate_authorities
keystore
in favor ofssl_keystore_path
keystore_password
in favor ofssl_keystore_password
Closes elastic/logstash#14923
Closes #164
Closes #49