[
https://issues.apache.org/jira/browse/HDFS-12895?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16289556#comment-16289556
]
Íñigo Goiri edited comment on HDFS-12895 at 12/13/17 7:56 PM:
--------------------------------------------------------------
Thanks [~linyiqun] for taking care of the comments. A couple more based on
[^HDFS-12895.005.patch]:
* It seems we are defaulting to nulls when nothing there. The entries created
early still show without permissions. We probably want to check for empty
{{getMode()}} at {{MountTableImpl}}. Not sure about user and group; show a
default one? Super user? This is what I currently get for old entries:
{code}
Mount Table Entries:
Source Destinations Owner
Group Mode
/ ns0->/ null
null ---------
/ns0 ns0->/ null
null ---------
/ns1 ns1->/ null
null ---------
/ns2 ns2->/ null
null ---------
/ns3 ns3->/ null
null ---------
{code}
* {{removeMountTableEntry()}} could initialize {{deleteEntry}} right away
without the null: {{final MountTable deleteEntry =
getDriver().get(getRecordClass(), query);}}
* For consistency, {{removeMountTableEntry()}} could do the same order of
{{if}} as {{addMountTableEntry()}} and {{updateMountTableEntry()}} and avoid
the {{pc}} init if no entry. In addition, the check status could use this if
structure:
{code}
boolean status = false;
if (deleteEntry != null) {
RouterPermissionChecker pc = RouterAdminServer.getPermissionChecker();
if (pc != null) {
pc.checkPermission(deleteEntry, FsAction.WRITE);
}
status = getDriver().remove(deleteEntry);
}
{code}
* Typo in the name {{testMountTableDefalutACL}}
* Internally this fails {{assertEquals(ugi.getShortUserName(),
mountTable.getGroupName());}}, I think it shouldn't be {{getShortUserName()}}
to compare with the group.
Other than this, this is ready to go.
was (Author: elgoiri):
Thanks [~linyiqun] for taking care of the comments. A couple more based on
[^HDFS-12895.005.patch]:
* It seems we are defaulting to nulls when nothing there. The entries created
early still show without permissions. We probably want to check for empty
{{getMode()}} at {{MountTableImpl}}. Not sure about user and group; show a
default one? Super user? This is what I currently get for old entries:
{code}
Mount Table Entries:
Source Destinations Owner
Group Mode
/ ns0->/ null
null ---------
/ns0 ns0->/ null
null ---------
/ns1 ns1->/ null
null ---------
/ns2 ns2->/ null
null ---------
/ns3 ns3->/ null
null ---------
{code}
* {{removeMountTableEntry()}} could initialize {{deleteEntry}} right away
without the null: {{final MountTable deleteEntry =
getDriver().get(getRecordClass(), query);}}
* For consistency, {{removeMountTableEntry()}} could do the same order of
{{if}} as {{addMountTableEntry()}} and {{updateMountTableEntry()}} and avoid
the {{pc}} init if no entry. In addition, the check status could use this if
structure:
{code}
boolean status = false;
if (deleteEntry != null) {
RouterPermissionChecker pc = RouterAdminServer.getPermissionChecker();
if (pc != null) {
pc.checkPermission(deleteEntry, FsAction.WRITE);
}
status = getDriver().remove(deleteEntry);
}
{code}
* Typo in the name {{testMountTableDefalutACL}}
Other than this, this is ready to go.
> RBF: Add ACL support for mount table
> ------------------------------------
>
> Key: HDFS-12895
> URL: https://issues.apache.org/jira/browse/HDFS-12895
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Affects Versions: 3.0.0-alpha3
> Reporter: Yiqun Lin
> Assignee: Yiqun Lin
> Labels: RBF
> Attachments: HDFS-12895.001.patch, HDFS-12895.002.patch,
> HDFS-12895.003.patch, HDFS-12895.004.patch, HDFS-12895.005.patch
>
>
> Adding ACL support for the Mount Table management. Following is the initial
> design of ACL control for the mount table management.
> Each mount table has its owner, group name and permission.
> The mount table permissions (FsPermission), here we use
> {{org.apache.hadoop.fs.permission.FsPermission}} to do the access check:
> # READ permission: you can read the mount table info.
> # WRITE permission: you can add remove or update this mount table info.
> # EXECUTE permission: This won't be used.
> The add command of mount table will be extended like this
> {noformat}
> $HADOOP_HOME/bin/hdfs dfsrouteradmin [-add <source> <nameservice>
> <destination> [-owner <owner>] [-group <group>] [-mode <mode>]]
> {noformat}
> *<mode> is UNIX-style permissions for the mount table. Permissions are
> specified in octal, e.g. 0755. By default, this is set to 0755*.
> If we want update the ACL info of specfied mount table, just execute add
> command again. This command not only adding for new mount talle but also
> updating mount table once it finds given mount table is existed.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
