Skip to content

Commit

Permalink
HADOOP-19089: [ABFS] Reverting Back Support of setXAttr() and getXAtt…
Browse files Browse the repository at this point in the history 
…r() 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
  • Loading branch information
@anujmodi2021
anujmodi2021 committed Mar 25, 2024
1 parent 5c7e40f commit c4fa1b6
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 34 deletions.
30 changes: 7 additions & 23 deletions ...p-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -979,32 +979,22 @@ public void setXAttr(final Path path,
TracingContext tracingContext = new TracingContext(clientCorrelationId,
fileSystemId, FSOperationType.SET_ATTR, true, tracingHeaderFormat,
listener);
Hashtable<String, String> properties;
Hashtable<String, String> 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
Expand All @@ -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<String, String> properties;
Hashtable<String, String> 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);
Expand Down
5 changes: 5 additions & 0 deletions hadoop-tools/hadoop-azure/src/site/markdown/abfs.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'.

## <a name="KnownIssues"></a> 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`

## <a name="testing"></a> Testing ABFS

See the relevant section in [Testing Azure](testing_azure.html).
41 changes: 30 additions & 11 deletions ...azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemAttributes.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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);
}
Expand All @@ -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);
Expand All @@ -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, () -> {
Expand All @@ -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 {

Expand Down Expand Up @@ -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);
}
}

0 comments on commit c4fa1b6

Please sign in to comment.