adoroszlai commented on a change in pull request #3163:
URL: https://github.com/apache/ozone/pull/3163#discussion_r822000993
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/RequestAuditor.java
##########
@@ -53,6 +55,14 @@ AuditMessage buildAuditMessage(AuditAction op,
*/
Map<String, String> buildVolumeAuditMap(String volume);
+ /**
+ * Build auditMap with specified Delegation Token.
+ *
+ * @param token Delegation token.
+ * @return auditMap.
+ */
+ Map<String, String> buildTokenAuditMap(Token<OzoneTokenIdentifier> token);
Review comment:
I think we should avoid adding this method to the `RequestAuditor`
interface and to the base `OMClientRequest` base class. It only applies to a
small subset of request types. The implementation does not use any state of
the request, only formats the token given in parameter, so it can be a static
utility method.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/security/OMRenewDelegationTokenRequest.java
##########
@@ -142,13 +151,18 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
} catch (IOException ex) {
LOG.error("Error in Updating Renew DelegationToken {}",
ozoneTokenIdentifierToken, ex);
+ exception = ex;
omClientResponse = new OMRenewDelegationTokenResponse(null, -1L,
createErrorOMResponse(omResponse, ex));
} finally {
addResponseToDoubleBuffer(transactionLogIndex, omClientResponse,
ozoneManagerDoubleBufferHelper);
}
+ auditLog(auditLogger,
+ buildAuditMessage(OMAction.GET_DELEGATION_TOKEN, auditMap, exception,
Review comment:
This should be `RENEW_DELEGATION_TOKEN`.
A more serious problem is that token renewal failure is not being logged,
because it happens in `preExecute`. We will never reach this code in case of
failure.
https://github.com/apache/ozone/blob/1c58d79c11ff9133e52f7cf511fcbd202e8e8a41/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/security/OMRenewDelegationTokenRequest.java#L62-L65
OM log shows the error:
```
om_1 | 2022-03-08 17:23:53,785 [IPC Server handler 34 on default port
9862] ERROR om.OzoneManager: Delegation token renewal failed for dt id: ...,
cause: token (OzoneToken owner=testuser/[email protected], ...) can't be found
in cache
```
but the request is missing from audit log.
On second look this problem applies to "get token" and "cancel token"
requests, too, but I don't know how to trigger an error there.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -2818,19 +2820,30 @@ public OzoneManagerHttpServer getHttpServer() {
.setType(ServicePort.Type.RPC)
.setValue(omRpcAddress.getPort())
.build());
+
+ auditMap.put(OzoneConsts.OZONE_OM_SERVICE_HOSTNAME,
+ omRpcAddress.getHostName());
+ auditMap.put("RPC_PORT", String.valueOf(omRpcAddress.getPort()));
Review comment:
Usually audit log should include information about the request, not the
response. Only status (success / failure) needs to be added about the
response. So for `getServiceList` request, which does not have parameters, I
think we can simply log success or failure.
##########
File path:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -449,4 +455,8 @@ private OzoneConsts() {
public static final String OZONE_HTTP_SECURITY_ENABLED_SECURE = "true";
public static final String OZONE_HTTP_FILTER_INITIALIZERS_SECURE =
"org.apache.hadoop.security.AuthenticationFilterInitializer";
+
+ public static final String DELEGATION_TOKEN_KIND = "delegationTokenKind";
+ public static final String DELEGATION_TOKEN_SERVICE =
+ "delegationTokenService";
Review comment:
I'd prefer shorter values to make the log message less verbose, as these
will be used only in the context of delegation tokens:
```suggestion
public static final String DELEGATION_TOKEN_KIND = "kind";
public static final String DELEGATION_TOKEN_SERVICE = "service";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
