From c4fa1b65fbcad54a27d6cd48117499e3732c3b00 Mon Sep 17 00:00:00 2001 From: Anuj Modi <128447756+anujmodi2021@users.noreply.github.com> Date: Mon, 25 Mar 2024 07:13:24 -0700 Subject: [PATCH] HADOOP-19089: [ABFS] Reverting Back Support of setXAttr() and getXAttr() on root path (#6592) This reverts most of HADOOP-18869: [ABFS] Fix behavior of a File System APIs on root path (#6003). Calling getXAttr("/") or setXAttr("/") on an abfs container will fail with `Operation failed: "The request URI is invalid.", HTTP 400 Bad Request` This change is to ensure: * Consistency across ADLS clients * Consistency across authentication mechanisms. Contributed by Anuj Modi --- .../fs/azurebfs/AzureBlobFileSystem.java | 30 ++++---------- .../hadoop-azure/src/site/markdown/abfs.md | 5 +++ .../ITestAzureBlobFileSystemAttributes.java | 41 ++++++++++++++----- 3 files changed, 42 insertions(+), 34 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java index b234f76d5d9dd..8b6bc337fb21c 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java @@ -952,7 +952,7 @@ public void setOwner(final Path path, final String owner, final String group) } /** - * Set the value of an attribute for a path. + * Set the value of an attribute for a non-root path. * * @param path The path on which to set the attribute * @param name The attribute to set @@ -979,32 +979,22 @@ public void setXAttr(final Path path, TracingContext tracingContext = new TracingContext(clientCorrelationId, fileSystemId, FSOperationType.SET_ATTR, true, tracingHeaderFormat, listener); - Hashtable properties; + Hashtable properties = abfsStore + .getPathStatus(qualifiedPath, tracingContext); String xAttrName = ensureValidAttributeName(name); - - if (path.isRoot()) { - properties = abfsStore.getFilesystemProperties(tracingContext); - } else { - properties = abfsStore.getPathStatus(qualifiedPath, tracingContext); - } - boolean xAttrExists = properties.containsKey(xAttrName); XAttrSetFlag.validate(name, xAttrExists, flag); String xAttrValue = abfsStore.decodeAttribute(value); properties.put(xAttrName, xAttrValue); - if (path.isRoot()) { - abfsStore.setFilesystemProperties(properties, tracingContext); - } else { - abfsStore.setPathProperties(qualifiedPath, properties, tracingContext); - } + abfsStore.setPathProperties(qualifiedPath, properties, tracingContext); } catch (AzureBlobFileSystemException ex) { checkException(path, ex); } } /** - * Get the value of an attribute for a path. + * Get the value of an attribute for a non-root path. * * @param path The path on which to get the attribute * @param name The attribute to get @@ -1029,15 +1019,9 @@ public byte[] getXAttr(final Path path, final String name) TracingContext tracingContext = new TracingContext(clientCorrelationId, fileSystemId, FSOperationType.GET_ATTR, true, tracingHeaderFormat, listener); - Hashtable properties; + Hashtable properties = abfsStore + .getPathStatus(qualifiedPath, tracingContext); String xAttrName = ensureValidAttributeName(name); - - if (path.isRoot()) { - properties = abfsStore.getFilesystemProperties(tracingContext); - } else { - properties = abfsStore.getPathStatus(qualifiedPath, tracingContext); - } - if (properties.containsKey(xAttrName)) { String xAttrValue = properties.get(xAttrName); value = abfsStore.encodeAttribute(xAttrValue); diff --git a/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md b/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md index 3f2e89ad6f502..008cb143542a4 100644 --- a/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md +++ b/hadoop-tools/hadoop-azure/src/site/markdown/abfs.md @@ -1229,6 +1229,11 @@ The fix is to mimic the ownership to the local OS user, by adding the below prop Once the above properties are configured, `hdfs dfs -ls abfs://container1@abfswales1.dfs.core.windows.net/` shows the ADLS Gen2 files/directories are now owned by 'user1'. +## Known Issues + +Following failures are known and expected to fail as of now. +1. AzureBlobFileSystem.setXAttr() and AzureBlobFileSystem.getXAttr() will fail when attempted on root ("/") path with `Operation failed: "The request URI is invalid.", HTTP 400 Bad Request` + ## Testing ABFS See the relevant section in [Testing Azure](testing_azure.html). diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemAttributes.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemAttributes.java index 7fe229c519fb6..a4ad0d207c3fd 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemAttributes.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemAttributes.java @@ -27,8 +27,11 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.XAttrSetFlag; import org.apache.hadoop.fs.azurebfs.constants.FSOperationType; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; import org.apache.hadoop.fs.azurebfs.utils.TracingHeaderValidator; +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ROOT_PATH; import static org.apache.hadoop.test.LambdaTestUtils.intercept; /** @@ -45,8 +48,7 @@ public ITestAzureBlobFileSystemAttributes() throws Exception { @Test public void testSetGetXAttr() throws Exception { AzureBlobFileSystem fs = getFileSystem(); - AbfsConfiguration conf = fs.getAbfsStore().getAbfsConfiguration(); - final Path testPath = path("setGetXAttr"); + final Path testPath = path(getMethodName()); fs.create(testPath); testGetSetXAttrHelper(fs, testPath); } @@ -56,7 +58,7 @@ public void testSetGetXAttrCreateReplace() throws Exception { AzureBlobFileSystem fs = getFileSystem(); byte[] attributeValue = fs.getAbfsStore().encodeAttribute("one"); String attributeName = "user.someAttribute"; - Path testFile = path("createReplaceXAttr"); + Path testFile = path(getMethodName()); // after creating a file, it must be possible to create a new xAttr touch(testFile); @@ -75,7 +77,7 @@ public void testSetGetXAttrReplace() throws Exception { byte[] attributeValue1 = fs.getAbfsStore().encodeAttribute("one"); byte[] attributeValue2 = fs.getAbfsStore().encodeAttribute("two"); String attributeName = "user.someAttribute"; - Path testFile = path("replaceXAttr"); + Path testFile = path(getMethodName()); // after creating a file, it must not be possible to replace an xAttr intercept(IOException.class, () -> { @@ -91,13 +93,6 @@ public void testSetGetXAttrReplace() throws Exception { .containsExactly(attributeValue2); } - @Test - public void testGetSetXAttrOnRoot() throws Exception { - AzureBlobFileSystem fs = getFileSystem(); - final Path testPath = new Path("/"); - testGetSetXAttrHelper(fs, testPath); - } - private void testGetSetXAttrHelper(final AzureBlobFileSystem fs, final Path testPath) throws Exception { @@ -154,4 +149,28 @@ private void testGetSetXAttrHelper(final AzureBlobFileSystem fs, .describedAs("Retrieved Attribute Does not Matches in Decoded Form") .isEqualTo(decodedAttributeValue2); } + + @Test + public void testGetSetXAttrOnRoot() throws Exception { + AzureBlobFileSystem fs = getFileSystem(); + String attributeName = "user.attribute1"; + byte[] attributeValue = fs.getAbfsStore().encodeAttribute("hi"); + final Path testPath = new Path(ROOT_PATH); + + AbfsRestOperationException ex = intercept(AbfsRestOperationException.class, () -> { + fs.getXAttr(testPath, attributeName); + }); + + Assertions.assertThat(ex.getStatusCode()) + .describedAs("GetXAttr() on root should fail with Bad Request") + .isEqualTo(HTTP_BAD_REQUEST); + + ex = intercept(AbfsRestOperationException.class, () -> { + fs.setXAttr(testPath, attributeName, attributeValue, CREATE_FLAG); + }); + + Assertions.assertThat(ex.getStatusCode()) + .describedAs("SetXAttr() on root should fail with Bad Request") + .isEqualTo(HTTP_BAD_REQUEST); + } }