lordgamez commented on code in PR #2016:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2016#discussion_r2329841490
##########
extensions/azure/controllerservices/AzureStorageCredentialsService.h:
##########
@@ -42,33 +43,44 @@ class AzureStorageCredentialsService : public
core::controller::ControllerServic
.build();
EXTENSIONAPI static constexpr auto StorageAccountKey =
core::PropertyDefinitionBuilder<>::createProperty("Storage Account Key")
.withDescription("The storage account key. This is an admin-like
password providing access to every container in this account. "
- "It is recommended one uses Shared Access Signature (SAS) token
instead for fine-grained control with policies.")
+ "It is recommended one uses Shared Access Signature (SAS) token
instead for fine-grained control with policies "
+ "if Credential Configuration Strategy is set to From Properties. If
set, SAS Token must be empty.")
.isSensitive(true)
.build();
EXTENSIONAPI static constexpr auto SASToken =
core::PropertyDefinitionBuilder<>::createProperty("SAS Token")
- .withDescription("Shared Access Signature token. Specify either SAS
Token (recommended) or Storage Account Key together with Storage Account Name
if Managed Identity is not used.")
+ .withDescription("Shared Access Signature token. Specify either SAS
Token (recommended) or Storage Account Key together with Storage Account Name "
+ "if Credential Configuration Strategy is set to From Properties. If
set, Storage Account Key must be empty.")
.isSensitive(true)
.build();
EXTENSIONAPI static constexpr auto CommonStorageAccountEndpointSuffix =
core::PropertyDefinitionBuilder<>::createProperty("Common Storage Account
Endpoint Suffix")
.withDescription("Storage accounts in public Azure always use a common
FQDN suffix. Override this endpoint suffix with a "
"different suffix in certain circumstances (like Azure Stack or
non-public Azure regions).")
.build();
EXTENSIONAPI static constexpr auto ConnectionString =
core::PropertyDefinitionBuilder<>::createProperty("Connection String")
- .withDescription("Connection string used to connect to Azure Storage
service. This overrides all other set credential properties if Managed Identity
is not used.")
+ .withDescription("Connection string used to connect to Azure Storage
service. This overrides all other set credential properties "
+ "if Credential Configuration Strategy is set to From Properties.")
.build();
- EXTENSIONAPI static constexpr auto UseManagedIdentityCredentials =
core::PropertyDefinitionBuilder<>::createProperty("Use Managed Identity
Credentials")
- .withDescription("If true Managed Identity credentials will be used
together with the Storage Account Name for authentication.")
+ EXTENSIONAPI static constexpr auto CredentialConfigurationStrategy =
+
core::PropertyDefinitionBuilder<magic_enum::enum_count<CredentialConfigurationStrategyOption>()>::createProperty("Credential
Configuration Strategy")
+ .withDescription("The strategy to use for credential configuration. If
set to From Properties, the credentials are parsed from the SAS Token, Storage
Account Key, "
+ "and Connection String properties. In other cases, the
selected Azure identity source is used.")
Review Comment:
Updated in
https://github.com/apache/nifi-minifi-cpp/pull/2016/commits/fa8b04f30e3b383886bd9004babe21fa327ac070
##########
extensions/azure/controllerservices/AzureStorageCredentialsService.cpp:
##########
@@ -39,13 +43,13 @@ void AzureStorageCredentialsService::onEnable() {
credentials_.setSasToken(*sas_token);
}
if (auto common_storage_account_endpoint_suffix =
getProperty(CommonStorageAccountEndpointSuffix.name)) {
- credentials_.setEndpontSuffix(*common_storage_account_endpoint_suffix);
+ credentials_.setEndpointSuffix(*common_storage_account_endpoint_suffix);
}
if (auto connection_String = getProperty(ConnectionString.name)) {
credentials_.setConnectionString(*connection_String);
}
- if (auto use_managed_identity_credentials =
getProperty(UseManagedIdentityCredentials.name) |
utils::andThen(parsing::parseBool)) {
-
credentials_.setUseManagedIdentityCredentials(*use_managed_identity_credentials);
+ if (auto managed_identity_client_id =
getProperty(ManagedIdentityClientId.name)) {
+ credentials_.setManagedIdentityClientId(*managed_identity_client_id);
Review Comment:
Replacing the old `Use Managed Identity Credentials` property with the new
`Credential Configuration Strategy` is a breaking change, adding the new
`Managed Identity Client Id` property is new a feature for using user assigned
managed identity, that is not a breaking change just an additional option.
##########
extensions/azure/storage/AzureBlobStorageClient.cpp:
##########
@@ -56,13 +56,13 @@ AzureBlobStorageClient::AzureBlobStorageClient() {
}
Azure::Storage::Blobs::BlobContainerClient
AzureBlobStorageClient::createClient(const AzureStorageCredentials
&credentials, const std::string &container_name) {
- if (credentials.getUseManagedIdentityCredentials()) {
- auto storage_client = Azure::Storage::Blobs::BlobServiceClient(
- "https://"; + credentials.getStorageAccountName() + ".blob." +
credentials.getEndpointSuffix(),
std::make_shared<Azure::Identity::ManagedIdentityCredential>());
- return storage_client.GetBlobContainerClient(container_name);
- } else {
+ if (credentials.getCredentialConfigurationStrategy() ==
CredentialConfigurationStrategyOption::FromProperties) {
return
Azure::Storage::Blobs::BlobContainerClient::CreateFromConnectionString(credentials.buildConnectionString(),
container_name);
}
+
+ auto storage_client = Azure::Storage::Blobs::BlobServiceClient("https://"; +
credentials.getStorageAccountName() + ".blob." +
credentials.getEndpointSuffix(),
+ credentials.createAzureTokenCredential());
Review Comment:
Updated in
https://github.com/apache/nifi-minifi-cpp/pull/2016/commits/fa8b04f30e3b383886bd9004babe21fa327ac070
##########
extensions/azure/storage/AzureDataLakeStorageClient.cpp:
##########
@@ -42,14 +42,14 @@
std::unique_ptr<Azure::Storage::Files::DataLake::DataLakeFileSystemClient> Azure
options.Retry.MaxRetries = gsl::narrow<int32_t>(*number_of_retries);
}
- if (credentials.getUseManagedIdentityCredentials()) {
- auto datalake_service_client =
Azure::Storage::Files::DataLake::DataLakeServiceClient(
- "https://"; + credentials.getStorageAccountName() + ".dfs." +
credentials.getEndpointSuffix(),
std::make_shared<Azure::Identity::ManagedIdentityCredential>(), options);
- return
std::make_unique<Azure::Storage::Files::DataLake::DataLakeFileSystemClient>(datalake_service_client.GetFileSystemClient(file_system_name));
- } else {
+ if (credentials.getCredentialConfigurationStrategy() ==
CredentialConfigurationStrategyOption::FromProperties) {
return
std::make_unique<Azure::Storage::Files::DataLake::DataLakeFileSystemClient>(
-
Azure::Storage::Files::DataLake::DataLakeFileSystemClient::CreateFromConnectionString(credentials.buildConnectionString(),
file_system_name, options));
+
Azure::Storage::Files::DataLake::DataLakeFileSystemClient::CreateFromConnectionString(credentials.buildConnectionString(),
file_system_name, options));
Review Comment:
Updated in
https://github.com/apache/nifi-minifi-cpp/pull/2016/commits/fa8b04f30e3b383886bd9004babe21fa327ac070
##########
extensions/azure/storage/AzureDataLakeStorageClient.cpp:
##########
@@ -42,14 +42,14 @@
std::unique_ptr<Azure::Storage::Files::DataLake::DataLakeFileSystemClient> Azure
options.Retry.MaxRetries = gsl::narrow<int32_t>(*number_of_retries);
}
- if (credentials.getUseManagedIdentityCredentials()) {
- auto datalake_service_client =
Azure::Storage::Files::DataLake::DataLakeServiceClient(
- "https://"; + credentials.getStorageAccountName() + ".dfs." +
credentials.getEndpointSuffix(),
std::make_shared<Azure::Identity::ManagedIdentityCredential>(), options);
- return
std::make_unique<Azure::Storage::Files::DataLake::DataLakeFileSystemClient>(datalake_service_client.GetFileSystemClient(file_system_name));
- } else {
+ if (credentials.getCredentialConfigurationStrategy() ==
CredentialConfigurationStrategyOption::FromProperties) {
return
std::make_unique<Azure::Storage::Files::DataLake::DataLakeFileSystemClient>(
-
Azure::Storage::Files::DataLake::DataLakeFileSystemClient::CreateFromConnectionString(credentials.buildConnectionString(),
file_system_name, options));
+
Azure::Storage::Files::DataLake::DataLakeFileSystemClient::CreateFromConnectionString(credentials.buildConnectionString(),
file_system_name, options));
}
+
+ auto datalake_service_client =
Azure::Storage::Files::DataLake::DataLakeServiceClient(
+ "https://"; + credentials.getStorageAccountName() + ".dfs." +
credentials.getEndpointSuffix(), credentials.createAzureTokenCredential(),
options);
Review Comment:
Updated in
https://github.com/apache/nifi-minifi-cpp/pull/2016/commits/fa8b04f30e3b383886bd9004babe21fa327ac070
##########
extensions/azure/storage/AzureStorageCredentials.cpp:
##########
@@ -90,16 +102,37 @@ std::string
AzureStorageCredentials::buildConnectionString() const {
}
bool AzureStorageCredentials::isValid() const {
- return (getUseManagedIdentityCredentials() &&
!getStorageAccountName().empty()) ||
- (!getUseManagedIdentityCredentials() &&
!buildConnectionString().empty());
+ return (credential_configuration_strategy_ !=
CredentialConfigurationStrategyOption::FromProperties &&
!getStorageAccountName().empty()) ||
+ (credential_configuration_strategy_ ==
CredentialConfigurationStrategyOption::FromProperties &&
!buildConnectionString().empty());
Review Comment:
Updated in
https://github.com/apache/nifi-minifi-cpp/pull/2016/commits/fa8b04f30e3b383886bd9004babe21fa327ac070
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
