[
https://issues.apache.org/jira/browse/SOLR-16192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17603238#comment-17603238
]
Lamine edited comment on SOLR-16192 at 9/12/22 7:43 PM:
--------------------------------------------------------
Hi [~dsmiley] Updated the description.
There was an old JIRA before this PR got split into two "sub PRs". The second
PR is in progress.
was (Author: JIRAUSER282315):
Hi [~dsmiley] Updated the description.
There was an [old JIRA|https://issues.apache.org/jira/browse/SOLR-15857] before
this PR got split into two "sub PRs".
> Add ZK credentials injectors
> ----------------------------
>
> Key: SOLR-16192
> URL: https://issues.apache.org/jira/browse/SOLR-16192
> Project: Solr
> Issue Type: Improvement
> Reporter: Lamine
> Priority: Minor
> Time Spent: 7h
> Remaining Estimate: 0h
>
> 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.
> h2. 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.
> h2. 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 (a kind of Strategy
> pattern) but that would require more refactoring, it can be achieved in a
> future contribution if this one is accepted.
> Thank you in advance for your time and your comments.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
