[
https://issues.apache.org/jira/browse/HADOOP-11335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14280013#comment-14280013
]
Arun Suresh commented on HADOOP-11335:
--------------------------------------
Thanks for working this [~dian.fu]
I have taken an first pass at the latest patch.. my initial comments follow
(will send out more comments once I do further reviews).
1) JavaKeyStoreProvider:
* createKey() : I think it might be a bit odd that we have metadata with
version == 0
* deleteKey() : Am sorry, I might be missing something but, I did not quite
understand why we can't just delete the metadata from the cache when the key is
deleted
2) KeyProvider:
* The Metadata Class now has a dependency on {{KeyOpType}} which is defined
in {{KeyProviderAuthorizationExtension}}. The Extension classes were meant to
add functionality to a KeyProvider. It seems a bit weird that the KeyProvider
class should have a dependency on an Extension.
* Not very confortable adding setters to Metadata. I Guess the original
implementaion conciously made a choice to not allow modification of metadata
once it is created (except for the version)
3) KeyProviderExtension:
* Do we really need the read and write locks ? The undelying KeyProvider
should take care of the synchronization. (for eg. {{JavaKeyStoreProvider}})
does infact use write and read locks for createKey etc.. this would probably
lead to unnecessary double locking.
4) KeyProviderAuthorizationExtension:
* Same as above.. do we really need the read and write locks ? I feel the
Extension class should handle its own concurrency semantics
5) MetadataKeyAuthorizer
* Remove commented code
Looking at the commented code in {{MetadataKeyAuthorizer}}, I see that you had
initially toyed with having an extended {{MetadataWithACL}} class. Any reason
why you did not pursue that design ? It seems to me like that could have been a
way to probably avoid having to modify the {{JavaKeyStoreProvider}} and
{{KeyProvider}}. One suggestion would have been to templatized {{KeyProvider}}
like so :
{noformat}
public class KeyProvider<M extends Metadata>
...
{noformat}
and have different implemenetation of a {{KeyProvider}} like :
{noformat}
public classs KeyProviderWithACls extends KeyProvider<MetadataWithACLs>
...
{noformat}
> KMS ACL in meta data or database
> --------------------------------
>
> Key: HADOOP-11335
> URL: https://issues.apache.org/jira/browse/HADOOP-11335
> Project: Hadoop Common
> Issue Type: Improvement
> Components: kms
> Affects Versions: 2.6.0
> Reporter: Jerry Chen
> Assignee: Dian Fu
> Labels: Security
> Attachments: HADOOP-11335.001.patch, HADOOP-11335.002.patch, KMS ACL
> in metadata or database.pdf
>
> Original Estimate: 504h
> Remaining Estimate: 504h
>
> Currently Hadoop KMS has implemented ACL for keys and the per key ACL are
> stored in the configuration file kms-acls.xml.
> The management of ACL in configuration file would not be easy in enterprise
> usage and it is put difficulties for backup and recovery.
> It is ideal to store the ACL for keys in the key meta data similar to what
> file system ACL does. In this way, the backup and recovery that works on
> keys should work for ACL for keys too.
> On the other hand, with the ACL in meta data, the ACL of each key can be
> easily manipulate with API or command line tool and take effect instantly.
> This is very important for enterprise level access control management. This
> feature can be addressed by separate JIRA. While with the configuration file,
> these would be hard to provide.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)