turcsanyip commented on code in PR #10482:
URL: https://github.com/apache/nifi/pull/10482#discussion_r2570061962
##########
nifi-extension-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/eventhub/ConsumeAzureEventHub.java:
##########
@@ -527,9 +539,42 @@ protected Collection<ValidationResult>
customValidate(ValidationContext validati
.valid(false)
.build());
}
+ if (blobIdentityFederationProviderSet) {
+ results.add(new ValidationResult.Builder()
+
.subject(BLOB_STORAGE_IDENTITY_FEDERATION_TOKEN_PROVIDER.getDisplayName())
+ .explanation("%s must not be set when %s is %s."
+
.formatted(BLOB_STORAGE_IDENTITY_FEDERATION_TOKEN_PROVIDER.getDisplayName(),
+
BLOB_STORAGE_AUTHENTICATION_STRATEGY.getDisplayName(),
+
BlobStorageAuthenticationStrategy.OAUTH2.getDisplayName()))
+ .valid(false)
+ .build());
+ }
+ } else if (blobStorageAuthenticationStrategy ==
BlobStorageAuthenticationStrategy.IDENTITY_FEDERATION) {
+ if (StringUtils.isNotBlank(storageAccountKey)) {
+ results.add(new ValidationResult.Builder()
+ .subject(STORAGE_ACCOUNT_KEY.getDisplayName())
+ .explanation("%s must not be set when %s is %s."
+
.formatted(STORAGE_ACCOUNT_KEY.getDisplayName(),
+
BLOB_STORAGE_AUTHENTICATION_STRATEGY.getDisplayName(),
+
BlobStorageAuthenticationStrategy.IDENTITY_FEDERATION.getDisplayName()))
+ .valid(false)
+ .build());
+ }
Review Comment:
@pvillard31 Thanks for addig identity federation support in Azure
processors. My plan is to review it in more detail.
Just an initial question: Are these extensive validation steps really
needed? I mean the authentication related properties are controlled by the
`Authentication Strategy` property: only the relevant properties are shown and
it does not matter what values the other properties contain. If we keep the
validation steps as well, then the user will get warnings related to properties
that are not visible and have no effect when the processor runs.
For example:
1. The user chooses `Authentication Strategy = Storage Account Key` and
fills in `Storage Account Key` property
2. Then they change their mind and select `Authentication Strategy =
Identity Federation` and provide the configuration
3. After Apply, they get a validation error saying "Storage Account Key must
not be set when Blob Storage Authentication Strategy is Identity Federation."
4. So they have to go back to the other option, clear the fields, and choose
the desired option again which
I believe it is not the ideal user experience. However, there may be other
advantages to keeping the `customValidate()` rules that I'm not aware of.
--
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]
