thelabdude opened a new pull request #2010: URL: https://github.com/apache/lucene-solr/pull/2010
# Description See JIRA for description of the issue: https://issues.apache.org/jira/browse/SOLR-12182 # Solution This PR takes the approach of storing the URL scheme (http or https) of the `base_url` for replicas in state.json using a replaceable parameter syntax `${scheme}://` instead of the actual scheme at the time of the write to ZK. The `${scheme}` var is replaced with the current value of the `urlScheme` when `state.json` is read from ZK. I even added some checking to heal existing state.json so we can avoid needing a migration tool. Although, that might be too subtle of a side-effect, so I'm happy to remove that check and just work with base_url's that start with `${scheme}://` I initially tried setting a `ThreadLocal` that gives access to the `urlScheme` whenever we need to read these props from ZK. However, that ended up being problematic because we tend to read ZkNodeProps from ZK in many places. What I landed on is just setting a global var (which I don't love doing), but `urlScheme` really is a global variable that should be set once during initialization by reading from the cluster property stored in ZK. So I felt trying to treat this global as something that was highly dynamic made the code overly cumbersome. I also tried to get rid of the `urlScheme` cluster property (re: https://issues.apache.org/jira/browse/SOLR-10202) but I'm not sure how SolrCloud client applications can resolve the correct `urlScheme` for the cluster without this property? On the server-side, sure we can just get the urlScheme from a Java System Property, but that won't be set for remote client applications that initialize via a connection to ZooKeeper. So I'm keeping the cluster property `urlScheme` for now. I'm also not clear about the `urlScheme` Java System Property ... our tests seem to set it if using `https` and many test solr.xml refer to it. # Tests Existing tests cover these code changes. Re-enabled SSLMigrationTest # Checklist Please review the following and check all that apply: - [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability. - [x] I have created a Jira issue and added the issue ID to my pull request title. - [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended) - [x] I have developed this patch against the `master` branch. - [x] I have run `./gradlew check`. - [x] I have added tests for my changes. - [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only). ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
