This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new d8d141caa0 [#13242] fix(common): resolve the Hadoop FileSystem from
the fetched hdfs uri (#13243)
d8d141caa0 is described below
commit d8d141caa0d4dde50275d701e2c601d0f6ea83a0
Author: YangJie <[email protected]>
AuthorDate: Fri Sep 18 09:19:45 2026 -0400
[#13242] fix(common): resolve the Hadoop FileSystem from the fetched hdfs
uri (#13243)
### What changes were proposed in this pull request?
`copyHdfsFileToLocal` now calls `FileSystem.get(URI, Configuration)`
instead of `FileSystem.get(Configuration)`, resolving the filesystem
from the fetched uri rather than from `fs.defaultFS`.
### Why are the changes needed?
Resolving from `fs.defaultFS` ignored the fetched uri's authority, so
fetching from a cluster other than the default failed with `Wrong FS` or
copied from the wrong cluster.
Fix: #13242
### Does this PR introduce _any_ user-facing change?
No API change. Fetching an `hdfs://` uri whose authority differs from
`fs.defaultFS` now resolves the correct filesystem instead of failing
with `Wrong FS` or copying from the wrong cluster.
### How was this patch tested?
Added `TestFileFetcher` coverage that pins `copyHdfsFileToLocal`
resolving the filesystem from the fetched uri's authority; because the
`common` module has no Hadoop dependency, it runs against minimal
`org.apache.hadoop` test stubs whose default-FS overload reproduces
Hadoop's `Wrong FS` behavior. It fails on the pre-fix tree and passes
after the fix.
---
.../org/apache/gravitino/utils/FileFetcher.java | 4 +-
.../apache/gravitino/utils/TestFileFetcher.java | 26 ++++++++--
.../java/org/apache/hadoop/conf/Configuration.java | 27 ++++++++++
.../test/java/org/apache/hadoop/fs/FileSystem.java | 60 ++++++++++++++++++++++
.../src/test/java/org/apache/hadoop/fs/Path.java | 34 ++++++++++++
5 files changed, 147 insertions(+), 4 deletions(-)
diff --git a/common/src/main/java/org/apache/gravitino/utils/FileFetcher.java
b/common/src/main/java/org/apache/gravitino/utils/FileFetcher.java
index dacd0cdcb8..7a64b4e78f 100644
--- a/common/src/main/java/org/apache/gravitino/utils/FileFetcher.java
+++ b/common/src/main/java/org/apache/gravitino/utils/FileFetcher.java
@@ -202,7 +202,9 @@ public final class FileFetcher {
Class<?> pathClass = Class.forName("org.apache.hadoop.fs.Path");
Object fileSystem =
- fileSystemClass.getMethod("get", configurationClass).invoke(null,
configuration);
+ fileSystemClass
+ .getMethod("get", URI.class, configurationClass)
+ .invoke(null, uri, configuration);
Object srcPath = pathClass.getConstructor(URI.class).newInstance(uri);
Object destPath =
pathClass.getConstructor(URI.class).newInstance(destFile.toURI());
fileSystemClass
diff --git
a/common/src/test/java/org/apache/gravitino/utils/TestFileFetcher.java
b/common/src/test/java/org/apache/gravitino/utils/TestFileFetcher.java
index e088e3a8bf..34397512a1 100644
--- a/common/src/test/java/org/apache/gravitino/utils/TestFileFetcher.java
+++ b/common/src/test/java/org/apache/gravitino/utils/TestFileFetcher.java
@@ -23,6 +23,7 @@ import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.net.InetSocketAddress;
+import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -35,6 +36,8 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
@@ -42,9 +45,11 @@ import org.junit.jupiter.api.io.TempDir;
/**
* Tests for {@link FileFetcher}.
*
- * <p>The hdfs happy path is exercised reflectively against Hadoop and is
covered by the catalog
- * Kerberos integration tests; the common module has no Hadoop on its test
classpath, so here we
- * only assert that an hdfs uri without a Hadoop configuration is rejected.
+ * <p>The hdfs happy path is exercised reflectively against minimal Hadoop
test stubs (see {@code
+ * org.apache.hadoop} under this test source set), which assert that the
filesystem is resolved from
+ * the fetched uri rather than fs.defaultFS; the catalog Kerberos integration
tests cover the path
+ * against real Hadoop. Here we also assert that an hdfs uri without a Hadoop
configuration is
+ * rejected.
*/
public class TestFileFetcher {
@@ -278,6 +283,21 @@ public class TestFileFetcher {
() -> FileFetcher.get().fetchFileFromUri("hdfs://namenode/keytab",
destFile, 10, null));
}
+ @Test
+ public void testHdfsFetchResolvesFileSystemFromUriNotDefaultFs() throws
Exception {
+ File destFile = new File(tempDir, "dest_hdfs_uri");
+ Configuration hadoopConf = new Configuration();
+ FileSystem.uriOverloadUsed.set(null);
+
+ // Before the fix, the fetcher resolved the filesystem via
FileSystem.get(Configuration),
+ // which keys off fs.defaultFS and ignores the fetched uri's authority;
the stub's
+ // default-FS overload throws the "Wrong FS" error real Hadoop produces in
that case.
+ FileFetcher.get().fetchFileFromUri("hdfs://namenode:8020/dir/keytab",
destFile, 10, hadoopConf);
+
+ Assertions.assertEquals(
+ new URI("hdfs://namenode:8020/dir/keytab"),
FileSystem.uriOverloadUsed.get());
+ }
+
@Test
public void testUnsupportedSchemeShouldFail() {
File destFile = new File(tempDir, "dest_scp");
diff --git a/common/src/test/java/org/apache/hadoop/conf/Configuration.java
b/common/src/test/java/org/apache/hadoop/conf/Configuration.java
new file mode 100644
index 0000000000..8998242ce9
--- /dev/null
+++ b/common/src/test/java/org/apache/hadoop/conf/Configuration.java
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hadoop.conf;
+
+/**
+ * Test stub of the Hadoop configuration class, so the FileFetcher reflective
Hadoop path can be
+ * exercised without a Hadoop dependency. It only has to exist; the fetcher
never calls methods on
+ * it.
+ */
+public class Configuration {}
diff --git a/common/src/test/java/org/apache/hadoop/fs/FileSystem.java
b/common/src/test/java/org/apache/hadoop/fs/FileSystem.java
new file mode 100644
index 0000000000..a6626fbde1
--- /dev/null
+++ b/common/src/test/java/org/apache/hadoop/fs/FileSystem.java
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hadoop.fs;
+
+import com.google.errorprone.annotations.DoNotCall;
+import java.net.URI;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.hadoop.conf.Configuration;
+
+/**
+ * Test stub of the Hadoop FileSystem recording which {@code get} overload the
FileFetcher uses. In
+ * real Hadoop, {@code get(Configuration)} resolves the filesystem from {@code
fs.defaultFS} and
+ * ignores the fetched URI's authority, while {@code get(URI, Configuration)}
resolves it from the
+ * URI — which is the contract the fetcher must uphold.
+ */
+public abstract class FileSystem {
+
+ /** The URI passed to {@code get(URI, Configuration)} by the code under
test, if any. */
+ public static final AtomicReference<URI> uriOverloadUsed = new
AtomicReference<>();
+
+ /**
+ * The single-arg {@code get(Configuration)} overload resolved from
fs.defaultFS. Called only
+ * reflectively by the code under test, so the {@code @DoNotCall} annotation
affects no
+ * compile-time caller.
+ */
+ @DoNotCall("Always throws: the default-FS overload must not be used to fetch
a specific uri")
+ public static FileSystem get(Configuration conf) {
+ // Mirrors real Hadoop: the default filesystem, whatever the fetched URI
points at.
+ throw new IllegalArgumentException(
+ "Wrong FS: the filesystem was resolved from fs.defaultFS, not from the
fetched uri");
+ }
+
+ /** The two-arg {@code get(URI, Configuration)} overload resolving from the
URI. */
+ public static FileSystem get(URI uri, Configuration conf) {
+ uriOverloadUsed.set(uri);
+ return new LocalStub();
+ }
+
+ /** Copy that succeeds; the stub only records the call through the overload
bookkeeping. */
+ public void copyToLocalFile(Path src, Path dst) {}
+
+ private static final class LocalStub extends FileSystem {}
+}
diff --git a/common/src/test/java/org/apache/hadoop/fs/Path.java
b/common/src/test/java/org/apache/hadoop/fs/Path.java
new file mode 100644
index 0000000000..caf84238ec
--- /dev/null
+++ b/common/src/test/java/org/apache/hadoop/fs/Path.java
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hadoop.fs;
+
+import java.net.URI;
+
+/** Test stub of the Hadoop Path class; the fetcher only constructs it from a
{@link URI}. */
+public class Path {
+
+ /**
+ * Constructs a path from a URI; the stub keeps no state because the fetcher
only passes it to
+ * {@code copyToLocalFile}.
+ *
+ * @param uri the source or destination URI
+ */
+ public Path(URI uri) {}
+}
