implement buildSecureClientTransportEngine with serverName parameter

[orages]

Description

This PR add support for SecureTransportSettingsProvider.buildSecureClientTransportEngine(Settings settings, String serverName, String hostname, int port) introduced in opensearch-project/OpenSearch#20321

This change add the possibility to create SSL Engine with the TLS Server Name Indication (SNI) extension populated with a given serverName.

Issues Resolved

opensearch-project/OpenSearch#17316

Testing

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR
• API changes companion pull request created
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.68%. Comparing base (f05045f) to head (28d7393).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...org/opensearch/security/ssl/SslContextHandler.java	16.66%	2 Missing and 3 partials ⚠️
.../security/ssl/OpenSearchSecureSettingsFactory.java	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5894      +/-   ##
==========================================
- Coverage   73.69%   73.68%   -0.01%     
==========================================
  Files         437      437              
  Lines       26649    26655       +6     
  Branches     3943     3945       +2     
==========================================
+ Hits        19640    19642       +2     
- Misses       5140     5141       +1     
- Partials     1869     1872       +3     

Files with missing lines	Coverage Δ
.../security/ssl/OpenSearchSecureSettingsFactory.java	55.22% <66.66%> (-0.17%)	⬇️
...org/opensearch/security/ssl/SslContextHandler.java	92.23% <16.66%> (-3.73%)	⬇️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[reta]

Thank you, @orages , please do not forget to update CHANGELOG,md as well

[cwperks]

@orages @reta will the changes in this PR work? It looks like its always passing null for serverName.

Dismissing

[reta]

@orages @reta will the changes in this PR work? It looks like its always passing null for serverName.

Thanks @cwperks Yes, I think it should work, see please https://github.com/opensearch-project/OpenSearch/pull/20321/changes#diff-6267a8e9b6166356b484dc5363c982e8d4b2e4c62cdb9199f06a8908ebf4b485R312 which basically invokes this new factory method with server name.

new ClientSSLHandler(
                            settings,
                            secureTransportSettingsProvider,
                            hostnameVerificationEnabled,
                            hostnameVerificationResolveHostName,
                            node.getAttributes().get("server_name")
                        )

[cwperks]

Oh I see now, we are overloading buildSecureClientTransportEngine in core. Any reason why we are overloading it instead of modifying the existing signature? I'm not terribly concerned about bwc bc the security plugin would be the only consumer.

[reta]

Oh I see now, we are overloading buildSecureClientTransportEngine in core. Any reason why we are overloading it instead of modifying the existing signature? I'm not terribly concerned about bwc bc the security plugin would be the only consumer.

I think we could definitely use the new method signature only, the weak motivation behind keeping 2 options is that in 90% cases, the server name would not be needed