NIFI-15890 Add Token Request Endpoint property to AwsRdsIamDatabasePasswordProvider#11189
Open
yagipy wants to merge 2 commits intoapache:mainfrom
Open
NIFI-15890 Add Token Request Endpoint property to AwsRdsIamDatabasePasswordProvider#11189yagipy wants to merge 2 commits intoapache:mainfrom
yagipy wants to merge 2 commits intoapache:mainfrom
Conversation
…ider Added optional RDS Endpoint property to override hostname and port used for IAM token signing when JDBC URL points to an intermediary such as a Network Load Balancer in front of an RDS Proxy Signed-off-by: yagipy <email@yagipy.me>
exceptionfactory
requested changes
May 2, 2026
Contributor
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for proposing this optional property @yagipy. The general approach looks good, I recommend a few implementation and naming adjustments.
Comment on lines
+190
to
+191
| // Prepend a dummy scheme to delegate host and port parsing to the standard URI parser | ||
| final URI uri = URI.create("tcp://" + endpoint); |
Contributor
There was a problem hiding this comment.
I recommend creating a format for the endpoint URI to be used as follows:
Suggested change
| // Prepend a dummy scheme to delegate host and port parsing to the standard URI parser | |
| final URI uri = URI.create("tcp://" + endpoint); | |
| final URI uri = URI.create(URI_ENDPOINT_FORMAT.formatted(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); |
Contributor
There was a problem hiding this comment.
The port should be required, so the ternary check seems unnecessary.
Suggested change
| return new ParsedEndpoint(uri.getHost(), uri.getPort() >= 0 ? uri.getPort() : null); | |
| return new ParsedEndpoint(uri.getHost(), uri.getPort()); |
| final int port = resolvePort(parsedEndpoint); | ||
| final String hostname; | ||
| final int port; | ||
| if (rdsEndpoint != null) { |
Contributor
There was a problem hiding this comment.
Minor styling, I recommend reversing the logic so that it reads: if (rdsEndpoint == null) { ... } else { ... }
Comment on lines
+64
to
+65
| static final PropertyDescriptor RDS_ENDPOINT = new PropertyDescriptor.Builder() | ||
| .name("RDS Endpoint") |
Contributor
There was a problem hiding this comment.
For clarity of behavior, it seems like this should be named something like Token Request Endpoint or Token Request RDS Endpoint.
Author
There was a problem hiding this comment.
Renamed to Token Request Endpoint.
Author
|
Thank you for the review, @exceptionfactory. I have pushed a commit addressing all of your comments. |
yagipy
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NIFI-15890
Added an optional
Token Request Endpointproperty toAwsRdsIamDatabasePasswordProviderto support architectures where the JDBC URL points to an intermediary such as a Network Load Balancer in front of an RDS Proxy.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation