[ https://issues.apache.org/jira/browse/HADOOP-19137?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17848977#comment-17848977 ]
ASF GitHub Bot commented on HADOOP-19137: ----------------------------------------- steveloughran commented on code in PR #6752: URL: https://github.com/apache/hadoop/pull/6752#discussion_r1611784761 ########## hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java: ########## @@ -390,9 +386,34 @@ private AzureBlobFileSystem getCPKEnabledFS() throws IOException { conf.set(FS_AZURE_ENCRYPTION_ENCODED_CLIENT_PROVIDED_KEY_SHA + "." + getAccountName(), cpkEncodedSHA); conf.unset(FS_AZURE_ENCRYPTION_CONTEXT_PROVIDER_TYPE); - AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(conf); - fileSystemsOpenedInTest.add(fs); - return fs; + return getAzureBlobFileSystem(conf); + } + + private AzureBlobFileSystem getAzureBlobFileSystem(final Configuration conf) { + try { + AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance( + conf); + fileSystemsOpenedInTest.add(fs); + Assertions.assertThat( + getConfiguration().getBoolean(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, + false)) + .describedAs("Encryption tests should run only on namespace enabled account") + .isTrue(); + return fs; + } catch (IOException ex) { + Assertions.assertThat(ex.getMessage()) Review Comment: use GenericTestUtils.assertExceptionContains ########## hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java: ########## @@ -184,8 +182,8 @@ public void testCustomEncryptionCombinations() throws Exception { AzureBlobFileSystem fs = getOrCreateFS(); Path testPath = path("/testFile"); String relativePath = fs.getAbfsStore().getRelativePath(testPath); - MockEncryptionContextProvider ecp = - (MockEncryptionContextProvider) createEncryptedFile(testPath); + MockEncryptionContextProvider ecp Review Comment: revert ########## hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java: ########## @@ -165,5 +165,8 @@ public static ApiVersion getCurrentVersion() { */ public static final Integer HTTP_STATUS_CATEGORY_QUOTIENT = 100; + public static final String CPK_IN_NON_HNS_ACCOUNT_ERROR_MESSAGE = + "Non HNS account can not have CPK configs enabled."; Review Comment: 1. please use Client Provided Keys, maybe Non Hierarchical Storage too. We cannot assume users know these acronyms. 2. please add a javadoc with an {@value} clause for IDE assistance ########## hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java: ########## @@ -390,9 +386,34 @@ private AzureBlobFileSystem getCPKEnabledFS() throws IOException { conf.set(FS_AZURE_ENCRYPTION_ENCODED_CLIENT_PROVIDED_KEY_SHA + "." + getAccountName(), cpkEncodedSHA); conf.unset(FS_AZURE_ENCRYPTION_CONTEXT_PROVIDER_TYPE); - AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(conf); - fileSystemsOpenedInTest.add(fs); - return fs; + return getAzureBlobFileSystem(conf); + } + + private AzureBlobFileSystem getAzureBlobFileSystem(final Configuration conf) { + try { + AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance( Review Comment: this needs to be closed on any failure > [ABFS]:Extra getAcl call while calling the very first API of FileSystem > ----------------------------------------------------------------------- > > Key: HADOOP-19137 > URL: https://issues.apache.org/jira/browse/HADOOP-19137 > Project: Hadoop Common > Issue Type: Sub-task > Components: fs/azure > Affects Versions: 3.4.0 > Reporter: Pranav Saxena > Assignee: Pranav Saxena > Priority: Major > Labels: pull-request-available > > Store doesn't flow in the namespace information to the client. > In https://github.com/apache/hadoop/pull/6221, getIsNamespaceEnabled is added > in client methods which checks if namespace information is there or not, and > if not there, it will make getAcl call and set the field. Once the field is > set, it would be used in future getIsNamespaceEnabled method calls for a > given AbfsClient. > Since, CPK both global and encryptionContext are only for hns account, the > fix that is proposed is that we would fail fs init if its non-hns account and > cpk config is given. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org