diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index d407b1654f5b5..6fe1dd3695986 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -101,7 +101,6 @@ void AzureOptions::ExtractFromUriSchemeAndHierPart(const Uri& uri, } Status AzureOptions::ExtractFromUriQuery(const Uri& uri) { - const auto account_key = uri.password(); std::optional credential_kind; std::optional credential_kind_value; std::string tenant_id; @@ -155,10 +154,6 @@ Status AzureOptions::ExtractFromUriQuery(const Uri& uri) { } if (credential_kind) { - if (!account_key.empty()) { - return Status::Invalid("Password must not be specified with credential_kind=", - *credential_kind_value); - } if (!tenant_id.empty()) { return Status::Invalid("tenant_id must not be specified with credential_kind=", *credential_kind_value); @@ -190,40 +185,25 @@ Status AzureOptions::ExtractFromUriQuery(const Uri& uri) { break; } } else { - if (!account_key.empty()) { - // With password - if (!tenant_id.empty()) { - return Status::Invalid("tenant_id must not be specified with password"); - } - if (!client_id.empty()) { - return Status::Invalid("client_id must not be specified with password"); - } - if (!client_secret.empty()) { - return Status::Invalid("client_secret must not be specified with password"); + if (tenant_id.empty() && client_id.empty() && client_secret.empty()) { + // No related parameters + if (account_name.empty()) { + RETURN_NOT_OK(ConfigureAnonymousCredential()); + } else { + // Default credential } - RETURN_NOT_OK(ConfigureAccountKeyCredential(account_key)); } else { - // Without password - if (tenant_id.empty() && client_id.empty() && client_secret.empty()) { - // No related parameters - if (account_name.empty()) { - RETURN_NOT_OK(ConfigureAnonymousCredential()); - } else { - // Default credential - } + // One or more tenant_id, client_id or client_secret are specified + if (client_id.empty()) { + return Status::Invalid("client_id must be specified"); + } + if (tenant_id.empty() && client_secret.empty()) { + RETURN_NOT_OK(ConfigureManagedIdentityCredential(client_id)); + } else if (!tenant_id.empty() && !client_secret.empty()) { + RETURN_NOT_OK( + ConfigureClientSecretCredential(tenant_id, client_id, client_secret)); } else { - // One or more tenant_id, client_id or client_secret are specified - if (client_id.empty()) { - return Status::Invalid("client_id must be specified"); - } - if (tenant_id.empty() && client_secret.empty()) { - RETURN_NOT_OK(ConfigureManagedIdentityCredential(client_id)); - } else if (!tenant_id.empty() && !client_secret.empty()) { - RETURN_NOT_OK( - ConfigureClientSecretCredential(tenant_id, client_id, client_secret)); - } else { - return Status::Invalid("Both of tenant_id and client_secret must be specified"); - } + return Status::Invalid("Both of tenant_id and client_secret must be specified"); } } } diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index ebbe00c4ee784..c5e5091256959 100644 --- a/cpp/src/arrow/filesystem/azurefs.h +++ b/cpp/src/arrow/filesystem/azurefs.h @@ -144,12 +144,10 @@ struct ARROW_EXPORT AzureOptions { /// /// Supported formats: /// - /// 1. abfs[s]://[:\@]\.blob.core.windows.net - /// [/\[/\]] - /// 2. abfs[s]://\[:\]\@\.dfs.core.windows.net[/path] - /// 3. abfs[s]://[\]@]\[\<:port\>] - /// [/\[/path]] - /// 4. abfs[s]://[\]@]\[/path] + /// 1. abfs[s]://\.blob.core.windows.net[/\[/\]] + /// 2. abfs[s]://\\@\.dfs.core.windows.net[/path] + /// 3. abfs[s]://[\[\<:port\>][/\[/path]] + /// 4. abfs[s]://[\[/path] /// /// (1) and (2) are compatible with the Azure Data Lake Storage Gen2 URIs /// [1], (3) is for Azure Blob Storage compatible service including Azurite, diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index a8dc923476752..a8087bfc16d78 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -562,16 +562,15 @@ class TestAzureOptions : public ::testing::Test { void TestFromUriAbfs() { std::string path; - ASSERT_OK_AND_ASSIGN( - auto options, - AzureOptions::FromUri( - "abfs://account:password@127.0.0.1:10000/container/dir/blob", &path)); + ASSERT_OK_AND_ASSIGN(auto options, + AzureOptions::FromUri( + "abfs://account@127.0.0.1:10000/container/dir/blob", &path)); ASSERT_EQ(options.account_name, "account"); ASSERT_EQ(options.blob_storage_authority, "127.0.0.1:10000"); ASSERT_EQ(options.dfs_storage_authority, "127.0.0.1:10000"); ASSERT_EQ(options.blob_storage_scheme, "https"); ASSERT_EQ(options.dfs_storage_scheme, "https"); - ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kStorageSharedKey); + ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kDefault); ASSERT_EQ(path, "container/dir/blob"); ASSERT_EQ(options.background_writes, true); } @@ -579,43 +578,42 @@ class TestAzureOptions : public ::testing::Test { void TestFromUriAbfss() { std::string path; ASSERT_OK_AND_ASSIGN( - auto options, - AzureOptions::FromUri( - "abfss://account:password@127.0.0.1:10000/container/dir/blob", &path)); + auto options, AzureOptions::FromUri( + "abfss://account@127.0.0.1:10000/container/dir/blob", &path)); ASSERT_EQ(options.account_name, "account"); ASSERT_EQ(options.blob_storage_authority, "127.0.0.1:10000"); ASSERT_EQ(options.dfs_storage_authority, "127.0.0.1:10000"); ASSERT_EQ(options.blob_storage_scheme, "https"); ASSERT_EQ(options.dfs_storage_scheme, "https"); - ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kStorageSharedKey); + ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kDefault); ASSERT_EQ(path, "container/dir/blob"); ASSERT_EQ(options.background_writes, true); } void TestFromUriEnableTls() { std::string path; - ASSERT_OK_AND_ASSIGN(auto options, - AzureOptions::FromUri( - "abfs://account:password@127.0.0.1:10000/container/dir/blob?" - "enable_tls=false", - &path)); + ASSERT_OK_AND_ASSIGN( + auto options, + AzureOptions::FromUri("abfs://account@127.0.0.1:10000/container/dir/blob?" + "enable_tls=false", + &path)); ASSERT_EQ(options.account_name, "account"); ASSERT_EQ(options.blob_storage_authority, "127.0.0.1:10000"); ASSERT_EQ(options.dfs_storage_authority, "127.0.0.1:10000"); ASSERT_EQ(options.blob_storage_scheme, "http"); ASSERT_EQ(options.dfs_storage_scheme, "http"); - ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kStorageSharedKey); + ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kDefault); ASSERT_EQ(path, "container/dir/blob"); ASSERT_EQ(options.background_writes, true); } void TestFromUriDisableBackgroundWrites() { std::string path; - ASSERT_OK_AND_ASSIGN(auto options, - AzureOptions::FromUri( - "abfs://account:password@127.0.0.1:10000/container/dir/blob?" - "background_writes=false", - &path)); + ASSERT_OK_AND_ASSIGN( + auto options, + AzureOptions::FromUri("abfs://account@127.0.0.1:10000/container/dir/blob?" + "background_writes=false", + &path)); ASSERT_EQ(options.background_writes, false); } @@ -637,15 +635,6 @@ class TestAzureOptions : public ::testing::Test { ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kAnonymous); } - void TestFromUriCredentialStorageSharedKey() { - ASSERT_OK_AND_ASSIGN( - auto options, - AzureOptions::FromUri( - "abfs://:password@account.blob.core.windows.net/container/dir/blob", - nullptr)); - ASSERT_EQ(options.credential_kind_, AzureOptions::CredentialKind::kStorageSharedKey); - } - void TestFromUriCredentialClientSecret() { ASSERT_OK_AND_ASSIGN( auto options, @@ -767,9 +756,6 @@ TEST_F(TestAzureOptions, FromUriDisableBackgroundWrites) { } TEST_F(TestAzureOptions, FromUriCredentialDefault) { TestFromUriCredentialDefault(); } TEST_F(TestAzureOptions, FromUriCredentialAnonymous) { TestFromUriCredentialAnonymous(); } -TEST_F(TestAzureOptions, FromUriCredentialStorageSharedKey) { - TestFromUriCredentialStorageSharedKey(); -} TEST_F(TestAzureOptions, FromUriCredentialClientSecret) { TestFromUriCredentialClientSecret(); }