Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HADOOP-19471. ABFS: Support Fixed SAS Token at Container Level #7461

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public class AbfsConfiguration{

private final Configuration rawConfig;
private final String accountName;
private final String fsName;
// Service type identified from URL used to initialize FileSystem.
private final AbfsServiceType fsConfiguredServiceType;
private final boolean isSecure;
Expand Down Expand Up @@ -451,17 +452,20 @@ public class AbfsConfiguration{
* Constructor for AbfsConfiguration for specified service type.
* @param rawConfig used to initialize the configuration.
* @param accountName the name of the azure storage account.
* @param fsName the name of the file system (container name).
* @param fsConfiguredServiceType service type configured for the file system.
* @throws IllegalAccessException if the field is not accessible.
* @throws IOException if an I/O error occurs.
*/
public AbfsConfiguration(final Configuration rawConfig,
String accountName,
String fsName,
AbfsServiceType fsConfiguredServiceType)
throws IllegalAccessException, IOException {
this.rawConfig = ProviderUtils.excludeIncompatibleCredentialProviders(
rawConfig, AzureBlobFileSystem.class);
this.accountName = accountName;
this.fsName = fsName;
this.fsConfiguredServiceType = fsConfiguredServiceType;
this.isSecure = getBoolean(FS_AZURE_SECURE_MODE, false);

Expand Down Expand Up @@ -493,7 +497,7 @@ public AbfsConfiguration(final Configuration rawConfig,
*/
public AbfsConfiguration(final Configuration rawConfig, String accountName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we should add a separate constructor with this new param

throws IllegalAccessException, IOException {
this(rawConfig, accountName, AbfsServiceType.DFS);
this(rawConfig, accountName, EMPTY_STRING, AbfsServiceType.DFS);
}

/**
Expand Down Expand Up @@ -589,6 +593,16 @@ public String accountConf(String key) {
return key + "." + accountName;
}

/**
* Appends the container, account name to a configuration key yielding the
* container-specific form.
* @param key Account-agnostic configuration key
* @return Container-specific configuration key
*/
public String containerConf(String key) {
return key + "." + fsName + "." + accountName;
}

/**
* Returns the account-specific value if it exists, then looks for an
* account-agnostic value.
Expand Down Expand Up @@ -644,16 +658,20 @@ public int getInt(String key, int defaultValue) {
}

/**
* Returns the account-specific password in string form if it exists, then
* Returns the container-specific password if it exists,
* else searches for the account-specific password, else finally
* looks for an account-agnostic value.
* @param key Account-agnostic configuration key
* @return value in String form if one exists, else null
* @throws IOException if parsing fails.
*/
public String getPasswordString(String key) throws IOException {
char[] passchars = rawConfig.getPassword(accountConf(key));
if (passchars == null) {
passchars = rawConfig.getPassword(key);
char[] passchars = rawConfig.getPassword(containerConf(key));
if(passchars == null) {
passchars = rawConfig.getPassword(accountConf(key));
if(passchars == null){
passchars = rawConfig.getPassword(key);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be simplified as :- char[] passchars = rawConfig.getPassword(containerConf(key)) != null ?
rawConfig.getPassword(containerConf(key)) :
rawConfig.getPassword(accountConf(key)) != null ?
rawConfig.getPassword(accountConf(key)) :
rawConfig.getPassword(key);

}
if (passchars != null) {
return new String(passchars);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public AzureBlobFileSystemStore(

try {
this.abfsConfiguration = new AbfsConfiguration(abfsStoreBuilder.configuration,
accountName, getAbfsServiceTypeFromUrl());
accountName, fileSystemName, getAbfsServiceTypeFromUrl());
} catch (IllegalAccessException exception) {
throw new FileSystemOperationUnhandledException(exception);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ public static String accountProperty(String property, String account) {
return property + "." + account;
}

public static String containerProperty(String property, String fsName, String account) {
return property + "." + fsName + "." + account;
}

public static final String FS_AZURE_ENABLE_DELEGATION_TOKEN = "fs.azure.enable.delegation.token";
public static final String FS_AZURE_DELEGATION_TOKEN_PROVIDER_TYPE = "fs.azure.delegation.token.provider.type";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ protected AbstractAbfsIntegrationTest() throws Exception {
final String abfsUrl = this.getFileSystemName() + "@" + this.getAccountName();
URI defaultUri = null;

abfsConfig = new AbfsConfiguration(rawConfig, accountName, identifyAbfsServiceTypeFromUrl(abfsUrl));
abfsConfig = new AbfsConfiguration(rawConfig, accountName, getFileSystemName(), identifyAbfsServiceTypeFromUrl(abfsUrl));

authType = abfsConfig.getEnum(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.SharedKey);
assumeValidAuthConfigsPresent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@
import org.apache.hadoop.fs.azurebfs.services.AuthType;
import org.apache.hadoop.fs.azurebfs.services.FixedSASTokenProvider;
import org.apache.hadoop.fs.azurebfs.utils.AccountSASGenerator;
import org.apache.hadoop.fs.azurebfs.utils.ServiceSASGenerator;
import org.apache.hadoop.fs.azurebfs.utils.Base64;

import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_SAS_FIXED_TOKEN;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.accountProperty;
import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.containerProperty;
import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_TEST_APP_ID;
import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_TEST_APP_SECRET;
import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_TEST_APP_SERVICE_PRINCIPAL_OBJECT_ID;
Expand All @@ -50,6 +52,7 @@
public class ITestAzureBlobFileSystemChooseSAS extends AbstractAbfsIntegrationTest{

private String accountSAS = null;
private String containerSAS = null;
private static final String TEST_PATH = "testPath";

/**
Expand All @@ -69,6 +72,7 @@ public void setup() throws Exception {
super.setup();
createFilesystemWithTestFileForSASTests(new Path(TEST_PATH));
generateAccountSAS();
generateContainerSAS();
}

/**
Expand All @@ -85,6 +89,22 @@ private void generateAccountSAS() throws AzureBlobFileSystemException {
accountSAS = configAccountSASGenerator.getAccountSAS(getAccountName());
}

/**
* Generates a Container SAS Token using the Account Shared Key to be used as a fixed SAS Token.
* Container SAS used here will have only read permissions to resources.
* This will be used by individual tests to set in the configurations.
* @throws AzureBlobFileSystemException
*/
private void generateContainerSAS() throws AzureBlobFileSystemException {
final byte[] accountKey = Base64.decode(
getConfiguration().getStorageAccountKey());
ServiceSASGenerator configServiceSASGenerator = new ServiceSASGenerator(
accountKey);
// Setting only read permissions.
configServiceSASGenerator.setPermissions("r");
containerSAS = configServiceSASGenerator.getContainerSASWithFullControl(
getAccountName(), getFileSystemName());
}
/**
* Tests the scenario where both the custom SASTokenProvider and a fixed SAS token are configured.
* Custom implementation of SASTokenProvider class should be chosen and User Delegation SAS should be used.
Expand Down Expand Up @@ -126,6 +146,42 @@ public void testBothProviderFixedTokenConfigured() throws Exception {
}
}

/**
* Helper method to get the Fixed SAS token value
*/
private String getFixedSASToken(AbfsConfiguration config) throws Exception {
return config.getSASTokenProvider().getSASToken(this.getAccountName(), this.getFileSystemName(), getMethodName(), "read");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use constants for string

}

/**
* Tests the implementation sequence if all fixed SAS configs are set.
* The expected sequence is Container Specific Fixed SAS, Account Specific Fixed SAS, Account Agnostic Fixed SAS.
* @throws IOException
*/
@Test
public void testFixedTokenPreference() throws Exception {
AbfsConfiguration testAbfsConfig = new AbfsConfiguration(
getRawConfiguration(), this.getAccountName(), this.getFileSystemName(), getAbfsServiceType());

// setting all types of Fixed SAS configs (container-specific, account-specific, account-agnostic)
removeAnyPresetConfiguration(testAbfsConfig);
testAbfsConfig.set(containerProperty(FS_AZURE_SAS_FIXED_TOKEN, this.getFileSystemName(), this.getAccountName()), containerSAS);
testAbfsConfig.set(accountProperty(FS_AZURE_SAS_FIXED_TOKEN, this.getAccountName()), accountSAS);
testAbfsConfig.set(FS_AZURE_SAS_FIXED_TOKEN, accountSAS);

// Assert that Container Specific Fixed SAS is used
Assertions.assertThat(getFixedSASToken(testAbfsConfig)).contains("sr=c");

// Assert that Account Specific Fixed SAS is used if container SAS isn't set
testAbfsConfig.unset(containerProperty(FS_AZURE_SAS_FIXED_TOKEN, this.getFileSystemName(), this.getAccountName()));
Assertions.assertThat(getFixedSASToken(testAbfsConfig)).contains("ss=bf");

//Assert that Account-Agnostic fixed SAS is used if no other fixed SAS configs are set.
// The token is the same as the Account Specific Fixed SAS.
testAbfsConfig.unset(accountProperty(FS_AZURE_SAS_FIXED_TOKEN, this.getAccountName()));
Assertions.assertThat(getFixedSASToken(testAbfsConfig)).contains("ss=bf");
}

/**
* Tests the scenario where only the fixed token is configured, and no token provider class is set.
* Account SAS Token configured as fixed SAS should be used.
Expand Down Expand Up @@ -189,5 +245,6 @@ private void removeAnyPresetConfiguration(AbfsConfiguration testAbfsConfig) {
testAbfsConfig.unset(FS_AZURE_SAS_FIXED_TOKEN);
testAbfsConfig.unset(accountProperty(FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, this.getAccountName()));
testAbfsConfig.unset(accountProperty(FS_AZURE_SAS_FIXED_TOKEN, this.getAccountName()));
testAbfsConfig.unset(containerProperty(FS_AZURE_SAS_FIXED_TOKEN, this.getFileSystemName(), this.getAccountName()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ public ServiceSASGenerator(byte[] accountKey) {
super(accountKey);
}

private String permissions = "racwdl";
public String getContainerSASWithFullControl(String accountName, String containerName) throws
InvalidConfigurationValueException {
accountName = getCanonicalAccountName(accountName);
String sp = "rcwdl";
String sp = permissions;
String sv = AuthenticationVersion.Feb20.toString();
String sr = "c";
String st = ISO_8601_FORMATTER.format(Instant.now().minus(FIVE_MINUTES));
Expand Down Expand Up @@ -96,4 +97,13 @@ private String computeSignatureForSAS(String sp, String st, String se, String sv
LOG.debug("Service SAS stringToSign: " + stringToSign.replace("\n", "."));
return computeHmac256(stringToSign);
}

/**
* By default, Container SAS has all the available permissions. Use this to
* override the default permissions and set as per the requirements.
* @param permissions
*/
public void setPermissions(final String permissions) {
this.permissions = permissions;
}
}