From 4bfacdde46d4e45720018fb0f67405c7b0ef857d Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Wed, 20 Dec 2023 13:08:28 +0000 Subject: [PATCH 1/8] Add managed identity --- cpp/src/arrow/filesystem/azurefs.cc | 8 ++++++++ cpp/src/arrow/filesystem/azurefs.h | 3 +++ cpp/src/arrow/filesystem/azurefs_test.cc | 10 ++++++++++ 3 files changed, 21 insertions(+) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 26c27618860..43a489739b2 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -129,6 +129,14 @@ Status AzureOptions::ConfigureDefaultCredential(const std::string& account_name) return Status::OK(); } +Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& account_name, + std::string const& clientId) { + credential_kind_ = CredentialKind::kTokenCredential; + token_credential_ = + std::make_shared(clientId); + return Status::OK(); +} + Status AzureOptions::ConfigureWorkloadIdentityCredential( const std::string& account_name) { credential_kind_ = CredentialKind::kTokenCredential; diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index 346dd349e93..97b5108c9f0 100644 --- a/cpp/src/arrow/filesystem/azurefs.h +++ b/cpp/src/arrow/filesystem/azurefs.h @@ -105,6 +105,9 @@ struct ARROW_EXPORT AzureOptions { Status ConfigureDefaultCredential(const std::string& account_name); + Status ConfigureManagedIdentityCredential(const std::string& account_name, + std::string const& clientId = std::string()); + Status ConfigureWorkloadIdentityCredential(const std::string& account_name); Status ConfigureAccountKeyCredential(const std::string& account_name, diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 62c5ef22320..5d1b2e5ece2 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -284,6 +284,16 @@ TEST(AzureFileSystem, InitializeFilesystemWithDefaultCredential) { EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); } +TEST(AzureFileSystem, InitializeFilesystemWithManagedIdentityCredential) { + AzureOptions options; + ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("dummy-account-name")); + EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); + + ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("dummy-account-name", + "specific-client-id")); + EXPECT_OK_AND_ASSIGN(fs, AzureFileSystem::Make(options)); +} + TEST(AzureFileSystem, InitializeFilesystemWithWorkloadIdentityCredential) { AzureOptions options; ARROW_EXPECT_OK(options.ConfigureWorkloadIdentityCredential("dummy-account-name")); From f6e40aad22f7f9eb7dd0ed190adfd9e54a129276 Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 12:30:46 +0000 Subject: [PATCH 2/8] Ensure filesystem initialisation fails without account name --- cpp/src/arrow/filesystem/azurefs.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 43a489739b2..ee6dc25f7d9 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -146,6 +146,9 @@ Status AzureOptions::ConfigureWorkloadIdentityCredential( Result> AzureOptions::MakeBlobServiceClient() const { + if (account_name_.empty()) { + return Status::Invalid("AzureOptions doesn't contain a valid account name"); + } switch (credential_kind_) { case CredentialKind::kAnonymous: break; @@ -161,6 +164,9 @@ Result> AzureOptions::MakeBlobServiceC Result> AzureOptions::MakeDataLakeServiceClient() const { + if (account_name_.empty()) { + return Status::Invalid("AzureOptions doesn't contain a valid account name"); + } switch (credential_kind_) { case CredentialKind::kAnonymous: break; From d4af8409d2f5489a9cd83487e5660846515a37ad Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 12:34:33 +0000 Subject: [PATCH 3/8] Fix broken auths --- cpp/src/arrow/filesystem/azurefs.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index ee6dc25f7d9..2585bb4fc0c 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -117,6 +117,7 @@ Status AzureOptions::ConfigureClientSecretCredential(const std::string& account_ const std::string& tenant_id, const std::string& client_id, const std::string& client_secret) { + account_name_ = account_name; credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared( tenant_id, client_id, client_secret); @@ -124,6 +125,7 @@ Status AzureOptions::ConfigureClientSecretCredential(const std::string& account_ } Status AzureOptions::ConfigureDefaultCredential(const std::string& account_name) { + account_name_ = account_name; credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(); return Status::OK(); @@ -131,6 +133,7 @@ Status AzureOptions::ConfigureDefaultCredential(const std::string& account_name) Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& account_name, std::string const& clientId) { + account_name_ = account_name; credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(clientId); @@ -139,6 +142,7 @@ Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& accou Status AzureOptions::ConfigureWorkloadIdentityCredential( const std::string& account_name) { + account_name_ = account_name; credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(); return Status::OK(); From 3a9b89f6f93be62d881ea24c6f546df7114d83bc Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 12:35:24 +0000 Subject: [PATCH 4/8] style fix --- cpp/src/arrow/filesystem/azurefs.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 2585bb4fc0c..ff58d253b00 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -132,11 +132,11 @@ Status AzureOptions::ConfigureDefaultCredential(const std::string& account_name) } Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& account_name, - std::string const& clientId) { + const std::string& client_id) { account_name_ = account_name; credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = - std::make_shared(clientId); + std::make_shared(client_id); return Status::OK(); } From 414f582d54289e978f7881b1c18499dcad112fb3 Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 18:06:38 +0000 Subject: [PATCH 5/8] another code style fix --- cpp/src/arrow/filesystem/azurefs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index 97b5108c9f0..99f06e3421f 100644 --- a/cpp/src/arrow/filesystem/azurefs.h +++ b/cpp/src/arrow/filesystem/azurefs.h @@ -106,7 +106,7 @@ struct ARROW_EXPORT AzureOptions { Status ConfigureDefaultCredential(const std::string& account_name); Status ConfigureManagedIdentityCredential(const std::string& account_name, - std::string const& clientId = std::string()); + const std::string& client_id = std::string()); Status ConfigureWorkloadIdentityCredential(const std::string& account_name); From de0a869193322e7f0b2c05bf2c28a82fc454abaa Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 19:27:26 +0000 Subject: [PATCH 6/8] Move account_name to AzureOptions constructor --- cpp/src/arrow/filesystem/azurefs.cc | 25 +++++++------------- cpp/src/arrow/filesystem/azurefs.h | 15 +++++------- cpp/src/arrow/filesystem/azurefs_test.cc | 30 +++++++++++------------- 3 files changed, 29 insertions(+), 41 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index ff58d253b00..dce2773c53f 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -47,7 +47,9 @@ using HNSSupport = internal::HierarchicalNamespaceSupport; // ----------------------------------------------------------------------- // AzureOptions Implementation -AzureOptions::AzureOptions() = default; +// AzureOptions::AzureOptions(const std::string& account_name) = default; +AzureOptions::AzureOptions(const std::string& account_name) + : account_name_(account_name) {} AzureOptions::~AzureOptions() = default; @@ -104,45 +106,36 @@ std::string AzureOptions::AccountDfsUrl(const std::string& account_name) const { return BuildBaseUrl(dfs_storage_scheme, dfs_storage_authority, account_name); } -Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_name, - const std::string& account_key) { +Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_key) { credential_kind_ = CredentialKind::kStorageSharedKeyCredential; - account_name_ = account_name; storage_shared_key_credential_ = - std::make_shared(account_name, account_key); + std::make_shared(account_name_, account_key); return Status::OK(); } -Status AzureOptions::ConfigureClientSecretCredential(const std::string& account_name, - const std::string& tenant_id, +Status AzureOptions::ConfigureClientSecretCredential(const std::string& tenant_id, const std::string& client_id, const std::string& client_secret) { - account_name_ = account_name; credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared( tenant_id, client_id, client_secret); return Status::OK(); } -Status AzureOptions::ConfigureDefaultCredential(const std::string& account_name) { - account_name_ = account_name; +Status AzureOptions::ConfigureDefaultCredential() { credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(); return Status::OK(); } -Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& account_name, - const std::string& client_id) { - account_name_ = account_name; +Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& client_id) { credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(client_id); return Status::OK(); } -Status AzureOptions::ConfigureWorkloadIdentityCredential( - const std::string& account_name) { - account_name_ = account_name; +Status AzureOptions::ConfigureWorkloadIdentityCredential() { credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(); return Status::OK(); diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index 99f06e3421f..2e635984d4e 100644 --- a/cpp/src/arrow/filesystem/azurefs.h +++ b/cpp/src/arrow/filesystem/azurefs.h @@ -100,21 +100,18 @@ struct ARROW_EXPORT AzureOptions { storage_shared_key_credential_; public: - AzureOptions(); + AzureOptions(const std::string& account_name); ~AzureOptions(); - Status ConfigureDefaultCredential(const std::string& account_name); + Status ConfigureDefaultCredential(); - Status ConfigureManagedIdentityCredential(const std::string& account_name, - const std::string& client_id = std::string()); + Status ConfigureManagedIdentityCredential(const std::string& client_id = std::string()); - Status ConfigureWorkloadIdentityCredential(const std::string& account_name); + Status ConfigureWorkloadIdentityCredential(); - Status ConfigureAccountKeyCredential(const std::string& account_name, - const std::string& account_key); + Status ConfigureAccountKeyCredential(const std::string& account_key); - Status ConfigureClientSecretCredential(const std::string& account_name, - const std::string& tenant_id, + Status ConfigureClientSecretCredential(const std::string& tenant_id, const std::string& client_id, const std::string& client_secret); diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 5d1b2e5ece2..ab23b0b3fc7 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -272,36 +272,35 @@ class AzureHierarchicalNSEnv : public AzureEnvImpl { }; TEST(AzureFileSystem, InitializeFilesystemWithClientSecretCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureClientSecretCredential( - "dummy-account-name", "tenant_id", "client_id", "client_secret")); + AzureOptions options("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, InitializeFilesystemWithDefaultCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureDefaultCredential("dummy-account-name")); + AzureOptions options("dummy-account-name"); + ARROW_EXPECT_OK(options.ConfigureDefaultCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, InitializeFilesystemWithManagedIdentityCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("dummy-account-name")); + AzureOptions options("dummy-account-name"); + ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); - ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("dummy-account-name", - "specific-client-id")); + ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("specific-client-id")); EXPECT_OK_AND_ASSIGN(fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, InitializeFilesystemWithWorkloadIdentityCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureWorkloadIdentityCredential("dummy-account-name")); + AzureOptions options("dummy-account-name"); + ARROW_EXPECT_OK(options.ConfigureWorkloadIdentityCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, OptionsCompare) { - AzureOptions options; + AzureOptions options("account-name"); EXPECT_TRUE(options.Equals(options)); } @@ -375,7 +374,7 @@ class TestAzureFileSystem : public ::testing::Test { std::unique_ptr datalake_service_client_; public: - TestAzureFileSystem() : rng_(std::random_device()()) {} + TestAzureFileSystem() : rng_(std::random_device()()), options_("temp") {} virtual Result GetAzureEnv() const = 0; virtual HNSSupport CachedHNSSupport(const BaseAzureEnv& env) const = 0; @@ -392,7 +391,7 @@ class TestAzureFileSystem : public ::testing::Test { } static Result MakeOptions(BaseAzureEnv* env) { - AzureOptions options; + AzureOptions options(env->account_name()); switch (env->backend()) { case AzureBackend::kAzurite: options.blob_storage_authority = "127.0.0.1:10000"; @@ -404,8 +403,7 @@ class TestAzureFileSystem : public ::testing::Test { // Use the default values break; } - ARROW_EXPECT_OK( - options.ConfigureAccountKeyCredential(env->account_name(), env->account_key())); + ARROW_EXPECT_OK(options.ConfigureAccountKeyCredential(env->account_key())); return options; } From 8cdcf4b9ec7b856570664e813278abbbc8c5b79d Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 19:50:11 +0000 Subject: [PATCH 7/8] Revert AzureOptions constructor. Just make account_name a public member. --- cpp/src/arrow/filesystem/azurefs.cc | 23 ++++++++++++----------- cpp/src/arrow/filesystem/azurefs.h | 6 ++++-- cpp/src/arrow/filesystem/azurefs_test.cc | 19 ++++++++++++------- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index dce2773c53f..21350a49041 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -47,9 +47,7 @@ using HNSSupport = internal::HierarchicalNamespaceSupport; // ----------------------------------------------------------------------- // AzureOptions Implementation -// AzureOptions::AzureOptions(const std::string& account_name) = default; -AzureOptions::AzureOptions(const std::string& account_name) - : account_name_(account_name) {} +AzureOptions::AzureOptions() = default; AzureOptions::~AzureOptions() = default; @@ -60,7 +58,7 @@ bool AzureOptions::Equals(const AzureOptions& other) const { blob_storage_scheme == other.blob_storage_scheme && dfs_storage_scheme == other.dfs_storage_scheme && default_metadata == other.default_metadata && - account_name_ == other.account_name_ && + account_name == other.account_name && credential_kind_ == other.credential_kind_; if (!equals) { return false; @@ -108,8 +106,11 @@ std::string AzureOptions::AccountDfsUrl(const std::string& account_name) const { Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_key) { credential_kind_ = CredentialKind::kStorageSharedKeyCredential; + if (account_name.empty()) { + return Status::Invalid("AzureOptions doesn't contain a valid account name"); + } storage_shared_key_credential_ = - std::make_shared(account_name_, account_key); + std::make_shared(account_name, account_key); return Status::OK(); } @@ -143,17 +144,17 @@ Status AzureOptions::ConfigureWorkloadIdentityCredential() { Result> AzureOptions::MakeBlobServiceClient() const { - if (account_name_.empty()) { + if (account_name.empty()) { return Status::Invalid("AzureOptions doesn't contain a valid account name"); } switch (credential_kind_) { case CredentialKind::kAnonymous: break; case CredentialKind::kTokenCredential: - return std::make_unique(AccountBlobUrl(account_name_), + return std::make_unique(AccountBlobUrl(account_name), token_credential_); case CredentialKind::kStorageSharedKeyCredential: - return std::make_unique(AccountBlobUrl(account_name_), + return std::make_unique(AccountBlobUrl(account_name), storage_shared_key_credential_); } return Status::Invalid("AzureOptions doesn't contain a valid auth configuration"); @@ -161,7 +162,7 @@ Result> AzureOptions::MakeBlobServiceC Result> AzureOptions::MakeDataLakeServiceClient() const { - if (account_name_.empty()) { + if (account_name.empty()) { return Status::Invalid("AzureOptions doesn't contain a valid account name"); } switch (credential_kind_) { @@ -169,10 +170,10 @@ AzureOptions::MakeDataLakeServiceClient() const { break; case CredentialKind::kTokenCredential: return std::make_unique( - AccountDfsUrl(account_name_), token_credential_); + AccountDfsUrl(account_name), token_credential_); case CredentialKind::kStorageSharedKeyCredential: return std::make_unique( - AccountDfsUrl(account_name_), storage_shared_key_credential_); + AccountDfsUrl(account_name), storage_shared_key_credential_); } return Status::Invalid("AzureOptions doesn't contain a valid auth configuration"); } diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index 2e635984d4e..78e0a8148c6 100644 --- a/cpp/src/arrow/filesystem/azurefs.h +++ b/cpp/src/arrow/filesystem/azurefs.h @@ -48,6 +48,9 @@ class TestAzureFileSystem; /// Options for the AzureFileSystem implementation. struct ARROW_EXPORT AzureOptions { + /// \brief account name of the Azure Storage account. + std::string account_name; + /// \brief hostname[:port] of the Azure Blob Storage Service. /// /// If the hostname is a relative domain name (one that starts with a '.'), then storage @@ -94,13 +97,12 @@ struct ARROW_EXPORT AzureOptions { kStorageSharedKeyCredential, } credential_kind_ = CredentialKind::kAnonymous; - std::string account_name_; std::shared_ptr token_credential_; std::shared_ptr storage_shared_key_credential_; public: - AzureOptions(const std::string& account_name); + AzureOptions(); ~AzureOptions(); Status ConfigureDefaultCredential(); diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index ab23b0b3fc7..146e23651c4 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -272,20 +272,23 @@ class AzureHierarchicalNSEnv : public AzureEnvImpl { }; TEST(AzureFileSystem, InitializeFilesystemWithClientSecretCredential) { - AzureOptions options("dummy-account-name"); + 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, InitializeFilesystemWithDefaultCredential) { - AzureOptions options("dummy-account-name"); + 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) { - AzureOptions options("dummy-account-name"); + AzureOptions options; + options.account_name = "dummy-account-name"; ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); @@ -294,13 +297,14 @@ TEST(AzureFileSystem, InitializeFilesystemWithManagedIdentityCredential) { } TEST(AzureFileSystem, InitializeFilesystemWithWorkloadIdentityCredential) { - AzureOptions options("dummy-account-name"); + AzureOptions options; + options.account_name = "dummy-account-name"; ARROW_EXPECT_OK(options.ConfigureWorkloadIdentityCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, OptionsCompare) { - AzureOptions options("account-name"); + AzureOptions options; EXPECT_TRUE(options.Equals(options)); } @@ -374,7 +378,7 @@ class TestAzureFileSystem : public ::testing::Test { std::unique_ptr datalake_service_client_; public: - TestAzureFileSystem() : rng_(std::random_device()()), options_("temp") {} + TestAzureFileSystem() : rng_(std::random_device()()) {} virtual Result GetAzureEnv() const = 0; virtual HNSSupport CachedHNSSupport(const BaseAzureEnv& env) const = 0; @@ -391,7 +395,8 @@ class TestAzureFileSystem : public ::testing::Test { } static Result MakeOptions(BaseAzureEnv* env) { - AzureOptions options(env->account_name()); + AzureOptions options; + options.account_name = env->account_name(); switch (env->backend()) { case AzureBackend::kAzurite: options.blob_storage_authority = "127.0.0.1:10000"; From bfa2cc505143ed60aa4d8cf17b0f3bc52b968ebd Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 19:54:02 +0000 Subject: [PATCH 8/8] Add a test asserting initialisation fails without an account name --- cpp/src/arrow/filesystem/azurefs_test.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 146e23651c4..f6af9f722db 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -271,6 +271,15 @@ class AzureHierarchicalNSEnv : public AzureEnvImpl { bool WithHierarchicalNamespace() const final { return true; } }; +TEST(AzureFileSystem, InitializingFilesystemWithoutAccountNameFails) { + AzureOptions options; + ASSERT_RAISES(Invalid, options.ConfigureAccountKeyCredential("account_key")); + + ARROW_EXPECT_OK( + options.ConfigureClientSecretCredential("tenant_id", "client_id", "client_secret")); + ASSERT_RAISES(Invalid, AzureFileSystem::Make(options)); +} + TEST(AzureFileSystem, InitializeFilesystemWithClientSecretCredential) { AzureOptions options; options.account_name = "dummy-account-name";