HDDS-2628. Make AuditMessage parameters strongly typed

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/audit/AuditMessage.java

@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with this
  * work for additional information regarding copyright ownership.  The ASF
@@ -23,13 +23,14 @@
 /**
  * Defines audit message structure.
  */
-public class AuditMessage implements Message {
-
-  private String message;
-  private Throwable throwable;
+public final class AuditMessage implements Message {
 
-  public AuditMessage(){
+  private final String message;
+  private final Throwable throwable;
 
+  private AuditMessage(String message, Throwable throwable) {
+    this.message = message;
+    this.throwable = throwable;
   }
 
   @Override
@@ -52,26 +53,6 @@ public Throwable getThrowable() {
     return throwable;
   }
 
-  /**
-   * Use when there are custom string to be added to default msg.
-   * @param customMessage custom string
-   */
-  private void appendMessage(String customMessage) {
-    this.message += customMessage;
-  }
-
-  public String getMessage() {
-    return message;
-  }
-
-  public void setMessage(String message) {
-    this.message = message;
-  }
-
-  public void setThrowable(Throwable throwable) {
-    this.throwable = throwable;
-  }
-
   /**
    * Builder class for AuditMessage.
    */
@@ -83,10 +64,6 @@ public static class Builder {
     private Map<String, String> params;
     private String ret;
 
-    public Builder(){
-
-    }
-
     public Builder setUser(String usr){
       this.user = usr;
       return this;
@@ -97,8 +74,8 @@ public Builder atIp(String ipAddr){
       return this;
     }
 
-    public Builder forOperation(String operation){
-      this.op = operation;
+    public Builder forOperation(AuditAction action) {
+      this.op = action.getAction();
       return this;
     }
 
@@ -107,8 +84,8 @@ public Builder withParams(Map<String, String> args){
       return this;
     }
 
-    public Builder withResult(String result){
-      this.ret = result;
+    public Builder withResult(AuditEventStatus result) {
+      this.ret = result.getStatus();
       return this;
     }
 
@@ -118,11 +95,9 @@ public Builder withException(Throwable ex){
     }
 
     public AuditMessage build(){
-      AuditMessage auditMessage = new AuditMessage();
-      auditMessage.message = "user=" + this.user + " | ip=" + this.ip + " | " +
+      String message = "user=" + this.user + " | ip=" + this.ip + " | " +
           "op=" + this.op + " " + this.params + " | " + "ret=" + this.ret;
-      auditMessage.throwable = this.throwable;
-      return auditMessage;
+      return new AuditMessage(message, throwable);
     }
   }
 }

hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/audit/TestOzoneAuditLogger.java

@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -29,6 +29,9 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.apache.hadoop.ozone.audit.AuditEventStatus.FAILURE;
+import static org.apache.hadoop.ozone.audit.AuditEventStatus.SUCCESS;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -42,9 +45,6 @@ public class TestOzoneAuditLogger {
   private static final AuditLogger AUDIT =
       new AuditLogger(AuditLoggerType.OMLOGGER);
 
-  private static final String SUCCESS = AuditEventStatus.SUCCESS.name();
-  private static final String FAILURE = AuditEventStatus.FAILURE.name();
-
   private static final Map<String, String> PARAMS =
       new DummyEntity().toAuditMap();
 
@@ -55,7 +55,7 @@ public class TestOzoneAuditLogger {
       new AuditMessage.Builder()
           .setUser(USER)
           .atIp(IP_ADDRESS)
-          .forOperation(DummyAction.CREATE_VOLUME.name())
+          .forOperation(DummyAction.CREATE_VOLUME)
           .withParams(PARAMS)
           .withResult(FAILURE)
           .withException(null).build();
@@ -64,7 +64,7 @@ public class TestOzoneAuditLogger {
       new AuditMessage.Builder()
           .setUser(USER)
           .atIp(IP_ADDRESS)
-          .forOperation(DummyAction.CREATE_VOLUME.name())
+          .forOperation(DummyAction.CREATE_VOLUME)
           .withParams(PARAMS)
           .withResult(SUCCESS)
           .withException(null).build();
@@ -73,7 +73,7 @@ public class TestOzoneAuditLogger {
       new AuditMessage.Builder()
           .setUser(USER)
           .atIp(IP_ADDRESS)
-          .forOperation(DummyAction.READ_VOLUME.name())
+          .forOperation(DummyAction.READ_VOLUME)
           .withParams(PARAMS)
           .withResult(FAILURE)
           .withException(null).build();
@@ -82,7 +82,7 @@ public class TestOzoneAuditLogger {
       new AuditMessage.Builder()
           .setUser(USER)
           .atIp(IP_ADDRESS)
-          .forOperation(DummyAction.READ_VOLUME.name())
+          .forOperation(DummyAction.READ_VOLUME)
           .withParams(PARAMS)
           .withResult(SUCCESS)
           .withException(null).build();
@@ -127,12 +127,12 @@ public void verifyDefaultLogLevelForFailure() throws IOException {
 
   @Test
   public void messageIncludesAllParts() {
-    String message = WRITE_FAIL_MSG.getMessage();
+    String message = WRITE_FAIL_MSG.getFormattedMessage();
     assertTrue(message, message.contains(USER));
     assertTrue(message, message.contains(IP_ADDRESS));
     assertTrue(message, message.contains(DummyAction.CREATE_VOLUME.name()));
     assertTrue(message, message.contains(PARAMS.toString()));
-    assertTrue(message, message.contains(FAILURE));
+    assertTrue(message, message.contains(FAILURE.getStatus()));
   }
 
   /**
@@ -174,6 +174,6 @@ private void verifyNoLog() throws IOException {
     File file = new File("audit.log");
     List<String> lines = FileUtils.readLines(file, (String)null);
     // When no log entry is expected, the log file must be empty
-    assertTrue(lines.size() == 0);
+    assertEquals(0, lines.size());
   }
 }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java

@@ -608,26 +608,27 @@ private void audit(AuditAction action, EventType eventType,
   @Override
   public AuditMessage buildAuditMessageForSuccess(AuditAction op,
       Map<String, String> auditMap) {
+
     return new AuditMessage.Builder()
         .setUser(null)
         .atIp(null)
-        .forOperation(op.getAction())
+        .forOperation(op)
         .withParams(auditMap)
-        .withResult(AuditEventStatus.SUCCESS.toString())
-        .withException(null)
+        .withResult(AuditEventStatus.SUCCESS)
         .build();
   }
 
   //TODO: use GRPC to fetch user and ip details
   @Override
   public AuditMessage buildAuditMessageForFailure(AuditAction op,
       Map<String, String> auditMap, Throwable throwable) {
+
     return new AuditMessage.Builder()
         .setUser(null)
         .atIp(null)
-        .forOperation(op.getAction())
+        .forOperation(op)
         .withParams(auditMap)
-        .withResult(AuditEventStatus.FAILURE.toString())
+        .withResult(AuditEventStatus.FAILURE)
         .withException(throwable)
         .build();
   }

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/ServerUtils.java

@@ -22,6 +22,8 @@
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.scm.ScmConfigKeys;
 import org.apache.hadoop.ipc.RPC;
+import org.apache.hadoop.ipc.Server;
+import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -224,4 +226,9 @@ public static File getDBPath(Configuration conf, String key) {
         HddsConfigKeys.OZONE_METADATA_DIRS);
     return ServerUtils.getOzoneMetaDirPath(conf);
   }
+
+  public static String getRemoteUserName() {
+    UserGroupInformation remoteUser = Server.getRemoteUser();
+    return remoteUser != null ? remoteUser.getUserName() : null;
+  }
 }

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java

@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license
  * agreements. See the NOTICE file distributed with this work for additional
@@ -65,6 +65,7 @@
 import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_HANDLER_COUNT_DEFAULT;
 import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_HANDLER_COUNT_KEY;
 import static org.apache.hadoop.hdds.scm.server.StorageContainerManager.startRpcServer;
+import static org.apache.hadoop.hdds.server.ServerUtils.getRemoteUserName;
 import static org.apache.hadoop.hdds.server.ServerUtils.updateRPCListenAddress;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -336,29 +337,26 @@ public List<DatanodeDetails> sortDatanodes(List<String> nodes,
   @Override
   public AuditMessage buildAuditMessageForSuccess(
       AuditAction op, Map<String, String> auditMap) {
+
     return new AuditMessage.Builder()
-        .setUser((Server.getRemoteUser() == null) ? null :
-            Server.getRemoteUser().getUserName())
-        .atIp((Server.getRemoteIp() == null) ? null :
-            Server.getRemoteIp().getHostAddress())
-        .forOperation(op.getAction())
+        .setUser(getRemoteUserName())
+        .atIp(Server.getRemoteAddress())
+        .forOperation(op)
         .withParams(auditMap)
-        .withResult(AuditEventStatus.SUCCESS.toString())
-        .withException(null)
+        .withResult(AuditEventStatus.SUCCESS)
         .build();
   }
 
   @Override
   public AuditMessage buildAuditMessageForFailure(AuditAction op, Map<String,
       String> auditMap, Throwable throwable) {
+
     return new AuditMessage.Builder()
-        .setUser((Server.getRemoteUser() == null) ? null :
-            Server.getRemoteUser().getUserName())
-        .atIp((Server.getRemoteIp() == null) ? null :
-            Server.getRemoteIp().getHostAddress())
-        .forOperation(op.getAction())
+        .setUser(getRemoteUserName())
+        .atIp(Server.getRemoteAddress())
+        .forOperation(op)
         .withParams(auditMap)
-        .withResult(AuditEventStatus.FAILURE.toString())
+        .withResult(AuditEventStatus.FAILURE)
         .withException(throwable)
         .build();
   }

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license
  * agreements. See the NOTICE file distributed with this work for additional
@@ -63,7 +63,6 @@
 import org.apache.hadoop.ozone.audit.SCMAction;
 import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocolServerSideTranslatorPB;
 import org.apache.hadoop.ozone.protocolPB.ProtocolMessageMetrics;
-import org.apache.hadoop.security.UserGroupInformation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -87,6 +86,7 @@
     .OZONE_SCM_HANDLER_COUNT_DEFAULT;
 import static org.apache.hadoop.hdds.scm.ScmConfigKeys
     .OZONE_SCM_HANDLER_COUNT_KEY;
+import static org.apache.hadoop.hdds.server.ServerUtils.getRemoteUserName;
 import static org.apache.hadoop.hdds.server.ServerUtils.updateRPCListenAddress;
 import static org.apache.hadoop.hdds.scm.server.StorageContainerManager
     .startRpcServer;
@@ -181,8 +181,7 @@ public void join() throws InterruptedException {
 
   @VisibleForTesting
   public String getRpcRemoteUsername() {
-    UserGroupInformation user = ProtobufRpcEngine.Server.getRemoteUser();
-    return user == null ? null : user.getUserName();
+    return getRemoteUserName();
   }
 
   @Override
@@ -564,29 +563,26 @@ private Set<DatanodeDetails> queryNodeState(HddsProtos.NodeState nodeState) {
   @Override
   public AuditMessage buildAuditMessageForSuccess(
       AuditAction op, Map<String, String> auditMap) {
+
     return new AuditMessage.Builder()
-        .setUser((Server.getRemoteUser() == null) ? null :
-            Server.getRemoteUser().getUserName())
-        .atIp((Server.getRemoteIp() == null) ? null :
-            Server.getRemoteIp().getHostAddress())
-        .forOperation(op.getAction())
+        .setUser(getRemoteUserName())
+        .atIp(Server.getRemoteAddress())
+        .forOperation(op)
         .withParams(auditMap)
-        .withResult(AuditEventStatus.SUCCESS.toString())
-        .withException(null)
+        .withResult(AuditEventStatus.SUCCESS)
         .build();
   }
 
   @Override
   public AuditMessage buildAuditMessageForFailure(AuditAction op, Map<String,
       String> auditMap, Throwable throwable) {
+
     return new AuditMessage.Builder()
-        .setUser((Server.getRemoteUser() == null) ? null :
-            Server.getRemoteUser().getUserName())
-        .atIp((Server.getRemoteIp() == null) ? null :
-            Server.getRemoteIp().getHostAddress())
-        .forOperation(op.getAction())
+        .setUser(getRemoteUserName())
+        .atIp(Server.getRemoteAddress())
+        .forOperation(op)
         .withParams(auditMap)
-        .withResult(AuditEventStatus.FAILURE.toString())
+        .withResult(AuditEventStatus.FAILURE)
         .withException(throwable)
         .build();
   }