Skip to content

Comments

NIFI-10882 prevent setting of both BASIC and API_KEY Authorization Scheme properties in ElasticSearchClientService#6722

Closed
ChrisSamo632 wants to merge 2 commits intoapache:mainfrom
ChrisSamo632:NIFI-10882
Closed

NIFI-10882 prevent setting of both BASIC and API_KEY Authorization Scheme properties in ElasticSearchClientService#6722
ChrisSamo632 wants to merge 2 commits intoapache:mainfrom
ChrisSamo632:NIFI-10882

Conversation

@ChrisSamo632
Copy link
Contributor

@ChrisSamo632 ChrisSamo632 commented Nov 28, 2022

Summary

NIFI-10882 prevent setting of both BASIC and API_KEY Authorization Scheme properties in ElasticSearchClientService

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 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 mvn clean install -P contrib-check
    • JDK 8
    • JDK 11
    • JDK 17

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

Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

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

Thanks for working on this issue @ChrisSamo632! This sounds similar to an issue with ListenSyslog and SSL Context Service, resolved in PR #6441.

This issue is slightly complicated by the handling of dependent properties in the framework and the UI. Based on the approach for ListenSyslog, however, the better solution seems to be setting the appropriate Authorization header based on the selected Authorization Scheme. That avoids the situation where the component is invalid, but the properties that need to be removed are not visible to the user.

ChrisSamo632 and others added 2 commits November 28, 2022 19:25
@ChrisSamo632
Copy link
Contributor Author

Thanks for working on this issue @ChrisSamo632! This sounds similar to an issue with ListenSyslog and SSL Context Service, resolved in PR #6441.
...

Yeah, I umm'd and ah'd a bit about which of the approaches to take, then decided I'd opt for replicating what was originally done for NIFI-10760 in #6619 (removed for NIFI-10776 in #6662, but flagged as a non-blocking issue during the 1.19.0 release vote).

Figured raising a PR would cause the discussion to decide which way to go. Another alternative would be for hidden properties to be reset to their default values (at the NiFi Framework level), but that seems like it should be a separate ticket with more considered discussion.

All that said... I've pushed the changes to remove the new customValidation and instead base the setting of Auth properties based upon the selected AUTHORIZATION_SCHEME, except for PKI and SSLContext as the latter may be needed even when not using PKI for mutual TLS-based IAAA.

Copy link
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

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

Thanks for giving it some thought and making the adjustments @ChrisSamo632, the updated approach looks good. +1 merging

asfgit pushed a commit that referenced this pull request Nov 30, 2022
…thorizationScheme

This closes #6722

Signed-off-by: David Handermann <exceptionfactory@apache.org>
lizhizhou pushed a commit to lizhizhou/nifi that referenced this pull request Jan 2, 2023
…thorizationScheme

This closes apache#6722

Signed-off-by: David Handermann <exceptionfactory@apache.org>
@ChrisSamo632 ChrisSamo632 deleted the NIFI-10882 branch January 23, 2023 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants