turcsanyip commented on a change in pull request #4369:
URL: https://github.com/apache/nifi/pull/4369#discussion_r450057671
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/AbstractAzureDataLakeStorageProcessor.java
##########
@@ -158,41 +173,51 @@
public static Collection<ValidationResult>
validateCredentialProperties(final ValidationContext validationContext) {
final List<ValidationResult> results = new ArrayList<>();
- final boolean useManagedIdentity =
validationContext.getProperty(USE_MANAGED_IDENTITY).asBoolean();
- final boolean accountKeyIsSet =
validationContext.getProperty(ACCOUNT_KEY).isSet();
- final boolean sasTokenIsSet =
validationContext.getProperty(SAS_TOKEN).isSet();
-
- int credential_config_found = 0;
- if(useManagedIdentity) credential_config_found++;
- if(accountKeyIsSet) credential_config_found++;
- if(sasTokenIsSet) credential_config_found++;
-
- if(credential_config_found == 0){
- final String msg = String.format(
- "At least one of ['%s', '%s', '%s'] should be set",
- ACCOUNT_KEY.getDisplayName(),
- SAS_TOKEN.getDisplayName(),
- USE_MANAGED_IDENTITY.getDisplayName()
- );
- results.add(new ValidationResult.Builder().subject("Credentials
config").valid(false).explanation(msg).build());
- } else if(credential_config_found > 1) {
- final String msg = String.format(
- "Only one of ['%s', '%s', '%s'] should be set",
- ACCOUNT_KEY.getDisplayName(),
- SAS_TOKEN.getDisplayName(),
- USE_MANAGED_IDENTITY.getDisplayName()
- );
- results.add(new ValidationResult.Builder().subject("Credentials
config").valid(false).explanation(msg).build());
+ if
(!validationContext.getProperty(AzureStorageUtils.STORAGE_CREDENTIALS_SERVICE).isSet())
{
Review comment:
When the CS defined, the processor level credential properties should
not be defined. It should be validated.
Another option would be to get rid of the processor level credential
properties. That would make the validation easier and the whole code simpler.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/AbstractAzureDataLakeStorageProcessor.java
##########
@@ -158,41 +173,51 @@
public static Collection<ValidationResult>
validateCredentialProperties(final ValidationContext validationContext) {
final List<ValidationResult> results = new ArrayList<>();
- final boolean useManagedIdentity =
validationContext.getProperty(USE_MANAGED_IDENTITY).asBoolean();
- final boolean accountKeyIsSet =
validationContext.getProperty(ACCOUNT_KEY).isSet();
- final boolean sasTokenIsSet =
validationContext.getProperty(SAS_TOKEN).isSet();
-
- int credential_config_found = 0;
- if(useManagedIdentity) credential_config_found++;
- if(accountKeyIsSet) credential_config_found++;
- if(sasTokenIsSet) credential_config_found++;
-
- if(credential_config_found == 0){
- final String msg = String.format(
- "At least one of ['%s', '%s', '%s'] should be set",
- ACCOUNT_KEY.getDisplayName(),
- SAS_TOKEN.getDisplayName(),
- USE_MANAGED_IDENTITY.getDisplayName()
- );
- results.add(new ValidationResult.Builder().subject("Credentials
config").valid(false).explanation(msg).build());
- } else if(credential_config_found > 1) {
- final String msg = String.format(
- "Only one of ['%s', '%s', '%s'] should be set",
- ACCOUNT_KEY.getDisplayName(),
- SAS_TOKEN.getDisplayName(),
- USE_MANAGED_IDENTITY.getDisplayName()
- );
- results.add(new ValidationResult.Builder().subject("Credentials
config").valid(false).explanation(msg).build());
+ if
(!validationContext.getProperty(AzureStorageUtils.STORAGE_CREDENTIALS_SERVICE).isSet())
{
+ final boolean useManagedIdentity =
validationContext.getProperty(USE_MANAGED_IDENTITY).asBoolean();
+ final boolean accountKeyIsSet =
validationContext.getProperty(ACCOUNT_KEY).isSet();
+ final boolean sasTokenIsSet =
validationContext.getProperty(SAS_TOKEN).isSet();
+
+ int credential_config_found = 0;
+ if (useManagedIdentity) credential_config_found++;
+ if (accountKeyIsSet) credential_config_found++;
+ if (sasTokenIsSet) credential_config_found++;
+
+ if (credential_config_found == 0) {
+ final String msg = String.format(
+ "At least one of ['%s', '%s', '%s'] should be set",
+ ACCOUNT_KEY.getDisplayName(),
+ SAS_TOKEN.getDisplayName(),
+ USE_MANAGED_IDENTITY.getDisplayName()
+ );
+ results.add(new
ValidationResult.Builder().subject("Credentials
config").valid(false).explanation(msg).build());
+ } else if (credential_config_found > 1) {
+ final String msg = String.format(
+ "Only one of ['%s', '%s', '%s'] should be set",
+ ACCOUNT_KEY.getDisplayName(),
+ SAS_TOKEN.getDisplayName(),
+ USE_MANAGED_IDENTITY.getDisplayName()
+ );
+ results.add(new
ValidationResult.Builder().subject("Credentials
config").valid(false).explanation(msg).build());
+ }
}
+
return results;
}
public static DataLakeServiceClient getStorageClient(PropertyContext
context, FlowFile flowFile) {
- final Map<String, String> attributes = flowFile != null ?
flowFile.getAttributes() : Collections.emptyMap();
- final String accountName =
context.getProperty(ACCOUNT_NAME).evaluateAttributeExpressions(attributes).getValue();
- final String accountKey =
context.getProperty(ACCOUNT_KEY).evaluateAttributeExpressions(attributes).getValue();
- final String sasToken =
context.getProperty(SAS_TOKEN).evaluateAttributeExpressions(attributes).getValue();
- final String endpointSuffix =
context.getProperty(ENDPOINT_SUFFIX).evaluateAttributeExpressions(attributes).getValue();
+ AzureStorageCredentialsDetails storageCredentialsDetails =
AzureStorageUtils.getStorageCredentialsDetails(context, flowFile);
+
+ final String accountName =
storageCredentialsDetails.getStorageAccountName();
+ final String accountKey = storageCredentialsDetails.getAccountKey();
+ final String sasToken = storageCredentialsDetails.getSasToken();
+ final AccessToken accessToken =
storageCredentialsDetails.getAccessToken();
+
+ // ControllerService - if set - may or may not have endpoint suffix
defined
+ final String endpointSuffix = Optional
+ .ofNullable(storageCredentialsDetails.getStorageSuffix())
+ .orElse(context.getProperty(ENDPOINT_SUFFIX).getValue());
Review comment:
The processor level property is not a fallback option. When the CS
defined, the processor level properties should be null (see my comment on
`validateCredentialProperties()`).
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/AbstractAzureDataLakeStorageProcessor.java
##########
@@ -60,7 +71,8 @@
.sensitive(true).build();
public static final PropertyDescriptor ACCOUNT_KEY = new
PropertyDescriptor.Builder()
- .name("storage-account-key").displayName("Storage Account Key")
+ .name(STORAGE_ACCOUNT_KEY_PROPERTY_DESCRIPTOR_NAME)
Review comment:
The property descriptor definition seems to be the same as in
`AzureStorageUtils`.
The same descriptors could be used here for the Account Key and SAS Token
(maybe for Endpoint Override too).
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/AbstractAzureDataLakeStorageProcessor.java
##########
@@ -204,6 +229,11 @@ public static DataLakeServiceClient
getStorageClient(PropertyContext context, Fl
} else if (StringUtils.isNotBlank(sasToken)) {
storageClient = new
DataLakeServiceClientBuilder().endpoint(endpoint).sasToken(sasToken)
.buildClient();
+ } else if (accessToken != null) {
+ TokenCredential credential = tokenRequestContext ->
Mono.just(accessToken);
+
+ storageClient = new
DataLakeServiceClientBuilder().endpoint(endpoint).credential(credential)
+ .buildClient();
} else if(useManagedIdentity){
Review comment:
Use Managed Identity option should be available on the CS too.
----------------------------------------------------------------
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:
us...@infra.apache.org
