turcsanyip commented on a change in pull request #4843:
URL: https://github.com/apache/nifi/pull/4843#discussion_r583125355
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/ADLSCredentialsControllerService.java
##########
@@ -74,12 +75,56 @@
.addValidator(StandardValidators.BOOLEAN_VALIDATOR)
.build();
+ public static final PropertyDescriptor SERVICE_PRINCIPAL_TENANT_ID = new
PropertyDescriptor.Builder()
+ .name("service-principal-tenant-id")
+ .displayName("Service Principal Tenant ID")
+ .description("Tenant ID of the Azure Active Directory hosting the
Service Principal. The property is required when Service Principal
authentication is used.")
+ .sensitive(true)
+ .required(false)
+ .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+
.expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
Review comment:
The concept of supporting EL for the sensitive `Account Name / Key` came
from the Blob credential service (the Blob counterpart of this CS) and from the
use case described by @jfrazee. I followed this "pattern" with the new Service
Principal properties.
However, there was a bug in this ADLS controller service and ELs were not
evaluated at all (this PR would fix it). So at the moment nobody can use the
ADLS service with EL and therefore removing EL support for `Account Name / Key`
would not arise backward compatibility questions here.
I think we can safely go ahead with removing EL support from all (old and
new) sensitive properties if that is preferred. We can add it later if needed.
Implementing a lookup service (like
`AzureStorageCredentialsControllerServiceLookup`) is also an option. It can
support multiple credentials within the same flow (though it cannot be so
dynamic as EL).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
