Skip to content

NIFI-16191: Add OAuth support for ConfluentSchemaRegistry controller service - #11534

Open
takraj wants to merge 2 commits into
apache:mainfrom
takraj:pr/NIFI-16191_oauth_confluent_schemareg
Open

NIFI-16191: Add OAuth support for ConfluentSchemaRegistry controller service#11534
takraj wants to merge 2 commits into
apache:mainfrom
takraj:pr/NIFI-16191_oauth_confluent_schemareg

Conversation

@takraj

@takraj takraj commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

NIFI-16191

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
  • Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using ./mvnw clean install -P contrib-check
    • JDK 21
    • JDK 25

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@takraj
takraj force-pushed the pr/NIFI-16191_oauth_confluent_schemareg branch 2 times, most recently from 27676d1 to c49c203 Compare August 13, 2026 08:57
@takraj
takraj force-pushed the pr/NIFI-16191_oauth_confluent_schemareg branch 3 times, most recently from 175cc27 to 16569f2 Compare August 26, 2026 12:40
@takraj
NIFI-16191: Add OAuth support for ConfluentSchemaRegistry controller …
730f9c1 
…service

Signed-off-by: Rajmund Takacs <takraj@gmail.com>
@takraj
takraj force-pushed the pr/NIFI-16191_oauth_confluent_schemareg branch from 16569f2 to 730f9c1 Compare August 26, 2026 14:35
exceptionfactory
exceptionfactory requested changes Aug 26, 2026
View reviewed changes

@exceptionfactory exceptionfactory left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for proposing this addition @takraj.

Add supporting for OAuth 2 requires careful lifecycle handling due to the limited expiration of Access Tokens. The current approach only retrieves the Access Token in the constructor, which will lead to expiration and permanent failure after a period of time, so a different approach is needed.

Comment thread .../src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java Outdated
this(baseUrls, timeoutMillis, sslContextProvider, logger, httpHeaders);

if (oauth2AccessTokenProvider != null) {
final String accessToken = oauth2AccessTokenProvider.getAccessDetails().getAccessToken();

@exceptionfactory exceptionfactory Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retrieving the Access Token once in the constructor will run into expiration issues over the lifecycle, so a different approach is needed.

@takraj takraj Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@exceptionfactory Good catch. I'll move this part to the applyRequestHeaders() then...

@takraj takraj Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ffea1b0

@takraj
Follow up: Acquire token on each request
ffea1b0 
Signed-off-by: Rajmund Takacs <takraj@gmail.com>
@takraj
takraj force-pushed the pr/NIFI-16191_oauth_confluent_schemareg branch from a09e9a2 to ffea1b0 Compare August 27, 2026 11:08
exceptionfactory
exceptionfactory requested changes Aug 27, 2026
View reviewed changes

@exceptionfactory exceptionfactory left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recent changes appear to have caused build timeouts on all platforms. There was a system issue with GitHub Actions yesterday, but I'm not seeing anything today. I recommend reviewing the changes to ensure there are no issues with tests based on the request token handling.

tpalfy
tpalfy approved these changes Aug 27, 2026
View reviewed changes

@tpalfy tpalfy left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1
Tested token refresh working properly as well.

@exceptionfactory

exceptionfactory commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

LGTM +1 Tested token refresh working properly as well.

The functional approach looks good now, but I would like to see successful workflow builds before approving and merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@exceptionfactory exceptionfactory exceptionfactory requested changes
@tpalfy tpalfy tpalfy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@takraj @exceptionfactory @tpalfy