HDDS-10615. ETag change detected in S3A contract test (apache#6519)

xichen01 · Apr 16, 2024 · 92f2449 · 92f2449
1 parent e0a4033
commit 92f2449
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 33 deletions.
diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
@@ -62,6 +62,7 @@
 import java.util.NoSuchElementException;
 import java.util.stream.Collectors;
 
+import static org.apache.hadoop.ozone.OzoneConsts.ETAG;
 import static org.apache.hadoop.ozone.OzoneConsts.QUOTA_RESET;
 import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND;
@@ -1276,25 +1277,33 @@ protected void initDelimiterKeyPrefix() {
     protected List<OzoneKey> buildKeysWithKeyPrefix(
         List<OzoneFileStatusLight> statuses) {
       return statuses.stream()
-          .map(status -> {
-            BasicOmKeyInfo keyInfo = status.getKeyInfo();
-            String keyName = keyInfo.getKeyName();
-            if (status.isDirectory()) {
-              // add trailing slash to represent directory
-              keyName = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
-            }
-            return new OzoneKey(keyInfo.getVolumeName(),
-                keyInfo.getBucketName(), keyName,
-                keyInfo.getDataSize(), keyInfo.getCreationTime(),
-                keyInfo.getModificationTime(),
-                keyInfo.getReplicationConfig(), keyInfo.isFile());
-          })
+          .map(OzoneBucket::toOzoneKey)
           .filter(key -> StringUtils.startsWith(key.getName(), getKeyPrefix()))
           .collect(Collectors.toList());
     }
 
   }
 
+  private static OzoneKey toOzoneKey(OzoneFileStatusLight status) {
+    BasicOmKeyInfo keyInfo = status.getKeyInfo();
+    String keyName = keyInfo.getKeyName();
+    final Map<String, String> metadata;
+    if (status.isDirectory()) {
+      // add trailing slash to represent directory
+      keyName = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
+      metadata = Collections.emptyMap();
+    } else {
+      metadata = Collections.singletonMap(ETAG, keyInfo.getETag());
+    }
+    return new OzoneKey(keyInfo.getVolumeName(),
+        keyInfo.getBucketName(), keyName,
+        keyInfo.getDataSize(), keyInfo.getCreationTime(),
+        keyInfo.getModificationTime(),
+        keyInfo.getReplicationConfig(),
+        metadata,
+        keyInfo.isFile());
+  }
+
 
   /**
    * An Iterator to iterate over {@link OzoneKey} list.
@@ -1662,21 +1671,7 @@ private boolean getChildrenKeys(String keyPrefix, String startKey,
       for (int indx = 0; indx < statuses.size(); indx++) {
         OzoneFileStatusLight status = statuses.get(indx);
         BasicOmKeyInfo keyInfo = status.getKeyInfo();
-        String keyName = keyInfo.getKeyName();
-
-        OzoneKey ozoneKey;
-        // Add dir to the dirList
-        if (status.isDirectory()) {
-          // add trailing slash to represent directory
-          keyName = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
-        }
-        ozoneKey = new OzoneKey(keyInfo.getVolumeName(),
-            keyInfo.getBucketName(), keyName,
-            keyInfo.getDataSize(), keyInfo.getCreationTime(),
-            keyInfo.getModificationTime(),
-            keyInfo.getReplicationConfig(),
-            keyInfo.isFile());
-
+        OzoneKey ozoneKey = toOzoneKey(status);
         keysResultList.add(ozoneKey);
 
         if (status.isDirectory()) {

diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BasicOmKeyInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/BasicOmKeyInfo.java
@@ -25,6 +25,8 @@
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.BasicKeyInfo;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.ListKeysRequest;
 
+import static org.apache.hadoop.ozone.OzoneConsts.ETAG;
+
 /**
  * Lightweight OmKeyInfo class.
  */
@@ -38,6 +40,7 @@ public final class BasicOmKeyInfo {
   private final long modificationTime;
   private final ReplicationConfig replicationConfig;
   private final boolean isFile;
+  private final String eTag;
 
   private BasicOmKeyInfo(Builder b) {
     this.volumeName = b.volumeName;
@@ -48,6 +51,7 @@ private BasicOmKeyInfo(Builder b) {
     this.modificationTime = b.modificationTime;
     this.replicationConfig = b.replicationConfig;
     this.isFile = b.isFile;
+    this.eTag = b.eTag;
   }
 
   private BasicOmKeyInfo(OmKeyInfo b) {
@@ -59,6 +63,7 @@ private BasicOmKeyInfo(OmKeyInfo b) {
     this.modificationTime = b.getModificationTime();
     this.replicationConfig = b.getReplicationConfig();
     this.isFile = b.isFile();
+    this.eTag = b.getMetadata().get(ETAG);
   }
 
   public String getVolumeName() {
@@ -93,6 +98,10 @@ public boolean isFile() {
     return isFile;
   }
 
+  public String getETag() {
+    return eTag;
+  }
+
   /**
    * Builder of BasicOmKeyInfo.
    */
@@ -105,6 +114,7 @@ public static class Builder {
     private long modificationTime;
     private ReplicationConfig replicationConfig;
     private boolean isFile;
+    private String eTag;
 
     public Builder setVolumeName(String volumeName) {
       this.volumeName = volumeName;
@@ -146,6 +156,11 @@ public Builder setIsFile(boolean isFile) {
       return this;
     }
 
+    public Builder setETag(String etag) {
+      this.eTag = etag;
+      return this;
+    }
+
     public BasicOmKeyInfo build() {
       return new BasicOmKeyInfo(this);
     }
@@ -164,6 +179,9 @@ public BasicKeyInfo getProtobuf() {
     } else {
       builder.setFactor(ReplicationConfig.getLegacyFactor(replicationConfig));
     }
+    if (eTag != null) {
+      builder.setETag(eTag);
+    }
 
     return builder.build();
   }
@@ -188,6 +206,7 @@ public static BasicOmKeyInfo getFromProtobuf(BasicKeyInfo basicKeyInfo,
             basicKeyInfo.getType(),
             basicKeyInfo.getFactor(),
             basicKeyInfo.getEcReplicationConfig()))
+        .setETag(basicKeyInfo.getETag())
         .setIsFile(!keyName.endsWith("/"));
 
     return builder.build();
@@ -212,6 +231,7 @@ public static BasicOmKeyInfo getFromProtobuf(String volumeName,
             basicKeyInfo.getType(),
             basicKeyInfo.getFactor(),
             basicKeyInfo.getEcReplicationConfig()))
+        .setETag(basicKeyInfo.getETag())
         .setIsFile(!keyName.endsWith("/"));
 
     return builder.build();
@@ -232,6 +252,7 @@ public boolean equals(Object o) {
         creationTime == basicOmKeyInfo.creationTime &&
         modificationTime == basicOmKeyInfo.modificationTime &&
         replicationConfig.equals(basicOmKeyInfo.replicationConfig) &&
+        Objects.equals(eTag, basicOmKeyInfo.eTag) &&
         isFile == basicOmKeyInfo.isFile;
   }
 

diff --git a/hadoop-ozone/dist/src/main/compose/common/s3a-test.sh b/hadoop-ozone/dist/src/main/compose/common/s3a-test.sh
@@ -79,11 +79,11 @@ EOF
 
   # Some tests are skipped due to known issues.
   # - ITestS3AContractDistCp: HDDS-10616
-  # - ITestS3AContractEtag, ITestS3AContractRename: HDDS-10615
   # - ITestS3AContractGetFileStatusV1List: HDDS-10617
   # - ITestS3AContractMkdir: HDDS-10572
+  # - ITestS3AContractRename: HDDS-10665
   mvn -B -V --fail-never --no-transfer-progress \
-    -Dtest='ITestS3AContract*, !ITestS3AContractDistCp, !ITestS3AContractEtag, !ITestS3AContractGetFileStatusV1List, !ITestS3AContractMkdir, !ITestS3AContractRename' \
+    -Dtest='ITestS3AContract*, !ITestS3AContractDistCp, !ITestS3AContractGetFileStatusV1List, !ITestS3AContractMkdir, !ITestS3AContractRename' \
     clean test
 
   local target="${RESULT_DIR}/junit/${bucket}/target"

diff --git a/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto b/hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
@@ -1117,6 +1117,7 @@ message BasicKeyInfo {
     optional hadoop.hdds.ReplicationType type = 5;
     optional hadoop.hdds.ReplicationFactor factor = 6;
     optional hadoop.hdds.ECReplicationConfig ecReplicationConfig = 7;
+    optional string eTag = 8;
 }
 
 message DirectoryInfo {

diff --git a/...-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/RequestAuditor.java b/...-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/RequestAuditor.java
@@ -32,6 +32,8 @@
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos
     .UserInfo;
 
+import static org.apache.hadoop.ozone.OzoneConsts.ETAG;
+
 /**
  * Interface for OM Requests to convert to audit objects.
  */
@@ -80,6 +82,11 @@ default Map<String, String> buildKeyArgsAuditMap(KeyArgs keyArgs) {
         auditMap.put(OzoneConsts.REPLICATION_CONFIG,
             ECReplicationConfig.toString(keyArgs.getEcReplicationConfig()));
       }
+      for (HddsProtos.KeyValue item : keyArgs.getMetadataList()) {
+        if (ETAG.equals(item.getKey())) {
+          auditMap.put(ETAG, item.getValue());
+        }
+      }
       return auditMap;
     }
   }

diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
@@ -69,6 +69,7 @@
 import java.util.List;
 import java.util.Set;
 
+import static org.apache.hadoop.ozone.OzoneConsts.ETAG;
 import static org.apache.hadoop.ozone.audit.AuditLogger.PerformanceStringBuilder;
 import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
 import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER;
@@ -709,7 +710,10 @@ private void addKey(ListObjectResponse response, OzoneKey next) {
     keyMetadata.setKey(EncodingTypeObject.createNullable(next.getName(),
         response.getEncodingType()));
     keyMetadata.setSize(next.getDataSize());
-    keyMetadata.setETag("" + next.getModificationTime());
+    String eTag = next.getMetadata().get(ETAG);
+    if (eTag != null) {
+      keyMetadata.setETag(ObjectEndpoint.wrapInQuotes(eTag));
+    }
     if (next.getReplicationType().toString().equals(ReplicationType
         .STAND_ALONE.toString())) {
       keyMetadata.setStorageClass(S3StorageType.REDUCED_REDUNDANCY.toString());

diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
@@ -611,7 +611,7 @@ public Response head(
       // Should not return ETag header if the ETag is not set
       // doing so will result in "null" string being returned instead
       // which breaks some AWS SDK implementation
-      response.header(ETAG, "" + wrapInQuotes(key.getMetadata().get(ETAG)));
+      response.header(ETAG, wrapInQuotes(key.getMetadata().get(ETAG)));
     }
 
     addLastModifiedDate(response, key);
@@ -1356,7 +1356,7 @@ public boolean isDatastreamEnabled() {
     return datastreamEnabled;
   }
 
-  private String wrapInQuotes(String value) {
+  static String wrapInQuotes(String value) {
     return "\"" + value + "\"";
   }