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

bug(opensearch-sink) Setting 'insecure' should override 'cert' #5268

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions data-prepper-plugins/opensearch/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pipeline:

- `hosts`: A list of IP addresses of OpenSearch nodes.

- `cert`(optional): CA certificate that is pem encoded. Accepts both .pem or .crt. This enables the client to trust the CA that has signed the certificate that the OpenSearch cluster is using.
- `cert`(optional): CA certificate that is pem encoded. Accepts both `.pem` or `.crt`. This enables the client to trust the CA that has signed the certificate that the OpenSearch cluster is using. This setting has no effect if `insecure` is set to `true`.
Default is null.

- `aws_sigv4`: A boolean flag to sign the HTTP request with AWS credentials. Only applies to Amazon OpenSearch Service. See [security](security.md) for details. Default to `false`.
Expand All @@ -118,7 +118,7 @@ Default is null.

- `aws_sts_header_overrides`: An optional map of header overrides to make when assuming the IAM role for the sink plugin.

- `insecure`: A boolean flag to turn off SSL certificate verification. If set to true, CA certificate verification will be turned off and insecure HTTP requests will be sent. Default to `false`.
- `insecure`: A boolean flag to turn off SSL certificate verification. If set to true, CA certificate verification will be turned off and insecure HTTP requests will be sent. Setting this will override any `cert` configured. Default to `false`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the same documentation clarification on the source side as on the sink side. Peeking in the configuration code for source, it appears that insecure: true already overrides cert, so this is just a clarification.


- `aws` (Optional) : AWS configurations. See [AWS Configuration](#aws_configuration) for details. SigV4 is enabled by default when this option is used. If this option is present, `aws_` options are not expected to be present. If any of `aws_` options are present along with this, error is thrown.

Expand Down Expand Up @@ -674,10 +674,10 @@ and then every hour after the first time the index is processed.

### <a name="connection_configuration">Connection Configuration</a>

* `insecure` (Optional): A boolean flag to turn off SSL certificate verification. If set to true, CA certificate verification will be turned off and insecure HTTP requests will be sent. Default to false.
* `insecure` (Optional): A boolean flag to turn off SSL certificate verification. If set to true, CA certificate verification will be turned off and insecure HTTP requests will be sent. Setting this will override any `cert` configured. Default to false.


* `cert` (Optional) : CA certificate that is pem encoded. Accepts both .pem or .crt. This enables the client to trust the CA that has signed the certificate that the OpenSearch cluster is using. Default is null.
* `cert` (Optional) : CA certificate that is pem encoded. Accepts both `.pem` or `.crt`. This enables the client to trust the CA that has signed the certificate that the OpenSearch cluster is using. This setting has no effect if `insecure` is set to `true`. Default is null.


* `socket_timeout` (Optional) : A String that indicates the timeout duration for waiting for data. Supports ISO_8601 notation Strings ("PT20.345S", "PT15M", etc.) as well as simple notation Strings for seconds ("60s") and milliseconds ("1500ms"). If this timeout value not set, the underlying Apache HttpClient would rely on operating system settings for managing socket timeouts.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,11 @@ public static ConnectionConfiguration readConnectionConfiguration(final PluginSe

final String certPath = pluginSetting.getStringOrDefault(CERT_PATH, null);
final boolean insecure = pluginSetting.getBooleanOrDefault(INSECURE, false);
if (certPath != null) {
builder = builder.withCert(certPath);
} else {
//We will set insecure flag only if certPath is null
builder = builder.withInsecure(insecure);
// Insecure == true will override configured certPath
if (insecure) {
builder.withInsecure(insecure);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found no way to log a warning if both options are configured at the same time, so this is just a silent override.

} else if (certPath != null) {
builder.withCert(certPath);
}
final String proxy = pluginSetting.getStringOrDefault(PROXY, null);
builder = builder.withProxy(proxy);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,20 @@ void testCreateClientWithCertPath() throws IOException {
client.close();
}

@Test
@Test
void testCreateClientWithInsecureAndCertPath() throws IOException {
// Insecure should take precedence over cert path when both are set
final PluginSetting pluginSetting = generatePluginSetting(
TEST_HOSTS, TEST_USERNAME, TEST_PASSWORD, TEST_CONNECT_TIMEOUT, TEST_SOCKET_TIMEOUT, false, null, null, TEST_CERT_PATH, true);
final ConnectionConfiguration connectionConfiguration =
ConnectionConfiguration.readConnectionConfiguration(pluginSetting);
assertNull(connectionConfiguration.getCertPath());
final RestHighLevelClient client = connectionConfiguration.createClient(awsCredentialsSupplier);
assertNotNull(client);
client.close();
}

@Test
void testCreateOpenSearchClientWithCertPath() throws IOException {
final PluginSetting pluginSetting = generatePluginSetting(
TEST_HOSTS, TEST_USERNAME, TEST_PASSWORD, TEST_CONNECT_TIMEOUT, TEST_SOCKET_TIMEOUT, false, null, null, TEST_CERT_PATH, false);
Expand Down
Loading