This is an automated email from the ASF dual-hosted git repository. gerlowskija pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 49fa3abf1a0b28c48ce5834d001b1d869c2b699f Author: Lamine <[email protected]> AuthorDate: Mon May 22 09:18:38 2023 -0500 SOLR-16687: Add a SolrClassLoader to SolrZkClient (#1508) Allows ZkCredentialsProviders from other modules or plugins to be found by SolrZkClient. --------- Co-authored-by: Lamine Idjeraoui <[email protected]> Co-authored-by: Jason Gerlowski <[email protected]> --- solr/CHANGES.txt | 2 + .../src/java/org/apache/solr/core/NodeConfig.java | 12 +++- .../org/apache/solr/core/TestCoreContainer.java | 2 +- .../org/apache/solr/common/cloud/SolrZkClient.java | 64 +++++++++++++++------- .../apache/solr/common/cloud/SolrZkClientTest.java | 32 +++++++++++ ...DigestZkACLAndCredentialsProvidersTestBase.java | 2 - 6 files changed, 89 insertions(+), 25 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index e739184bf34..bd4df75b193 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -107,6 +107,8 @@ Improvements * SOLR-16470: `/coreName/replication?commit=indexversion` now has a v2 equivalent, available at `GET /api/cores/coreName/replication/indexversion` (Matthew Biscocho via Jason Gerlowski) +* SOLR-16687: Add support of SolrClassLoader to SolrZkClient (Lamine Idjeraoui via Jason Gerlowski & Houston Putman) + Optimizations --------------------- diff --git a/solr/core/src/java/org/apache/solr/core/NodeConfig.java b/solr/core/src/java/org/apache/solr/core/NodeConfig.java index 53df61cc987..fbf5cc4fe65 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeConfig.java +++ b/solr/core/src/java/org/apache/solr/core/NodeConfig.java @@ -206,6 +206,8 @@ public class NodeConfig { log.warn( "Solr property solr.solrxml.location is no longer supported. Will automatically load solr.xml from ZooKeeper if it exists"); } + final SolrResourceLoader loader = new SolrResourceLoader(solrHome); + initModules(loader, null); nodeProperties = SolrXmlConfig.wrapAndSetZkHostFromSysPropIfNeeded(nodeProperties); String zkHost = nodeProperties.getProperty(SolrXmlConfig.ZK_HOST); if (StrUtils.isNotNullOrEmpty(zkHost)) { @@ -216,6 +218,7 @@ public class NodeConfig { .withUrl(zkHost) .withTimeout(startUpZkTimeOut, TimeUnit.MILLISECONDS) .withConnTimeOut(startUpZkTimeOut, TimeUnit.MILLISECONDS) + .withSolrClassLoader(loader) .build()) { if (zkClient.exists("/solr.xml", true)) { log.info("solr.xml found in ZooKeeper. Loading..."); @@ -259,7 +262,7 @@ public class NodeConfig { * * @return path to install dir or null if solr.install.dir not set. */ - public Path getSolrInstallDir() { + public static Path getSolrInstallDir() { String prop = System.getProperty(SolrDispatchFilter.SOLR_INSTALL_DIR_ATTRIBUTE); if (prop == null || prop.isBlank()) { log.debug("solr.install.dir property not initialized."); @@ -483,7 +486,12 @@ public class NodeConfig { // Adds modules to shared classpath private void initModules() { - var moduleNames = ModuleUtils.resolveModulesFromStringOrSyspropOrEnv(getModules()); + initModules(loader, getModules()); + } + + // can't we move this to ModuleUtils? + public static void initModules(SolrResourceLoader loader, String modules) { + var moduleNames = ModuleUtils.resolveModulesFromStringOrSyspropOrEnv(modules); boolean modified = false; Path solrInstallDir = getSolrInstallDir(); diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java index f5a70c5dc5e..e6b194858c9 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java +++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java @@ -490,7 +490,7 @@ public class TestCoreContainer extends SolrTestCaseJ4 { final CoreContainer cores = init(CONFIGSETS_SOLR_XML); try { - Path solrInstallDir = cores.getConfig().getSolrInstallDir(); + Path solrInstallDir = NodeConfig.getSolrInstallDir(); assertTrue( "solrInstallDir was " + solrInstallDir, solrInstallDir != null && installDirPath.toString().equals(solrInstallDir.toString())); diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java index ccfb71b726e..b7aeec05b7d 100644 --- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java +++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/SolrZkClient.java @@ -100,6 +100,7 @@ public class SolrZkClient implements Closeable { private ZkACLProvider zkACLProvider; private ZkCredentialsInjector zkCredentialsInjector; private String zkServerAddress; + private SolrClassLoader solrClassLoader; private IsClosed higherLevelIsClosed; @@ -117,7 +118,8 @@ public class SolrZkClient implements Closeable { builder.beforeReconnect, builder.zkACLProvider, builder.higherLevelIsClosed, - builder.compressor); + builder.compressor, + builder.solrClassLoader); } private SolrZkClient( @@ -129,7 +131,8 @@ public class SolrZkClient implements Closeable { BeforeReconnect beforeReconnect, ZkACLProvider zkACLProvider, IsClosed higherLevelIsClosed, - Compressor compressor) { + Compressor compressor, + SolrClassLoader solrClassLoader) { if (zkServerAddress == null) { // only tests should create one without server address @@ -144,6 +147,7 @@ public class SolrZkClient implements Closeable { } this.zkClientConnectionStrategy = strat; + this.solrClassLoader = solrClassLoader; if (!strat.hasZkCredentialsToAddAutomatically()) { zkCredentialsInjector = createZkCredentialsInjector(); ZkCredentialsProvider zkCredentialsToAddAutomatically = @@ -236,10 +240,13 @@ public class SolrZkClient implements Closeable { try { log.info("Using ZkCredentialsProvider: {}", zkCredentialsProviderClassName); ZkCredentialsProvider zkCredentialsProvider = - Class.forName(zkCredentialsProviderClassName) - .asSubclass(ZkCredentialsProvider.class) - .getConstructor() - .newInstance(); + solrClassLoader == null + ? Class.forName(zkCredentialsProviderClassName) + .asSubclass(ZkCredentialsProvider.class) + .getConstructor() + .newInstance() + : solrClassLoader.newInstance( + zkCredentialsProviderClassName, ZkCredentialsProvider.class); zkCredentialsProvider.setZkCredentialsInjector(zkCredentialsInjector); return zkCredentialsProvider; } catch (Exception e) { @@ -261,16 +268,21 @@ public class SolrZkClient implements Closeable { try { log.info("Using ZkACLProvider: {}", zkACLProviderClassName); ZkACLProvider zkACLProvider = - Class.forName(zkACLProviderClassName) - .asSubclass(ZkACLProvider.class) - .getConstructor() - .newInstance(); + solrClassLoader == null + ? Class.forName(zkACLProviderClassName) + .asSubclass(ZkACLProvider.class) + .getConstructor() + .newInstance() + : solrClassLoader.newInstance(zkACLProviderClassName, ZkACLProvider.class); zkACLProvider.setZkCredentialsInjector(zkCredentialsInjector); return zkACLProvider; } catch (Exception e) { - // just ignore - go default - log.warn( - "VM param zkACLProvider does not point to a class implementing ZkACLProvider and with a non-arg constructor", + // Fail-fast. If the instantiation fails better fail-fast rather than use the default unsafe + // ZkACLProvider + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, + "VM param zkACLProvider does not point to a class implementing " + + "ZkACLProvider and with a non-arg constructor", e); } } @@ -288,14 +300,20 @@ public class SolrZkClient implements Closeable { if (StrUtils.isNotNullOrEmpty(zkCredentialsInjectorClassName)) { try { log.info("Using ZkCredentialsInjector: {}", zkCredentialsInjectorClassName); - return Class.forName(zkCredentialsInjectorClassName) - .asSubclass(ZkCredentialsInjector.class) - .getConstructor() - .newInstance(); + return solrClassLoader == null + ? Class.forName(zkCredentialsInjectorClassName) + .asSubclass(ZkCredentialsInjector.class) + .getConstructor() + .newInstance() + : solrClassLoader.newInstance( + zkCredentialsInjectorClassName, ZkCredentialsInjector.class); } catch (Exception e) { - // just ignore - go default - log.warn( - "VM param ZkCredentialsInjector does not point to a class implementing ZkCredentialsInjector and with a non-arg constructor", + // Fail-fast. If the instantiation fails better fail-fast rather than use the default unsafe + // ZkCredentialsInjector + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, + "VM param zkCredentialsInjector does not point to a class implementing " + + "ZkCredentialsInjector and with a non-arg constructor", e); } } @@ -1116,6 +1134,7 @@ public class SolrZkClient implements Closeable { public ZkClientConnectionStrategy connectionStrategy; public ZkACLProvider zkACLProvider; public IsClosed higherLevelIsClosed; + public SolrClassLoader solrClassLoader; public Compressor compressor; @@ -1164,6 +1183,11 @@ public class SolrZkClient implements Closeable { return this; } + public Builder withSolrClassLoader(SolrClassLoader solrClassLoader) { + this.solrClassLoader = solrClassLoader; + return this; + } + public SolrZkClient build() { return new SolrZkClient(this); } diff --git a/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java b/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java index aee07343b97..9d1eacd8950 100644 --- a/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java +++ b/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java @@ -33,6 +33,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.cloud.AbstractZkTestCase; import org.apache.solr.cloud.SolrCloudTestCase; import org.apache.solr.cloud.ZkTestServer; +import org.apache.solr.core.SolrResourceLoader; import org.apache.solr.util.ExternalPaths; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.WatchedEvent; @@ -281,4 +282,35 @@ public class SolrZkClientTest extends SolrCloudTestCase { SolrZkClient.checkInterrupted(new InterruptedException()); assertTrue(Thread.currentThread().isInterrupted()); } + + @Test + public void testInstantiationWithSolrResourceLoader() { + enableCustomCredentialsProvider(); + SolrResourceLoader solrResourceLoader = + new SolrResourceLoader(Path.of("."), ClassLoader.getSystemClassLoader()); + new SolrZkClient.Builder() + .withUrl(zkServer.getZkHost()) + .withTimeout(AbstractZkTestCase.TIMEOUT, TimeUnit.MILLISECONDS) + .withSolrClassLoader(solrResourceLoader) + .build() + .close(); // no more tests needed. We only test class instantiation + } + + private static void enableCustomCredentialsProvider() { + System.setProperty( + SolrZkClient.ZK_CRED_PROVIDER_CLASS_NAME_VM_PARAM_NAME, + DigestZkCredentialsProvider.class.getName()); + System.setProperty( + SolrZkClient.ZK_ACL_PROVIDER_CLASS_NAME_VM_PARAM_NAME, DigestZkACLProvider.class.getName()); + System.setProperty( + SolrZkClient.ZK_CREDENTIALS_INJECTOR_CLASS_NAME_VM_PARAM_NAME, + CustomZkCredentialsInjector.class.getName()); + } + + public static class CustomZkCredentialsInjector implements ZkCredentialsInjector { + @Override + public List<ZkCredential> getZkCredentials() { + return List.of(new ZkCredential("someuser", "somepass", ZkCredential.Perms.READ)); + } + } } diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractDigestZkACLAndCredentialsProvidersTestBase.java b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractDigestZkACLAndCredentialsProvidersTestBase.java index 80dfaa93f01..dbbf308bad1 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/AbstractDigestZkACLAndCredentialsProvidersTestBase.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/AbstractDigestZkACLAndCredentialsProvidersTestBase.java @@ -57,8 +57,6 @@ public class AbstractDigestZkACLAndCredentialsProvidersTestBase extends SolrTest private static final String READONLY_USERNAME = "readonlyACLUsername"; private static final String READONLY_PASSWORD = "readonlyACLPassword"; - public static final String SECRET_NAME = "zkCredentialsSecret"; - protected ZkTestServer zkServer; protected Path zkDir;
