From 0d88ed279425da14c8876be9d7c0a1181fb0d5c7 Mon Sep 17 00:00:00 2001 From: Mehakmeet Singh Date: Thu, 16 Jul 2020 21:59:37 +0530 Subject: [PATCH] HADOOP-17129. Validating storage keys in ABFS correctly (#2141) Contributed by Mehakmeet Singh Change-Id: I8016ee2f9ffbc86ea867f4a3d960b134e507d099 --- .../hadoop/fs/azurebfs/AbfsConfiguration.java | 14 ------------- .../azurebfs/services/SimpleKeyProvider.java | 20 ++++++++++++++++++- ...TestAbfsConfigurationFieldsValidation.java | 4 ++-- 3 files changed, 21 insertions(+), 17 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 354176f5ff9..7996ec10988 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 @@ -20,7 +20,6 @@ package org.apache.hadoop.fs.azurebfs; import java.io.IOException; import java.lang.reflect.Field; -import java.util.Map; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -215,8 +214,6 @@ public class AbfsConfiguration{ DefaultValue = DEFAULT_SAS_TOKEN_RENEW_PERIOD_FOR_STREAMS_IN_SECONDS) private long sasTokenRenewPeriodForStreamsInSeconds; - private Map storageAccountKeys; - public AbfsConfiguration(final Configuration rawConfig, String accountName) throws IllegalAccessException, InvalidConfigurationValueException, IOException { this.rawConfig = ProviderUtils.excludeIncompatibleCredentialProviders( @@ -224,7 +221,6 @@ public class AbfsConfiguration{ this.accountName = accountName; this.isSecure = getBoolean(FS_AZURE_SECURE_MODE, false); - validateStorageAccountKeys(); Field[] fields = this.getClass().getDeclaredFields(); for (Field field : fields) { field.setAccessible(true); @@ -665,16 +661,6 @@ public class AbfsConfiguration{ } } - void validateStorageAccountKeys() throws InvalidConfigurationValueException { - Base64StringConfigurationBasicValidator validator = new Base64StringConfigurationBasicValidator( - FS_AZURE_ACCOUNT_KEY_PROPERTY_NAME, "", true); - this.storageAccountKeys = rawConfig.getValByRegex(FS_AZURE_ACCOUNT_KEY_PROPERTY_NAME_REGX); - - for (Map.Entry account : storageAccountKeys.entrySet()) { - validator.validate(account.getValue()); - } - } - int validateInt(Field field) throws IllegalAccessException, InvalidConfigurationValueException { IntegerConfigurationValidatorAnnotation validator = field.getAnnotation(IntegerConfigurationValidatorAnnotation.class); String value = get(validator.ConfigurationKey()); diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/SimpleKeyProvider.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/SimpleKeyProvider.java index 727e1b3fd3f..bb1ec9e4a3f 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/SimpleKeyProvider.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/SimpleKeyProvider.java @@ -25,6 +25,7 @@ import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.KeyProviderException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException; +import org.apache.hadoop.fs.azurebfs.diagnostics.Base64StringConfigurationBasicValidator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,7 +44,10 @@ public class SimpleKeyProvider implements KeyProvider { try { AbfsConfiguration abfsConfig = new AbfsConfiguration(rawConfig, accountName); key = abfsConfig.getPasswordString(ConfigurationKeys.FS_AZURE_ACCOUNT_KEY_PROPERTY_NAME); - } catch(IllegalAccessException | InvalidConfigurationValueException e) { + + // Validating the key. + validateStorageAccountKey(key); + } catch (IllegalAccessException | InvalidConfigurationValueException e) { throw new KeyProviderException("Failure to initialize configuration", e); } catch(IOException ioe) { LOG.warn("Unable to get key from credential providers. {}", ioe); @@ -51,4 +55,18 @@ public class SimpleKeyProvider implements KeyProvider { return key; } + + /** + * A method to validate the storage key. + * + * @param key the key to be validated. + * @throws InvalidConfigurationValueException + */ + private void validateStorageAccountKey(String key) + throws InvalidConfigurationValueException { + Base64StringConfigurationBasicValidator validator = new Base64StringConfigurationBasicValidator( + ConfigurationKeys.FS_AZURE_ACCOUNT_KEY_PROPERTY_NAME, "", true); + + validator.validate(key); + } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java index 0f550d825e1..898667fe6d2 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsConfigurationFieldsValidation.java @@ -30,7 +30,7 @@ import org.apache.hadoop.fs.azurebfs.contracts.annotations.ConfigurationValidati import org.apache.hadoop.fs.azurebfs.contracts.annotations.ConfigurationValidationAnnotations.StringConfigurationValidatorAnnotation; import org.apache.hadoop.fs.azurebfs.contracts.annotations.ConfigurationValidationAnnotations.LongConfigurationValidatorAnnotation; import org.apache.hadoop.fs.azurebfs.contracts.annotations.ConfigurationValidationAnnotations.Base64StringConfigurationValidatorAnnotation; -import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.KeyProviderException; import org.apache.hadoop.fs.azurebfs.utils.Base64; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_SSL_CHANNEL_MODE_KEY; @@ -155,7 +155,7 @@ public class TestAbfsConfigurationFieldsValidation { assertEquals(this.encodedAccountKey, accountKey); } - @Test(expected = ConfigurationPropertyNotFoundException.class) + @Test(expected = KeyProviderException.class) public void testGetAccountKeyWithNonExistingAccountName() throws Exception { Configuration configuration = new Configuration(); configuration.addResource(TestConfigurationKeys.TEST_CONFIGURATION_FILE_NAME);