From a5321f1563d7d1ced7631a2a80ebc9e79376e371 Mon Sep 17 00:00:00 2001
From: Jyotinder Singh <jyotinder.singh@cloudera.com>
Date: Thu, 3 Feb 2022 21:21:32 +0530
Subject: [PATCH 1/3] HDDS-6245. Add BucketLayout logging to Audit Logs

---
 .../org/apache/hadoop/ozone/OzoneConsts.java  |  1 +
 .../hadoop/ozone/om/helpers/OmBucketInfo.java |  1 +
 .../apache/hadoop/ozone/om/OzoneManager.java  | 18 ++++++-----
 .../hadoop/ozone/om/ResolvedBucket.java       | 30 +++++++++++++++++++
 .../ozone/om/request/key/OMKeyRequest.java    |  2 +-
 .../om/request/key/OMKeysDeleteRequest.java   |  2 +-
 .../om/request/key/OMKeysRenameRequest.java   |  2 +-
 7 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
index 133f39d09654..cb0362352bd0 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
@@ -333,6 +333,7 @@ private OzoneConsts() {
   public static final String UNDELETED_KEYS_LIST = "unDeletedKeysList";
   public static final String SOURCE_VOLUME = "sourceVolume";
   public static final String SOURCE_BUCKET = "sourceBucket";
+  public static final String BUCKET_LAYOUT = "bucketLayout";
 
 
 
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
index adbf3986c5a5..ad81c8b016fc 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
@@ -319,6 +319,7 @@ public Map<String, String> toAuditMap() {
     Map<String, String> auditMap = new LinkedHashMap<>();
     auditMap.put(OzoneConsts.VOLUME, this.volumeName);
     auditMap.put(OzoneConsts.BUCKET, this.bucketName);
+    auditMap.put(OzoneConsts.BUCKET_LAYOUT, String.valueOf(this.bucketLayout));
     auditMap.put(OzoneConsts.GDPR_FLAG,
         this.metadata.get(OzoneConsts.GDPR_FLAG));
     auditMap.put(OzoneConsts.ACLS,
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index 900babde032a..817af7b56919 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -2644,7 +2644,8 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException {
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
+    Map<String, String> auditMap =
+        bucket.auditWithBucketLayout(args.toAuditMap(), this);
 
     args = bucket.update(args);
 
@@ -2677,7 +2678,7 @@ public List<OmKeyInfo> listKeys(String volumeName, String bucketName,
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap = bucket.audit();
+    Map<String, String> auditMap = bucket.auditWithBucketLayout(this);
     auditMap.put(OzoneConsts.START_KEY, startKey);
     auditMap.put(OzoneConsts.MAX_KEYS, String.valueOf(maxKeys));
     auditMap.put(OzoneConsts.KEY_PREFIX, keyPrefix);
@@ -2928,7 +2929,7 @@ public OmMultipartUploadListParts listParts(final String volumeName,
 
     ResolvedBucket bucket = resolveBucketLink(Pair.of(volumeName, bucketName));
 
-    Map<String, String> auditMap = bucket.audit();
+    Map<String, String> auditMap = bucket.auditWithBucketLayout(this);
     auditMap.put(OzoneConsts.KEY, keyName);
     auditMap.put(OzoneConsts.UPLOAD_ID, uploadID);
     auditMap.put(OzoneConsts.PART_NUMBER_MARKER,
@@ -2957,7 +2958,7 @@ public OmMultipartUploadList listMultipartUploads(String volumeName,
 
     ResolvedBucket bucket = resolveBucketLink(Pair.of(volumeName, bucketName));
 
-    Map<String, String> auditMap = bucket.audit();
+    Map<String, String> auditMap = bucket.auditWithBucketLayout(this);
     auditMap.put(OzoneConsts.PREFIX, prefix);
 
     metrics.incNumListMultipartUploads();
@@ -2983,7 +2984,8 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException {
     ResolvedBucket bucket = resolveBucketLink(args);
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
+    Map<String, String> auditMap =
+        bucket.auditWithBucketLayout(args.toAuditMap(), this);
 
     args = bucket.update(args);
 
@@ -3021,7 +3023,8 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws IOException {
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
+    Map<String, String> auditMap =
+        bucket.auditWithBucketLayout(args.toAuditMap(), this);
 
     args = bucket.update(args);
 
@@ -3054,7 +3057,8 @@ public List<OzoneFileStatus> listStatus(OmKeyArgs args, boolean recursive,
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
+    Map<String, String> auditMap =
+        bucket.auditWithBucketLayout(args.toAuditMap(), this);
 
     args = bucket.update(args);
 
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
index fef9b2e35a27..2af33068eb06 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
@@ -21,7 +21,10 @@
 import org.apache.hadoop.ozone.OzoneConsts;
 import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -36,6 +39,8 @@ public class ResolvedBucket {
 
   private final Pair<String, String> requested;
   private final Pair<String, String> resolved;
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ResolvedBucket.class);
 
   public ResolvedBucket(Pair<String, String> requested,
       Pair<String, String> resolved) {
@@ -93,6 +98,10 @@ public Map<String, String> audit() {
     return audit(new LinkedHashMap<>());
   }
 
+  public Map<String, String> auditWithBucketLayout(OzoneManager ozoneManager) {
+    return auditWithBucketLayout(new LinkedHashMap<>(), ozoneManager);
+  }
+
   /**
    * Adds audit information about the bucket (and if it's a link, then the
    * real bucket, too) to {@code auditMap}.
@@ -108,4 +117,25 @@ public Map<String, String> audit(Map<String, String> auditMap) {
     return auditMap;
   }
 
+  /**
+   * Adds audit information about the bucket and its bucket layout (and if it's
+   * a link, then the real bucket, too) to {@code auditMap}.
+   *
+   * @return the same map for convenience
+   */
+  public Map<String, String> auditWithBucketLayout(Map<String, String> auditMap,
+                                                   OzoneManager om) {
+    audit(auditMap);
+    try {
+      // Add BucketLayout to auditMap from the resolved Bucket.
+      auditMap.put(OzoneConsts.BUCKET_LAYOUT, String.valueOf(
+          om.getBucketInfo(realVolume(), realBucket()).getBucketLayout()
+      ));
+    } catch (IOException ioe) {
+      LOG.error("Failed to get bucket layout for Volume: {}, Bucket: {}",
+          realVolume(), realBucket(), ioe);
+    }
+    return auditMap;
+  }
+
 }
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
index b4528b889c19..ea5e714c7110 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
@@ -114,7 +114,7 @@ protected KeyArgs resolveBucketLink(
       Map<String, String> auditMap) throws IOException {
     ResolvedBucket bucket = ozoneManager.resolveBucketLink(keyArgs, this);
     keyArgs = bucket.update(keyArgs);
-    bucket.audit(auditMap);
+    bucket.auditWithBucketLayout(auditMap, ozoneManager);
     return keyArgs;
   }
 
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
index 59d87736f58f..40ce903d2f8e 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
@@ -115,7 +115,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     try {
       ResolvedBucket bucket = ozoneManager.resolveBucketLink(
           Pair.of(volumeName, bucketName), this);
-      bucket.audit(auditMap);
+      bucket.auditWithBucketLayout(auditMap, ozoneManager);
       volumeName = bucket.realVolume();
       bucketName = bucket.realBucket();
 
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
index c480d3fd24bf..d9ca93963464 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
@@ -110,7 +110,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     try {
       ResolvedBucket bucket = ozoneManager.resolveBucketLink(
           Pair.of(volumeName, bucketName), this);
-      bucket.audit(auditMap);
+      bucket.auditWithBucketLayout(auditMap, ozoneManager);
       volumeName = bucket.realVolume();
       bucketName = bucket.realBucket();
       acquiredLock =

From d81f739f44c958ee643278057ae70c239712c5df Mon Sep 17 00:00:00 2001
From: Jyotinder Singh <jyotinder.singh@cloudera.com>
Date: Thu, 3 Feb 2022 23:42:04 +0530
Subject: [PATCH 2/3] Address reviews. Remove bucket layout from getting
 audited in unrelated logs.

---
 .../apache/hadoop/ozone/om/OzoneManager.java  | 18 ++++++-------
 .../hadoop/ozone/om/ResolvedBucket.java       | 25 -------------------
 .../ozone/om/request/key/OMKeyRequest.java    |  2 +-
 .../om/request/key/OMKeysDeleteRequest.java   |  2 +-
 .../om/request/key/OMKeysRenameRequest.java   |  2 +-
 5 files changed, 10 insertions(+), 39 deletions(-)

diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index 817af7b56919..900babde032a 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -2644,8 +2644,7 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException {
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap =
-        bucket.auditWithBucketLayout(args.toAuditMap(), this);
+    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
 
     args = bucket.update(args);
 
@@ -2678,7 +2677,7 @@ public List<OmKeyInfo> listKeys(String volumeName, String bucketName,
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap = bucket.auditWithBucketLayout(this);
+    Map<String, String> auditMap = bucket.audit();
     auditMap.put(OzoneConsts.START_KEY, startKey);
     auditMap.put(OzoneConsts.MAX_KEYS, String.valueOf(maxKeys));
     auditMap.put(OzoneConsts.KEY_PREFIX, keyPrefix);
@@ -2929,7 +2928,7 @@ public OmMultipartUploadListParts listParts(final String volumeName,
 
     ResolvedBucket bucket = resolveBucketLink(Pair.of(volumeName, bucketName));
 
-    Map<String, String> auditMap = bucket.auditWithBucketLayout(this);
+    Map<String, String> auditMap = bucket.audit();
     auditMap.put(OzoneConsts.KEY, keyName);
     auditMap.put(OzoneConsts.UPLOAD_ID, uploadID);
     auditMap.put(OzoneConsts.PART_NUMBER_MARKER,
@@ -2958,7 +2957,7 @@ public OmMultipartUploadList listMultipartUploads(String volumeName,
 
     ResolvedBucket bucket = resolveBucketLink(Pair.of(volumeName, bucketName));
 
-    Map<String, String> auditMap = bucket.auditWithBucketLayout(this);
+    Map<String, String> auditMap = bucket.audit();
     auditMap.put(OzoneConsts.PREFIX, prefix);
 
     metrics.incNumListMultipartUploads();
@@ -2984,8 +2983,7 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException {
     ResolvedBucket bucket = resolveBucketLink(args);
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap =
-        bucket.auditWithBucketLayout(args.toAuditMap(), this);
+    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
 
     args = bucket.update(args);
 
@@ -3023,8 +3021,7 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws IOException {
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap =
-        bucket.auditWithBucketLayout(args.toAuditMap(), this);
+    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
 
     args = bucket.update(args);
 
@@ -3057,8 +3054,7 @@ public List<OzoneFileStatus> listStatus(OmKeyArgs args, boolean recursive,
     }
 
     boolean auditSuccess = true;
-    Map<String, String> auditMap =
-        bucket.auditWithBucketLayout(args.toAuditMap(), this);
+    Map<String, String> auditMap = bucket.audit(args.toAuditMap());
 
     args = bucket.update(args);
 
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
index 2af33068eb06..0626c6abafc0 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
@@ -98,10 +98,6 @@ public Map<String, String> audit() {
     return audit(new LinkedHashMap<>());
   }
 
-  public Map<String, String> auditWithBucketLayout(OzoneManager ozoneManager) {
-    return auditWithBucketLayout(new LinkedHashMap<>(), ozoneManager);
-  }
-
   /**
    * Adds audit information about the bucket (and if it's a link, then the
    * real bucket, too) to {@code auditMap}.
@@ -117,25 +113,4 @@ public Map<String, String> audit(Map<String, String> auditMap) {
     return auditMap;
   }
 
-  /**
-   * Adds audit information about the bucket and its bucket layout (and if it's
-   * a link, then the real bucket, too) to {@code auditMap}.
-   *
-   * @return the same map for convenience
-   */
-  public Map<String, String> auditWithBucketLayout(Map<String, String> auditMap,
-                                                   OzoneManager om) {
-    audit(auditMap);
-    try {
-      // Add BucketLayout to auditMap from the resolved Bucket.
-      auditMap.put(OzoneConsts.BUCKET_LAYOUT, String.valueOf(
-          om.getBucketInfo(realVolume(), realBucket()).getBucketLayout()
-      ));
-    } catch (IOException ioe) {
-      LOG.error("Failed to get bucket layout for Volume: {}, Bucket: {}",
-          realVolume(), realBucket(), ioe);
-    }
-    return auditMap;
-  }
-
 }
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
index ea5e714c7110..b4528b889c19 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
@@ -114,7 +114,7 @@ protected KeyArgs resolveBucketLink(
       Map<String, String> auditMap) throws IOException {
     ResolvedBucket bucket = ozoneManager.resolveBucketLink(keyArgs, this);
     keyArgs = bucket.update(keyArgs);
-    bucket.auditWithBucketLayout(auditMap, ozoneManager);
+    bucket.audit(auditMap);
     return keyArgs;
   }
 
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
index 40ce903d2f8e..59d87736f58f 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
@@ -115,7 +115,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     try {
       ResolvedBucket bucket = ozoneManager.resolveBucketLink(
           Pair.of(volumeName, bucketName), this);
-      bucket.auditWithBucketLayout(auditMap, ozoneManager);
+      bucket.audit(auditMap);
       volumeName = bucket.realVolume();
       bucketName = bucket.realBucket();
 
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
index d9ca93963464..c480d3fd24bf 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysRenameRequest.java
@@ -110,7 +110,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
     try {
       ResolvedBucket bucket = ozoneManager.resolveBucketLink(
           Pair.of(volumeName, bucketName), this);
-      bucket.auditWithBucketLayout(auditMap, ozoneManager);
+      bucket.audit(auditMap);
       volumeName = bucket.realVolume();
       bucketName = bucket.realBucket();
       acquiredLock =

From c95aef42a36c0fe7c0049dcd65e35f3ddb02face Mon Sep 17 00:00:00 2001
From: Jyotinder Singh <jyotinder.singh@cloudera.com>
Date: Thu, 3 Feb 2022 23:42:57 +0530
Subject: [PATCH 3/3] remove unused vars

---
 .../main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
index 0626c6abafc0..fef9b2e35a27 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ResolvedBucket.java
@@ -21,10 +21,7 @@
 import org.apache.hadoop.ozone.OzoneConsts;
 import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyArgs;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -39,8 +36,6 @@ public class ResolvedBucket {
 
   private final Pair<String, String> requested;
   private final Pair<String, String> resolved;
-  private static final Logger LOG =
-      LoggerFactory.getLogger(ResolvedBucket.class);
 
   public ResolvedBucket(Pair<String, String> requested,
       Pair<String, String> resolved) {