From b24b416db5dd7d352d4a6146b712e2887b58847e Mon Sep 17 00:00:00 2001 From: Mingliang Liu Date: Tue, 28 Feb 2017 14:51:32 -0800 Subject: [PATCH] HADOOP-14135. Remove URI parameter in AWSCredentialProvider constructors. Contributed by Mingliang Liu (cherry picked from commit 2e30aa72e01de7b5774fcb312406a393221e0908) --- .../apache/hadoop/fs/s3a/S3AFileSystem.java | 2 +- .../org/apache/hadoop/fs/s3a/S3AUtils.java | 34 +++++--------- .../apache/hadoop/fs/s3a/S3ClientFactory.java | 7 ++- .../fs/s3a/SimpleAWSCredentialsProvider.java | 3 +- .../s3a/TemporaryAWSCredentialsProvider.java | 3 +- .../s3a/ITestS3AAWSCredentialsProvider.java | 44 +++++++++++++++++-- .../fs/s3a/ITestS3ATemporaryCredentials.java | 2 +- .../hadoop/fs/s3a/MockS3ClientFactory.java | 2 +- .../fs/s3a/TestS3AAWSCredentialsProvider.java | 16 +++---- 9 files changed, 69 insertions(+), 44 deletions(-) diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java index 53a0321b183..07d9730c40e 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java @@ -186,7 +186,7 @@ public class S3AFileSystem extends FileSystem { S3_CLIENT_FACTORY_IMPL, DEFAULT_S3_CLIENT_FACTORY_IMPL, S3ClientFactory.class); s3 = ReflectionUtils.newInstance(s3ClientFactoryClass, conf) - .createS3Client(name, uri); + .createS3Client(name); maxKeys = intOption(conf, MAX_PAGING_KEYS, DEFAULT_MAX_PAGING_KEYS, 1); listing = new Listing(this); diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java index 84f3c9964e2..6a11699ba5f 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java @@ -318,15 +318,12 @@ public final class S3AUtils { * Create the AWS credentials from the providers and the URI. * @param binding Binding URI, may contain user:pass login details * @param conf filesystem configuration - * @param fsURI fS URI —after any login details have been stripped. * @return a credentials provider list * @throws IOException Problems loading the providers (including reading * secrets from credential files). */ public static AWSCredentialProviderList createAWSCredentialProviderSet( - URI binding, - Configuration conf, - URI fsURI) throws IOException { + URI binding, Configuration conf) throws IOException { AWSCredentialProviderList credentials = new AWSCredentialProviderList(); Class[] awsClasses; @@ -351,11 +348,12 @@ public final class S3AUtils { SharedInstanceProfileCredentialsProvider.class.getName()); aClass = SharedInstanceProfileCredentialsProvider.class; } - credentials.add(createAWSCredentialProvider(conf, - aClass, - fsURI)); + credentials.add(createAWSCredentialProvider(conf, aClass)); } } + // make sure the logging message strips out any auth details + LOG.debug("For URI {}, using credentials {}", + S3xLoginHelper.toString(binding), credentials); return credentials; } @@ -365,8 +363,8 @@ public final class S3AUtils { * attempted in order: * *
    - *
  1. a public constructor accepting java.net.URI and - * org.apache.hadoop.conf.Configuration
    2. + *
  2. a public constructor accepting + * org.apache.hadoop.conf.Configuration
    3. *
  3. a public static method named getInstance that accepts no * arguments and returns an instance of * com.amazonaws.auth.AWSCredentialsProvider, or
    4. @@ -375,14 +373,11 @@ public final class S3AUtils { * * @param conf configuration * @param credClass credential class - * @param uri URI of the FS * @return the instantiated class * @throws IOException on any instantiation failure. */ static AWSCredentialsProvider createAWSCredentialProvider( - Configuration conf, - Class credClass, - URI uri) throws IOException { + Configuration conf, Class credClass) throws IOException { AWSCredentialsProvider credentials = null; String className = credClass.getName(); if (!AWSCredentialsProvider.class.isAssignableFrom(credClass)) { @@ -394,11 +389,10 @@ public final class S3AUtils { LOG.debug("Credential provider class is {}", className); try { - // new X(uri, conf) - Constructor cons = getConstructor(credClass, URI.class, - Configuration.class); + // new X(conf) + Constructor cons = getConstructor(credClass, Configuration.class); if (cons != null) { - credentials = (AWSCredentialsProvider)cons.newInstance(uri, conf); + credentials = (AWSCredentialsProvider)cons.newInstance(conf); return credentials; } @@ -420,16 +414,12 @@ public final class S3AUtils { // no supported constructor or factory method found throw new IOException(String.format("%s " + CONSTRUCTOR_EXCEPTION + ". A class specified in %s must provide a public constructor " - + "accepting URI and Configuration, or a public factory method named " + + "accepting Configuration, or a public factory method named " + "getInstance that accepts no arguments, or a public default " + "constructor.", className, AWS_CREDENTIALS_PROVIDER)); } catch (ReflectiveOperationException | IllegalArgumentException e) { // supported constructor or factory method found, but the call failed throw new IOException(className + " " + INSTANTIATION_EXCEPTION +".", e); - } finally { - if (credentials != null) { - LOG.debug("Using {} for {}.", credentials, uri); - } } } diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ClientFactory.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ClientFactory.java index 2b549756fc8..7ccdc0632f2 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ClientFactory.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ClientFactory.java @@ -52,11 +52,10 @@ interface S3ClientFactory { * because both values may be useful in logging. * * @param name raw input S3A file system URI - * @param uri validated form of S3A file system URI * @return S3 client * @throws IOException IO problem */ - AmazonS3 createS3Client(URI name, URI uri) throws IOException; + AmazonS3 createS3Client(URI name) throws IOException; /** * The default factory implementation, which calls the AWS SDK to configure @@ -68,10 +67,10 @@ interface S3ClientFactory { private static final Logger LOG = S3AFileSystem.LOG; @Override - public AmazonS3 createS3Client(URI name, URI uri) throws IOException { + public AmazonS3 createS3Client(URI name) throws IOException { Configuration conf = getConf(); AWSCredentialsProvider credentials = - createAWSCredentialProviderSet(name, conf, uri); + createAWSCredentialProviderSet(name, conf); ClientConfiguration awsConf = new ClientConfiguration(); initConnectionSettings(conf, awsConf); initProxySupport(conf, awsConf); diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/SimpleAWSCredentialsProvider.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/SimpleAWSCredentialsProvider.java index 13c139d5e0a..ec372bd6136 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/SimpleAWSCredentialsProvider.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/SimpleAWSCredentialsProvider.java @@ -28,7 +28,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.security.ProviderUtils; import java.io.IOException; -import java.net.URI; import static org.apache.hadoop.fs.s3a.Constants.ACCESS_KEY; import static org.apache.hadoop.fs.s3a.Constants.SECRET_KEY; @@ -51,7 +50,7 @@ public class SimpleAWSCredentialsProvider implements AWSCredentialsProvider { private String secretKey; private IOException lookupIOE; - public SimpleAWSCredentialsProvider(URI uri, Configuration conf) { + public SimpleAWSCredentialsProvider(Configuration conf) { try { Configuration c = ProviderUtils.excludeIncompatibleCredentialProviders( conf, S3AFileSystem.class); diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/TemporaryAWSCredentialsProvider.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/TemporaryAWSCredentialsProvider.java index 883ae86394f..22b23a4ad7b 100644 --- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/TemporaryAWSCredentialsProvider.java +++ b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/TemporaryAWSCredentialsProvider.java @@ -24,7 +24,6 @@ import com.amazonaws.auth.AWSCredentials; import org.apache.commons.lang.StringUtils; import java.io.IOException; -import java.net.URI; import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; @@ -51,7 +50,7 @@ public class TemporaryAWSCredentialsProvider implements AWSCredentialsProvider { private String sessionToken; private IOException lookupIOE; - public TemporaryAWSCredentialsProvider(URI uri, Configuration conf) { + public TemporaryAWSCredentialsProvider(Configuration conf) { try { Configuration c = ProviderUtils.excludeIncompatibleCredentialProviders( conf, S3AFileSystem.class); diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAWSCredentialsProvider.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAWSCredentialsProvider.java index 99454f4d38e..22c4f7ee41f 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAWSCredentialsProvider.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAWSCredentialsProvider.java @@ -19,13 +19,14 @@ package org.apache.hadoop.fs.s3a; import java.io.IOException; -import java.net.URI; import java.nio.file.AccessDeniedException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.test.GenericTestUtils; + import org.junit.Rule; import org.junit.Test; import org.junit.rules.Timeout; @@ -40,6 +41,7 @@ import org.slf4j.LoggerFactory; import static org.apache.hadoop.fs.s3a.Constants.*; import static org.apache.hadoop.fs.s3a.S3ATestConstants.*; +import static org.apache.hadoop.fs.s3a.S3AUtils.*; import static org.junit.Assert.*; /** @@ -66,6 +68,42 @@ public class ITestS3AAWSCredentialsProvider { } } + /** + * A bad CredentialsProvider which has no suitable constructor. + * + * This class does not provide a public constructor accepting Configuration, + * or a public factory method named getInstance that accepts no arguments, + * or a public default constructor. + */ + static class BadCredentialsProviderConstructor + implements AWSCredentialsProvider { + + @SuppressWarnings("unused") + public BadCredentialsProviderConstructor(String fsUri, Configuration conf) { + } + + @Override + public AWSCredentials getCredentials() { + return new BasicAWSCredentials("dummy_key", "dummy_secret"); + } + + @Override + public void refresh() { + } + } + + @Test + public void testBadCredentialsConstructor() throws Exception { + Configuration conf = new Configuration(); + conf.set(AWS_CREDENTIALS_PROVIDER, + BadCredentialsProviderConstructor.class.getName()); + try { + createFailingFS(conf); + } catch (IOException e) { + GenericTestUtils.assertExceptionContains(CONSTRUCTOR_EXCEPTION, e); + } + } + /** * Create a filesystem, expect it to fail by raising an IOException. * Raises an assertion exception if in fact the FS does get instantiated. @@ -81,7 +119,7 @@ public class ITestS3AAWSCredentialsProvider { static class BadCredentialsProvider implements AWSCredentialsProvider { @SuppressWarnings("unused") - public BadCredentialsProvider(URI name, Configuration conf) { + public BadCredentialsProvider(Configuration conf) { } @Override @@ -108,7 +146,7 @@ public class ITestS3AAWSCredentialsProvider { static class GoodCredentialsProvider extends AWSCredentialsProviderChain { @SuppressWarnings("unused") - public GoodCredentialsProvider(URI name, Configuration conf) { + public GoodCredentialsProvider(Configuration conf) { super(new BasicAWSCredentialsProvider(conf.get(ACCESS_KEY), conf.get(SECRET_KEY)), InstanceProfileCredentialsProvider.getInstance()); diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java index 84aad3c9b7f..4abe2b79e9e 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java @@ -126,7 +126,7 @@ public class ITestS3ATemporaryCredentials extends AbstractS3ATestBase { conf.set(SECRET_KEY, "secretkey"); conf.set(SESSION_TOKEN, ""); TemporaryAWSCredentialsProvider provider - = new TemporaryAWSCredentialsProvider(getFileSystem().getUri(), conf); + = new TemporaryAWSCredentialsProvider(conf); try { AWSCredentials credentials = provider.getCredentials(); fail("Expected a CredentialInitializationException," diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java index 41f04ee728b..9e0a5e42b62 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/MockS3ClientFactory.java @@ -31,7 +31,7 @@ import com.amazonaws.services.s3.AmazonS3; public class MockS3ClientFactory implements S3ClientFactory { @Override - public AmazonS3 createS3Client(URI name, URI uri) { + public AmazonS3 createS3Client(URI name) { String bucket = name.getHost(); AmazonS3 s3 = mock(AmazonS3.class); when(s3.doesBucketExist(bucket)).thenReturn(true); diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java index c29d7254b84..33740c8ab93 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java @@ -93,7 +93,7 @@ public class TestS3AAWSCredentialsProvider { URI uri = testFile.toUri(); AWSCredentialProviderList list = S3AUtils.createAWSCredentialProviderSet( - uri, conf, uri); + uri, conf); List> expectedClasses = Arrays.asList( TemporaryAWSCredentialsProvider.class, @@ -107,9 +107,9 @@ public class TestS3AAWSCredentialsProvider { URI uri1 = new URI("s3a://bucket1"), uri2 = new URI("s3a://bucket2"); Configuration conf = new Configuration(); AWSCredentialProviderList list1 = S3AUtils.createAWSCredentialProviderSet( - uri1, conf, uri1); + uri1, conf); AWSCredentialProviderList list2 = S3AUtils.createAWSCredentialProviderSet( - uri2, conf, uri2); + uri2, conf); List> expectedClasses = Arrays.asList( BasicAWSCredentialsProvider.class, @@ -132,9 +132,9 @@ public class TestS3AAWSCredentialsProvider { AnonymousAWSCredentialsProvider.class); conf.set(AWS_CREDENTIALS_PROVIDER, buildClassListString(expectedClasses)); AWSCredentialProviderList list1 = S3AUtils.createAWSCredentialProviderSet( - uri1, conf, uri1); + uri1, conf); AWSCredentialProviderList list2 = S3AUtils.createAWSCredentialProviderSet( - uri2, conf, uri2); + uri2, conf); assertCredentialProviders(expectedClasses, list1); assertCredentialProviders(expectedClasses, list2); assertSameInstanceProfileCredentialsProvider(list1.getProviders().get(1), @@ -150,9 +150,9 @@ public class TestS3AAWSCredentialsProvider { InstanceProfileCredentialsProvider.class); conf.set(AWS_CREDENTIALS_PROVIDER, buildClassListString(expectedClasses)); AWSCredentialProviderList list1 = S3AUtils.createAWSCredentialProviderSet( - uri1, conf, uri1); + uri1, conf); AWSCredentialProviderList list2 = S3AUtils.createAWSCredentialProviderSet( - uri2, conf, uri2); + uri2, conf); assertCredentialProviders(expectedClasses, list1); assertCredentialProviders(expectedClasses, list2); assertSameInstanceProfileCredentialsProvider(list1.getProviders().get(0), @@ -226,7 +226,7 @@ public class TestS3AAWSCredentialsProvider { conf.getTrimmed(KEY_CSVTEST_FILE, DEFAULT_CSVTEST_FILE)); expectException(IOException.class, expectedErrorText); URI uri = testFile.toUri(); - S3AUtils.createAWSCredentialProviderSet(uri, conf, uri); + S3AUtils.createAWSCredentialProviderSet(uri, conf); } /**