Hi Riven, I think it may be possible to accomplish most or all of this without a KIP.
RE ConfigDef.NonNullAndEmptyString: is there a case that this validator satisfies that isn't satisfied by the existing ConfigDef.NonEmptyString validator [1] ? If a property is required but a user doesn't specify a value for it, that should already trigger a validation error, and if we don't want to permit null values for a property, we should just make it required instead of optional but with an additional custom validator that disallows null values. If there is a use case that requires a check for both null and empty values, we could also use a ConfigDef.CompositeValidator [2] to combine the ConfigDef.NonEmptyString and ConfigDef.NonNullValidator [3] validators. RE SslClientAuth::names and CompressionType::names: It seems like these can be replaced with calls to Utils::enumOptions [4], which was implemented for exactly this kind of case: create a string array containing all the possible values for an enum, which can then be passed to ConfigDef.ValidLIst::in [5] to create a validator. RE BrokerSecurityConfigs.SaslEnabledMechanismsValidator: This is the one case that I don't think can be accomplished without implementing a new custom validator. But since this class isn't part of the public API, we don't have to go through the KIP process to add it. Cheers, Chris [1] - https://github.com/apache/kafka/blob/c93b717836b6d92d0f2e9fb101ea6ff3e823ffca/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java#L1070-L1084 [2] - https://github.com/apache/kafka/blob/c93b717836b6d92d0f2e9fb101ea6ff3e823ffca/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java#L1038-L1068 [3] - https://github.com/apache/kafka/blob/c93b717836b6d92d0f2e9fb101ea6ff3e823ffca/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java#L998-L1010 [4] - https://github.com/apache/kafka/blob/c93b717836b6d92d0f2e9fb101ea6ff3e823ffca/clients/src/main/java/org/apache/kafka/common/utils/Utils.java#L1418-L1433 [5] - https://github.com/apache/kafka/blob/c93b717836b6d92d0f2e9fb101ea6ff3e823ffca/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java#L928-L930 On Wed, Apr 13, 2022 at 5:26 AM Lolov, Christo <lol...@amazon.com.invalid> wrote: > Hello! > > I am struggling to understand the context of the suggested change. Is the > point to add dynamic validation? In other words, is the idea that when a > person adds another SASL module it would automatically be picked up by the > validation framework and the validation list would automatically change to > ("GSSAPI", "PLAIN", "SCRAM-SHA-256", "SCRAM-SHA-512", "OAUTHBEARER", > "MY-AWESOME-CUSTOM-MECHANISM")? If so, could this be explained a bit more > clearly in the motivation - when I read the KIP for the first time I was > left with the impression that you are proposing to add validation to a > configuration which didn't have any validation to begin with. > > Best, > Christo > > On 08/04/2022, 09:54, "Riven Sun" <riven....@zoom.us.INVALID> wrote: > > Hi devs, I've created a KIP that aims to add the corresponding > validator > to the config where the validator is missing. > In order for the program to detect these incorrect configurations > during > initialization: > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-828%3A+Add+the+corresponding+validator+to+the+configuration+where+the+validator+is+missing > Please take a look and let me know if you have any feedback. Thanks. > Riven > Sun > >
