HADOOP-19595. ABFS: AbfsConfiguration should store account type information (HNS or FNS)

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java

@@ -1528,8 +1528,23 @@ void setMaxBackoffIntervalMilliseconds(int maxBackoffInterval) {
     this.maxBackoffInterval = maxBackoffInterval;
   }
 
+  /**
+   * Sets the namespace enabled account flag.
+   *
+   * @param isNamespaceEnabledAccount boolean value indicating if the account is namespace enabled.
+   */
+  void setIsNamespaceEnabledAccount(boolean isNamespaceEnabledAccount) {
+    this.isNamespaceEnabled = Trilean.getTrilean(isNamespaceEnabledAccount);
+  }
+
+  /**
+   * Sets the namespace enabled account flag for testing purposes.
+   * Use this method only for testing scenarios.
+   *
+   * @param isNamespaceEnabledAccount Trilean value indicating if the account is namespace enabled.
+   */
   @VisibleForTesting
-  public void setIsNamespaceEnabledAccount(Trilean isNamespaceEnabledAccount) {
+  void setIsNamespaceEnabledAccountForTesting(Trilean isNamespaceEnabledAccount) {
     this.isNamespaceEnabled = isNamespaceEnabledAccount;
   }
 

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java

@@ -369,12 +369,12 @@ private String[] authorityParts(URI uri) throws InvalidUriAuthorityException, In
   }
 
   /**
-   * Resolves namespace information of the filesystem from the state of {@link #isNamespaceEnabled}.
+   * Resolves namespace information of the filesystem from the state of {@link #isNamespaceEnabled()}.
    * if the state is UNKNOWN, it will be determined by making a GET_ACL request
    * to the root of the filesystem. GET_ACL call is synchronized to ensure a single
    * call is made to determine the namespace information in case multiple threads are
    * calling this method at the same time. The resolution of namespace information
-   * would be stored back as state of {@link #isNamespaceEnabled}.
+   * would be stored back as {@link #setNamespaceEnabled(boolean)}.
    *
    * @param tracingContext tracing context
    * @return true if namespace is enabled, false otherwise.
@@ -392,30 +392,40 @@ public boolean getIsNamespaceEnabled(TracingContext tracingContext)
     return getNamespaceEnabledInformationFromServer(tracingContext);
   }
 
+  /**
+   * In case the namespace configuration is not set or invalid, this method will
+   * make a call to the server to determine if namespace is enabled or not.
+   * This method is synchronized to ensure that only one thread
+   * is making the call to the server to determine the namespace
+   *
+   * @param tracingContext tracing context
+   * @return true if namespace is enabled, false otherwise.
+   * @throws AzureBlobFileSystemException server errors.
+   */
   private synchronized boolean getNamespaceEnabledInformationFromServer(
       final TracingContext tracingContext) throws AzureBlobFileSystemException {
-    if (abfsConfiguration.getIsNamespaceEnabledAccount() != Trilean.UNKNOWN) {
+    if (getAbfsConfiguration().getIsNamespaceEnabledAccount() != Trilean.UNKNOWN) {
       return isNamespaceEnabled();
     }
     try {
       LOG.debug("Get root ACL status");
       getClient(AbfsServiceType.DFS).getAclStatus(AbfsHttpConstants.ROOT_PATH, tracingContext);
       // If getAcl succeeds, namespace is enabled.
-      setNamespaceEnabled(Trilean.TRUE);
+      setNamespaceEnabled(true);
     } catch (AbfsRestOperationException ex) {
       // Get ACL status is a HEAD request, its response doesn't contain errorCode
       // So can only rely on its status code to determine account type.
       if (HttpURLConnection.HTTP_BAD_REQUEST != ex.getStatusCode()) {
         // If getAcl fails with anything other than 400, namespace is enabled.
-        setNamespaceEnabled(Trilean.TRUE);
+        setNamespaceEnabled(true);
         // Continue to throw exception as earlier.
         LOG.debug("Failed to get ACL status with non 400. Inferring namespace enabled", ex);
         throw ex;
       }
       // If getAcl fails with 400, namespace is disabled.
       LOG.debug("Failed to get ACL status with 400. "
           + "Inferring namespace disabled and ignoring error", ex);
-      setNamespaceEnabled(Trilean.FALSE);
+      setNamespaceEnabled(false);
     } catch (AzureBlobFileSystemException ex) {
       throw ex;
     }
@@ -428,7 +438,7 @@ private synchronized boolean getNamespaceEnabledInformationFromServer(
    */
   @VisibleForTesting
   boolean isNamespaceEnabled() throws TrileanConversionException {
-    return abfsConfiguration.getIsNamespaceEnabledAccount().toBoolean();
+    return getAbfsConfiguration().getIsNamespaceEnabledAccount().toBoolean();
   }
 
   @VisibleForTesting
@@ -2011,9 +2021,8 @@ DataBlocks.BlockFactory getBlockFactory() {
     return blockFactory;
   }
 
-  @VisibleForTesting
-  void setNamespaceEnabled(Trilean isNamespaceEnabled){
-    abfsConfiguration.setIsNamespaceEnabledAccount(isNamespaceEnabled);
+  void setNamespaceEnabled(boolean isNamespaceEnabled){
+    getAbfsConfiguration().setIsNamespaceEnabledAccount(isNamespaceEnabled);
   }
 
   @VisibleForTesting

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java

@@ -66,6 +66,7 @@
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidAbfsRestOperationException;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidFileSystemPropertyException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidUriException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.SASTokenProviderException;
@@ -442,7 +443,7 @@ protected List<AbfsHttpHeader> createCommonHeaders(ApiVersion xMsVersion) {
     requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, xMsVersion.toString()));
     requestHeaders.add(new AbfsHttpHeader(ACCEPT_CHARSET, UTF_8));
     requestHeaders.add(new AbfsHttpHeader(CONTENT_TYPE, EMPTY_STRING));
-    requestHeaders.add(new AbfsHttpHeader(USER_AGENT, userAgent));
+    requestHeaders.add(new AbfsHttpHeader(USER_AGENT, getUserAgent()));
     return requestHeaders;
   }
 
@@ -1322,8 +1323,9 @@ String initializeUserAgent(final AbfsConfiguration abfsConfiguration,
     sb.append(abfsConfiguration.getClusterType());
 
     // Add a unique identifier in FNS-Blob user agent string
-    if (!getIsNamespaceEnabled()
-        && abfsConfiguration.getFsConfiguredServiceType() == BLOB) {
+    // Current filesystem init restricts HNS-Blob combination
+    // so namespace check not required.
+    if (BLOB == abfsConfiguration.getFsConfiguredServiceType()) {
       sb.append(SEMICOLON)
           .append(SINGLE_WHITE_SPACE)
           .append(FNS_BLOB_USER_AGENT_IDENTIFIER);
@@ -1724,16 +1726,19 @@ protected String getUserAgent() {
 
   /**
    * Checks if the namespace is enabled.
+   * Filesystem init will fail if namespace is not correctly configured,
+   * so instead of swallowing the exception, we should throw the exception
+   * in case namespace is not configured correctly.
    *
    * @return True if the namespace is enabled, false otherwise.
-   * @throws AbfsDriverException if the conversion fails.
+   * @throws AzureBlobFileSystemException if the conversion fails.
    */
-  public boolean getIsNamespaceEnabled() throws AbfsDriverException {
+  public boolean getIsNamespaceEnabled() throws AzureBlobFileSystemException {
     try {
-      return abfsConfiguration.getIsNamespaceEnabledAccount().toBoolean();
+      return getAbfsConfiguration().getIsNamespaceEnabledAccount().toBoolean();
     } catch (TrileanConversionException ex) {
       LOG.error("Failed to convert namespace enabled account property to boolean", ex);
-      throw new AbfsDriverException("Failed to determine if namespace is enabled", ex);
+      throw new InvalidConfigurationValueException("Failed to determine account type", ex);
     }
   }
 

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemCheckAccess.java

@@ -102,8 +102,8 @@ private void setTestUserFs() throws Exception {
     conf.setBoolean(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isHNSEnabled);
     this.testUserFs = FileSystem.newInstance(conf);
     // Resetting the namespace enabled flag to unknown after file system init.
-    ((AzureBlobFileSystem) testUserFs).getAbfsStore().setNamespaceEnabled(
-        Trilean.UNKNOWN);
+    ((AzureBlobFileSystem) testUserFs).getAbfsStore()
+        .getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
   }
 
   private void setTestFsConf(final String fsConfKey,

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java

@@ -82,7 +82,7 @@ public void testGetAclCallOnHnsConfigAbsence() throws Exception {
     AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore());
     AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient(AbfsServiceType.DFS));
     Mockito.doReturn(client).when(store).getClient(AbfsServiceType.DFS);
-    store.setNamespaceEnabled(Trilean.UNKNOWN);
+    store.getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
 
     TracingContext tracingContext = getSampleTracingContext(fs, true);
     Mockito.doReturn(Mockito.mock(AbfsRestOperation.class))

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestGetNameSpaceEnabled.java

@@ -30,6 +30,7 @@
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.azurebfs.constants.AbfsServiceType;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException;
 import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
 import org.apache.hadoop.fs.azurebfs.services.AbfsDfsClient;
 import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
@@ -159,7 +160,8 @@ public void testFailedRequestWhenFSNotExist() throws Exception {
             + testUri.substring(testUri.indexOf("@"));
     config.setBoolean(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isUsingXNSAccount);
     AzureBlobFileSystem fs = this.getFileSystem(nonExistingFsUrl);
-    fs.getAbfsStore().setNamespaceEnabled(Trilean.UNKNOWN);
+    fs.getAbfsStore().getAbfsConfiguration()
+        .setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
 
     FileNotFoundException ex = intercept(FileNotFoundException.class, ()-> {
       fs.getFileStatus(new Path("/")); // Run a dummy FS call
@@ -207,7 +209,8 @@ public void testEnsureGetAclCallIsMadeOnceWhenConfigIsNotPresent()
   private void ensureGetAclCallIsMadeOnceForInvalidConf(String invalidConf)
       throws Exception {
     this.getFileSystem().getAbfsStore()
-        .setNamespaceEnabled(Trilean.getTrilean(invalidConf));
+        .getAbfsConfiguration()
+        .setIsNamespaceEnabledAccountForTesting(Trilean.getTrilean(invalidConf));
     AbfsClient mockClient =
         callAbfsGetIsNamespaceEnabledAndReturnMockAbfsClient();
     verify(mockClient, times(1))
@@ -217,15 +220,17 @@ private void ensureGetAclCallIsMadeOnceForInvalidConf(String invalidConf)
   private void ensureGetAclCallIsNeverMadeForValidConf(String validConf)
       throws Exception {
     this.getFileSystem().getAbfsStore()
-        .setNamespaceEnabled(Trilean.getTrilean(validConf));
+        .getAbfsConfiguration()
+        .setIsNamespaceEnabledAccountForTesting(Trilean.getTrilean(validConf));
     AbfsClient mockClient =
         callAbfsGetIsNamespaceEnabledAndReturnMockAbfsClient();
     verify(mockClient, never())
         .getAclStatus(anyString(), any(TracingContext.class));
   }
 
   private void unsetConfAndEnsureGetAclCallIsMadeOnce() throws IOException {
-    this.getFileSystem().getAbfsStore().setNamespaceEnabled(Trilean.UNKNOWN);
+    this.getFileSystem().getAbfsStore()
+        .getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
     AbfsClient mockClient =
         callAbfsGetIsNamespaceEnabledAndReturnMockAbfsClient();
     verify(mockClient, times(1))
@@ -262,7 +267,7 @@ private void ensureGetAclDetermineHnsStatusAccuratelyInternal(int statusCode,
       boolean expectedValue, boolean isExceptionExpected) throws Exception {
     AzureBlobFileSystemStore store = Mockito.spy(getFileSystem().getAbfsStore());
     AbfsClient mockClient = mock(AbfsClient.class);
-    store.setNamespaceEnabled(Trilean.UNKNOWN);
+    store.getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
     doReturn(mockClient).when(store).getClient(AbfsServiceType.DFS);
     AbfsRestOperationException ex = new AbfsRestOperationException(
         statusCode, null, Integer.toString(statusCode), null);
@@ -354,30 +359,123 @@ public void testAccountSpecificConfig() throws Exception {
    */
   @Test
   public void testNameSpaceConfig() throws Exception {
-    Configuration configuration = new Configuration();
-    configuration.unset(FS_AZURE_ACCOUNT_IS_HNS_ENABLED);
+    Configuration configuration = getConfigurationWithoutHnsConfig();
+    AzureBlobFileSystem abfs = (AzureBlobFileSystem) FileSystem.newInstance(configuration);
     AbfsConfiguration abfsConfig = new AbfsConfiguration(configuration, "bogusAccountName");
 
     // Test that the namespace value when config is not set
     Assertions.assertThat(abfsConfig.getIsNamespaceEnabledAccount())
         .describedAs("Namespace enabled should be unknown in case config is not set")
         .isEqualTo(Trilean.UNKNOWN);
 
+    // In case no namespace config is present, file system init calls getAcl() to determine account type.
+    Assertions.assertThat(abfs.getIsNamespaceEnabled(getTestTracingContext(abfs, false)))
+        .describedAs("getIsNamespaceEnabled should return account type based on getAcl() call")
+        .isEqualTo(abfs.getAbfsClient().getIsNamespaceEnabled());
+
+    // In case no namespace config is present, file system init calls getAcl() to determine account type.
+    Assertions.assertThat(abfs.getAbfsStore().getAbfsConfiguration().getIsNamespaceEnabledAccount())
+        .describedAs("getIsNamespaceEnabled() should return updated account type based on getAcl() call")
+        .isNotEqualTo(Trilean.UNKNOWN);
+
     configuration.set(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, TRUE_STR);
+    abfs = (AzureBlobFileSystem) FileSystem.newInstance(configuration);
     abfsConfig = new AbfsConfiguration(configuration, "bogusAccountName");
 
     // Test that the namespace enabled config is set correctly
     Assertions.assertThat(abfsConfig.getIsNamespaceEnabledAccount())
         .describedAs("Namespace enabled should be true in case config is set to true")
         .isEqualTo(Trilean.TRUE);
 
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getIsNamespaceEnabled(getTestTracingContext(abfs, false)))
+        .describedAs("getIsNamespaceEnabled() should return true when config is set to true")
+        .isEqualTo(true);
+
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getAbfsClient().getIsNamespaceEnabled())
+        .describedAs("Client's getIsNamespaceEnabled() should return true when config is set to true")
+        .isEqualTo(true);
+
     configuration.set(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, FALSE_STR);
+    abfs = (AzureBlobFileSystem) FileSystem.newInstance(configuration);
     abfsConfig = new AbfsConfiguration(configuration, "bogusAccountName");
 
     // Test that the namespace enabled config is set correctly
     Assertions.assertThat(abfsConfig.getIsNamespaceEnabledAccount())
         .describedAs("Namespace enabled should be false in case config is set to false")
         .isEqualTo(Trilean.FALSE);
+
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getIsNamespaceEnabled(getTestTracingContext(abfs, false)))
+        .describedAs("getIsNamespaceEnabled() should return false when config is set to false")
+        .isEqualTo(false);
+
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getAbfsClient().getIsNamespaceEnabled())
+        .describedAs("Client's getIsNamespaceEnabled() should return false when config is set to false")
+        .isEqualTo(false);
+  }
+
+  /**
+   * Tests to check that the namespace configuration is set correctly
+   * during the initialization of the AzureBlobFileSystem.
+   *
+   *
+   * @throws Exception if any error occurs during configuration setup or evaluation
+   */
+  @Test
+  public void testFsInitShouldSetNamespaceConfig() throws Exception {
+    // Mock the AzureBlobFileSystem and its dependencies
+    AzureBlobFileSystem mockFileSystem = Mockito.spy((AzureBlobFileSystem)
+        FileSystem.newInstance(getConfigurationWithoutHnsConfig()));
+    AzureBlobFileSystemStore mockStore = Mockito.spy(mockFileSystem.getAbfsStore());
+    AbfsClient abfsClient = Mockito.spy(mockStore.getClient());
+    Mockito.doReturn(abfsClient).when(mockStore).getClient();
+    Mockito.doReturn(abfsClient).when(mockStore).getClient(any());
+    abfsClient.getIsNamespaceEnabled();
+    // Verify that getAclStatus is called once during initialization
+    Mockito.verify(abfsClient, times(0))
+        .getAclStatus(anyString(), any(TracingContext.class));
+
+    mockStore.getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
+    // In case someone wrongly configured the namespace in between the processing,
+    // abfsclient.getIsNamespaceEnabled() should throw an exception.
+    String errorMessage = intercept(InvalidConfigurationValueException.class, () -> {
+      abfsClient.getIsNamespaceEnabled();
+    }).getMessage();
+    Assertions.assertThat(errorMessage)
+        .describedAs("Client should throw exception when namespace is unknown")
+        .contains("Failed to determine account type");
+
+    // In case of unknown namespace, store's getIsNamespaceEnabled should call getAclStatus
+    // to determine the namespace status.
+    mockStore.getIsNamespaceEnabled(getTestTracingContext(mockFileSystem, false));
+    Mockito.verify(abfsClient, times(1))
+        .getAclStatus(anyString(), any(TracingContext.class));
+
+    abfsClient.getIsNamespaceEnabled();
+    // Verify that client's getNamespaceEnabled will not call getAclStatus again
+    Mockito.verify(abfsClient, times(1))
+        .getAclStatus(anyString(), any(TracingContext.class));
+  }
+
+  /**
+   * Returns the configuration without the HNS config set.
+   *
+   * @return Configuration without HNS config
+   */
+  private Configuration getConfigurationWithoutHnsConfig() {
+    Configuration rawConfig = new Configuration();
+    rawConfig.addResource(TEST_CONFIGURATION_FILE_NAME);
+    rawConfig.unset(FS_AZURE_ACCOUNT_IS_HNS_ENABLED);
+    rawConfig.unset(accountProperty(FS_AZURE_ACCOUNT_IS_HNS_ENABLED,
+        this.getAccountName()));
+    String testAccountName = "testAccount.dfs.core.windows.net";
+    String defaultUri = this.getTestUrl().replace(this.getAccountName(), testAccountName);
+    // Assert that account specific config takes precedence
+    rawConfig.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, defaultUri);
+    return rawConfig;
   }
 
   private void assertFileSystemInitWithExpectedHNSSettings(

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestTracingContext.java

@@ -200,15 +200,15 @@ public void testExternalOps() throws Exception {
         0));
 
     // unset namespaceEnabled to call getAcl -> trigger tracing header validator
-    fs.getAbfsStore().setNamespaceEnabled(Trilean.UNKNOWN);
+    fs.getAbfsStore().getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.UNKNOWN);
     fs.hasPathCapability(new Path("/"), CommonPathCapabilities.FS_ACLS);
 
     Assume.assumeTrue(getIsNamespaceEnabled(getFileSystem()));
     Assume.assumeTrue(getConfiguration().isCheckAccessEnabled());
     Assume.assumeTrue(getAuthType() == AuthType.OAuth);
 
     fs.setListenerOperation(FSOperationType.ACCESS);
-    fs.getAbfsStore().setNamespaceEnabled(Trilean.TRUE);
+    fs.getAbfsStore().getAbfsConfiguration().setIsNamespaceEnabledAccountForTesting(Trilean.TRUE);
     fs.access(new Path("/"), FsAction.READ);
   }
 

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestAbfsRenameRetryRecovery.java

@@ -460,8 +460,8 @@ public void testRenameRecoveryUnsupportedForFlatNamespace() throws Exception {
     // In DFS endpoint, renamePath is O(1) API call and idempotency issue can happen.
     // For blob endpoint, client orchestrates the rename operation.
     assumeDfsServiceType();
+    assumeHnsDisabled();
     AzureBlobFileSystem fs = getFileSystem();
-    Assume.assumeTrue(!getIsNamespaceEnabled(fs));
     AzureBlobFileSystemStore abfsStore = fs.getAbfsStore();
     TracingContext testTracingContext = getTestTracingContext(fs, false);