[GitHub] [hadoop] anmolanmol1234 commented on a diff in pull request #5148: HADOOP-18516. ABFS: Support fixed SAS token config in addition to SASTokenProvider class

[GitBox <gi...@apache.org>]

anmolanmol1234 commented on code in PR #5148:
URL: https://github.com/apache/hadoop/pull/5148#discussion_r1044131234


##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -885,31 +885,50 @@ public AccessTokenProvider getTokenProvider() throws TokenAccessProviderExceptio
     }
   }
 
+  /**
+   * @return sasTokenProvider object
+   * @throws AzureBlobFileSystemException
+   * The following method chooses between a configured fixed sas token, and a user implementation of the SASTokenProvider interface,
+   * depending on which one is available. In case a user SASTokenProvider implementation is not present, and a fixed token is configured,
+   * it simply returns null, to set the sasTokenProvider object for current configuration instance to null.
+   * The fixed token is read and used later. This is done to:
+   * 1. check for cases where both are not set, while initializing AbfsConfiguration,
+   * to not proceed further than thi stage itself when none of the options are available.
+   * 2. avoid using  similar tokenProvider implementation to just read the configured fixed token,
+   * as this could create confusion. The configuration is introduced
+   * primarily to avoid using any tokenProvider class/interface. Also,implementing the SASTokenProvider requires relying on the raw configurations.
+   * It is more stable to depend on the AbfsConfiguration with which a filesystem is initialized,
+   * and eliminate chances of dynamic modifications and spurious situations.
+   */
+
   public SASTokenProvider getSASTokenProvider() throws AzureBlobFileSystemException {
     AuthType authType = getEnum(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.SharedKey);
     if (authType != AuthType.SAS) {
-      throw new SASTokenProviderException(String.format(
-        "Invalid auth type: %s is being used, expecting SAS", authType));
+      throw new SASTokenProviderException(String.format("Invalid auth type: %s is being used, expecting SAS", authType));
     }
 
     try {
-      String configKey = FS_AZURE_SAS_TOKEN_PROVIDER_TYPE;
-      Class<? extends SASTokenProvider> sasTokenProviderClass =
-          getTokenProviderClass(authType, configKey, null,
-              SASTokenProvider.class);
-
-      Preconditions.checkArgument(sasTokenProviderClass != null,
-          String.format("The configuration value for \"%s\" is invalid.", configKey));
-
-      SASTokenProvider sasTokenProvider = ReflectionUtils
-          .newInstance(sasTokenProviderClass, rawConfig);
-      Preconditions.checkArgument(sasTokenProvider != null,
-          String.format("Failed to initialize %s", sasTokenProviderClass));
-
-      LOG.trace("Initializing {}", sasTokenProviderClass.getName());
-      sasTokenProvider.initialize(rawConfig, accountName);
-      LOG.trace("{} init complete", sasTokenProviderClass.getName());
-      return sasTokenProvider;
+      Class<? extends SASTokenProvider> sasTokenProviderImplementation =
+              getTokenProviderClass(authType, FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, null,
+                      SASTokenProvider.class);
+      String configuredFixedToken = this.rawConfig.get(FS_AZURE_SAS_FIXED_TOKEN, null);
+
+      Preconditions.checkArgument(sasTokenProviderImplementation != null || configuredFixedToken != null,
+              String.format("The value for both \"%s\" and \"%s\" cannot be invalid.", FS_AZURE_SAS_TOKEN_PROVIDER_TYPE, FS_AZURE_SAS_FIXED_TOKEN));
+
+      if (sasTokenProviderImplementation != null) {
+        LOG.trace("Using SASTokenProvider class instead of config although both are available for use");

Review Comment:
   How are we ensuring that both are available for use because in checkArgument we have used or.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org