SOLR-18055 Use solr.ssl.enabled property for url scheme detection by VishnuPriyaChandraSekar · Pull Request #4272 · apache/solr

VishnuPriyaChandraSekar · 2026-04-09T01:08:13Z

https://issues.apache.org/jira/browse/SOLR-18055

Description

Currently, the client is compelled to provide cluster property in order to indicate the url scheme. This is annoying. In this PR, the url scheme is detected using the "solr.ssl.enabled" system property instead of cluster property.

Solution

Improved HttpShardHandlerFactory's url scheme detection by giving higher precedence for "solr.ssl.enabled" over explicit configuration from solr.xml
Improved ZkStateReader's url scheme detection by giving higher precedence for "solr.ssl.enabled" over cluster property
Deprecated the usage of Cluster property in ZkController, ReplicaMutator, SliceMutator and SystemInfoProvider. These classes will depend on ClusterStateProvider to know the url scheme.

Tests

Added new test cases in ZKStateReader to verify the correctness of getUrlScheme
Replaced "urlScheme" system property by "solr.ssl.enabled" in the following test files
- SolrTestCaseJ4.java
- TestMiniSolrCloudClusterSSL.java
Removed the usage of urlScheme in the solr configuration files
Modified the existing tests in solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide
I have added a changelog entry for my change

dsmiley

Thanks.
Remember to ./gradle writeChangelog too

dsmiley · 2026-04-09T01:39:41Z

  }

  private void testUrlSchemeDefault(SolrZkClient client) throws Exception {
+    if (isSSLMode()) {


why are you this back & forth? in some methods?

Looks like the baseClass (SolrTestCaseJ4) randomly runs the server in SSL mode. Thus, I had to put back the system property if the server runs in https (If I don't do this, the next consecutive tests becomes flaky)

dsmiley · 2026-04-09T01:52:29Z

+  /**
+   * Deduce url scheme using environment variable if available otherwise from cluster property
+   *
+   * @return url scheme of host


show the 2 examples please

Will add some in the next revision

dsmiley · 2026-04-09T03:08:00Z

      new RequestReplicaListTransformerGenerator();

-  // URL scheme to be used in distributed search.
-  static final String INIT_URL_SCHEME = "urlScheme";


Curious; was urlScheme arg documented in the ref guide or present in a published solr.xml that people use? If so we'll want to keep it.

Looks like urlScheme is an optional field in solr.xml [1]. In order to provide backward compatibility, I think the precedence to lookup the urlScheme should be 1) solr.ssl.enabled 2) urlScheme from solr.xml 3) default (null).

[1] https://solr.apache.org/guide/solr/latest/configuration-guide/configuring-solr-xml.html

I agree. Thanks for looking into it.

…nd HttpSharedHandlerFactory

dsmiley · 2026-04-16T04:54:17Z

@@ -1,8 +1,15 @@
 # See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
-title: Improved CloudSolrClient's urlScheme detection by using the scheme of provided Solr URLs, or looking at "solr.ssl.enabled".
+title: Improved url scheme detection in client API and core components


IMO this has become much more vague to a reader of the changelog who is anyone other than you and me. The "title" label is misleading -- add more sentences.

How about this "Standardize urlScheme detection with precedence for "solr.ssl.enabled" ?

dsmiley · 2026-04-16T05:02:11Z

    url: https://issues.apache.org/jira/browse/SOLR-18056
+  - name: SOLR-18055
+    url: https://issues.apache.org/jira/browse/SOLR-18055
+important_notes:


This is the first I've seen of this... and you are the first to attempt to use it :-) Our only references to it thus far don't even truly say what it's for.
Documentation. Seems for "breaking changes" or presumably anything befitting of that label. What you added doesn't qualify IMO.

Well the "deprecated" part does... albeit I think I said it's premature to deprecate that.

Well, there is no other field I could use for.
Based on this, I will just have a self descriptive title (IMO this is the hardest part) like "Standardize urlScheme detection by prioritizing "solr.ssl.enabled" over solr.xml and cluster property"

dsmiley · 2026-04-16T05:07:54Z

+   *
+   * <p>Examples:
+   *
+   * <ul>
+   *   <li>Returns {@code "https"} when SSL is enabled via configuration.
+   *   <li>Returns {@code null} when ssl is disabled or no explicit configuration is provided in
+   *       solr.xml.
+   * </ul>


sorry, this is way too much documentation for a private method

dsmiley · 2026-04-16T05:12:19Z

+   *
+   * @return http or https or null
+   */
+  private String getUrlScheme(NamedList<?> args, StringBuilder sb) {


maybe prefix with 'init' not 'get' to emphasize this is for initialization and not any method in this class that might want it. On further reflection, there's getParameter called by init() a bunch of times. This method here was merely extracted from init(). You could put it back. Or call it getParameterUrlScheme

That makes sense :)

SOLR-18055 Use solr.ssl.enabled property for url scheme detection

b036676

github-actions bot added test-framework client:solrj module:ltr module:opentelemetry tests cat:search cat:cloud module:clustering labels Apr 9, 2026

dsmiley reviewed Apr 9, 2026

View reviewed changes

VishnuPriyaChandraSekar and others added 3 commits April 15, 2026 14:20

Added changelog, improved url scheme in ZkController, ZkStateReader a…

deba3e4

…nd HttpSharedHandlerFactory

Merge branch 'apache:main' into SOLR-18055-zkstateReader-urlScheme

d362ed8

Modified changelog

a5ec761

VishnuPriyaChandraSekar marked this pull request as draft April 15, 2026 22:18

dsmiley reviewed Apr 16, 2026

View reviewed changes

Conversation

VishnuPriyaChandraSekar commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Solution

Tests

Checklist

Uh oh!

dsmiley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VishnuPriyaChandraSekar Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

VishnuPriyaChandraSekar commented Apr 9, 2026 •

edited

Loading

VishnuPriyaChandraSekar Apr 9, 2026 •

edited

Loading