Merge pull request apache#19 from ahmarsuhail/HADOOP-18073-cred-provider-fix-test

fixes failing test

Author: passaro

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java

@@ -21,7 +21,6 @@
 import com.amazonaws.ClientConfiguration;
 import com.amazonaws.Protocol;
 import com.amazonaws.auth.AWSCredentialsProvider;
-import com.amazonaws.auth.EnvironmentVariableCredentialsProvider;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
@@ -50,6 +49,7 @@
 import org.slf4j.LoggerFactory;
 
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
+import software.amazon.awssdk.auth.credentials.EnvironmentVariableCredentialsProvider;
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
 import software.amazon.awssdk.core.exception.AbortedException;
 import software.amazon.awssdk.core.exception.SdkException;
@@ -76,6 +76,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -621,6 +622,22 @@ public static List<Class<?>> loadAWSProviderClasses(Configuration conf,
     }
   }
 
+  /**
+   * Maps V1 credential providers to either their equivalent SDK V2 class or hadoop provider.
+   */
+  private static Map<String, Class> initCredentialProvidersMap() {
+    Map<String, Class> v1v2CredentialProviderMap = new HashMap<>();
+
+    v1v2CredentialProviderMap.put("EnvironmentVariableCredentialsProvider",
+        EnvironmentVariableCredentialsProvider.class);
+    v1v2CredentialProviderMap.put("EC2ContainerCredentialsProviderWrapper",
+        IAMInstanceCredentialsProvider.class);
+    v1v2CredentialProviderMap.put("InstanceProfileCredentialsProvider",
+        IAMInstanceCredentialsProvider.class);
+
+    return v1v2CredentialProviderMap;
+  }
+
   /**
    * Load list of AWS credential provider/credential provider factory classes;
    * support a forbidden list to prevent loops, mandate full secrets, etc.
@@ -643,6 +660,8 @@ public static AWSCredentialProviderList buildAWSProviderList(
     List<Class<?>> awsClasses = loadAWSProviderClasses(conf,
         key,
         defaultValues.toArray(new Class[defaultValues.size()]));
+
+    Map<String, Class> v1v2CredentialProviderMap = initCredentialProvidersMap();
     // and if the list is empty, switch back to the defaults.
     // this is to address the issue that configuration.getClasses()
     // doesn't return the default if the config value is just whitespace.
@@ -659,11 +678,11 @@ public static AWSCredentialProviderList buildAWSProviderList(
             + " in option " + key + ": " + aClass);
       }
 
-      // TODO: Not sure what to do here yet.
-      //  There were some V1 providers for which we suppressed warnings, how do we map those?
-      if (aClass.getName().contains(AWS_AUTH_CLASS_PREFIX)) {
-        // V2Migration.v1ProviderReferenced(aClass.getName());
-
+      if (v1v2CredentialProviderMap.containsKey(aClass.getSimpleName()) &&
+          aClass.getName().contains(AWS_AUTH_CLASS_PREFIX)){
+        providers.add(createAWSV2CredentialProvider(conf,
+            v1v2CredentialProviderMap.get(aClass.getSimpleName()), binding));
+      } else if (AWSCredentialsProvider.class.isAssignableFrom(aClass)) {
         providers.add(createAWSV1CredentialProvider(conf,
             aClass, binding));
       } else {
@@ -823,7 +842,7 @@ private static AwsCredentialsProvider createAWSV2CredentialProvider(
 
       // X.getInstance()
       Method factory = getFactoryMethod(credClass, AwsCredentialsProvider.class,
-          "getInstance");
+          "create");
       if (factory != null) {
         credentials = (AwsCredentialsProvider)factory.invoke(null);
         return credentials;
@@ -840,7 +859,7 @@ private static AwsCredentialsProvider createAWSV2CredentialProvider(
       throw new IOException(String.format("%s " + CONSTRUCTOR_EXCEPTION
               + ".  A class specified in %s must provide a public constructor "
               + "of a supported signature, or a public factory method named "
-              + "getInstance that accepts no arguments.",
+              + "create that accepts no arguments.",
           className, AWS_CREDENTIALS_PROVIDER));
     } catch (InvocationTargetException e) {
       // TODO: Can probably be moved to a common method, but before doing this, check if we still

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java

@@ -28,13 +28,14 @@
 
 import com.amazonaws.auth.AWSCredentials;
 import com.amazonaws.auth.AWSCredentialsProvider;
-import com.amazonaws.auth.EnvironmentVariableCredentialsProvider;
-import com.amazonaws.auth.InstanceProfileCredentialsProvider;
 import org.apache.hadoop.util.Sets;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import software.amazon.awssdk.auth.credentials.AwsCredentials;
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
+import software.amazon.awssdk.auth.credentials.EnvironmentVariableCredentialsProvider;
+import software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvider;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
@@ -52,6 +53,7 @@
 /**
  * Unit tests for {@link Constants#AWS_CREDENTIALS_PROVIDER} logic.
  */
+// TODO: Add new tests that use a mix of V1 and V2 providers and assert that everything works ok.
 public class TestS3AAWSCredentialsProvider {
 
   /**
@@ -66,7 +68,7 @@ public class TestS3AAWSCredentialsProvider {
   @Test
   public void testProviderWrongClass() throws Exception {
     expectProviderInstantiationFailure(this.getClass(),
-        NOT_AWS_PROVIDER);
+        NOT_AWS_V2_PROVIDER);
   }
 
   @Test
@@ -220,21 +222,18 @@ public void refresh() {
    * A credential provider whose constructor raises an NPE.
    */
   static class ConstructorFailureProvider
-      implements AWSCredentialsProvider {
+      implements AwsCredentialsProvider {
 
     @SuppressWarnings("unused")
     public ConstructorFailureProvider() {
       throw new NullPointerException("oops");
     }
 
     @Override
-    public AWSCredentials getCredentials() {
+    public AwsCredentials resolveCredentials() {
       return null;
     }
 
-    @Override
-    public void refresh() {
-    }
   }
 
   @Test