gerlowskija commented on code in PR #826: URL: https://github.com/apache/solr/pull/826#discussion_r868371995
########## solr/modules/aws-secret-provider/README.md: ########## @@ -0,0 +1,80 @@ +Apache Solr - AWS Secret Provider +=========================== + +An implementation of `SecretCredentialsProvider` that pulls Zookeeper credentials from an AWS Secret Manager. + +This plugin uses the [default AWS credentials provider chain](https://docs.aws.amazon.com/sdk-for-java/v2/developer-guide/credentials.html), so ensure that your credentials are set appropriately (e.g., via env var, or in `~/.aws/credentials`, etc.). + +## Usage + +- To enable this feature copy the jar files in `modules/aws-secret-provider/lib` to `SOLR_INSTALL/server/solr-webapp/webapp/WEB-INF/lib/` and add follow the below steps before restarting Solr. Review Comment: > The existing (new feature) to pass modules through environment variable SOLR_MODULES doesn't work, only -Dsolr.modules=scripting option works. Hmm, it works for me. Not sure what's up there, but that's a discussion for a different JIRA and a different PR I guess... > ZKController also needs to create a zkClient, and the call is made in SolrZKClient (solrj) which doesn’t have access to solr core class loaders. True enough - but that default classloader usage is conditional - SolrZkClient only uses it when [the connection-strategy object doesn't already have a ZkCredentialsProvider](https://github.com/apache/solr/blob/main/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java#L155). AFAICT from reading the code and some manual tests, the default classloader block will never run on the Solr server because Solr [always sets a ZkCredentialsProvider](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/cloud/ZkController.java#L340) in ZkController. That's why I suggested biting the bullet and creating a ZkCredentialsInjector impl that's AWS-specific, rather than creating the extra layer of indirection (SecretCredentialsProvider) that we use to make SecretCredentialInjector sm-agnostic. That's what has us run afoul of the classloader stuff: solr-core already has logic to ensure that ZkCredentialsProvider+ZkCredentialsInjector impl's don't hit classloader issues, but we're trying to instantiate some other type that we don't have special handling for in ZkController. We can get around the classloader issues with the additional classloader solution you proposed too, but it makes me leery from a "principle of least change" perspective. Adding a new classloader to Solr and SolrJ is a big change: conceptually if not in terms of LOC. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
