From d3b5c4033e9beac0f905286f172afefc85c009bf Mon Sep 17 00:00:00 2001 From: Mehakmeet Singh Date: Wed, 15 Jul 2020 18:04:31 +0530 Subject: [PATCH 1/3] HADOOP-17129. Validating storage keys in ABFS correctly --- .../hadoop/fs/azurebfs/AbfsConfiguration.java | 14 ---------- .../azurebfs/services/SimpleKeyProvider.java | 27 +++++++++++++++++-- 2 files changed, 25 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 43021c0fa8b87..4cdb3c95eccfb 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 @@ 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; @@ -219,8 +218,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( @@ -228,7 +225,6 @@ public AbfsConfiguration(final Configuration rawConfig, String accountName) this.accountName = accountName; this.isSecure = getBoolean(FS_AZURE_SECURE_MODE, false); - validateStorageAccountKeys(); Field[] fields = this.getClass().getDeclaredFields(); for (Field field : fields) { field.setAccessible(true); @@ -740,16 +736,6 @@ public SASTokenProvider getSASTokenProvider() throws AzureBlobFileSystemExceptio } } - 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 727e1b3fd3fdd..a8a8874dea180 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 @@ -20,13 +20,15 @@ import java.io.IOException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.apache.hadoop.conf.Configuration; 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.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.apache.hadoop.fs.azurebfs.diagnostics.Base64StringConfigurationBasicValidator; /** * Key provider that simply returns the storage account key from the @@ -49,6 +51,27 @@ public String getStorageAccountKey(String accountName, Configuration rawConfig) LOG.warn("Unable to get key from credential providers. {}", ioe); } + // Validating the key. + try { + validateStorageAccountKey(key); + } catch (InvalidConfigurationValueException e) { + e.printStackTrace(); + } + 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); + } } From d21076af7eeafd397a9b228d2136549dae0f4c89 Mon Sep 17 00:00:00 2001 From: Mehakmeet Singh Date: Wed, 15 Jul 2020 21:13:42 +0530 Subject: [PATCH 2/3] HADOOP-17129. Fixing review comments --- .../fs/azurebfs/services/KeyProvider.java | 3 ++- .../services/ShellDecryptionKeyProvider.java | 3 ++- .../azurebfs/services/SimpleKeyProvider.java | 23 +++++++++---------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java index 09491c520bdcd..1ce3ea58f0f2c 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java @@ -19,6 +19,7 @@ package org.apache.hadoop.fs.azurebfs.services; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.KeyProviderException; /** @@ -39,5 +40,5 @@ public interface KeyProvider { * the storage account key. */ String getStorageAccountKey(String accountName, Configuration conf) - throws KeyProviderException; + throws KeyProviderException, ConfigurationPropertyNotFoundException; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java index bdac922fb3a79..208a87e55ffb1 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java @@ -24,6 +24,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.KeyProviderException; import org.apache.hadoop.util.Shell; import org.slf4j.Logger; @@ -38,7 +39,7 @@ public class ShellDecryptionKeyProvider extends SimpleKeyProvider { @Override public String getStorageAccountKey(String accountName, Configuration rawConfig) - throws KeyProviderException { + throws KeyProviderException, ConfigurationPropertyNotFoundException { String envelope = super.getStorageAccountKey(accountName, rawConfig); AbfsConfiguration abfsConfig; 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 a8a8874dea180..4c5fbd89b94a3 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 @@ -20,15 +20,15 @@ import java.io.IOException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; 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; /** * Key provider that simply returns the storage account key from the @@ -39,25 +39,24 @@ public class SimpleKeyProvider implements KeyProvider { @Override public String getStorageAccountKey(String accountName, Configuration rawConfig) - throws KeyProviderException { + throws KeyProviderException, ConfigurationPropertyNotFoundException { String key = null; 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) { + if (key == null) { + throw new ConfigurationPropertyNotFoundException(accountName); + } throw new KeyProviderException("Failure to initialize configuration", e); } catch(IOException ioe) { LOG.warn("Unable to get key from credential providers. {}", ioe); } - // Validating the key. - try { - validateStorageAccountKey(key); - } catch (InvalidConfigurationValueException e) { - e.printStackTrace(); - } - return key; } From c5ab869069958e6e7032f30e39b01d73a32b0cc4 Mon Sep 17 00:00:00 2001 From: Mehakmeet Singh Date: Wed, 15 Jul 2020 23:11:07 +0530 Subject: [PATCH 3/3] HADOOP-17129. Fixing test --- .../org/apache/hadoop/fs/azurebfs/services/KeyProvider.java | 3 +-- .../fs/azurebfs/services/ShellDecryptionKeyProvider.java | 3 +-- .../hadoop/fs/azurebfs/services/SimpleKeyProvider.java | 6 +----- .../fs/azurebfs/TestAbfsConfigurationFieldsValidation.java | 4 ++-- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java index 1ce3ea58f0f2c..09491c520bdcd 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/KeyProvider.java @@ -19,7 +19,6 @@ package org.apache.hadoop.fs.azurebfs.services; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.KeyProviderException; /** @@ -40,5 +39,5 @@ public interface KeyProvider { * the storage account key. */ String getStorageAccountKey(String accountName, Configuration conf) - throws KeyProviderException, ConfigurationPropertyNotFoundException; + throws KeyProviderException; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java index 208a87e55ffb1..bdac922fb3a79 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ShellDecryptionKeyProvider.java @@ -24,7 +24,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys; -import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.KeyProviderException; import org.apache.hadoop.util.Shell; import org.slf4j.Logger; @@ -39,7 +38,7 @@ public class ShellDecryptionKeyProvider extends SimpleKeyProvider { @Override public String getStorageAccountKey(String accountName, Configuration rawConfig) - throws KeyProviderException, ConfigurationPropertyNotFoundException { + throws KeyProviderException { String envelope = super.getStorageAccountKey(accountName, rawConfig); AbfsConfiguration abfsConfig; 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 4c5fbd89b94a3..bb1ec9e4a3fb5 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 @@ -23,7 +23,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys; -import org.apache.hadoop.fs.azurebfs.contracts.exceptions.ConfigurationPropertyNotFoundException; 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; @@ -39,7 +38,7 @@ public class SimpleKeyProvider implements KeyProvider { @Override public String getStorageAccountKey(String accountName, Configuration rawConfig) - throws KeyProviderException, ConfigurationPropertyNotFoundException { + throws KeyProviderException { String key = null; try { @@ -49,9 +48,6 @@ public String getStorageAccountKey(String accountName, Configuration rawConfig) // Validating the key. validateStorageAccountKey(key); } catch (IllegalAccessException | InvalidConfigurationValueException e) { - if (key == null) { - throw new ConfigurationPropertyNotFoundException(accountName); - } throw new KeyProviderException("Failure to initialize configuration", e); } catch(IOException ioe) { LOG.warn("Unable to get key from credential providers. {}", ioe); 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 d8711876fefc5..bda845bb45ad5 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.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 void testGetAccountKey() throws Exception { 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);