Details
-
Improvement
-
Status: Closed
-
Minor
-
Resolution: Done
-
None
-
None
-
None
Description
TLTR;
This PR adds ZkCredentialsInjector support. What is a ZkCredentialsInjector? It’s a class (interface) that loads ZK/Solr creds from an external source and injects them into ZkACLProvider/ZkCredentialsProvider. This PR uses a Strategy pattern approach where the credentials are injected as a dependency. This allows decoupling the process of reading the creds from the one using them (Single responsibility).
Benefits:
- No more code duplication.
- Adding a new ZK credentials source is as easy as adding a new ZkCredentialsInjector implementation (one class, instead of two).
- Classes implementing ZkCredentialsInjector are completely encapsulated. Therefore, the implementation can be changed without affecting the existing ZkACLProvider/ZkCredntialsProvider classes.
- Different ZkCredentialsInjector can be used interchangeably to alter the creds source without changing the whole architecture.
- Run time interchangeability (think of those situations where you have a client that has to hit two different clusters).
- Backward compatible. Existing and custom providers can still work as we are not breaking the existing ZkACLProvider/ZkCredntialsProvider interfaces.
Problems with the current design
The same code to load the credentials is called twice, first by ZkCredentialsProvider to connect to Zookeeper and a second time by ZkACLProvider to create ACLs. The code is also duplicated, although it's only reading from system props.
Adding a custom pair of ZkCredentialsProvider/ZkACLProvider to load the credentials from another source (ex a Secret Manager) would require duplicating the code and making repetitive calls to extract the same credentials.
The purpose of this PR is to allow retrieving Solr/ZL creds from any source before injecting them into ZK creds/acl providers without duplicating the existing code.
Without this new feature, adding custom ZK credentials provider requires:
1 - Either, duplicating VMParamsAllAndReadonlyDigestZkACLProvider and VMParamsSingleSetCredentialsDigestZkCredentialsProvider code. That means a new pair of classes, 90% identical to the existing classes, for every ZK credentials provider. Adding another ZK credentials provider? Add 2 more duplicate classes…
2 - Or, creating a super abstract class and use inheritance to reuse the code. Still, the code to load the creds would be duplicated and executed twice (two calls to load the same creds from the same source)
I think 1) is not an option.
For 2) I believe this is one of those situations where composition should be favored over inheritance.
Proposed solution
- Refactor the way how the credentials are injected by passing them as a dependency. One code, called once and injected into the client class. Here the client classes are ZkCredentialsProvider and ZkACLProvider.
- Favor composition over inheritance to inject custom credentials loaders without changing the composing (container) class.
- Add a third interface ZkCredentialsInjector whose implementations load ZK credentials from a credentials source to be injected into ZkCredentialsProvider and ZkACLProvider
- The dataflow is: Credentials source —> ZkCredentialsInjector —> ZkCredentialsProvider/ZkACLProvider —> Zookeeper
- The ZkCredentialsInjector gets the creds from an external source which get injected into zkCredentialsProvider and zkACLProvider. The "external source" here can be system props, a file, a Secret Manager, or any other local or remote source.
public interface ZkCredentialsInjector {
List<ZkCredential> getZkCredentials();
...
}
Any class implementing ZkCredentialsInjector can be injected via system props in solr.ini.sh/cmd and solr.xml.
In the below example VMParamsZkCredentialsInjector is injected.
Note: VMParamsAllAndReadonlyDigestZkACLProvider and VMParamsSingleSetCredentialsDigestZkCredentialsProvider would be deprecated and replaced with a combination of DigestZkACLProvider/DigestZkCredentialsProvider and VMParamsZkCredentialsInjector.
SOLR_ZK_CREDS_AND_ACLS=“
_-DzkACLProvider=org.apache.solr.common.cloud.acl.DigestZkACLProvider _
_-DzkCredentialsProvider=org.apache.solr.common.cloud.acl.DigestZkCredentialsProvider _
_-DzkCredentialsInjector=org.apache.solr.common.cloud.acl.VMParamsZkCredentialsInjector _
_-DzkDigestUsername=admin-user -DzkDigestPassword=CHANGEME-ADMIN-PASSWORD _
-DzkDigestReadonlyUsername=readonly-user -DzkDigestReadonlyPassword=CHANGEME-READONLY-PASSWORD"
SOLR_OPTS="$SOLR_OPTS $SOLR_ZK_CREDS_AND_ACLS"
Add DigestZkACLProvider/DigestZkCredentialsProvider classes to support Digest based scheme ZK authentication/authorization
Class DigestZkACLProvider implements ZkACLProvider{
CredentialsInjector credentialsInjector;
...
}
Class DigestZkCredentialsProvider implements ZkCredentialsProvider{
CredentialsInjector credentialsInjector;
...
}
This concept can be generalized to non-digest schemes but that would require more refactoring, it can be achieved in a future contribution if this one is accepted.
