[
https://issues.apache.org/jira/browse/HDFS-5758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13874078#comment-13874078
]
Chris Nauroth commented on HDFS-5758:
-------------------------------------
bq. It's difficult for the client to differentiate the ACLs you've generated
and the ones that the admins have specified.
This sounds like a scenario where one person is responsible for chmod and
another person is responsible for setfacl, and you want to be able to tell who
did which part of it. Is that right? In general, it's going to be impossible
to reliably tell the difference no matter which implementation choice we make.
This is because setfacl is allowed to change permission bits. Calling setfacl
\-\-set user::rwx,group::r\-\-,other::r\-\- on a file that has no extended ACL
changes just the permission bits. Likewise, chmod is allowed to change
extended ACL entries. It can change one specific extended ACL entry, the mask
entry, if the file has an extended ACL.
bq. We have to implement optimizations to get rid of minimal ACLs.
Does this refer to the code paths where we check for ACL size 3? I don't see a
way to eliminate this completely, but at least that logic is encapsulated
behind {{AclStorage}}. There are 4 distinct scenarios that can cause reduction
of an extended ACL to a minimal ACL:
# {{FileSystem#removeAclEntries}} passes an ACL spec that selectively removes
all of the extended ACL entries, both access and default.
# {{FileSystem#removeDefaultAcl}} removes all default ACL entries, and there
are no extended access ACL entries.
# {{FileSystem#removeAcl}} removes all extended ACL entries.
# {{FileSystem#setAcl}} on an inode that has an extended ACL and the caller
passes an ACL spec with exactly 3 entries corresponding to a minimal ACL,
overwriting the extended ACL with a minimal ACL.
bq. It seems to me that in my proposal removeAclEntries cannot simply drops the
ACL features in the inode. Am I missing something?
In addition to removing the {{AclFeature}} and toggling the ACL bit in the
{{FsPermission}}, we must also get the group permissions out of the old ACL and
put it back into the {{FsPermission}}. (Recall that for an inode with an
extended ACL, the {{FsPermission}} group bits are actually the mask, and this
implementation choice simplifies integration with a lot of legacy APIs.) See
{{TestNameNodeAcl#testRemoveAclEntriesMinimal}} for a unit test that shows an
example of this. Initially, we set an extended ACL that includes user:foo:rwx.
Because of this, the inferred mask is mask::rwx, and this gets stored into the
group bits of {{FsPermission}}. After calling {{removeAclEntries}}, we reduce
to a minimal ACL, and the group permissions get restored to group::rw-. If we
simply dropped the {{AclFeature}}, then we'd still be left with group::rwx in
the {{FsPermission}}, which would be incorrect.
> 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)
