Skip to content

Commit

Permalink
Move account_name to AzureOptions constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
Tom-Newton committed Dec 22, 2023
1 parent 414f582 commit de0a869
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 41 deletions.
25 changes: 9 additions & 16 deletions cpp/src/arrow/filesystem/azurefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Storage::StorageSharedKeyCredential>(account_name, account_key);
std::make_shared<Storage::StorageSharedKeyCredential>(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<Azure::Identity::ClientSecretCredential>(
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<Azure::Identity::DefaultAzureCredential>();
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<Azure::Identity::ManagedIdentityCredential>(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<Azure::Identity::WorkloadIdentityCredential>();
return Status::OK();
Expand Down
15 changes: 6 additions & 9 deletions cpp/src/arrow/filesystem/azurefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
30 changes: 14 additions & 16 deletions cpp/src/arrow/filesystem/azurefs_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,36 +272,35 @@ class AzureHierarchicalNSEnv : public AzureEnvImpl<AzureHierarchicalNSEnv> {
};

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));
}

Expand Down Expand Up @@ -375,7 +374,7 @@ class TestAzureFileSystem : public ::testing::Test {
std::unique_ptr<DataLake::DataLakeServiceClient> datalake_service_client_;

public:
TestAzureFileSystem() : rng_(std::random_device()()) {}
TestAzureFileSystem() : rng_(std::random_device()()), options_("temp") {}

virtual Result<BaseAzureEnv*> GetAzureEnv() const = 0;
virtual HNSSupport CachedHNSSupport(const BaseAzureEnv& env) const = 0;
Expand All @@ -392,7 +391,7 @@ class TestAzureFileSystem : public ::testing::Test {
}

static Result<AzureOptions> 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";
Expand All @@ -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;
}

Expand Down

0 comments on commit de0a869

Please sign in to comment.