[
https://issues.apache.org/jira/browse/HADOOP-1298?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498422
]
dhruba borthakur commented on HADOOP-1298:
------------------------------------------
Patch looks good. Here are a few comments that I have:
1. PosixConstant.init() invokes conf.get("dfs.default.permissions", "-1");
It might be a good idea to make it conf.get("dfs.default.permissions",
"777");
2. Why is OWNER_WRITEABLE, OWNER_READABLE public? Can we make them package
private?
3. FsShell allows changing permissions for a group of files. If it encounters
an error for one file, it stops processing the remaining files.
4. DFSClient.groups() may invoke checkOpen() before invoking the RPC to the
namenode.
5. Permissions.owner and Permissions.ownerGroup are Integers and not native
ints. This increases memory consumption in the namenode. I am guessing that you
have made them Integers because they are shared objects that are also inserted
into HashMaps in Users.java. Is that right?
6. FSEditLog, you have
if (logversion == LAYOUT) {
perm.readFields();
}
instead, it should be
if (logversion <= -5) {
perm.readFields();
}
7. You have added another parameter to logEdit() to write the permission
object. In future a few more fields will be added to the INODE (e.g. creation
times, mod times, etc). This will mean that the number of parameters to
logEdit() will increase significantly. Instead, you could leave logEdit() as it
is and pass in an array as the first parameter. This trick is already used by
logCreateFile(). Please see if we can do that same for the newly introduced
methods.
8. DistributedFileSystem.initialize() extracts the user from the conf file. It
also extracts the user from the uri that is being passed in as a parameter
(e.g. String user = uri.getUserInfo()). It uses the user from the uri to
construct the HDFS URI. Is this intended? I am thinking that we should be using
the user extracted from the conf file to create the HDFS uri.
9. DistributedFileSystem.getPath has been renamed to
DistributedFileSystem.getUTF8Path(). This might not be very related to this
issue. This changed caused quite a few code lines to get changed. Maybe we can
defer it to hadoop-1283.
10. NamenodeFsck invokes new ClientContext("root") for many of the files that
it encounters. Maybe keeping a static constant for root will help reduce memory
usage.
11. Some minor changes to bin/hadoop and build.xml that is intended for
debugging.
12. I did not see any code that prevents an user from reading a file that
he/she does not have access for. Similarly, deleting a file does not check for
access permissions. What did I miss? can you pl point me to the relevant code
for this one?
13. The code seems to allow setting permissions on directories. Is this
intended? This means that users will expect permissions to enforced for
directory traversal as well. Also, questions would be raised about inheriting
permissions from directories. My vote would be to not support permissions on
directories at all.
> adding user info to file
> ------------------------
>
> Key: HADOOP-1298
> URL: https://issues.apache.org/jira/browse/HADOOP-1298
> Project: Hadoop
> Issue Type: New Feature
> Components: dfs, fs
> Reporter: Kurtis Heimerl
> Attachments: hadoop-user-munncha.patch, hadoop-user-munncha.patch,
> hadoop-user-munncha.patch, hadoop-user-munncha.patch10,
> hadoop-user-munncha.patch11, hadoop-user-munncha.patch12,
> hadoop-user-munncha.patch13, hadoop-user-munncha.patch14,
> hadoop-user-munncha.patch15, hadoop-user-munncha.patch16,
> hadoop-user-munncha.patch4, hadoop-user-munncha.patch5,
> hadoop-user-munncha.patch6, hadoop-user-munncha.patch7,
> hadoop-user-munncha.patch8, hadoop-user-munncha.patch9
>
>
> I'm working on adding a permissions model to hadoop's DFS. The first step is
> this change, which associates user info with files. Following this I'll
> assoicate permissions info, then block methods based on that user info, then
> authorization of the user info.
> So, right now i've implemented adding user info to files. I'm looking for
> feedback before I clean this up and make it offical.
> I wasn't sure what release, i'm working off trunk.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
