tengqm commented on code in PR #5758:
URL: https://github.com/apache/gravitino/pull/5758#discussion_r1872410957


##########
bundles/aws-bundle/src/main/java/org/apache/gravitino/s3/fs/S3FileSystemProvider.java:
##########
@@ -41,18 +42,49 @@ public class S3FileSystemProvider implements 
FileSystemProvider {
           S3Properties.GRAVITINO_S3_ACCESS_KEY_ID, Constants.ACCESS_KEY,
           S3Properties.GRAVITINO_S3_SECRET_ACCESS_KEY, Constants.SECRET_KEY);
 
+  // We can't use Constants.AWS_CREDENTIALS_PROVIDER as in 2.7, this key does 
not exist.
+  private static final String S3_CREDENTIAL_KEY = 
"fs.s3a.aws.credentials.provider";
+  private static final String S3_SIMPLE_CREDENTIAL =
+      "org.apache.hadoop.fs.s3a.SimpleAWSCredentialsProvider";
+
   @Override
   public FileSystem getFileSystem(Path path, Map<String, String> config) 
throws IOException {
-    Configuration configuration = new Configuration();
-    Map<String, String> hadoopConfMap =
-        FileSystemUtils.toHadoopConfigMap(config, 
GRAVITINO_KEY_TO_S3_HADOOP_KEY);
 
-    if (!hadoopConfMap.containsKey(Constants.AWS_CREDENTIALS_PROVIDER)) {
-      configuration.set(
-          Constants.AWS_CREDENTIALS_PROVIDER, 
Constants.ASSUMED_ROLE_CREDENTIALS_DEFAULT);
+    try {
+      String hadoopVersion = VersionInfo.getVersion();
+      if (versionedClassLoader == null) {
+        versionedClassLoader = VersionedClassLoader.loadVersion(hadoopVersion);
+      }
+
+      ClassLoader oldClassLoader = 
Thread.currentThread().getContextClassLoader();
+      try {
+        Thread.currentThread().setContextClassLoader(versionedClassLoader);
+        Configuration configuration = new Configuration();
+        Map<String, String> hadoopConfMap =
+            FileSystemUtils.toHadoopConfigMap(config, 
GRAVITINO_KEY_TO_S3_HADOOP_KEY);
+
+        if (!hadoopConfMap.containsKey(S3_CREDENTIAL_KEY)) {
+          hadoopConfMap.put(S3_CREDENTIAL_KEY, S3_SIMPLE_CREDENTIAL);
+        }
+
+        hadoopConfMap.forEach(configuration::set);
+
+        // Hadoop-aws 2 does not support IAMInstanceCredentialsProvider
+        if (configuration.get(S3_CREDENTIAL_KEY) != null
+            && versionedClassLoader.getVersion().startsWith("hadoop2")
+            && 
configuration.get(S3_CREDENTIAL_KEY).contains("IAMInstanceCredentialsProvider"))
 {
+          configuration.set(S3_CREDENTIAL_KEY, S3_SIMPLE_CREDENTIAL);
+        }
+
+        return FileSystem.newInstance(path.toUri(), configuration);
+      } finally {
+        Thread.currentThread().setContextClassLoader(oldClassLoader);
+      }
+    } catch (ClassNotFoundException e) {
+      throw new IOException("Failed to load the Hadoop versioned class 
loader", e);

Review Comment:
   Shall we move this closer to where the exception might be thrown?



##########
bundles/aws-bundle/src/main/java/org/apache/gravitino/s3/fs/VersionedClassLoader.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.gravitino.s3.fs;
+
+import com.google.common.collect.Lists;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.Files;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import org.apache.commons.io.IOUtils;
+
+public class VersionedClassLoader extends URLClassLoader {

Review Comment:
   Maybe we should consider generalize this to support other providers, rather 
than
   making this Hadoop specific.
   



##########
bundles/aws-bundle/src/main/java/org/apache/gravitino/s3/fs/VersionedClassLoader.java:
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.gravitino.s3.fs;
+
+import com.google.common.collect.Lists;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.Files;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import org.apache.commons.io.IOUtils;
+
+public class VersionedClassLoader extends URLClassLoader {
+
+  private String version;
+
+  public VersionedClassLoader(URL[] urls, ClassLoader parent, String version) {
+    super(urls, parent);
+    this.version = version;
+  }
+
+  public String getVersion() {
+    return version;
+  }
+
+  public void setVersion(String version) {
+    this.version = version;
+  }
+
+  public static VersionedClassLoader loadVersion(String version) throws 
Exception {
+
+    // Default version 3.3 if the version is not provided
+    String hadoopVersion = "hadoop3_3";
+    String[] versionNumber = version.split("\\.");
+    if (versionNumber.length < 2 || versionNumber.length > 3) {
+      // Users may custom the version, so we need to handle the case that the 
version is not x.x
+      URL[] urls = loadFile(hadoopVersion);
+      return new VersionedClassLoader(
+          urls, VersionedClassLoader.class.getClassLoader(), hadoopVersion);
+    }
+
+    int mainVersion = Integer.parseInt(versionNumber[0]);
+    int subVersion = Integer.parseInt(versionNumber[1]);
+    int lastVersion = versionNumber.length == 3 ? 
Integer.parseInt(versionNumber[2]) : -1;

Review Comment:
   ```suggestion
       int majorVer = Integer.parseInt(versionNumber[0]);
       int minorVer = Integer.parseInt(versionNumber[1]);
       int patchVer = versionNumber.length == 3 ? 
Integer.parseInt(versionNumber[2]) : -1;
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to