From a9060b2694b58704364db10b2b8d6acececb3ace Mon Sep 17 00:00:00 2001 From: smadurawe-oss <83042578+smadurawe-oss@users.noreply.github.com> Date: Mon, 5 Aug 2024 15:19:10 -0700 Subject: [PATCH 1/2] Fix reflection logic of aws-java-sdk-v2 credential providers (#201) * Fix reflection logic of aws-java-sdk-v2 credential providers DynamoDB connector uses reflection to load custom credential providers. When the package was upgraded to use aws-java-sdk-v2, the package was only updated to fix the differing classpaths. SDK-v2 credential providers no longer use constructors but static create() methods to initialize the instance which was not handled in the previous implementation. This PR accounts for this use case and includes a fallback to the original logic to ensure backwards compatibility. * Refactor and fix javadoc --------- Co-authored-by: Sugath Madurawe --- .../hadoop/dynamodb/DynamoDBClient.java | 8 +- .../apache/hadoop/dynamodb/DynamoDBUtil.java | 28 +++++ .../util/DynamoDBReflectionUtils.java | 117 ++++++++++++++++++ .../hadoop/dynamodb/DynamoDBClientTest.java | 46 ++++++- 4 files changed, 191 insertions(+), 8 deletions(-) create mode 100644 emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/util/DynamoDBReflectionUtils.java diff --git a/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java index b19159cb..9c7042c6 100644 --- a/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java +++ b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java @@ -459,13 +459,7 @@ protected AwsCredentialsProvider getAwsCredentialsProvider(Configuration conf) { // initialized String providerClass = conf.get(DynamoDBConstants.CUSTOM_CREDENTIALS_PROVIDER_CONF); if (!Strings.isNullOrEmpty(providerClass)) { - try { - providersList.add( - (AwsCredentialsProvider) ReflectionUtils.newInstance(Class.forName(providerClass), conf) - ); - } catch (ClassNotFoundException e) { - throw new RuntimeException("Custom AWSCredentialsProvider not found: " + providerClass, e); - } + providersList.add(DynamoDBUtil.loadAwsCredentialsProvider(providerClass, conf)); } // try to fetch credentials from core-site diff --git a/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBUtil.java b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBUtil.java index e26fcb01..1c85171f 100644 --- a/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBUtil.java +++ b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBUtil.java @@ -41,6 +41,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.dynamodb.util.ClusterTopologyNodeCapacityProvider; +import org.apache.hadoop.dynamodb.util.DynamoDBReflectionUtils; import org.apache.hadoop.dynamodb.util.NodeCapacityProvider; import org.apache.hadoop.dynamodb.util.RoundRobinYarnContainerAllocator; import org.apache.hadoop.dynamodb.util.TaskCalculator; @@ -49,6 +50,7 @@ import org.apache.hadoop.mapred.JobConf; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; import software.amazon.awssdk.core.SdkBytes; import software.amazon.awssdk.regions.internal.util.EC2MetadataUtils; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; @@ -279,6 +281,32 @@ public static String getDynamoDBRegion(Configuration conf, String region) { return DynamoDBConstants.DEFAULT_AWS_REGION; } + /** + * + * Utility method to load an aws credentials provider from config via reflection. There are two + * strategies followed: + * 1. Load credential provider via its 'create' method. + * This is the intended credential provider construction mechanism with aws-java-sdk-v2 + * For more information, visit {@link Credential Provider Changes}. + * 2. If 'create' method is not found, fallback to default no-arg constructor. + * This is kept to ensure utility method maintains backwards compatibility with what it + * used to support. + * + * @param providerClass - class name loaded from conf used as custom credential provider + * @return - credential provider loaded via reflection using class name from conf + */ + public static AwsCredentialsProvider loadAwsCredentialsProvider( + String providerClass, + Configuration conf) { + if (DynamoDBReflectionUtils.hasFactoryMethod(providerClass, "create")) { + log.debug("Provider: " + providerClass + " contains required method for creation - create()"); + return DynamoDBReflectionUtils.createInstanceFromFactory(providerClass, conf, "create"); + } else { + log.debug("Falling back to default constructor."); + return DynamoDBReflectionUtils.createInstanceOf(providerClass, conf); + } + } + public static JobClient createJobClient(JobConf jobConf) { try { return new JobClient(jobConf); diff --git a/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/util/DynamoDBReflectionUtils.java b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/util/DynamoDBReflectionUtils.java new file mode 100644 index 00000000..9319ecbb --- /dev/null +++ b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/util/DynamoDBReflectionUtils.java @@ -0,0 +1,117 @@ +/** + * Copyright 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file + * except in compliance with the License. A copy of the License is located at + * + *     http://aws.amazon.com/apache2.0/ + * + * or in the "LICENSE.TXT" file accompanying this file. This file 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.dynamodb.util; + +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Optional; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.util.ReflectionUtils; + +/** + * Reflected-utility methods for DynamoDB connector pkg + */ +public class DynamoDBReflectionUtils { + + private static final Log log = LogFactory.getLog(DynamoDBReflectionUtils.class); + + // Default no-arg constructor reflection logic + public static T createInstanceOf(String className, Configuration conf) { + return createInstanceOfWithParams(className, conf, null, null); + } + + // constructor with-args reflection logic + @SuppressWarnings("unchecked") + public static T createInstanceOfWithParams( + String className, + Configuration conf, + Class[] paramTypes, + Object[] params) { + try { + Class clazz = getClass(className); + Constructor ctor = paramTypes == null + ? (Constructor) clazz.getDeclaredConstructor() + : (Constructor) clazz.getDeclaredConstructor(paramTypes); + ctor.setAccessible(true); + T instance = ctor.newInstance(params); + log.info("Successfully loaded class: " + className); + ReflectionUtils.setConf(instance, conf); + log.debug("Configured instance to use conf"); + return instance; + + } catch (NoSuchMethodException | InvocationTargetException e) { + throw new RuntimeException("Unable to find constructor of class: " + className, e); + } catch (IllegalAccessException e) { + throw new RuntimeException("Class being loaded is not accessible: " + className, e); + } catch (InstantiationException e) { + throw new RuntimeException("Unable to instantiate class: " + className, e); + } + } + + // checks if class has method available in it + public static boolean hasFactoryMethod(String className, String methodName) { + Class clazz = getClass(className); + return Arrays.stream(clazz.getMethods()) + .anyMatch(method -> method.getName().equals(methodName)); + } + + // factory-based reflection logic that uses a method for object construction + @SuppressWarnings("unchecked") + public static T createInstanceFromFactory( + String className, + Configuration conf, + String methodName) { + try { + Class clazz = getClass(className); + Method m = clazz.getDeclaredMethod(methodName); + m.setAccessible(true); + T instance = (T) m.invoke(null); + log.info("Successfully loaded class: " + className); + ReflectionUtils.setConf(instance, conf); + log.debug("Configured instance to use conf"); + return instance; + + } catch (NoSuchMethodException e) { + log.error("Method not found for object construction: " + methodName); + throw new RuntimeException("Unable to find static method to load class: " + className, e); + } catch (InvocationTargetException e) { + log.error("Exception found when invoking method for object construction: " + methodName); + throw new RuntimeException("Unable to load class: " + className, e); + } catch (IllegalAccessException e) { + throw new RuntimeException("Class being loaded is not accessible: " + className, e); + } + } + + // checks if class can be loaded + private static Class getClass(String className) { + try { + return Class.forName(className, true, getContextOrDefaultClassLoader()); + } catch (ClassNotFoundException e) { + throw new RuntimeException("Unable to locate class to load via reflection: " + className, e); + } + } + + private static ClassLoader getContextOrDefaultClassLoader() { + return Optional.of(Thread.currentThread().getContextClassLoader()) + .orElseGet(DynamoDBReflectionUtils::getDefaultClassLoader); + } + + private static ClassLoader getDefaultClassLoader() { + return DynamoDBReflectionUtils.class.getClassLoader(); + } +} diff --git a/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java b/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java index 90743a99..58d6a6a0 100644 --- a/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java +++ b/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java @@ -97,7 +97,7 @@ public void testDefaultCredentials() { } @Test - public void testCustomCredentialsProvider() { + public void testCustomCredentialsProviderWithConstructor() { final String MY_ACCESS_KEY = "abc"; final String MY_SECRET_KEY = "xyz"; Configuration conf = new Configuration(); @@ -112,6 +112,22 @@ public void testCustomCredentialsProvider() { Assert.assertEquals(MY_SECRET_KEY, provider.resolveCredentials().secretAccessKey()); } + @Test + public void testCustomCredentialsProviderWithMethod() { + final String MY_ACCESS_KEY = "abc"; + final String MY_SECRET_KEY = "xyz"; + Configuration conf = new Configuration(); + conf.set("my.accessKey", MY_ACCESS_KEY); + conf.set("my.secretKey", MY_SECRET_KEY); + conf.set(DynamoDBConstants.CUSTOM_CREDENTIALS_PROVIDER_CONF, MyFactoryCredentialsProvider.class + .getName()); + + DynamoDBClient dynamoDBClient = new DynamoDBClient(); + AwsCredentialsProvider provider = dynamoDBClient.getAwsCredentialsProvider(conf); + Assert.assertEquals(MY_ACCESS_KEY, provider.resolveCredentials().accessKeyId()); + Assert.assertEquals(MY_SECRET_KEY, provider.resolveCredentials().secretAccessKey()); + } + @Test public void testCustomProviderNotFound() { Configuration conf = new Configuration(); @@ -325,6 +341,7 @@ private void setProxyUsernameAndPassword(Configuration conf, String username, St } } + // Default Constructor-based credential provider private static class MyAWSCredentialsProvider implements AwsCredentialsProvider, Configurable { private Configuration conf; private String accessKey; @@ -352,4 +369,31 @@ public void setConf(Configuration configuration) { } } + // Method-based constructor credential provider + private static class MyFactoryCredentialsProvider implements AwsCredentialsProvider, Configurable { + private Configuration conf; + private String accessKey; + private String secretKey; + + public static MyFactoryCredentialsProvider create() { + return new MyFactoryCredentialsProvider(); + } + + @Override + public AwsCredentials resolveCredentials() { + return AwsBasicCredentials.create(accessKey, secretKey); + } + + @Override + public Configuration getConf() { + return this.conf; + } + + @Override + public void setConf(Configuration configuration) { + this.conf = configuration; + accessKey = conf.get("my.accessKey"); + secretKey = conf.get("my.secretKey"); + } + } } From b4edd1324cb905d3a5607ccb090717fde3dcb60d Mon Sep 17 00:00:00 2001 From: smadurawe-oss <83042578+smadurawe-oss@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:16:18 -0700 Subject: [PATCH 2/2] Set default credential provider to sdk's default provider (#203) * Set default credential provider to sdk's default provider The default credential provide dynamo-db connector falls back to is InstanceProfileCredentialProvider. However, instance profile is not available in all deployment environments (e.g. EMR Serverless). Instead, aws-java-sdk-v2 default provider should be used as the fallback. * Add unit test * nit: fix imports --------- Co-authored-by: Sugath Madurawe --- .../hadoop/dynamodb/DynamoDBClient.java | 6 ++--- .../hadoop/dynamodb/DynamoDBClientTest.java | 26 ++++++++++++++++--- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java index 9c7042c6..0be5222b 100644 --- a/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java +++ b/emr-dynamodb-hadoop/src/main/java/org/apache/hadoop/dynamodb/DynamoDBClient.java @@ -46,14 +46,13 @@ import org.apache.hadoop.dynamodb.filter.DynamoDBIndexInfo; import org.apache.hadoop.dynamodb.filter.DynamoDBQueryFilter; import org.apache.hadoop.mapred.Reporter; -import org.apache.hadoop.util.ReflectionUtils; import org.joda.time.Duration; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; import software.amazon.awssdk.auth.credentials.AwsCredentialsProviderChain; import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; -import software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvider; +import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider; import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; import software.amazon.awssdk.core.exception.SdkException; import software.amazon.awssdk.http.apache.ApacheHttpClient; @@ -476,7 +475,8 @@ protected AwsCredentialsProvider getAwsCredentialsProvider(Configuration conf) { } if (Strings.isNullOrEmpty(accessKey) || Strings.isNullOrEmpty(secretKey)) { - providersList.add(InstanceProfileCredentialsProvider.create()); + log.debug("Custom credential provider not found, loading default provider from sdk"); + providersList.add(DefaultCredentialsProvider.create()); } else if (!Strings.isNullOrEmpty(sessionKey)) { final AwsCredentials credentials = AwsSessionCredentials.create(accessKey, secretKey, sessionKey); diff --git a/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java b/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java index 58d6a6a0..a4b8c152 100644 --- a/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java +++ b/emr-dynamodb-hadoop/src/test/java/org/apache/hadoop/dynamodb/DynamoDBClientTest.java @@ -17,7 +17,6 @@ import static org.mockito.Mockito.mock; import com.google.common.base.Strings; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.apache.hadoop.conf.Configurable; @@ -28,17 +27,18 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.mockito.Mock; import org.mockito.Mockito; -import java.util.HashMap; +import java.lang.reflect.Field; import java.util.List; import java.util.Map; -import java.util.Set; + import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProviderChain; import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; +import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider; import software.amazon.awssdk.http.apache.ProxyConfiguration; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; import software.amazon.awssdk.services.dynamodb.model.AttributeValue; @@ -166,6 +166,24 @@ public void testBasicSessionCredentials(){ } + @Test + public void testDefaultCredentialProvider() { + DynamoDBClient dynamoDBClient = new DynamoDBClient(); + AwsCredentialsProvider provider = dynamoDBClient.getAwsCredentialsProvider(conf); + Assert.assertTrue(provider instanceof AwsCredentialsProviderChain); + AwsCredentialsProviderChain providerChain = (AwsCredentialsProviderChain) provider; + try { + Field providersField = AwsCredentialsProviderChain.class.getDeclaredField("credentialsProviders"); + providersField.setAccessible(true); + @SuppressWarnings("unchecked") + List providers = (List) providersField.get(providerChain); + Assert.assertEquals(1, providers.size()); + Assert.assertTrue(providers.get(0) instanceof DefaultCredentialsProvider); + } catch (Exception e) { + Assert.fail("Unexpected error thrown: " + e.getMessage()); + } + } + @Test public void setsClientConfigurationProxyHostAndPortWhenBothAreSupplied() { setTestProxyHostAndPort(conf);