[
https://issues.apache.org/jira/browse/HDFS-6962?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15383249#comment-15383249
]
Chris Nauroth commented on HDFS-6962:
-------------------------------------
Hello [~jzhuge]. I apologize for my delayed response. Thank you for working
on this tricky issue.
I think what you are proposing for configurability and extending the protocol
messages makes sense as a way to provide deployments with a choice of which
behavior to use. However, I'm reluctant to push it into 2.8.0 now due to the
complexity of the changes required to support it. Considering something like a
cross-cluster DistCp, with a mix of old and new versions in play, it could
become very confusing to explain the end results to users. Unless you consider
it urgent for 2.8.0, would you consider targeting it to the 3.x line, as I had
done a while ago?
I don't think we can realistically ship without the WebHDFS support in place.
At this point, there is a user expectation of feature parity for ACL commands
whether the target is an hdfs: path or a webhdfs: path. If you want to track
WebHDFS work in a separate JIRA, then I think that's fine, but I wouldn't want
to ship a non-alpha release lacking the WebHDFS support.
I am concerned about adding the {{createModes}} member to
{{INodeWithAdditionalFields}} because of the increased per-inode memory
footprint in the NameNode. Even for a {{null}}, there is still the pointer
cost. I assume this was done because it was the easiest way to get the masked
vs. unmasked information passed all the way down to
{{FSDirectory#copyINodeDefaultAcl}} during new file/directory creation. That
information is not valuable beyond the lifetime of the creation operation, so
paying memory to preserve it longer is unnecessary. I think we'll need to
explore passing the unmasked information along separately from the inode
object. Unfortunately, this likely will make the change more awkward,
requiring changes to method signatures to accept more arguments.
{code}
if (modes == null) {
LOG.warn("Received create request without unmasked create mode");
}
{code}
I expect this log statement would be noisy in practice. I recommend removing
it or changing it to debug level if you find it helpful.
The documentation of {{dfs.namenode.posix.acl.inheritance.enabled}} in
hdfs-default.xml and HdfsPermissionsGuide.md looks good overall. I saw one
typo in both places: "comppatible" instead of "compatible". Could you also add
a clarifying statement that umask would be ignored if the parent has a default
ACL? It could be as simple as "...will apply default ACLs from the parent
directory to the create mode and ignore umask."
In addition to the new tests you added to {{FSAclBaseTest}}, I recommend
testing through the shell. The XML-driven shell tests don't have a way to
reconfigure the mini-cluster under test. I expect you'll need to make a new
test suite, similar to {{TestAclCLI}}, but with
{{dfs.namenode.posix.acl.inheritance.enabled}} set to {{true}}.
bq. PermissionStatus#applyUMask never used, remove it?
bq. DFSClient#mkdirs and {{DFSClient#primitiveMkdir use file default if
permission is null. Should use dir default permission?
You might consider filing separate JIRAs for these 2 observations, so that we
keep the scope here focused on the ACL inheritance issue.
> ACLs inheritance conflict with umaskmode
> ----------------------------------------
>
> Key: HDFS-6962
> URL: https://issues.apache.org/jira/browse/HDFS-6962
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: security
> Affects Versions: 2.4.1
> Environment: CentOS release 6.5 (Final)
> Reporter: LINTE
> Assignee: John Zhuge
> Priority: Critical
> Labels: hadoop, security
> Attachments: HDFS-6962.001.patch, HDFS-6962.002.patch,
> HDFS-6962.003.patch, HDFS-6962.004.patch, HDFS-6962.005.patch,
> HDFS-6962.006.patch, HDFS-6962.1.patch, disabled_new_client.log,
> disabled_old_client.log, enabled_new_client.log, enabled_old_client.log, run
>
>
> In hdfs-site.xml
> <property>
> <name>dfs.umaskmode</name>
> <value>027</value>
> </property>
> 1/ Create a directory as superuser
> bash# hdfs dfs -mkdir /tmp/ACLS
> 2/ set default ACLs on this directory rwx access for group readwrite and user
> toto
> bash# hdfs dfs -setfacl -m default:group:readwrite:rwx /tmp/ACLS
> bash# hdfs dfs -setfacl -m default:user:toto:rwx /tmp/ACLS
> 3/ check ACLs /tmp/ACLS/
> bash# hdfs dfs -getfacl /tmp/ACLS/
> # file: /tmp/ACLS
> # owner: hdfs
> # group: hadoop
> user::rwx
> group::r-x
> other::---
> default:user::rwx
> default:user:toto:rwx
> default:group::r-x
> default:group:readwrite:rwx
> default:mask::rwx
> default:other::---
> user::rwx | group::r-x | other::--- matches with the umaskmode defined in
> hdfs-site.xml, everything ok !
> default:group:readwrite:rwx allow readwrite group with rwx access for
> inhéritance.
> default:user:toto:rwx allow toto user with rwx access for inhéritance.
> default:mask::rwx inhéritance mask is rwx, so no mask
> 4/ Create a subdir to test inheritance of ACL
> bash# hdfs dfs -mkdir /tmp/ACLS/hdfs
> 5/ check ACLs /tmp/ACLS/hdfs
> bash# hdfs dfs -getfacl /tmp/ACLS/hdfs
> # file: /tmp/ACLS/hdfs
> # owner: hdfs
> # group: hadoop
> user::rwx
> user:toto:rwx #effective:r-x
> group::r-x
> group:readwrite:rwx #effective:r-x
> mask::r-x
> other::---
> default:user::rwx
> default:user:toto:rwx
> default:group::r-x
> default:group:readwrite:rwx
> default:mask::rwx
> default:other::---
> Here we can see that the readwrite group has rwx ACL bu only r-x is effective
> because the mask is r-x (mask::r-x) in spite of default mask for inheritance
> is set to default:mask::rwx on /tmp/ACLS/
> 6/ Modifiy hdfs-site.xml et restart namenode
> <property>
> <name>dfs.umaskmode</name>
> <value>010</value>
> </property>
> 7/ Create a subdir to test inheritance of ACL with new parameter umaskmode
> bash# hdfs dfs -mkdir /tmp/ACLS/hdfs2
> 8/ Check ACL on /tmp/ACLS/hdfs2
> bash# hdfs dfs -getfacl /tmp/ACLS/hdfs2
> # file: /tmp/ACLS/hdfs2
> # owner: hdfs
> # group: hadoop
> user::rwx
> user:toto:rwx #effective:rw-
> group::r-x #effective:r--
> group:readwrite:rwx #effective:rw-
> mask::rw-
> other::---
> default:user::rwx
> default:user:toto:rwx
> default:group::r-x
> default:group:readwrite:rwx
> default:mask::rwx
> default:other::---
> So HDFS masks the ACL value (user, group and other -- exepted the POSIX
> owner -- ) with the group mask of dfs.umaskmode properties when creating
> directory with inherited ACL.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
