amogh-jahagirdar commented on code in PR #7066:
URL: https://github.com/apache/iceberg/pull/7066#discussion_r1140452655
##########
aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java:
##########
@@ -1249,6 +1314,39 @@ private boolean s3KeyIdAccessKeyBothConfigured() {
return (s3AccessKeyId == null) == (s3SecretAccessKey == null);
}
+ private AwsCredentialsProvider credentialsProvider(String
credentialsProviderClazz) {
+ try {
+ Class.forName(credentialsProviderClazz);
+ AwsCredentialsProvider provider;
+ if (this.credentialsProviderProperties == null
+ || this.credentialsProviderProperties.size() == 0) {
+ provider =
+
DynMethods.builder("create").impl(credentialsProviderClazz).buildChecked().invoke(null);
Review Comment:
Nit: I think we can just call it `credentialsProviderClass`, don't think we
need the `clazz` moniker
##########
aws/src/test/java/org/apache/iceberg/aws/TestAwsClientFactories.java:
##########
@@ -132,6 +139,152 @@ public void
testLakeFormationAwsClientFactorySerializable() throws IOException {
.isInstanceOf(LakeFormationAwsClientFactory.class);
}
+ @Test
+ public void testWithDummyValidCredentialsProvider() {
+ AwsClientFactory defaultAwsClientFactory =
+
getAwsClientFactoryByCredentialsProvider(DummyValidProvider.class.getName());
+ assertDefaultAwsClientFactory(defaultAwsClientFactory);
+ assertClientObjectsNotNull(defaultAwsClientFactory);
+ // Ensuring S3Exception thrown instead exception thrown by
resolveCredentials() implemented by
+ // test credentials provider
+ Assertions.assertThatThrownBy(() ->
defaultAwsClientFactory.s3().listBuckets())
+
.isInstanceOf(software.amazon.awssdk.services.s3.model.S3Exception.class)
+ .hasMessageContaining("The AWS Access Key Id you provided does not
exist in our records");
+ }
+
+ @Test
+ public void testWithNoCreateMethodCredentialsProvider() {
+ String providerClassName = NoCreateMethod.class.getName();
+ String containsMessage =
+ "it does not contain a static 'create' or 'create(Map<String,
String>)' method";
+ testProviderAndAssertThrownBy(providerClassName, containsMessage);
+ }
+
+ @Test
+ public void testWithNoArgCreateMethodCredentialsProvider() {
+ String providerClassName = CreateMethod.class.getName();
+ String containsMessage = "Unable to load credentials from " +
providerClassName;
+ testProviderAndAssertThrownBy(providerClassName, containsMessage);
+ }
+
+ @Test
+ public void testWithMapArgCreateMethodCredentialsProvider() {
+ String providerClassName = CreateMapMethod.class.getName();
+ String containsMessage = "Unable to load credentials from " +
providerClassName;
+ testProviderAndAssertThrownBy(providerClassName, containsMessage);
+ }
+
+ @Test
+ public void testWithClassDoesNotExistsCredentialsProvider() {
+ String providerClassName = "invalidClassName";
+ String containsMessage = "it does not exist in the classpath";
+ testProviderAndAssertThrownBy(providerClassName, containsMessage);
+ }
+
+ @Test
+ public void testWithClassDoesNotImplementCredentialsProvider() {
+ String providerClassName = NoInterface.class.getName();
+ String containsMessage =
+ "it does not implement
software.amazon.awssdk.auth.credentials.AwsCredentialsProvider";
+ testProviderAndAssertThrownBy(providerClassName, containsMessage);
+ }
+
+ private void testProviderAndAssertThrownBy(String providerClassName, String
containsMessage) {
+ AwsClientFactory defaultAwsClientFactory =
+ getAwsClientFactoryByCredentialsProvider(providerClassName);
+ assertDefaultAwsClientFactory(defaultAwsClientFactory);
+ assertAllClientObjectsThrownBy(defaultAwsClientFactory, containsMessage);
+ }
+
+ public void assertAllClientObjectsThrownBy(
+ AwsClientFactory defaultAwsClientFactory, String containsMessage) {
+ // invoking sdk client apis to ensure resolveCredentials() being called
+ assertThatThrownBy(() -> defaultAwsClientFactory.s3().listBuckets(),
containsMessage);
+ assertThatThrownBy(
+ () ->
defaultAwsClientFactory.glue().getTables(GetTablesRequest.builder().build()),
+ containsMessage);
+ assertThatThrownBy(() -> defaultAwsClientFactory.dynamo().listTables(),
containsMessage);
+ assertThatThrownBy(() -> defaultAwsClientFactory.kms().listAliases(),
containsMessage);
+ }
+
+ private void assertClientObjectsNotNull(AwsClientFactory
defaultAwsClientFactory) {
+ Assertions.assertThat(defaultAwsClientFactory.s3()).isNotNull();
+ Assertions.assertThat(defaultAwsClientFactory.dynamo()).isNotNull();
+ Assertions.assertThat(defaultAwsClientFactory.glue()).isNotNull();
+ Assertions.assertThat(defaultAwsClientFactory.kms()).isNotNull();
+ }
+
+ private void assertThatThrownBy(
+ ThrowableAssert.ThrowingCallable shouldRaiseThrowable, String
containsMessage) {
+ Assertions.assertThatThrownBy(shouldRaiseThrowable)
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining(containsMessage);
+ }
+
+ private void assertDefaultAwsClientFactory(AwsClientFactory
awsClientFactory) {
+ Assertions.assertThat(awsClientFactory)
+ .isInstanceOf(AwsClientFactories.DefaultAwsClientFactory.class);
+ }
+
+ private AwsClientFactory getAwsClientFactoryByCredentialsProvider(String
providerClass) {
+ Map<String, String> properties =
getDefaultClientFactoryProperties(providerClass);
+ AwsClientFactory defaultAwsClientFactory =
AwsClientFactories.from(properties);
+ return defaultAwsClientFactory;
+ }
+
+ private Map<String, String> getDefaultClientFactoryProperties(String
providerClass) {
+ Map<String, String> properties = Maps.newHashMap();
+ properties.put(AwsProperties.CLIENT_CREDENTIALS_PROVIDER + ".param1",
"value1");
+ properties.put(AwsProperties.CLIENT_REGION, Region.AWS_GLOBAL.toString());
+ properties.put(AwsProperties.CLIENT_CREDENTIALS_PROVIDER, providerClass);
+ return properties;
+ }
+
+ public static class NoInterface {}
+
+ public static class DummyValidProvider implements AwsCredentialsProvider {
+
+ public static DummyValidProvider create() {
+ return new DummyValidProvider();
+ }
+
+ @Override
+ public AwsCredentials resolveCredentials() {
+ return AwsBasicCredentials.create("test-accessKeyId",
"test-secretAccessKey");
+ }
+ }
+
+ public abstract static class ProviderTestBase implements
AwsCredentialsProvider {
+
+ @Override
+ public AwsCredentials resolveCredentials() {
+ throw new IllegalArgumentException(
+ "Unable to load credentials from " + this.getClass().getName());
+ }
+ }
+
+ public static class NoCreateMethod extends ProviderTestBase {}
+
+ public static class CreateMethod extends ProviderTestBase {
+ public static CreateMethod create() {
+ return new CreateMethod();
+ }
+ }
+
+ public static class CreateMapMethod extends ProviderTestBase {
Review Comment:
Just a nit since it's for a test, but I don't think these need to be public
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
