C0urante commented on code in PR #12010:
URL: https://github.com/apache/kafka/pull/12010#discussion_r853583209
##########
core/src/main/scala/kafka/server/KafkaConfig.scala:
##########
@@ -1324,9 +1324,9 @@ object KafkaConfig {
.define(SslEngineFactoryClassProp, CLASS, null, LOW,
SslEngineFactoryClassDoc)
/** ********* Sasl Configuration ****************/
- .define(SaslMechanismInterBrokerProtocolProp, STRING,
Defaults.SaslMechanismInterBrokerProtocol, MEDIUM,
SaslMechanismInterBrokerProtocolDoc)
+ .define(SaslMechanismInterBrokerProtocolProp, STRING,
Defaults.SaslMechanismInterBrokerProtocol, ConfigDef.CompositeValidator.of(new
ConfigDef.NonNullValidator(), new ConfigDef.NonEmptyString()), MEDIUM,
SaslMechanismInterBrokerProtocolDoc)
.define(SaslJaasConfigProp, PASSWORD, null, MEDIUM, SaslJaasConfigDoc)
- .define(SaslEnabledMechanismsProp, LIST, Defaults.SaslEnabledMechanisms,
MEDIUM, SaslEnabledMechanismsDoc)
+ .define(SaslEnabledMechanismsProp, LIST, Defaults.SaslEnabledMechanisms,
new BrokerSecurityConfigs.SaslEnabledMechanismsValidator(), MEDIUM,
SaslEnabledMechanismsDoc)
Review Comment:
(@RivenSun2 I think your second comment was meant as a reply to [this
one](https://github.com/apache/kafka/pull/12010#discussion_r852477821), right?)
In response to the first comment:
Similar point as above, but it's less about having to permit a value of
`,,,` because people would _want_ to set it, and more because it's not worth
breaking things for people who already do have it set and can get bitten by the
upgrade.
Interesting that this validator doesn't seem to take effect when SASL isn't
enabled on the broker, though. Have you tested this change at all? I understand
there are unit tests; I'm more wondering if it actually causes a different,
more-useful error message to get logged to the user when this property is
invalid (and is actually used by the broker)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
