KAFKA-15053: Use case insensitive validator for security.protocol config #13831

bogao007 · 2023-06-08T23:41:53Z

Fixed a regression described in KAFKA-15053 that security.protocol only allows uppercase values like PLAINTEXT, SSL, SASL_PLAINTEXT, SASL_SSL. With this fix, both lower case and upper case values will be supported (e.g. PLAINTEXT, SSL, SASL_PLAINTEXT, SASL_SSL, plaintext, ssl, sasl_plaintext, sasl_ssl)

Added new unit test to cover the case insensitive test case.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

divijvaidya

For consistency, we should allow case insensitive option at all configuration places for security protocol. e.g. at AdminClientConfig at

kafka/clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java

Line 226 in 7eea2a3

in(Utils.enumOptions(SecurityProtocol.class)),

and other places like ConsumerConfig, ProducerConfig etc.

We should also change the documentation to make it clear that this configuration is case insensitive.

streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java

…protocol config validation

bogao007 · 2023-06-10T00:24:07Z

For consistency, we should allow case insensitive option at all configuration places for security protocol. e.g. at AdminClientConfig at

kafka/clients/src/main/java/org/apache/kafka/clients/admin/AdminClientConfig.java

Line 226 in 7eea2a3

in(Utils.enumOptions(SecurityProtocol.class)),

and other places like ConsumerConfig, ProducerConfig etc.
We should also change the documentation to make it clear that this configuration is case insensitive.

Updated other places by comparing with the original PR, please help to take another look. Thanks! Also, what's the common process to update the documentation?

divijvaidya · 2023-06-14T13:40:28Z

Also, what's the common process to update the documentation?

As part of this PR, please modify the documentation at:

kafka/docs/security.html

Line 69 in 044d058

<p>Possible options for the security protocol are given below:</p>

bogao007 · 2023-06-21T23:44:47Z

@divijvaidya Added the doc changes, do you mind taking another look? Also, are we able to include this change to the 3.3.x minor release as well or it has to be in the next release (probably 3.5.1)?

divijvaidya

Left a minor nit. Yes, I will backport it to 3.3, 3.4 and 3.5 branches. This will be released with any new patch version release (if any) that we perform for these versions.

Also, we require a change at https://kafka.apache.org/documentation.html#adminclientconfigs_security.protocol, https://kafka.apache.org/documentation.html#brokerconfigs_security.inter.broker.protocol,
https://kafka.apache.org/documentation.html#streamsconfigs_security.protocol and all other configs that you have changed in this file. The change could mention case sensitivity similar to https://kafka.apache.org/documentation.html#connectconfigs_exactly.once.source.support

divijvaidya · 2023-06-26T14:35:24Z

docs/security.html

@@ -72,6 +72,10 @@ <h3 class="anchor-heading"><a id="listener_configuration" class="anchor-link"></
      <li>SSL</li>
      <li>SASL_PLAINTEXT</li>
      <li>SASL_SSL</li>
+      <li>plaintext</li>


Instead of adding all values, please consider the following:

Possible options (case insensitive) for the security protocol are given below:

C0urante · 2023-06-26T19:07:37Z

@divijvaidya we have logic to automatically generate these docs and populate our docs page with them during release; see here and here. I don't think we need to do anything beyond what's present in this PR to ensure that the docs are updated in the next release.

