[
https://issues.apache.org/jira/browse/KNOX-1187?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16422620#comment-16422620
]
Phil Zampino commented on KNOX-1187:
------------------------------------
[~smore]
*RemoteAliasService*
* ctor takes AliasService, but expects/requires it to be DefaultAliasService?
** Should take DefaultAliasService explicitly if that is what is
required/expected
* defaultAlias should be named defaultAliasService ?
* buildAliasName() --> buildAliasZNodeName() ?
* buildClusterName() --> buildClusterZNodeName() ?
* getAliasesForCluster
** Does the implementation detail explanation belong in the javadoc?
** defaultAlias.getAliasesForCluster never returns null
** defaultAlias.getAliasesForCluster is invoked before and potentially after
querying the remote aliases; why?
** This method structure could be simplified
* checkPathExists(RemoteConfigurationRegistryClient) can be static?
* addAliasForCluster, removeAliasForCluster:
** Is it better to invoke buildAliasName() once, and resuse the result, rather
than invoking it multiple times with the same args?
* The service depends on the RemoteConfigurationRegistryClient, which is not
ZK-specific, but some of your comments, exceptions and error messages are
ZK-specific.
** Even though the only RemoteConfigurationRegistryClient implementation is
ZK-specific today, that may not be the case in th future, and this is the
motivation for the abstraction. If using the abstraction, we should make as few
assumptions as possible about the actual implementation.
* generateAliasForCluster(String clusterName, String alias)
** Should it delegate to this.defaultAlias.generatePassword(), rather than
invoking DefaultAliasService.generatePassword() statically?
* getPasswordFromAliasForCluster(String clusterName, String alias, boolean
generate)
** final else doesn't need a check for password == null, since the method
already returns if password had been non-null
* getCertificateForGateway: typo in comment (deligate)
* You have methods xxxAliasForClusterLocally(), which you don't use internally;
Should you?
* Do RemoteAliasChildListener and RemoteAliasEntryListener need to be public?
* RemoteAliasChildListener#childEvent(),
RemoteAliasEntryListener#entryChanged():
** Is it safe to assume that
GatewayServer.getGatewayServices().getService(GatewayServices.ALIAS_SERVICE)
returns a RemoteAliasService
***Should the type be verified prior to casting?
*DefaultRemoteConfigurationMonitor*
* Why is this monitor listening for remote aliases? This smells bad to me.
** RemoteAliasService#start() should register the remote alias listener
*EncryptionAlgorithm*
* Is there a reason for the license comment to be placed between the imports
and the class declaration, rather than at the beginning of the file content?
* What is the need for this new class?
* Unit tests?
*ZKAliasServiceTest*
* Why tearDownSuite() does not delete PATH_KNOX_SECURITY instead of only
PATH_KNOX_ALIAS_STORE_TOPOLOGY?
*No CLI yet?* This service is +unusable+ without the CLI commands.
> Distributed Alias Service
> -------------------------
>
> Key: KNOX-1187
> URL: https://issues.apache.org/jira/browse/KNOX-1187
> Project: Apache Knox
> Issue Type: Bug
> Components: Server
> Affects Versions: 0.14.0, 1.0.0
> Reporter: Phil Zampino
> Assignee: Sandeep More
> Priority: Major
> Fix For: 1.1.0
>
> Attachments: KNOX-1187.001.patch
>
>
> Given the ability to manage provider configurations and descriptors in
> ZooKeeper, it would also be good to employ ZooKeeper for managing aliases
> since descriptors reference them for discovery authentication.
> The benefits of ZooKeeper-managed descriptors is limited by the current need
> to individually define the associated aliases at each and every Knox
> instance. Any Knox instance for which the referenced alias has not been
> defined will fail to generate/deploy the topology because service discovery
> will fail.
> The resolution of this issue will provide a Knox administrator the ability to
> define aliases in ZooKeeper, which will be consumed and applied by any Knox
> instance configured to monitor that same ZooKeeper, similar to the way
> provider configurations and descriptors are supported.
> In fact, the alias-related CLI commands could leverage the remote
> configuration monitor config to determine whether the aliases should be
> persisted to / read from ZooKeeper or locally. Knox could use the remote
> configuration client service to monitor the remote alias configuration, and
> apply changes locally.
> This will also require some kind of coordination of Knox master secrets; at a
> minimum, each participating Knox instance will have to have been configured
> with the same master secret.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
