Fix automatic addition of protocol in discovery endpoint

[heyanlong]

Description

Fix #11609

Related Issues

Resolves #11609

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)
• Public documentation issue/PR created

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.

[github-actions]

Compatibility status:

Checks if related components are compatible with change 1b277ab

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/performance-analyzer.git]

[github-actions]

✅ Gradle check result for 7263a02: SUCCESS

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (074bc6a) 71.31% compared to head (62866a8) 71.34%.
Report is 4 commits behind head on main.

❗ Current head 62866a8 differs from pull request most recent head 1b277ab. Consider uploading reports for the commit 1b277ab to get more accurate results

Files	Patch %	Lines
...rg/opensearch/discovery/ec2/AwsEc2ServiceImpl.java	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #11612      +/-   ##
============================================
+ Coverage     71.31%   71.34%   +0.03%     
- Complexity    59155    59162       +7     
============================================
  Files          4906     4906              
  Lines        278178   278178              
  Branches      40421    40421              
============================================
+ Hits         198388   198475      +87     
+ Misses        63260    63152     -108     
- Partials      16530    16551      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

✅ Gradle check result for 62866a8: SUCCESS

[dblock]

Before we merge this, any reason to hand-roll our own implementation vs. using something like HttpHost.create(endpoint).getHostName()?

[heyanlong]

Before we merge this, any reason to hand-roll our own implementation vs. using something like HttpHost.create(endpoint).getHostName()?

Yes, we can use HttpHost.create(endpoint).getScheme() to check if it has scheme, but I don't think this simple validation requires using a third party implementation

[github-actions]

❌ Gradle check result for b3b210a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

❕ Gradle check result for 1b277ab: UNSTABLE

• TEST FAILURES:

      1 org.opensearch.search.SearchWeightedRoutingIT.testShardRoutingWithNetworkDisruption_FailOpenEnabled

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[dblock]

Before we merge this, any reason to hand-roll our own implementation vs. using something like HttpHost.create(endpoint).getHostName()?

Yes, we can use HttpHost.create(endpoint).getScheme() to check if it has scheme, but I don't think this simple validation requires using a third party implementation

That's fine here. Generally I prefer well tested libraries that implement an RFC because, I think, you'll agree that this exact PR is a demonstration of the fact that validation may not be totally trivial. This fixes a bug introduced by rolling out custom validation.

[heyanlong]

Before we merge this, any reason to hand-roll our own implementation vs. using something like HttpHost.create(endpoint).getHostName()?

Yes, we can use HttpHost.create(endpoint).getScheme() to check if it has scheme, but I don't think this simple validation requires using a third party implementation

That's fine here. Generally I prefer well tested libraries that implement an RFC because, I think, you'll agree that this exact PR is a demonstration of the fact that validation may not be totally trivial. This fixes a bug introduced by rolling out custom validation.

It is like this, for stricter validation, libraries are a better choice.

[dblock]

It is like this, for stricter validation, libraries are a better choice.

@heyanlong feel free to PR a change that uses the library! we can compare looking at the code