hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Amar Kamat (JIRA)" <j...@apache.org>
Subject [jira] Commented: (MAPREDUCE-1543) Log messages of JobACLsManager should use security logging of HADOOP-6586
Date Tue, 16 Mar 2010 08:50:27 GMT

    [ https://issues.apache.org/jira/browse/MAPREDUCE-1543?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12845766#action_12845766
] 

Amar Kamat commented on MAPREDUCE-1543:
---------------------------------------

bq. I think AuditLogManager can be pulled into a separate class file.
+1.

bq. I would instead suggest that the Keys be defined as an Enum
+1

bq. I am not sure 'agent' is conveying the right meaning.
HDFS uses ugi. As we discussed, we would be logging the usernames in MapReduce. I think agent
can act as a common word for ugi or username. Agent is someone who is external to the system
and tries to cause a state change. I am ok with changing it to something better. I named the
IP key as 'ip' because hdfs kept it that way.

bq. Reason seems confusing.
I have removed it in my next patch. It doesnt add much value to MapReduce because we are dealing
with authorization events here. The reason why I made them enums was for ease of parsing.
Having strings in key=value formats can lead to issues to do with spaces etc.

bq. Don't tabs make the lines appear too long,
This is inline with hdfs.

bq. Some values like 'permissions' (if this is a free format string) can have whitespaces
I would rather prefer having one word representation for permissions for audit logs. Example
{noformat}
user{u1,u2,u3}:group{g1,g2,g3}
{noformat}
Key=Value pairs with free format strings can lead to issues. We can quote them but the parser
logic would become complex.

bq. Suggest the parameter 'operation' could be of type String as well.
I would rather prefer Enums and have JobTracker.Operations and JobInProgress.Operations. Note
we already have QueueManager.QueueOperations.

bq. If there's no cost to logging remote-ip,
+1

bq. The comment '//for testcases' is misleading. 
+1

bq. Why are we logging success again in JobTracker.addJob, wouldn't the same line come up
in QueueManager.checkAccess ?
Because they are 2 authorization checks involved : 
1) Queue access (success or failure)
2) Job submission (success or failure based on username in config)

bq.  Seems like Operations "REFRESH_NODES", "QUEUE_REFRESH" and target "Jobtracker" can be
static final variables. Should we pull all such AuditLogManager related constants into some
constants file ? Like AuditLogConstants or something ?
+1 for factoring it out.

bq. I don't think we need to give 'Queue:' prefix, 
+1

bq.Failure logging should be at WARN level - as done in the other audit log.
If audit logs are important then what is the point of making some of them WARN, ERROR or FATAL?
Shouldn't all the audit logs be INFO logs?

bq. Can we move the refreshAcls logging into QueueManager.refreshQueueAcls - so that failures
can be logged too.
+1

bq. Can we add a few simple unit tests that test the formatting of the log message alone
+1

> Log messages of JobACLsManager should use security logging of HADOOP-6586
> -------------------------------------------------------------------------
>
>                 Key: MAPREDUCE-1543
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-1543
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>          Components: security
>            Reporter: Vinod K V
>            Assignee: Amar Kamat
>             Fix For: 0.22.0
>
>         Attachments: mapreduce-1543-y20s.patch
>
>
> {{JobACLsManager}} added in MAPREDUCE-1307 logs the successes and failures w.r.t job-level
authorization in the corresponding Daemons' logs. The log messages should instead use security
logging of HADOOP-6586.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message