From 626f9c6c86743c345519cf30bc63641e53c9120d Mon Sep 17 00:00:00 2001 From: Bilahari T H Date: Tue, 22 Sep 2020 20:44:57 +0530 Subject: [PATCH 1/2] ABFS: Enabling checkaccess --- .../constants/FileSystemConfigurations.java | 2 +- .../src/site/markdown/testing_azure.md | 57 +++++++++ .../ITestAzureBlobFileSystemCheckAccess.java | 120 ++++++++++++------ 3 files changed, 137 insertions(+), 42 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java index 2b50a5ddfd40d..fa0ee6a89212d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java @@ -85,7 +85,7 @@ public final class FileSystemConfigurations { public static final boolean DEFAULT_ENABLE_HTTPS = true; public static final boolean DEFAULT_USE_UPN = false; - public static final boolean DEFAULT_ENABLE_CHECK_ACCESS = false; + public static final boolean DEFAULT_ENABLE_CHECK_ACCESS = true; public static final boolean DEFAULT_ABFS_LATENCY_TRACK = false; public static final long DEFAULT_SAS_TOKEN_RENEW_PERIOD_FOR_STREAMS_IN_SECONDS = 120; diff --git a/hadoop-tools/hadoop-azure/src/site/markdown/testing_azure.md b/hadoop-tools/hadoop-azure/src/site/markdown/testing_azure.md index 87cbb97aa044f..66b1ce593bbe9 100644 --- a/hadoop-tools/hadoop-azure/src/site/markdown/testing_azure.md +++ b/hadoop-tools/hadoop-azure/src/site/markdown/testing_azure.md @@ -868,6 +868,63 @@ hierarchical namespace enabled and set the following configuration settings: ``` +To run CheckAccess test cases you must register an app with no RBAC and set +the following configurations. +```xml + + + + + fs.azure.enable.check.access + true + By default the check access will be on. Checkaccess can + be turned off by changing this flag to false. + + + fs.azure.account.test.oauth2.client.id + {client id} + The client id(app id) for the app created on step 1 + + + + fs.azure.account.test.oauth2.client.secret + {client secret} + +The client secret(application's secret) for the app created on step 1 + + + + fs.azure.check.access.testuser.guid + {guid} + The guid fetched on step 2 + + + fs.azure.account.oauth2.client.endpoint.{account name}.dfs.core +.windows.net + https://login.microsoftonline.com/{TENANTID}/oauth2/token + +Token end point. This can be found through Azure portal. As part of CheckAccess +test cases. The access will be tested for an FS instance created with the +above mentioned client credentials. So this configuration is necessary to +create the test FS instance. + + + +``` + If running tests against an endpoint that uses the URL format http[s]://[ip]:[port]/[account]/[filesystem] instead of http[s]://[account][domain-suffix]/[filesystem], please use the following: diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java index 4189d666e7a70..367ac49684746 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java @@ -17,16 +17,19 @@ */ package org.apache.hadoop.fs.azurebfs; -import com.google.common.collect.Lists; - import java.io.FileNotFoundException; import java.io.IOException; +import java.lang.reflect.Field; import java.util.List; +import com.google.common.collect.Lists; import org.junit.Assume; import org.junit.Test; +import org.mockito.Mockito; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider; +import org.apache.hadoop.fs.azurebfs.services.AuthType; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.azurebfs.utils.AclTestHelpers; @@ -37,6 +40,9 @@ import org.apache.hadoop.security.AccessControlException; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION; +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; +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_ENABLE_CHECK_ACCESS; import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID; import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET; @@ -44,9 +50,15 @@ import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CLIENT_ID; import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CLIENT_SECRET; import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT; +import static org.apache.hadoop.test.LambdaTestUtils.intercept; /** * Test cases for AzureBlobFileSystem.access() + * + * Some of the tests in this class require additional configs set in the test + * config file. + * Refer testing_azure.md for how to set the configs. + * */ public class ITestAzureBlobFileSystemCheckAccess extends AbstractAbfsIntegrationTest { @@ -72,25 +84,27 @@ private void setTestUserFs() throws Exception { if (this.testUserFs != null) { return; } - String orgClientId = getConfiguration().get(FS_AZURE_BLOB_FS_CLIENT_ID); - String orgClientSecret = getConfiguration() - .get(FS_AZURE_BLOB_FS_CLIENT_SECRET); - Boolean orgCreateFileSystemDurungInit = getConfiguration() - .getBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, true); - getRawConfiguration().set(FS_AZURE_BLOB_FS_CLIENT_ID, - getConfiguration().get(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID)); - getRawConfiguration().set(FS_AZURE_BLOB_FS_CLIENT_SECRET, getConfiguration() - .get(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET)); - getRawConfiguration() - .setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, - false); - FileSystem fs = FileSystem.newInstance(getRawConfiguration()); - getRawConfiguration().set(FS_AZURE_BLOB_FS_CLIENT_ID, orgClientId); - getRawConfiguration().set(FS_AZURE_BLOB_FS_CLIENT_SECRET, orgClientSecret); - getRawConfiguration() - .setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, - orgCreateFileSystemDurungInit); - this.testUserFs = fs; + checkIfConfigIsSet(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT + + "." + getAccountName()); + Configuration conf = getRawConfiguration(); + setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_ID, + FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID); + setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_SECRET, + FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET); + conf.set(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name()); + conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME + "." + + getAccountName(), ClientCredsTokenProvider.class.getName()); + conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, + false); + this.testUserFs = FileSystem.newInstance(getRawConfiguration()); + } + + private void setTestFsConf(final String fsConfKey, + final String testFsConfKey) { + final String confKeyWithAccountName = fsConfKey + "." + getAccountName(); + final String confValue = getConfiguration() + .getString(testFsConfKey, ""); + getRawConfiguration().set(confKeyWithAccountName, confValue); } @Test(expected = IllegalArgumentException.class) @@ -100,15 +114,15 @@ public void testCheckAccessWithNullPath() throws IOException { @Test(expected = NullPointerException.class) public void testCheckAccessForFileWithNullFsAction() throws Exception { - assumeHNSAndCheckAccessEnabled(); + Assume.assumeTrue(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT + " is false", + isHNSEnabled); // NPE when trying to convert null FsAction enum superUserFs.access(new Path("test.txt"), null); } @Test(expected = FileNotFoundException.class) public void testCheckAccessForNonExistentFile() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path nonExistentFile = setupTestDirectoryAndUserAccess( "/nonExistentFile1.txt", FsAction.ALL); superUserFs.delete(nonExistentFile, true); @@ -153,15 +167,38 @@ public void testCheckAccessForAccountWithoutNS() throws Exception { getConfiguration() .getBoolean(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, true)); Assume.assumeTrue(FS_AZURE_ENABLE_CHECK_ACCESS + " is false", - isCheckAccessEnabled); + isCheckAccessEnabled); + checkIfConfigIsSet(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID); + checkIfConfigIsSet(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET); + checkIfConfigIsSet(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID); + setTestUserFs(); + + // When the driver does not know if the account is HNS enabled or not it + // makes a server call and fails + intercept(AccessControlException.class, + "\"This request is not authorized to perform this operation using " + + "this permission.\", 403", + () -> testUserFs.access(new Path("/"), FsAction.READ)); + + // When the driver has already determined if the account is HNS enabled + // or not, and as the account is non HNS the AzureBlobFileSystem#access + // acts as noop + AzureBlobFileSystemStore mockAbfsStore = + Mockito.mock(AzureBlobFileSystemStore.class); + Mockito.when(mockAbfsStore.getIsNamespaceEnabled()).thenReturn(true); + Field abfsStoreField = AzureBlobFileSystem.class.getDeclaredField( + "abfsStore"); + abfsStoreField.setAccessible(true); + abfsStoreField.set(testUserFs, mockAbfsStore); testUserFs.access(new Path("/"), FsAction.READ); + + superUserFs.access(new Path("/"), FsAction.READ); } @Test public void testFsActionNONE() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path testFilePath = setupTestDirectoryAndUserAccess("/test2.txt", FsAction.NONE); assertInaccessible(testFilePath, FsAction.EXECUTE); @@ -175,8 +212,7 @@ public void testFsActionNONE() throws Exception { @Test public void testFsActionEXECUTE() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path testFilePath = setupTestDirectoryAndUserAccess("/test3.txt", FsAction.EXECUTE); assertAccessible(testFilePath, FsAction.EXECUTE); @@ -191,8 +227,7 @@ public void testFsActionEXECUTE() throws Exception { @Test public void testFsActionREAD() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path testFilePath = setupTestDirectoryAndUserAccess("/test4.txt", FsAction.READ); assertAccessible(testFilePath, FsAction.READ); @@ -207,8 +242,7 @@ public void testFsActionREAD() throws Exception { @Test public void testFsActionWRITE() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path testFilePath = setupTestDirectoryAndUserAccess("/test5.txt", FsAction.WRITE); assertAccessible(testFilePath, FsAction.WRITE); @@ -223,8 +257,7 @@ public void testFsActionWRITE() throws Exception { @Test public void testFsActionREADEXECUTE() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path testFilePath = setupTestDirectoryAndUserAccess("/test6.txt", FsAction.READ_EXECUTE); assertAccessible(testFilePath, FsAction.EXECUTE); @@ -239,8 +272,7 @@ public void testFsActionREADEXECUTE() throws Exception { @Test public void testFsActionWRITEEXECUTE() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path testFilePath = setupTestDirectoryAndUserAccess("/test7.txt", FsAction.WRITE_EXECUTE); assertAccessible(testFilePath, FsAction.EXECUTE); @@ -255,8 +287,7 @@ public void testFsActionWRITEEXECUTE() throws Exception { @Test public void testFsActionALL() throws Exception { - assumeHNSAndCheckAccessEnabled(); - setTestUserFs(); + checkPrerequisites(); Path testFilePath = setupTestDirectoryAndUserAccess("/test8.txt", FsAction.ALL); assertAccessible(testFilePath, FsAction.EXECUTE); @@ -268,13 +299,20 @@ public void testFsActionALL() throws Exception { assertAccessible(testFilePath, FsAction.ALL); } - private void assumeHNSAndCheckAccessEnabled() { + private void checkPrerequisites() throws Exception { + setTestUserFs(); Assume.assumeTrue(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT + " is false", isHNSEnabled); Assume.assumeTrue(FS_AZURE_ENABLE_CHECK_ACCESS + " is false", isCheckAccessEnabled); + checkIfConfigIsSet(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID); + checkIfConfigIsSet(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET); + checkIfConfigIsSet(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID); + } - Assume.assumeNotNull(getRawConfiguration().get(FS_AZURE_BLOB_FS_CLIENT_ID)); + private void checkIfConfigIsSet(String configKey){ + AbfsConfiguration conf = getConfiguration(); + Assume.assumeNotNull(configKey + " config missing", conf.get(configKey)); } private void assertAccessible(Path testFilePath, FsAction fsAction) From ee68171f51f38ddf10349aba3e76def38f1e3fee Mon Sep 17 00:00:00 2001 From: Bilahari T H Date: Mon, 28 Sep 2020 01:56:07 +0530 Subject: [PATCH 2/2] Making the checkaccess test configs mandatory --- .../fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java index 367ac49684746..67b201c651480 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java @@ -22,6 +22,7 @@ import java.lang.reflect.Field; import java.util.List; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import org.junit.Assume; import org.junit.Test; @@ -116,6 +117,8 @@ public void testCheckAccessWithNullPath() throws IOException { public void testCheckAccessForFileWithNullFsAction() throws Exception { Assume.assumeTrue(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT + " is false", isHNSEnabled); + Assume.assumeTrue(FS_AZURE_ENABLE_CHECK_ACCESS + " is false", + isCheckAccessEnabled); // NPE when trying to convert null FsAction enum superUserFs.access(new Path("test.txt"), null); } @@ -312,7 +315,9 @@ private void checkPrerequisites() throws Exception { private void checkIfConfigIsSet(String configKey){ AbfsConfiguration conf = getConfiguration(); - Assume.assumeNotNull(configKey + " config missing", conf.get(configKey)); + String value = conf.get(configKey); + Preconditions.checkArgument((value != null && value.trim().length() > 1), + configKey + " config is mandatory for the test to run"); } private void assertAccessible(Path testFilePath, FsAction fsAction)