[
https://issues.apache.org/jira/browse/HDFS-11246?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16093132#comment-16093132
]
Brahma Reddy Battula edited comment on HDFS-11246 at 7/19/17 2:09 PM:
----------------------------------------------------------------------
[~kshukla] thanks for working on this.
bq.That would mean we log for IOExceptions and others as well with
allowed=false, which should ideally only be when we see an
AccessControlException. This was a bug fixed thru HDFS-9395. We can still have
the call in the finally block while making sure we don't log unconditionally.
Instead of double try-finally blocks can we log in finally block based on
some boolean condition like below such that it can be logged only on ACE..?
*Option 1:*
{code}
final String operationName = "setOwner";
- FileStatus auditStat;
+ boolean logAudit = false;
+ boolean allow = true;
+ FileStatus auditStat=null;
checkOperation(OperationCategory.WRITE);
writeLock();
try {
checkOperation(OperationCategory.WRITE);
checkNameNodeSafeMode("Cannot set owner for " + src);
auditStat = FSDirAttrOp.setOwner(dir, src, username, group);
+ logAudit = true;
} catch (AccessControlException e) {
- logAuditEvent(false, operationName, src);
+ logAudit = true;
+ allow = false;
+
throw e;
} finally {
writeUnlock(operationName);
+ if(logAudit)
+ logAuditEvent(allow, operationName, src, null, auditStat);
}
getEditLog().logSync();
- logAuditEvent(true, operationName, src, null, auditStat);
{code}
*Option 2:*
{code}
+ public enum AuditCheck {
+ SUCCESS, FAILURE, NONE
+ }
/**
* Set owner for an existing file.
* @throws IOException
@@ -1807,21 +1810,25 @@ void setPermission(String src, FsPermission permission)
throws IOException {
void setOwner(String src, String username, String group)
throws IOException {
final String operationName = "setOwner";
- FileStatus auditStat;
+ AuditCheck auditCheck = AuditCheck.NONE;
+ FileStatus auditStat = null;
checkOperation(OperationCategory.WRITE);
writeLock();
try {
checkOperation(OperationCategory.WRITE);
checkNameNodeSafeMode("Cannot set owner for " + src);
auditStat = FSDirAttrOp.setOwner(dir, src, username, group);
+ auditCheck = AuditCheck.SUCCESS;
} catch (AccessControlException e) {
- logAuditEvent(false, operationName, src);
+ auditCheck = AuditCheck.FAILURE;
throw e;
} finally {
writeUnlock(operationName);
+ if (auditCheck != AuditCheck.NONE)
+ logAuditEvent(auditCheck == AuditCheck.SUCCESS, operationName, src,
+ null, auditStat);
}
getEditLog().logSync();
- logAuditEvent(true, operationName, src, null, auditStat);
{code}
I prefer *option-2.*
was (Author: brahmareddy):
[~kshukla] thanks for working on this.
bq.That would mean we log for IOExceptions and others as well with
allowed=false, which should ideally only be when we see an
AccessControlException. This was a bug fixed thru HDFS-9395. We can still have
the call in the finally block while making sure we don't log unconditionally.
Instead of double try-finally blocks can we log in finally block based on
some boolean condition like below such that it can be logged only on ACE..?
{code}
final String operationName = "setOwner";
- FileStatus auditStat;
+ boolean logAudit = false;
+ boolean allow = true;
+ FileStatus auditStat=null;
checkOperation(OperationCategory.WRITE);
writeLock();
try {
checkOperation(OperationCategory.WRITE);
checkNameNodeSafeMode("Cannot set owner for " + src);
auditStat = FSDirAttrOp.setOwner(dir, src, username, group);
+ logAudit = true;
} catch (AccessControlException e) {
- logAuditEvent(false, operationName, src);
+ logAudit = true;
+ allow = false;
+
throw e;
} finally {
writeUnlock(operationName);
+ if(logAudit)
+ logAuditEvent(allow, operationName, src, null, auditStat);
}
getEditLog().logSync();
- logAuditEvent(true, operationName, src, null, auditStat);
{code}
{code}
> FSNameSystem#logAuditEvent should be called outside the read or write locks
> during operations like getContentSummary
> --------------------------------------------------------------------------------------------------------------------
>
> Key: HDFS-11246
> URL: https://issues.apache.org/jira/browse/HDFS-11246
> Project: Hadoop HDFS
> Issue Type: Bug
> Affects Versions: 2.7.3
> Reporter: Kuhu Shukla
> Assignee: Kuhu Shukla
> Attachments: HDFS-11246.001.patch, HDFS-11246.002.patch
>
>
> {code}
> readLock();
> boolean success = true;
> ContentSummary cs;
> try {
> checkOperation(OperationCategory.READ);
> cs = FSDirStatAndListingOp.getContentSummary(dir, src);
> } catch (AccessControlException ace) {
> success = false;
> logAuditEvent(success, operationName, src);
> throw ace;
> } finally {
> readUnlock(operationName);
> }
> {code}
> It would be nice to have audit logging outside the lock esp. in scenarios
> where applications hammer a given operation several times.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
