[
https://issues.apache.org/jira/browse/HDFS-5758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13873865#comment-13873865
]
Chris Nauroth commented on HDFS-5758:
-------------------------------------
bq. It seems to me that you can enforce the permission by the following:
Yes, this is basically what the HDFS-5612 enforcement patch does, except we
can't run the existing logic for checking {{FsPermission}} at all if an inode
has an ACL, so it would be more like:
{code}
if (hasAcl) {
decide whether acl allows the access
} else {
checkFsPermission();
}
{code}
Always checking {{FsPermission}} before the ACL could erroneously deny access
to someone who had been granted access through a named user or named group ACL
entry.
bq. I'm not sure why AclStorage#updateINodeAcl and AclStorage#readINodeAcl need
to construct entries from the old permissions.
There are a few different reasons for this:
# Every inode in the system really has a logical ACL. Even if the ACL bit is
off and there is no {{AclFeature}}, the inode still has a minimal ACL. Calling
{{getAclStatus}} on an inode with no ACL bit/no {{AclFeature}} still needs to
return the minimal ACL, so we construct 3 ACL entries based on the permission
bits.
# For an inode that has an ACL, the expected behavior of chmod is that it
changes the mask entry instead of the group permission bits. Likewise, file
listings should show the mask permissions in place of the group permissions.
By choosing to store the mask permission inside the group permission bits, we
minimize the impact on existing code like {{setPermission}} and {{listStatus}}.
Those APIs continue to read/write bits in {{FsPermission}} without needing
awareness that they are logically reading/writing the mask. Only the ACL APIs
need this awareness.
# Updates to the owner and other ACL entries are also supposed to update the
corresponding owner and other permission bits. For symmetry, if chmod changes
those permissions, then the change is also supposed to be visible in the
corresponding ACL entries. Similar to the above, by choosing to reuse the
corresponding {{FsPermission}} bits for this purpose, then a lot of existing
code just works with no changes. We also prevent the possibility of bugs in
trying to keep 2 data sources in sync (one copy in {{FsPermission}} and one
copy in an {{AclEntry}} instance).
# Finally, I expect a minor storage optimization as a result of this strategy.
We're keeping 3 of the "logical" ACL entries in {{FsPermission}}, so that's 3
fewer {{AclEntry}} instances to store inside each {{AclFeature}}.
Additionally, this will increase the likelihood of de-duplication ("Global ACL
Set" as described in the design doc) once we implement that. If 2 ACLs differ
only in their owner, mask or other entries, then their underlying
{{AclFeature}} instances will still have the same contents, and we can
de-duplicate them.
> NameNode: complete implementation of inode modifications for ACLs.
> ------------------------------------------------------------------
>
> Key: HDFS-5758
> URL: https://issues.apache.org/jira/browse/HDFS-5758
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Affects Versions: HDFS ACLs (HDFS-4685)
> Reporter: Chris Nauroth
> Assignee: Chris Nauroth
> Attachments: HDFS-5758.1.patch, HDFS-5758.2.patch
>
>
> This patch will complete the remaining logic for the ACL get and set APIs,
> including remaining work in {{FSNamesystem}}, {{FSDirectory}} and storage in
> the inodes.
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
