Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIFI-10919 Correct SCRAM SASL Mechanism for Kafka Components #6743

Closed
wants to merge 1 commit into from

Conversation

exceptionfactory
Copy link
Contributor

Summary

NIFI-10919 Corrects Kafka component handling of SCRAM-SHA-256 and SCRAM-SHA-512 SASL Mechanism property handling as a result of changes introduced in NIFI-10819.

The changes replace the use of SaslMechanism.valueOf() with SaslMechanism.getSaslMechanism() to find the matching property value instead of the enumeration name, which does not match property values. Additional changes include new unit tests to validate property handling in referencing classes.

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

@Kagee
Copy link

Kagee commented Dec 1, 2022

No need to check for Java 8 since it is deprecated for 1.19?

Support for Java 8 is deprecated: Java 11.0.16 is the minimum recommended version
https://cwiki.apache.org/confluence/display/NIFI/Release+Notes#ReleaseNotes-Version1.19.0

@exceptionfactory
Copy link
Contributor Author

@Kagee NiFi version 1 releases continue to support Java 8, although support is deprecated in 1.19.0. NiFi 2.0 will remove support for Java 8, but it remains supported for now.

final Class<?> propertyClass = propertyClassFound.get();
final Set<String> classPropertyNames = getStaticStringPropertyNames(propertyClass);
propertyNames.addAll(classPropertyNames);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

should there be an else clause here to flag the bad value (logger::warn)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a best effort approach, based on Kafka classes available at runtime. This influences the custom property validation, so any issues would be flagged as validation problems. For that reason, adding logging here seems unnecessary.

Copy link
Contributor

@greyp9 greyp9 left a comment

Choose a reason for hiding this comment

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

This looks good. Did some testing against a vanilla Kafka docker image. I can see the updated NiFi configuration settings being passed through to Kafka via the NiFi app log. As this was an enum constant issue, this seems sufficient to verify.

@greyp9
Copy link
Contributor

greyp9 commented Dec 1, 2022

...
	sasl.login.refresh.window.factor = 0.8
	sasl.login.refresh.window.jitter = 0.05
	sasl.mechanism = GSSAPI
	security.protocol = PLAINTEXT
	security.providers = null
...
	sasl.login.refresh.window.factor = 0.8
	sasl.login.refresh.window.jitter = 0.05
	sasl.mechanism = SCRAM-SHA-256
	security.protocol = PLAINTEXT
	security.providers = null
...
	sasl.login.refresh.window.factor = 0.8
	sasl.login.refresh.window.jitter = 0.05
	sasl.mechanism = SCRAM-SHA-512
	security.protocol = PLAINTEXT
	security.providers = null
...

@greyp9 greyp9 closed this in c3b0e1a Dec 1, 2022
asfgit pushed a commit that referenced this pull request Dec 1, 2022
This closes #6743
Signed-off-by: Paul Grey <greyp@apache.org>
lizhizhou pushed a commit to lizhizhou/nifi that referenced this pull request Jan 2, 2023
This closes apache#6743
Signed-off-by: Paul Grey <greyp@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants