exceptionfactory commented on code in PR #9605:
URL: https://github.com/apache/nifi/pull/9605#discussion_r1904301832
##########
nifi-extension-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service-api/src/main/java/org/apache/nifi/elasticsearch/ElasticSearchClientService.java:
##########
@@ -103,6 +124,16 @@ public interface ElasticSearchClientService extends
ControllerService, Verifiabl
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ PropertyDescriptor JWT_SHARED_SECRET = new PropertyDescriptor.Builder()
+ .name("jwt-shared-secret")
+ .displayName("JWT Shared Secret")
+ .description("JWT realm Shared Secret.")
+ .dependsOn(AUTHORIZATION_SCHEME, AuthorizationScheme.JWT)
+ .required(false)
Review Comment:
This can be changed to `required(true)` and the custom validation can be
removed. Also recommend moving this directly under the `OAuth2 Access Token
Provider` property.
```suggestion
.required(true)
```
##########
nifi-extension-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/pom.xml:
##########
@@ -83,23 +87,11 @@
<artifactId>jackson-annotations</artifactId>
<scope>provided</scope>
</dependency>
- <dependency>
- <groupId>commons-io</groupId>
- <artifactId>commons-io</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-compress</artifactId>
- </dependency>
Review Comment:
Are these dependencies not used?
##########
nifi-extension-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/main/java/org/apache/nifi/elasticsearch/ElasticSearchClientServiceImpl.java:
##########
@@ -153,6 +160,23 @@ protected Collection<ValidationResult>
customValidate(final ValidationContext va
);
}
+ if (authorizationScheme == AuthorizationScheme.JWT) {
+ if (oAuth2Provider == null) {
+ results.add(new
ValidationResult.Builder().subject(OAUTH2_ACCESS_TOKEN_PROVIDER.getName()).valid(false)
+ .explanation(String.format("if '%s' is '%s' then '%s'
must be set.",
+ AUTHORIZATION_SCHEME.getDisplayName(),
authorizationScheme.getDisplayName(),
OAUTH2_ACCESS_TOKEN_PROVIDER.getDisplayName())
+ ).build()
+ );
+ }
+ if (!jwtSharedSecretSet) {
+ results.add(new
ValidationResult.Builder().subject(JWT_SHARED_SECRET.getName()).valid(false)
+ .explanation(String.format("if '%s' is '%s' then '%s'
must be set.",
+ AUTHORIZATION_SCHEME.getDisplayName(),
authorizationScheme.getDisplayName(), JWT_SHARED_SECRET.getDisplayName())
+ ).build()
+ );
+ }
+ }
Review Comment:
This can be be removed given the use of dependent properties.
##########
nifi-extension-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service/src/main/java/org/apache/nifi/elasticsearch/ElasticSearchClientServiceImpl.java:
##########
@@ -102,10 +104,12 @@ public class ElasticSearchClientServiceImpl extends
AbstractControllerService im
private ObjectMapper mapper;
- private static final List<PropertyDescriptor> properties =
List.of(HTTP_HOSTS, PATH_PREFIX, AUTHORIZATION_SCHEME, USERNAME, PASSWORD,
API_KEY_ID, API_KEY,
- PROP_SSL_CONTEXT_SERVICE, PROXY_CONFIGURATION_SERVICE,
CONNECT_TIMEOUT, SOCKET_TIMEOUT, CHARSET,
- SUPPRESS_NULLS, COMPRESSION, SEND_META_HEADER, STRICT_DEPRECATION,
NODE_SELECTOR, SNIFF_CLUSTER_NODES,
- SNIFFER_INTERVAL, SNIFFER_REQUEST_TIMEOUT, SNIFF_ON_FAILURE,
SNIFFER_FAILURE_DELAY);
+ private static final List<PropertyDescriptor> properties =
List.of(HTTP_HOSTS, PATH_PREFIX, AUTHORIZATION_SCHEME, USERNAME, PASSWORD,
+ API_KEY_ID, API_KEY, JWT_SHARED_SECRET,
OAUTH2_ACCESS_TOKEN_PROVIDER, RUN_AS_USER, PROP_SSL_CONTEXT_SERVICE,
PROXY_CONFIGURATION_SERVICE,
+ CONNECT_TIMEOUT, SOCKET_TIMEOUT, CHARSET, SUPPRESS_NULLS,
COMPRESSION, SEND_META_HEADER, STRICT_DEPRECATION, NODE_SELECTOR,
+ SNIFF_CLUSTER_NODES, SNIFFER_INTERVAL, SNIFFER_REQUEST_TIMEOUT,
SNIFF_ON_FAILURE, SNIFFER_FAILURE_DELAY);
Review Comment:
This is a good opportunity to reformat these properties and list one per
line for easier readability.
##########
nifi-extension-bundles/nifi-elasticsearch-bundle/nifi-elasticsearch-client-service-api/src/main/java/org/apache/nifi/elasticsearch/ElasticSearchClientService.java:
##########
@@ -62,6 +64,25 @@ public interface ElasticSearchClientService extends
ControllerService, Verifiabl
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.build();
+ PropertyDescriptor OAUTH2_ACCESS_TOKEN_PROVIDER = new
PropertyDescriptor.Builder()
+ .name("el-cs-oauth2-token-provider")
+ .displayName("OAuth2 Access Token Provider")
+ .description("The OAuth2 Access Token Provider used to provide
JWTs for Bearer Token Authorization with Elasticsearch.")
+ .dependsOn(AUTHORIZATION_SCHEME, AuthorizationScheme.JWT)
+ .required(false)
Review Comment:
```suggestion
.required(true)
```
--
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]