The case-insensitive validator will also automatically note that the accepted values are case insensitive (which is what's done for the exactly.once.source.support property for distributed Kafka Connect workers, for example).

divijvaidya · 2023-06-26T19:15:45Z

@C0urante ah! we have fancy stuff. Thanks for letting me know.

@bogao007 seems like we might not need the docs change for config after all. The only remaining fix is the small nit. We should be ready to merge (assuming sane CI tests) after that (unless @C0urante has some additional comments?)

C0urante · 2023-06-26T19:27:29Z

No additional comments from me! Agree with the suggested change to the security.html page.

bogao007 · 2023-06-28T17:31:03Z

Thanks @divijvaidya , I've updated the security doc, could you help take another look?

Also, it seems that my previous CI build failed with error [Checks API] No suitable checks publisher found. (link), it doesn't seem to be related to my PR, do you know how to fix the build? Thanks!

divijvaidya · 2023-06-29T08:10:20Z

@bogao007 you can click on the "tests" tab on the top right to see the test failures.

In the latest run(number 7) all test failures are unrelated and known to be flaky.

[Build / JDK 8 and Scala 2.12 / org.apache.kafka.common.network.SslTransportLayerTest.[1] tlsProtocol=TLSv1.2, useInlinePem=false](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/org.apache.kafka.common.network/SslTransportLayerTest/Build___JDK_8_and_Scala_2_12____1__tlsProtocol_TLSv1_2__useInlinePem_false/)
[Build / JDK 8 and Scala 2.12 / kafka.api.PlaintextConsumerTest.testListTopics()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/kafka.api/PlaintextConsumerTest/Build___JDK_8_and_Scala_2_12___testListTopics__/)
[Build / JDK 8 and Scala 2.12 / org.apache.kafka.tools.MetadataQuorumCommandTest.[1] Type=Raft-Combined, Name=testDescribeQuorumStatusSuccessful, MetadataVersion=3.6-IV0, Security=PLAINTEXT](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/org.apache.kafka.tools/MetadataQuorumCommandTest/Build___JDK_8_and_Scala_2_12____1__Type_Raft_Combined__Name_testDescribeQuorumStatusSuccessful__MetadataVersion_3_6_IV0__Security_PLAINTEXT_2/)
[Build / JDK 17 and Scala 2.13 / integration.kafka.server.FetchFromFollowerIntegrationTest.testRackAwareRangeAssignor()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/integration.kafka.server/FetchFromFollowerIntegrationTest/Build___JDK_17_and_Scala_2_13___testRackAwareRangeAssignor__/)
[Build / JDK 17 and Scala 2.13 / integration.kafka.server.FetchFromFollowerIntegrationTest.testRackAwareRangeAssignor()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/integration.kafka.server/FetchFromFollowerIntegrationTest/Build___JDK_17_and_Scala_2_13___testRackAwareRangeAssignor___2/)
[Build / JDK 11 and Scala 2.13 / org.apache.kafka.tools.MetadataQuorumCommandTest.[6] Type=Raft-Isolated, Name=testDescribeQuorumStatusSuccessful, MetadataVersion=3.6-IV0, Security=PLAINTEXT](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/org.apache.kafka.tools/MetadataQuorumCommandTest/Build___JDK_11_and_Scala_2_13____6__Type_Raft_Isolated__Name_testDescribeQuorumStatusSuccessful__MetadataVersion_3_6_IV0__Security_PLAINTEXT/)
[Build / JDK 17 and Scala 2.13 / org.apache.kafka.controller.QuorumControllerTest.testBalancePartitionLeaders()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/org.apache.kafka.controller/QuorumControllerTest/Build___JDK_17_and_Scala_2_13___testBalancePartitionLeaders__/)
[Build / JDK 11 and Scala 2.13 / org.apache.kafka.controller.QuorumControllerTest.testBalancePartitionLeaders()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-13831/7/testReport/junit/org.apache.kafka.controller/QuorumControllerTest/Build___JDK_11_and_Scala_2_13___testBalancePartitionLeaders__/)

divijvaidya · 2023-06-29T08:19:05Z

Merged to trunk. I am backporting this to 3.3, 3.4 and 3.5 branches. Will update the JIRA when backport is complete.

…fig (#13831) Fixed a regression described in KAFKA-15053 that security.protocol only allows uppercase values like PLAINTEXT, SSL, SASL_PLAINTEXT, SASL_SSL. With this fix, both lower case and upper case values will be supported (e.g. PLAINTEXT, SSL, SASL_PLAINTEXT, SASL_SSL, plaintext, ssl, sasl_plaintext, sasl_ssl) Reviewers: Chris Egerton <chrise@aiven.io>, Divij Vaidya <diviv@amazon.com>

bogao007 · 2023-06-29T17:24:41Z

Thanks @divijvaidya!

Use case insensitive validator for security.protocol config

736ada0

divijvaidya reviewed Jun 9, 2023

View reviewed changes

streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java Outdated Show resolved Hide resolved

bogao007 added 3 commits June 9, 2023 17:15

Added case insensitive support for all the places that have security.…

db098ed

…protocol config validation

Simplified MirrorClientConfig

586a7b5

Fixed an issue

aa0e467

retrigger checks

616f3a8

divijvaidya self-assigned this Jun 14, 2023

divijvaidya requested a review from omkreddy June 14, 2023 17:26

Added doc changes

fcb790e

divijvaidya reviewed Jun 26, 2023

View reviewed changes

Addressed comment

9cc8727

divijvaidya approved these changes Jun 29, 2023

View reviewed changes

divijvaidya merged commit 0054168 into apache:trunk Jun 29, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-15053: Use case insensitive validator for security.protocol config #13831

KAFKA-15053: Use case insensitive validator for security.protocol config #13831

bogao007 commented Jun 8, 2023

divijvaidya left a comment

bogao007 commented Jun 10, 2023

divijvaidya commented Jun 14, 2023

bogao007 commented Jun 21, 2023 •

edited

divijvaidya left a comment •

edited

divijvaidya Jun 26, 2023

C0urante commented Jun 26, 2023

divijvaidya commented Jun 26, 2023

C0urante commented Jun 26, 2023

bogao007 commented Jun 28, 2023

divijvaidya commented Jun 29, 2023

divijvaidya commented Jun 29, 2023

bogao007 commented Jun 29, 2023

KAFKA-15053: Use case insensitive validator for security.protocol config #13831

KAFKA-15053: Use case insensitive validator for security.protocol config #13831

Conversation

bogao007 commented Jun 8, 2023

Committer Checklist (excluded from commit message)

divijvaidya left a comment

Choose a reason for hiding this comment

bogao007 commented Jun 10, 2023

divijvaidya commented Jun 14, 2023

bogao007 commented Jun 21, 2023 • edited

divijvaidya left a comment • edited

Choose a reason for hiding this comment

divijvaidya Jun 26, 2023

Choose a reason for hiding this comment

C0urante commented Jun 26, 2023

divijvaidya commented Jun 26, 2023

C0urante commented Jun 26, 2023

bogao007 commented Jun 28, 2023

divijvaidya commented Jun 29, 2023

divijvaidya commented Jun 29, 2023

bogao007 commented Jun 29, 2023

bogao007 commented Jun 21, 2023 •

edited

divijvaidya left a comment •

edited