hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alejandro Abdelnur (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-11017) KMS delegation token secret manager should be able to use zookeeper as store
Date Thu, 18 Sep 2014 22:48:35 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-11017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14139644#comment-14139644
] 

Alejandro Abdelnur commented on HADOOP-11017:
---------------------------------------------

another partial pass on the latest patch.

*AbstractDelegationTokenManager.java*:
* now that are accessor methods to the instance variables, they should be made private to
ensure all subclasses use the accessor methods. If this becomes too many changes through the
hadoop code, I’d suggest doing a separate JIRA just for that.

*DelegationTokenAutheticationHandler.java*:
* the {{new DelegationTokenManager(..)}} should receive a conf only, all the other params
are get from the conf, this can be moved from here to the {{DelegationTokenManager}} constructor.

*DelegationTokenManager.java*:
* If I recall correctly, the conf you get in the constructor has been trimmed from the prefix
(so, if in the conf file the value was 'hadoop.kms.delegation.token.manager.enable-zk' you
would get here 'delegation.token.manager.enable-zk'). If I’m correct, I would say the prefix
for all the configs here should be 'zk.dt.manager', so in the config file would be ie ’hadoop.kms.zk.dt.manager.numRetries'
(no need for the 'zk' prefix in the last part. And the ZK enable one would be 'zk.dt.manager.enable'

*ZKDelegationTokenSecretManager.java*:
* class should be annotated as Private
* for the {{DELEGATION_KEY_PREFIX}} and the {{DELEGATION_TOKEN_PREFIX}} use shorter constant
values, ie: {{DK_}} and {{DT_}}, less memory in ZK and the wire.
* line 104 {{Builder builder = CuratorFrameworkFactory.builder();}} it is re-created later,
this instance is never used.
* auth should be 'sasl' or 'none' explicitly', defaulting to 'none', failing if it is none
of both.
* is the system property name {{"zookeeper.authProvider.1"}} correct (the '.1’) ?
* will this JAAS config conflict wit the JAAS config of ZK for the hadoop-auth cookie? If
so, we should be able to use the same config for both.
* typo in exception messages 'retirving'
* in a couple of places, you are logging and rethrowing an exception, typically you do one
or the other to avoid double logging/reporting.
* {{updateDelegationKey()}} should be annotated with {{@Override}}


> KMS delegation token secret manager should be able to use zookeeper as store
> ----------------------------------------------------------------------------
>
>                 Key: HADOOP-11017
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11017
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: security
>    Affects Versions: 2.6.0
>            Reporter: Alejandro Abdelnur
>            Assignee: Arun Suresh
>         Attachments: HADOOP-11017.1.patch, HADOOP-11017.2.patch, HADOOP-11017.WIP.patch
>
>
> This will allow supporting multiple KMS instances behind a load balancer.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message