Skip to content

Conversation

@edmocosta
Copy link
Contributor

@edmocosta edmocosta commented Mar 2, 2023

What this PR does?

Added the following SSL settings:
ssl_enabled: Enable/disable the SSL settings. Infer the value from the hosts' scheme if neither the deprecate :ssl nor the new :ssl_enabled configs were set
ssl_certificate: OpenSSL-style X.509 certificate file to authenticate the client
ssl_key: OpenSSL-style RSA private key that corresponds to the ssl_certificate
ssl_truststore_path: he JKS truststore to validate the server's certificate
ssl_truststore_type: The format of the truststore file
ssl_truststore_password: The truststore password
ssl_keystore_path: The keystore used to present a certificate to the server
ssl_keystore_type: The format of the keystore file
ssl_keystore_password: The keystore password
ssl_cipher_suites: The list of cipher suites to use
ssl_supported_protocols: Supported protocols with versions

Reviewed and deprecated SSL settings to comply with Logstash's naming convention
ssl in favor of ssl_enabled:
ca_file in favor of ssl_certificate_authorities
ssl_certificate_verification in favor of ssl_verification_mode

The behavior standardization across plugins, such as the accepted certificate formats, default values, etc will be tackled in future PRs.


Closes elastic/logstash#14922
Closes #96
Closes #71
Closes #42
Closes #115

@edmocosta edmocosta changed the title [WIP] Standardize and add SSL settings Standardize and add SSL settings Mar 8, 2023
@edmocosta edmocosta marked this pull request as ready for review March 8, 2023 15:14
@roaksoax roaksoax requested a review from andsel March 9, 2023 02:51
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Left some comments, for your evaluation

@edmocosta edmocosta requested a review from andsel March 9, 2023 15:57
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

We are almost at the finish just a style nitpick

@edmocosta edmocosta force-pushed the standardize_ssl_settings branch from 36a2a6b to 4f291fa Compare March 10, 2023 10:34
@edmocosta
Copy link
Contributor Author

Left some comments, for your evaluation

Thank you again for reviewing it, @andsel. I've addressed your suggestions plus the suggestion #1 and suggestion #2 made on the output plugin.

@edmocosta edmocosta requested a review from andsel March 10, 2023 10:56
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

LGTM

@edmocosta edmocosta merged commit b37536a into logstash-plugins:main Mar 10, 2023
@edmocosta edmocosta deleted the standardize_ssl_settings branch March 10, 2023 16:44

[id="plugins-{type}s-{plugin}-deprecated-options"]
==== Elasticsearch Input deprecated configuration options

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a departure from the standard approach we've used for deprecated options in other plugins. For example, https://www.elastic.co/guide/en/logstash/current/plugins-inputs-beats.html#plugins-inputs-beats-options.

This approach is cleaner, but not as discoverable for users with older configs.

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