From a4053adae5626a02448f85c6073670ae29191049 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Carvalho Date: Wed, 3 Jan 2024 22:00:42 -0300 Subject: [PATCH] GH-39449: [C++] Use default Azure credentials implicitly and support anonymous credentials explictly --- cpp/src/arrow/filesystem/azurefs.cc | 39 +++++++++++++++++------- cpp/src/arrow/filesystem/azurefs.h | 20 ++++++------ cpp/src/arrow/filesystem/azurefs_test.cc | 24 +++++++++++---- 3 files changed, 55 insertions(+), 28 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 21350a490411a..e9b78530e9d1e 100644 --- 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::kDefaultCredential; + token_credential_ = std::make_shared(); + 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(); - return Status::OK(); -} - Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& client_id) { credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = @@ -148,8 +154,13 @@ Result> 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(AccountBlobUrl(account_name)); + case CredentialKind::kDefaultCredential: + if (!token_credential_) { + token_credential_ = std::make_shared(); + } + [[fallthrough]]; case CredentialKind::kTokenCredential: return std::make_unique(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( + AccountDfsUrl(account_name)); + case CredentialKind::kDefaultCredential: + if (!token_credential_) { + token_credential_ = std::make_shared(); + } + [[fallthrough]]; case CredentialKind::kTokenCredential: return std::make_unique( AccountDfsUrl(account_name), token_credential_); diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index 78e0a8148c616..a3a8331d60c33 100644 --- 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 token_credential_; std::shared_ptr storage_shared_key_credential_; + mutable std::shared_ptr 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 index f6af9f722dbac..d1108a84d3b86 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -280,22 +280,34 @@ 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)); + + AzureOptions explictly_default_options; + explictly_default_options.account_name = "dummy-account-name"; + ARROW_EXPECT_OK(explictly_default_options.ConfigureDefaultCredential()); + ASSERT_TRUE(options.Equals(explictly_default_options)); +} + +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, InitializeFilesystemWithManagedIdentityCredential) { +TEST(AzureFileSystem, InitializeWithManagedIdentityCredential) { AzureOptions options; options.account_name = "dummy-account-name"; ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential()); @@ -305,7 +317,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());