flink-connector-elasticsearch icon indicating copy to clipboard operation
flink-connector-elasticsearch copied to clipboard

[FLINK-34369][connectors/elasticsearch] Elasticsearch connector supports SSL context

Open liuml07 opened this issue 1 year ago • 5 comments

https://issues.apache.org/jira/browse/FLINK-34369

02/05/2024

This is a preliminary PR that was tested locally with secure ES clusters. Plan is to get early feedback, add some tests, and test with deployed jobs.

liuml07 avatar Feb 05 '24 20:02 liuml07

Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)

boring-cyborg[bot] avatar Feb 05 '24 20:02 boring-cyborg[bot]

@liuml07 could you please use the same configuration/API model as [1] does for SSL support in OpenSearch? Besides just having familiar configuration, the API is friendly to SQL connector (where providing hostname verifier could be challenging), thank you.

The idea basically is that NetworkClientConfig has a setting:

  • allowInsecure: boolean (uses trustall model in case of self-signed certs)
  • you could certainly also keep more elaborate configuration with SSLContext / SSLEngine / ... in case it is needed

Thank you.

[1] https://github.com/apache/flink-connector-opensearch/tree/main/flink-connector-opensearch

reta avatar Mar 21 '24 13:03 reta

@reta Thanks for the advice. I have updated the patch accordingly. I think the allowInsecure and the sslContextSupplier are mutually exclusive, so I added some validation and unit test for that. I can work on a separate on for table if this looks right.

liuml07 avatar Apr 16 '24 06:04 liuml07

@liuml07 thanks for the contribution it seems the ci failed because of spotless could you please apply spotless to the PR?

snuyanzin avatar Apr 16 '24 07:04 snuyanzin

@reta may I ask you to have another review iteration please?

snuyanzin avatar Apr 22 '24 07:04 snuyanzin

Thanks for reviewing and your approval. Could you help merge this @reta?

liuml07 avatar May 03 '24 05:05 liuml07

Thanks for the contribution @liuml07 Thanks for the review @reta

snuyanzin avatar May 03 '24 07:05 snuyanzin

Awesome work, congrats on your first merged pull request!

boring-cyborg[bot] avatar May 03 '24 07:05 boring-cyborg[bot]