apacheGH-39449: [C++] Use default Azure credentials implicitly and su…

…pport anonymous credentials explictly
felipecrv · Jan 4, 2024 · d2b77bd · d2b77bd
1 parent 0e597ab
commit d2b77bd
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 28 deletions.
diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc
@@ -64,7 +64,8 @@ bool AzureOptions::Equals(const AzureOptions& other) const {
     return false;
   }
   switch (credential_kind_) {
-    case CredentialKind::kAnonymous:
+    case CredentialKind::kDefaultCredential:
+    case CredentialKind::kAnonymousCredential:
       return true;
     case CredentialKind::kTokenCredential:
       return token_credential_ == other.token_credential_;
@@ -104,6 +105,17 @@ std::string AzureOptions::AccountDfsUrl(const std::string& account_name) const {
   return BuildBaseUrl(dfs_storage_scheme, dfs_storage_authority, account_name);
 }
 
+Status AzureOptions::ConfigureDefaultCredential() {
+  credential_kind_ = CredentialKind::kTokenCredential;
+  token_credential_ = std::make_shared<Azure::Identity::DefaultAzureCredential>();
+  return Status::OK();
+}
+
+Status AzureOptions::ConfigureAnonymousCredential() {
+  credential_kind_ = CredentialKind::kAnonymousCredential;
+  return Status::OK();
+}
+
 Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_key) {
   credential_kind_ = CredentialKind::kStorageSharedKeyCredential;
   if (account_name.empty()) {
@@ -123,12 +135,6 @@ Status AzureOptions::ConfigureClientSecretCredential(const std::string& tenant_i
   return Status::OK();
 }
 
-Status AzureOptions::ConfigureDefaultCredential() {
-  credential_kind_ = CredentialKind::kTokenCredential;
-  token_credential_ = std::make_shared<Azure::Identity::DefaultAzureCredential>();
-  return Status::OK();
-}
-
 Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& client_id) {
   credential_kind_ = CredentialKind::kTokenCredential;
   token_credential_ =
@@ -148,8 +154,13 @@ Result<std::unique_ptr<Blobs::BlobServiceClient>> AzureOptions::MakeBlobServiceC
     return Status::Invalid("AzureOptions doesn't contain a valid account name");
   }
   switch (credential_kind_) {
-    case CredentialKind::kAnonymous:
-      break;
+    case CredentialKind::kAnonymousCredential:
+      return std::make_unique<Blobs::BlobServiceClient>(AccountBlobUrl(account_name));
+    case CredentialKind::kDefaultCredential:
+      if (!token_credential_) {
+        token_credential_ = std::make_shared<Azure::Identity::DefaultAzureCredential>();
+      }
+      [[fallthrough]];
     case CredentialKind::kTokenCredential:
       return std::make_unique<Blobs::BlobServiceClient>(AccountBlobUrl(account_name),
                                                         token_credential_);
@@ -166,8 +177,14 @@ AzureOptions::MakeDataLakeServiceClient() const {
     return Status::Invalid("AzureOptions doesn't contain a valid account name");
   }
   switch (credential_kind_) {
-    case CredentialKind::kAnonymous:
-      break;
+    case CredentialKind::kAnonymousCredential:
+      return std::make_unique<DataLake::DataLakeServiceClient>(
+          AccountDfsUrl(account_name));
+    case CredentialKind::kDefaultCredential:
+      if (!token_credential_) {
+        token_credential_ = std::make_shared<Azure::Identity::DefaultAzureCredential>();
+      }
+      [[fallthrough]];
     case CredentialKind::kTokenCredential:
       return std::make_unique<DataLake::DataLakeServiceClient>(
           AccountDfsUrl(account_name), token_credential_);

diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h
@@ -48,7 +48,7 @@ class TestAzureFileSystem;
 
 /// Options for the AzureFileSystem implementation.
 struct ARROW_EXPORT AzureOptions {
-  /// \brief account name of the Azure Storage account.
+  /// \brief Name of the Azure Storage Account.
   std::string account_name;
 
   /// \brief hostname[:port] of the Azure Blob Storage Service.
@@ -92,30 +92,28 @@ struct ARROW_EXPORT AzureOptions {
 
  private:
   enum class CredentialKind {
-    kAnonymous,
-    kTokenCredential,
+    kDefaultCredential,
+    kAnonymousCredential,
     kStorageSharedKeyCredential,
-  } credential_kind_ = CredentialKind::kAnonymous;
+    kTokenCredential,
+  } credential_kind_ = CredentialKind::kDefaultCredential;
 
-  std::shared_ptr<Azure::Core::Credentials::TokenCredential> token_credential_;
   std::shared_ptr<Azure::Storage::StorageSharedKeyCredential>
       storage_shared_key_credential_;
+  mutable std::shared_ptr<Azure::Core::Credentials::TokenCredential> token_credential_;
 
  public:
   AzureOptions();
   ~AzureOptions();
 
   Status ConfigureDefaultCredential();
-
-  Status ConfigureManagedIdentityCredential(const std::string& client_id = std::string());
-
-  Status ConfigureWorkloadIdentityCredential();
-
+  Status ConfigureAnonymousCredential();
   Status ConfigureAccountKeyCredential(const std::string& account_key);
-
   Status ConfigureClientSecretCredential(const std::string& tenant_id,
                                          const std::string& client_id,
                                          const std::string& client_secret);
+  Status ConfigureManagedIdentityCredential(const std::string& client_id = std::string());
+  Status ConfigureWorkloadIdentityCredential();
 
   bool Equals(const AzureOptions& other) const;
 

diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc
@@ -280,22 +280,29 @@ TEST(AzureFileSystem, InitializingFilesystemWithoutAccountNameFails) {
   ASSERT_RAISES(Invalid, AzureFileSystem::Make(options));
 }
 
-TEST(AzureFileSystem, InitializeFilesystemWithClientSecretCredential) {
+TEST(AzureFileSystem, InitializeWithDefaultCredential) {
   AzureOptions options;
   options.account_name = "dummy-account-name";
-  ARROW_EXPECT_OK(
-      options.ConfigureClientSecretCredential("tenant_id", "client_id", "client_secret"));
+  ARROW_EXPECT_OK(options.ConfigureDefaultCredential());
   EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options));
 }
 
-TEST(AzureFileSystem, InitializeFilesystemWithDefaultCredential) {
+TEST(AzureFileSystem, InitializeWithDefaultCredentialImplicitly) {
   AzureOptions options;
   options.account_name = "dummy-account-name";
   ARROW_EXPECT_OK(options.ConfigureDefaultCredential());
   EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options));
 }
 
-TEST(AzureFileSystem, InitializeFilesystemWithManagedIdentityCredential) {
+TEST(AzureFileSystem, InitializeWithClientSecretCredential) {
+  AzureOptions options;
+  options.account_name = "dummy-account-name";
+  ARROW_EXPECT_OK(
+      options.ConfigureClientSecretCredential("tenant_id", "client_id", "client_secret"));
+  EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options));
+}
+
+TEST(AzureFileSystem, InitializeWithManagedIdentityCredential) {
   AzureOptions options;
   options.account_name = "dummy-account-name";
   ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential());
@@ -305,7 +312,7 @@ TEST(AzureFileSystem, InitializeFilesystemWithManagedIdentityCredential) {
   EXPECT_OK_AND_ASSIGN(fs, AzureFileSystem::Make(options));
 }
 
-TEST(AzureFileSystem, InitializeFilesystemWithWorkloadIdentityCredential) {
+TEST(AzureFileSystem, InitializeWithWorkloadIdentityCredential) {
   AzureOptions options;
   options.account_name = "dummy-account-name";
   ARROW_EXPECT_OK(options.ConfigureWorkloadIdentityCredential());