Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,10 @@ public static long dateToLong(final Date date) {
* <li>a public default constructor.</li>
* </ol>
*
* Uses the configuration's class loader, to respect the setting of
* {@link Constants#AWS_S3_CLASSLOADER_ISOLATION}. For backwards
* compatibility, if configuration is null, a default classloader is used.
*
* @param className name of class for which instance is to be created
* @param conf configuration
* @param uri URI of the FS
Expand All @@ -657,7 +661,18 @@ public static <InstanceT> InstanceT getInstanceFromReflection(String className,
String methodName,
String configKey) throws IOException {
try {
Class<?> instanceClass = S3AUtils.class.getClassLoader().loadClass(className);
ClassLoader classLoader;
if (conf != null) {
classLoader = conf.getClassLoader();
LOG.debug("Loading class {} with Configuration classloader {}",
className, classLoader);
} else {
classLoader = S3AUtils.class.getClassLoader();
LOG.debug("Loading class {} with default classloader {}",
className, classLoader);
}

Class<?> instanceClass = classLoader.loadClass(className);
if (Modifier.isAbstract(instanceClass.getModifiers())) {
throw isAbstract(uri, className, configKey);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public static Signer createSigner(String signerType, final Configuration conf, S
LOG.debug("Signer class from {} and key {} is {}", signerType, configKey, className);

Signer signer =
S3AUtils.getInstanceFromReflection(className, null, null, Signer.class, "create",
S3AUtils.getInstanceFromReflection(className, conf, null, Signer.class, "create",
configKey);
requireNonNull(conf);
if (signer instanceof Configurable sc) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,22 @@

import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import software.amazon.awssdk.auth.credentials.AwsCredentials;
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.s3a.impl.InstantiationIOException;

import static org.apache.hadoop.test.LambdaTestUtils.intercept;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;

/**
* Checks that classloader isolation for loading extension classes is applied
Expand All @@ -37,10 +45,33 @@
*/
public class ITestS3AFileSystemIsolatedClassloader extends AbstractS3ATestBase {

private static String customClassName = "custom.class.name";

private static class CustomCredentialsProvider implements AwsCredentialsProvider {

CustomCredentialsProvider() {
}

@Override
public AwsCredentials resolveCredentials() {
return null;
}

}

private static class CustomClassLoader extends ClassLoader {
}

private final ClassLoader customClassLoader = new CustomClassLoader();
private final ClassLoader customClassLoader = spy(new CustomClassLoader());
{
try {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nice way to simulate classloader pain.

doReturn(CustomCredentialsProvider.class)
.when(customClassLoader)
.loadClass(customClassName);
} catch (ClassNotFoundException ex) {
throw new RuntimeException(ex);
}
}

private S3AFileSystem createNewTestFs(Configuration conf) throws IOException {
S3AFileSystem fs = new S3AFileSystem();
Expand Down Expand Up @@ -87,8 +118,15 @@ private Map<String, String> mapOf(String key, String value) {
return m;
}

private Map<String, String> mapOf(String key1, String value1, String key2, String value2) {
HashMap<String, String> m = new HashMap<>();
m.put(key1, value1);
m.put(key2, value2);
return m;
}

@Test
public void defaultIsolatedClassloader() throws IOException {
public void defaultIsolatedClassloader() throws Exception {
assertInNewFilesystem(mapOf(), (fs) -> {
Assertions.assertThat(fs.getConf().getClassLoader())
.describedAs("The classloader used to load s3a fs extensions")
Expand All @@ -100,10 +138,21 @@ public void defaultIsolatedClassloader() throws IOException {
.isEqualTo(fs.getClass().getClassLoader())
.describedAs("the classloader that loaded the fs");
});

InstantiationIOException ex = intercept(
InstantiationIOException.class,
() -> assertInNewFilesystem(
mapOf(Constants.AWS_CREDENTIALS_PROVIDER, customClassName),
(fs) -> {}));

Assertions.assertThat(ex.getCause())
.describedAs("cause")
.isInstanceOf(ClassNotFoundException.class)
.hasMessageContaining(customClassName);
}

@Test
public void isolatedClassloader() throws IOException {
public void isolatedClassloader() throws Exception {
assertInNewFilesystem(mapOf(Constants.AWS_S3_CLASSLOADER_ISOLATION, "true"), (fs) -> {
Assertions.assertThat(fs.getConf().getClassLoader())
.describedAs("The classloader used to load s3a fs extensions")
Expand All @@ -115,11 +164,26 @@ public void isolatedClassloader() throws IOException {
.isEqualTo(fs.getClass().getClassLoader())
.describedAs("the classloader that loaded the fs");
});

InstantiationIOException ex = intercept(
InstantiationIOException.class,
() -> assertInNewFilesystem(
mapOf(Constants.AWS_S3_CLASSLOADER_ISOLATION, "true",
Constants.AWS_CREDENTIALS_PROVIDER, customClassName),
(fs) -> {}));

Assertions.assertThat(ex.getCause())
.describedAs("cause")
.isInstanceOf(ClassNotFoundException.class)
.hasMessageContaining(customClassName);
}

@Test
public void notIsolatedClassloader() throws IOException {
assertInNewFilesystem(mapOf(Constants.AWS_S3_CLASSLOADER_ISOLATION, "false"), (fs) -> {
Map<String, String> confToSet =
mapOf(Constants.AWS_S3_CLASSLOADER_ISOLATION, "false",
Constants.AWS_CREDENTIALS_PROVIDER, customClassName);
assertInNewFilesystem(confToSet, (fs) -> {
Assertions.assertThat(fs.getConf().getClassLoader())
.describedAs("The classloader used to load s3a fs extensions")
.isEqualTo(Thread.currentThread().getContextClassLoader())
Expand All @@ -129,6 +193,16 @@ public void notIsolatedClassloader() throws IOException {
.describedAs("The classloader used to load s3a fs extensions")
.isNotEqualTo(fs.getClass().getClassLoader())
.describedAs("the classloader that loaded the fs");

S3AFileSystem s3a = (S3AFileSystem) fs;
List<AwsCredentialsProvider> providers =
s3a.getS3AInternals().shareCredentials("test").getProviders();
Assertions.assertThat(providers)
.describedAs("providers")
.hasSize(1);
Assertions.assertThat(providers.get(0))
.describedAs("provider")
.isInstanceOf(CustomCredentialsProvider.class);
});
}
}