From 8f0ba9ee1b89901ba8961df3576a2b44f0e856cf Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Wed, 9 Jun 2021 02:33:03 +0530 Subject: [PATCH] HADOOP-17725. Improve error message for token providers in ABFS (#3041) Contributed by Viraj Jasani. --- .../hadoop/fs/azurebfs/AbfsConfiguration.java | 50 +++++++++++++++---- .../fs/azurebfs/TestAccountConfiguration.java | 49 ++++++++++++++++-- 2 files changed, 83 insertions(+), 16 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java index 1c4a09be3c..82a116ba83 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java @@ -402,6 +402,24 @@ public String getPasswordString(String key) throws IOException { return null; } + /** + * Returns a value for the key if the value exists and is not null. + * Otherwise, throws {@link ConfigurationPropertyNotFoundException} with + * key name. + * + * @param key Account-agnostic configuration key + * @return value if exists + * @throws IOException if error in fetching password or + * ConfigurationPropertyNotFoundException for missing key + */ + private String getMandatoryPasswordString(String key) throws IOException { + String value = getPasswordString(key); + if (value == null) { + throw new ConfigurationPropertyNotFoundException(key); + } + return value; + } + /** * Returns account-specific token provider class if it exists, else checks if * an account-agnostic setting is present for token provider class if AuthType @@ -742,25 +760,33 @@ public AccessTokenProvider getTokenProvider() throws TokenAccessProviderExceptio FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME, null, AccessTokenProvider.class); - AccessTokenProvider tokenProvider = null; + AccessTokenProvider tokenProvider; if (tokenProviderClass == ClientCredsTokenProvider.class) { - String authEndpoint = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT); - String clientId = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID); - String clientSecret = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET); + String authEndpoint = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT); + String clientId = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID); + String clientSecret = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET); tokenProvider = new ClientCredsTokenProvider(authEndpoint, clientId, clientSecret); LOG.trace("ClientCredsTokenProvider initialized"); } else if (tokenProviderClass == UserPasswordTokenProvider.class) { - String authEndpoint = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT); - String username = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_NAME); - String password = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_PASSWORD); + String authEndpoint = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT); + String username = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_NAME); + String password = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_USER_PASSWORD); tokenProvider = new UserPasswordTokenProvider(authEndpoint, username, password); LOG.trace("UserPasswordTokenProvider initialized"); } else if (tokenProviderClass == MsiTokenProvider.class) { String authEndpoint = getTrimmedPasswordString( FS_AZURE_ACCOUNT_OAUTH_MSI_ENDPOINT, AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_MSI_ENDPOINT); - String tenantGuid = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_MSI_TENANT); - String clientId = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID); + String tenantGuid = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_MSI_TENANT); + String clientId = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID); String authority = getTrimmedPasswordString( FS_AZURE_ACCOUNT_OAUTH_MSI_AUTHORITY, AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_MSI_AUTHORITY); @@ -772,8 +798,10 @@ public AccessTokenProvider getTokenProvider() throws TokenAccessProviderExceptio String authEndpoint = getTrimmedPasswordString( FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN_ENDPOINT, AuthConfigurations.DEFAULT_FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN_ENDPOINT); - String refreshToken = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN); - String clientId = getPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID); + String refreshToken = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_REFRESH_TOKEN); + String clientId = + getMandatoryPasswordString(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID); tokenProvider = new RefreshTokenBasedTokenProvider(authEndpoint, clientId, refreshToken); LOG.trace("RefreshTokenBasedTokenProvider initialized"); diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java index 4cb0961e93..86bb2adbe5 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAccountConfiguration.java @@ -19,18 +19,22 @@ package org.apache.hadoop.fs.azurebfs; import java.io.IOException; - -import org.assertj.core.api.Assertions; -import org.junit.Test; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.TokenAccessProviderException; import org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider; import org.apache.hadoop.fs.azurebfs.oauth2.CustomTokenProviderAdapter; import org.apache.hadoop.fs.azurebfs.services.AuthType; +import org.apache.hadoop.test.GenericTestUtils; +import org.apache.hadoop.test.LambdaTestUtils; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; +import org.assertj.core.api.Assertions; +import org.junit.Test; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT; @@ -38,6 +42,8 @@ import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; /** * Tests correct precedence of various configurations that might be returned. @@ -60,6 +66,12 @@ public class TestAccountConfiguration { private static final String TEST_CLIENT_ID = "clientId"; private static final String TEST_CLIENT_SECRET = "clientSecret"; + private static final List CONFIG_KEYS = + Collections.unmodifiableList(Arrays.asList( + FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT, + FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID, + FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET)); + @Test public void testStringPrecedence() throws IllegalAccessException, IOException, InvalidConfigurationValueException { @@ -361,6 +373,33 @@ public void testAccessTokenProviderPrecedence() testGlobalAndAccountOAuthPrecedence(abfsConf, null, AuthType.OAuth); } + @Test + public void testConfigPropNotFound() throws Throwable { + final String accountName = "account"; + + final Configuration conf = new Configuration(); + final AbfsConfiguration abfsConf = new AbfsConfiguration(conf, accountName); + + for (String key : CONFIG_KEYS) { + setAuthConfig(abfsConf, true, AuthType.OAuth); + abfsConf.unset(key + "." + accountName); + testMissingConfigKey(abfsConf, key); + } + + unsetAuthConfig(abfsConf, false); + unsetAuthConfig(abfsConf, true); + } + + private static void testMissingConfigKey(final AbfsConfiguration abfsConf, + final String confKey) throws Throwable { + GenericTestUtils.assertExceptionContains("Configuration property " + + confKey + " not found.", + LambdaTestUtils.verifyCause( + ConfigurationPropertyNotFoundException.class, + LambdaTestUtils.intercept(TokenAccessProviderException.class, + () -> abfsConf.getTokenProvider().getClass().getTypeName()))); + } + public void testGlobalAndAccountOAuthPrecedence(AbfsConfiguration abfsConf, AuthType globalAuthType, AuthType accountSpecificAuthType)