From c87ee5591525667c32ffa3945a58647bbdec752e Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Tue, 19 Dec 2023 07:32:00 +0000 Subject: [PATCH] GH-39262: [C++][Azure][FS] Add default credential auth configuration (#39263) ### Rationale for this change Default credential is a useful auth option. ### What changes are included in this PR? Implement `AzureOptions::ConfigureDefaultCredential` plus a little bit of plumbing to go around it. Created a simple test. ### Are these changes tested? Added a simple unittest that everything initialises happily. This does not actually test a successful authentication. I think to do a real authentication with Azure we would need to run the test against real blob storage and we would need to create various identities which are non-trivial to create. Personally I think this is ok because all the complexity is abstracted away by the Azure SDK. ### Are there any user-facing changes? * Closes: #39262 Lead-authored-by: Thomas Newton Co-authored-by: Sutou Kouhei Signed-off-by: Sutou Kouhei --- cpp/src/arrow/filesystem/azurefs.cc | 24 ++++++++++++++++++++++-- cpp/src/arrow/filesystem/azurefs.h | 7 +++++++ cpp/src/arrow/filesystem/azurefs_test.cc | 18 ++++++------------ 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 217885364089b..dd267aac36d35 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -18,6 +18,7 @@ #include "arrow/filesystem/azurefs.h" #include "arrow/filesystem/azurefs_internal.h" +#include #include #include @@ -61,6 +62,8 @@ bool AzureOptions::Equals(const AzureOptions& other) const { switch (credential_kind_) { case CredentialKind::kAnonymous: return true; + case CredentialKind::kTokenCredential: + return token_credential_ == other.token_credential_; case CredentialKind::kStorageSharedKeyCredential: return storage_shared_key_credential_->AccountName == other.storage_shared_key_credential_->AccountName; @@ -69,8 +72,7 @@ bool AzureOptions::Equals(const AzureOptions& other) const { return false; } -Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_name, - const std::string& account_key) { +void AzureOptions::SetUrlsForAccountName(const std::string& account_name) { if (this->backend == AzureBackend::kAzurite) { account_blob_url_ = "http://127.0.0.1:10000/" + account_name + "/"; account_dfs_url_ = "http://127.0.0.1:10000/" + account_name + "/"; @@ -78,6 +80,18 @@ Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_na account_dfs_url_ = "https://" + account_name + ".dfs.core.windows.net/"; account_blob_url_ = "https://" + account_name + ".blob.core.windows.net/"; } +} + +Status AzureOptions::ConfigureDefaultCredential(const std::string& account_name) { + AzureOptions::SetUrlsForAccountName(account_name); + credential_kind_ = CredentialKind::kTokenCredential; + token_credential_ = std::make_shared(); + return Status::OK(); +} + +Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_name, + const std::string& account_key) { + AzureOptions::SetUrlsForAccountName(account_name); credential_kind_ = CredentialKind::kStorageSharedKeyCredential; storage_shared_key_credential_ = std::make_shared(account_name, account_key); @@ -89,6 +103,9 @@ Result> AzureOptions::MakeBlobServiceC switch (credential_kind_) { case CredentialKind::kAnonymous: break; + case CredentialKind::kTokenCredential: + return std::make_unique(account_blob_url_, + token_credential_); case CredentialKind::kStorageSharedKeyCredential: return std::make_unique(account_blob_url_, storage_shared_key_credential_); @@ -101,6 +118,9 @@ AzureOptions::MakeDataLakeServiceClient() const { switch (credential_kind_) { case CredentialKind::kAnonymous: break; + case CredentialKind::kTokenCredential: + return std::make_unique(account_dfs_url_, + token_credential_); case CredentialKind::kStorageSharedKeyCredential: return std::make_unique( account_dfs_url_, storage_shared_key_credential_); diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index 1266aa2d02b86..b2c7010ff3758 100644 --- a/cpp/src/arrow/filesystem/azurefs.h +++ b/cpp/src/arrow/filesystem/azurefs.h @@ -70,16 +70,23 @@ struct ARROW_EXPORT AzureOptions { enum class CredentialKind { kAnonymous, + kTokenCredential, kStorageSharedKeyCredential, } credential_kind_ = CredentialKind::kAnonymous; std::shared_ptr storage_shared_key_credential_; + std::shared_ptr token_credential_; + + void SetUrlsForAccountName(const std::string& account_name); + public: AzureOptions(); ~AzureOptions(); + Status ConfigureDefaultCredential(const std::string& account_name); + Status ConfigureAccountKeyCredential(const std::string& account_name, const std::string& account_key); diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 463ff4e8daf3d..799f3992a2210 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -43,9 +43,6 @@ #include #include #include -#include -#include -#include #include #include #include @@ -266,15 +263,12 @@ class AzureHierarchicalNSEnv : public AzureEnvImpl { bool WithHierarchicalNamespace() const final { return true; } }; -// Placeholder tests -// TODO: GH-18014 Remove once a proper test is added -TEST(AzureFileSystem, InitializeCredentials) { - auto default_credential = std::make_shared(); - auto managed_identity_credential = - std::make_shared(); - auto service_principal_credential = - std::make_shared("tenant_id", "client_id", - "client_secret"); +TEST(AzureFileSystem, InitializeFilesystemWithDefaultCredential) { + AzureOptions options; + options.backend = AzureBackend::kAzurite; // Irrelevant for this test because it + // doesn't connect to the server. + ARROW_EXPECT_OK(options.ConfigureDefaultCredential("dummy-account-name")); + EXPECT_OK_AND_ASSIGN(auto default_credential_fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, OptionsCompare) {