gerlowskija commented on code in PR #826:
URL: https://github.com/apache/solr/pull/826#discussion_r866976673


##########
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:
   I spent the morning trying out the `SOLR_MODULES` and `<sharedLib>` setups 
and can confirm that I also hit ClassNotFoundExceptions on Solr startup.
   
   Both of those config options (`SOLR_MODULES` and `<sharedLib>`) work by 
adding the lib directory for the contrib/module to the classloader used by 
Solr's "CoreContainer" class.  From there it's the solr server's job to use 
this particular classloader when instantiating any pluggable classes.
   
   We are doing this correctly for at least some of the relevant instantiations 
(e.g. the ZkCredentialsInjector and ZkCredentialsProvider instantiations 
[here](https://github.com/apache/solr/pull/826/files#diff-5b63503605ede4384429e74d1fa0c410adc5da8f3246e8c36e49feff2f3ea692R333)).
  But we _aren't_ doing this when instantiating "SecretCredentialsProvider": in 
that case [the instantiating code in SecretCredentialInjector uses the default 
classloader](https://github.com/apache/solr/pull/826/files#diff-a01f74c25f0b6595287532fe1e307d201c32adeed1bb9e9eb03a38115887449dR77)
 so it can't "see" AWSSecretCredentialsProvider.
   
   The way I see it, we've got a couple options:
   
   1. We could add a setter to the "ZkCredentialInjector" interface to provide 
injectors with a classloader that they can use to load any pluggable classes 
they might need.  The Solr server code could invoke this setter to pass the 
CoreContainer loader down to the SolrJ code.  This strikes me as a little 
kludgey, but I don't have any particularly good reason not to like it.
   2. Alternatively: the server code in ZkController already uses the correct 
classloader when it instantiates ZkCredentialInjector instances.  If we made a 
ZkCredentialInjector that was AWS specific (in effect merging 
AWSSecretCredentialProvider and SecretCredentialInjector), then the loading 
problem would disappear entirely.  Don't get me wrong, I loved the design you 
came up with to make SecretCredInjector reusable for different Secret Managers, 
but...it might be a tolerable sacrifice to get around the classloading issue.
   3. We could just accept the current setup and require Solr admin's to 
manually copy around jars to use this component.
   
   Anyone have thoughts?



-- 
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]

Reply via email to