exceptionfactory commented on code in PR #11189:
URL: https://github.com/apache/nifi/pull/11189#discussion_r3176951412
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/rds/AwsRdsIamDatabasePasswordProvider.java:
##########
@@ -95,22 +105,31 @@ public void onEnabled(final ConfigurationContext context) {
awsCredentialsProvider =
credentialsService.getAwsCredentialsProvider();
awsRegion = getRegion(context);
rdsUtilities = createRdsUtilities(awsRegion, awsCredentialsProvider);
+ rdsEndpoint = getRdsEndpoint(context);
}
@OnDisabled
public void onDisabled() {
awsCredentialsProvider = null;
rdsUtilities = null;
awsRegion = null;
+ rdsEndpoint = null;
}
@Override
public char[] getPassword(final DatabasePasswordRequestContext
requestContext) {
Objects.requireNonNull(requestContext, "Database Password Request
Context required");
- final ParsedEndpoint parsedEndpoint =
parseEndpoint(requestContext.getJdbcUrl());
- final String hostname = resolveHostname(parsedEndpoint,
requestContext.getJdbcUrl());
- final int port = resolvePort(parsedEndpoint);
+ final String hostname;
+ final int port;
+ if (rdsEndpoint != null) {
Review Comment:
Minor styling, I recommend reversing the logic so that it reads: `if
(rdsEndpoint == null) { ... } else { ... }`
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/rds/AwsRdsIamDatabasePasswordProvider.java:
##########
@@ -60,15 +61,24 @@ public class AwsRdsIamDatabasePasswordProvider extends
AbstractControllerService
.required(true)
.build();
+ static final PropertyDescriptor RDS_ENDPOINT = new
PropertyDescriptor.Builder()
+ .name("RDS Endpoint")
Review Comment:
For clarity of behavior, it seems like this should be named something like
`Token Request Endpoint` or `Token Request RDS Endpoint`.
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/rds/AwsRdsIamDatabasePasswordProvider.java:
##########
@@ -160,6 +179,22 @@ private String resolveUsername(final String contextUser) {
return username;
}
+ private ParsedEndpoint getRdsEndpoint(final ConfigurationContext context) {
+ return context.getProperty(RDS_ENDPOINT).isSet()
+ ?
parseRdsEndpoint(context.getProperty(RDS_ENDPOINT).getValue())
+ : null;
+ }
+
+ private ParsedEndpoint parseRdsEndpoint(final String endpoint) {
+ try {
+ // Prepend a dummy scheme to delegate host and port parsing to the
standard URI parser
+ final URI uri = URI.create("tcp://" + endpoint);
+ return new ParsedEndpoint(uri.getHost(), uri.getPort() >= 0 ?
uri.getPort() : null);
Review Comment:
The `port` should be required, so the ternary check seems unnecessary.
```suggestion
return new ParsedEndpoint(uri.getHost(), uri.getPort());
```
##########
nifi-extension-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/rds/AwsRdsIamDatabasePasswordProvider.java:
##########
@@ -160,6 +179,22 @@ private String resolveUsername(final String contextUser) {
return username;
}
+ private ParsedEndpoint getRdsEndpoint(final ConfigurationContext context) {
+ return context.getProperty(RDS_ENDPOINT).isSet()
+ ?
parseRdsEndpoint(context.getProperty(RDS_ENDPOINT).getValue())
+ : null;
+ }
+
+ private ParsedEndpoint parseRdsEndpoint(final String endpoint) {
+ try {
+ // Prepend a dummy scheme to delegate host and port parsing to the
standard URI parser
+ final URI uri = URI.create("tcp://" + endpoint);
Review Comment:
I recommend creating a format for the endpoint URI to be used as follows:
```suggestion
final URI uri =
URI.create(URI_ENDPOINT_FORMAT.formatted(endpoint));
```
--
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]
