[ 
https://issues.apache.org/jira/browse/HDFS-12895?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16288226#comment-16288226
 ] 

Íñigo Goiri edited comment on HDFS-12895 at 12/12/17 8:35 PM:
--------------------------------------------------------------

[~linyiqun], thanks for [^HDFS-12895.003.patch]; I tested in our cluster and I 
have a few comments:
* The javadoc in {{RouterPermissionChecker}} has a typo: ferdertion
* {{MountTableStoreImpl#updateMountTableEntry()}} is repeating 
{{request.getEntry()}}
* Could {{MountTableStoreImpl#removeMountTableEntry()}} still use the {{Query}} 
to do the get?
* {{MountTableStoreImpl#getMountTableEntries()}} could use a different if 
structure:
{code}
        } else if (pc != null) {
          // do the READ permission check
          try {
            pc.checkPermission(record, FsAction.READ);
          } catch(AccessControlException ignored) {
            // Remove this mount table entry if it cannot
            // be accessed by current user.
            it.remove();
          }
        }
{code}
* In {{MountTable#toString()}}, we should use {{append}} instead of {{+}}.
* {{RouterAdmin#printUsage}} should open/close the brackets.
* In {{RouterAdmin}}, the new breaks across {{if}} are a little too much (this 
is more a personal taste thing, ignore if so).
* The headers for the output in {{RouterAdmin}} have inconsitent capitalization 
(e.g., "Destinations", "owner").
* The entries that where created before show all the fields as null. Should we 
do some better default? In addition, they don't show for other users when they 
are default, if null, we should assume 755 or so.
* When listing the entries, they show without a particular order; they should 
be sorted by source I'd say.
* We should add these permissions to the Web UI.
* Changing the order of the ORDER in the proto messes with previously created 
entries, I'd prefer to keep the order and add the permissions starting in field 
10 or so.


was (Author: elgoiri):
[~linyiqun], thanks for [^HDFS-12895.003.patch]; I tested in our cluster and I 
have a few comments:
* The javadoc in {{RouterPermissionChecker}} has a typo: ferdertion
* {{MountTableStoreImpl#updateMountTableEntry()}} is repeating 
{{request.getEntry()}}
* Could {{MountTableStoreImpl#removeMountTableEntry()}} still use the {{Query}} 
to do the get?
* {{MountTableStoreImpl#getMountTableEntries()}} could use a different if 
structure:
{code}
        } else if (pc != null) {
          // do the READ permission check
          try {
            pc.checkPermission(record, FsAction.READ);
          } catch(AccessControlException ignored) {
            // Remove this mount table entry if it cannot
            // be accessed by current user.
            it.remove();
          }
        }
{code}
* In {{MountTable#toString()}}, we should use {{append}} instead of {{+}}.
* {{RouterAdmin#printUsage}} should open/close the brackets.
* In {{RouterAdmin}}, the new breaks across {{if}} are a little too much (this 
is more a personal taste thing, ignore if so).
* The headers for the output in {{RouterAdmin}} have inconsitent capitalization 
(e.g., "Destinations", "owner").
* The entries that where created before show all the fields as null. Should we 
do some better default? In addition, they don't show for other users when they 
are default, if null, we should assume 755 or so.
* When listing the entries, they show without a particular order; they should 
be sorted by source I'd say.
* We should add these permissions to the Web UI.

> 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
>
>
> 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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to