[
https://issues.apache.org/jira/browse/HDFS-5658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13852358#comment-13852358
]
Chris Nauroth commented on HDFS-5658:
-------------------------------------
Hi, Haohui. This patch looks good. A couple of comments:
# In {{FSDirectory#getAclStatus}}, can we take care of sticky bit in this patch
by calling {{node.getFsPermission().getStickyBit()}}?
# Minor: In {{resolveINodeWithAdditionalField}}, the throw statement is
indented 4 spaces instead of 2.
I think that's all that needs to be addressed before commit to the feature
branch.
Here are a couple of other comments that I don't think need to be addressed in
this patch. These are just some things for us to keep in mind. I think
they're going to be easier to address in other patches.
# We'll want to add more tests in {{TestNameNodeAcl}}. I think this will make
more sense in a separate patch to finish out the wiring of all methods through
{{NameNodeRpcServer}} and {{FSNamesystem}}.
# Right now, {{getAclStatus}} returns an empty entry list for inodes that don't
have an ACL. We'll need to change this so that it returns a minimal ACL:
exactly 3 ACL entries for owner, group and other, inferred from the
{{FsPermission}} bits. I think this will make more sense in a separate patch
to finish out the interactions with classic permission bits and chmod.
# Regarding the {{FileNotFoundException}} thrown from
{{resolveINodeWithAdditionalField}}, I think the only kind of {{INode}} that
won't be an {{INodeWithAdditionalFields}} is an {{INodeReference}} (used inside
a snapshot). At this point, the code should never see one of those, because
{{INodeDirectory#getINodesInPath4Write}} throws a
{{SnapshotAccessControlException}} for attempts to modify a snapshot path. I
think the only way we'd hit this code path is if there was a bug, so
{{FileNotFoundException}} might not be the right choice. I think this decision
is easier to finalize as part of HDFS-5614, which tracks interaction of ACLs
with snapshots.
> Implement ACL as a INode feature
> --------------------------------
>
> Key: HDFS-5658
> URL: https://issues.apache.org/jira/browse/HDFS-5658
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client, namenode, security
> Reporter: Haohui Mai
> Assignee: Haohui Mai
> Attachments: HDFS-5658.000.patch, HDFS-5658.001.patch
>
>
> HDFS-5284 introduces features as generic abstractions to extend the
> functionality of the inodes. The implementation of ACL should leverage the
> new abstractions.
--
This message was sent by Atlassian JIRA
(v6.1.4#6159)
