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

Chris Nauroth commented on HDFS-6570:
-------------------------------------

Jitendra, thanks for incorporating the feedback.  I think this is almost ready. 
 I see just one more thing to fix, and I have recommendations on a few more 
test cases to add.  I expect the patch is already correct for all of these 
suggested test cases, so adding them would just be helpful for preventing 
regressions in the future.

# {{GetOpParam}}: It looks like the convention on WebHDFS operation names is to 
put all the words together, not separated by underscore.  Let's change 
{{CHECK_ACCESS}} to {{CHECKACCESS}}.  This is actually how you named the 
operation in the docs already.
# {{TestPermissionSymlinks}}: Let's add a test asserting that a call to check 
access for a symlink checks the permissions of its target.  (Symlinks always 
have 777, so it wouldn't be correct to check the symlink inode directly.)
# {{TestSafeMode#testOperationsWhileInSafeMode}}: Let's make a small change 
here to add a call to check access while in safe mode.  This is a read-only 
operation, so we expect it to work during safe mode.
# {{TestAclWithSnapshot}}: If there is a snapshot, and the original inode's 
permissions change, then checking access on the snapshot inode must still 
enforce the old permissions, and checking access on the current version of the 
inode must reflect the changes.  I think the current patch does this correctly, 
but let's test to make sure.  Snapshot tests like this need a lot of setup, so 
I recommend we just add a few quick access check calls to the 4 existing 
{{testOriginalAclEnforced*}} tests in this suite.  That way, we can get a free 
ride on the setup code that's already done here.  :-)
# BTW, I agree with what you did for audit logging in this version of the 
patch.  HDFS-5730 has more discussion on making audit logging consistent across 
all APIs.

bq. -1 core tests. The patch failed these unit tests in 
hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs:

These look like spurious test failures.  They passed for me locally.

> add api that enables checking if a user has certain permissions on a file
> -------------------------------------------------------------------------
>
>                 Key: HDFS-6570
>                 URL: https://issues.apache.org/jira/browse/HDFS-6570
>             Project: Hadoop HDFS
>          Issue Type: Bug
>            Reporter: Thejas M Nair
>            Assignee: Jitendra Nath Pandey
>         Attachments: HDFS-6570-prototype.1.patch, HDFS-6570.2.patch, 
> HDFS-6570.3.patch
>
>
> For some of the authorization modes in Hive, the servers in Hive check if a 
> given user has permissions on a certain file or directory. For example, the 
> storage based authorization mode allows hive table metadata to be modified 
> only when the user has access to the corresponding table directory on hdfs. 
> There are likely to be such use cases outside of Hive as well.
> HDFS does not provide an api for such checks. As a result, the logic to check 
> if a user has permissions on a directory gets replicated in Hive. This 
> results in duplicate logic and there introduces possibilities for 
> inconsistencies in the interpretation of the permission model. This becomes a 
> bigger problem with the complexity of ACL logic.
> HDFS should provide an api that provides functionality that is similar to 
> access function in unistd.h - http://linux.die.net/man/2/access .



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to