From 784d8cae01d1d880c81b80b164c82b6c59c7c61f Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Fri, 12 Apr 2024 12:44:08 +0200 Subject: [PATCH 1/3] HDDS-10615. ETag change detected in S3A contract test --- .../hadoop/ozone/client/OzoneBucket.java | 51 +++++++++---------- .../ozone/om/helpers/BasicOmKeyInfo.java | 21 ++++++++ .../dist/src/main/compose/common/s3a-test.sh | 4 +- .../src/main/proto/OmClientProtocol.proto | 1 + .../ozone/om/request/RequestAuditor.java | 7 +++ .../ozone/s3/endpoint/BucketEndpoint.java | 6 ++- 6 files changed, 59 insertions(+), 31 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 index 112c76f8c0a8..6976d1842cd6 100644 --- 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 buildKeysWithKeyPrefix( List 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 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 index 044cc17f5e57..fdbbef8aec05 100644 --- 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 index 85dbc5feced2..d58b91b4e526 100644 --- 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 index b0d26020c8d2..6569ea7f2446 100644 --- 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/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/RequestAuditor.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/RequestAuditor.java index c0872db0fd61..78e67bb8ed5c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/RequestAuditor.java +++ b/hadoop-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 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 index b7a5af73403e..7a78c34478c6 100644 --- 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(eTag); + } if (next.getReplicationType().toString().equals(ReplicationType .STAND_ALONE.toString())) { keyMetadata.setStorageClass(S3StorageType.REDUCED_REDUNDANCY.toString()); From f2a568628b5d3bf8584e3fdc0e3fd65fceefa117 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 16 Apr 2024 11:37:13 +0200 Subject: [PATCH 2/3] wrap in quotes --- .../org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java | 2 +- .../org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 index 7a78c34478c6..14b5b8ffebae 100644 --- 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 @@ -712,7 +712,7 @@ private void addKey(ListObjectResponse response, OzoneKey next) { keyMetadata.setSize(next.getDataSize()); String eTag = next.getMetadata().get(ETAG); if (eTag != null) { - keyMetadata.setETag(eTag); + keyMetadata.setETag(ObjectEndpoint.wrapInQuotes(eTag)); } if (next.getReplicationType().toString().equals(ReplicationType .STAND_ALONE.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 index 26e51a6d6661..642ea0c900e7 100644 --- 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 @@ -1356,7 +1356,7 @@ public boolean isDatastreamEnabled() { return datastreamEnabled; } - private String wrapInQuotes(String value) { + static String wrapInQuotes(String value) { return "\"" + value + "\""; } From c896b17a326c3724aa49637a530d2dd384863aa0 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Tue, 16 Apr 2024 11:37:52 +0200 Subject: [PATCH 3/3] remove unnecessary concatenation --- .../org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 642ea0c900e7..68daf80cfbca 100644 --- 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);