address comments

minor change
facebookincubator · Dec 3, 2024 · fcaf015 · fcaf015
1 parent bc6cc65
commit fcaf015
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 11 deletions.
diff --git a/velox/connectors/hive/storage_adapters/abfs/AbfsConfig.cpp b/velox/connectors/hive/storage_adapters/abfs/AbfsConfig.cpp
@@ -45,11 +45,11 @@ AbfsConfig::AbfsConfig(
 
   auto authTypeKey =
       fmt::format("{}.{}", kAzureAccountAuthType, accountNameWithSuffix_);
-  authType_ = "SharedKey";
+  authType_ = kAzureSharedKeyAuthType;
   if (config.valueExists(authTypeKey)) {
     authType_ = config.get<std::string>(authTypeKey).value();
   }
-  if (authType_ == "SharedKey") {
+  if (authType_ == kAzureSharedKeyAuthType) {
     auto credKey =
         fmt::format("{}.{}", kAzureAccountKey, accountNameWithSuffix_);
     VELOX_USER_CHECK(
@@ -69,7 +69,7 @@ AbfsConfig::AbfsConfig(
     }
     ss << ";";
     connectionString_ = ss.str();
-  } else if (authType_ == "OAuth") {
+  } else if (authType_ == kAzureOAuthAuthType) {
     auto clientIdKey = fmt::format(
         "{}.{}", kAzureAccountOAuth2ClientId, accountNameWithSuffix_);
     auto clientSecretKey = fmt::format(
@@ -99,7 +99,7 @@ AbfsConfig::AbfsConfig(
             config.get<std::string>(clientIdKey).value(),
             config.get<std::string>(clientSecretKey).value(),
             options);
-  } else if (authType_ == "SAS") {
+  } else if (authType_ == kAzureSASAuthType) {
     auto sasKey = fmt::format("{}.{}", kAzureSASKey, accountNameWithSuffix_);
     VELOX_USER_CHECK(config.valueExists(sasKey), "Config {} not found", sasKey);
     sas_ = config.get<std::string>(sasKey).value();
@@ -111,10 +111,10 @@ AbfsConfig::AbfsConfig(
 }
 
 std::unique_ptr<BlobClient> AbfsConfig::getReadFileClient() {
-  if (authType_ == "SAS") {
+  if (authType_ == kAzureSASAuthType) {
     auto url = getUrl(true);
     return std::make_unique<BlobClient>(fmt::format("{}?{}", url, sas_));
-  } else if (authType_ == "OAuth") {
+  } else if (authType_ == kAzureOAuthAuthType) {
     auto url = getUrl(true);
     return std::make_unique<BlobClient>(url, tokenCredential_);
   } else {
@@ -124,11 +124,11 @@ std::unique_ptr<BlobClient> AbfsConfig::getReadFileClient() {
 }
 
 std::unique_ptr<DataLakeFileClient> AbfsConfig::getWriteFileClient() {
-  if (authType_ == "SAS") {
+  if (authType_ == kAzureSASAuthType) {
     auto url = getUrl(false);
     return std::make_unique<DataLakeFileClient>(
         fmt::format("{}?{}", url, sas_));
-  } else if (authType_ == "OAuth") {
+  } else if (authType_ == kAzureOAuthAuthType) {
     auto url = getUrl(false);
     return std::make_unique<DataLakeFileClient>(url, tokenCredential_);
   } else {

diff --git a/velox/connectors/hive/storage_adapters/abfs/AbfsConfig.h b/velox/connectors/hive/storage_adapters/abfs/AbfsConfig.h
@@ -45,6 +45,7 @@ static constexpr const char* kAzureSASKey = "fs.azure.sas.fixed.token";
 
 static constexpr const char* kAzureAccountOAuth2ClientId =
     "fs.azure.account.oauth2.client.id";
+
 static constexpr const char* kAzureAccountOAuth2ClientSecret =
     "fs.azure.account.oauth2.client.secret";
 
@@ -53,6 +54,12 @@ static constexpr const char* kAzureAccountOAuth2ClientSecret =
 static constexpr const char* kAzureAccountOAuth2ClientEndpoint =
     "fs.azure.account.oauth2.client.endpoint";
 
+static constexpr const char* kAzureSharedKeyAuthType = "SharedKey";
+
+static constexpr const char* kAzureOAuthAuthType = "OAuth";
+
+static constexpr const char* kAzureSASAuthType = "SAS";
+
 class AbfsConfig {
  public:
   explicit AbfsConfig(std::string_view path, const config::ConfigBase& config);

diff --git a/velox/docs/configs.rst b/velox/docs/configs.rst
@@ -681,12 +681,47 @@ These semantics are similar to the `Apache Hadoop-Aws module <https://hadoop.apa
      - Type
      - Default Value
      - Description
+   * - fs.azure.account.auth.type.<storage-account>.dfs.core.windows.net
+     - string
+     - SharedKey
+     - Specifies the authentication mechanism to use for Azure storage accounts. 
+       **Allowed values:**: 
+        "SharedKey": Uses the storage account name and key for authentication.
+        "OAuth": Utilizes OAuth tokens for secure authentication.
+        "SAS": Employs Shared Access Signatures for granular access control.
+     -
    * - fs.azure.account.key.<storage-account>.dfs.core.windows.net
      - string
      -
-     -  The credentials to access the specific Azure Blob Storage account, replace <storage-account> with the name of your Azure Storage account.
-        This property aligns with how Spark configures Azure account key credentials for accessing Azure storage, by setting this property multiple
-        times with different storage account names, you can access multiple Azure storage accounts.
+     - The credentials to access the specific Azure Blob Storage account, replace <storage-account> with the name of your Azure Storage account.
+       This property aligns with how Spark configures Azure account key credentials for accessing Azure storage, by setting this property multiple
+       times with different storage account names, you can access multiple Azure storage accounts.
+     -
+   * - fs.azure.sas.fixed.token.<storage-account>.dfs.core.windows.net
+     - string
+     -
+     - Specifies a fixed SAS (Shared Access Signature) token for accessing Azure storage.
+       This token provides scoped and time-limited access to specific resources.
+       Use this property when a pre-generated SAS token is used for authentication.
+     -
+   * - fs.azure.account.oauth2.client.id.<storage-account>.dfs.core.windows.net
+     - string
+     -
+     - Specifies the client ID of the Azure AD application used for OAuth 2.0 authentication.
+       This client ID is required when using OAuth as the authentication type.
+     -
+   * - fs.azure.account.oauth2.client.secret.<storage-account>.dfs.core.windows.net
+     - string
+     -
+     - Specifies the client secret of the Azure AD application used for OAuth 2.0 authentication.
+       This secret is required in conjunction with the client ID to authenticate the application.
+     -
+   * - fs.azure.account.oauth2.client.endpoint.<storage-account>.dfs.core.windows.net
+     - string
+     -
+     - Specifies the OAuth 2.0 token endpoint URL for the Azure AD application.  
+       This endpoint is used to acquire access tokens for authenticating with Azure storage.
+       The URL follows the format: https://login.microsoftonline.com/<tenant-id>/oauth2/token.
 
 Presto-specific Configuration
 -----------------------------