Hi Kirk,

Thanks for clarifying.

> mb-5 : Regarding configs.push.allowed.keys, would it help if there is a
> deny list which can override allow list? or to avoid any sensitive keys
> just in case.

>>  Can you provide a use case that would require that?

Since configs.push.allowed.keys is an exact-match list, if an org has keys
like org.batching.strategy, org.auth.token, it could be easily overlooked
and sensitive keys can end up there, as they don't get filtered with
default ssl.*.

In the same context, if wildcards are allowed or being planned, then the
risk becomes high and denylist can become more important.

Thanks,
Murali


On Tue, May 5, 2026 at 5:12 PM Kirk True <[email protected]> wrote:

> Hi Mickael,
>
> On Mon, May 4, 2026, at 12:10 PM, Mickael Maison wrote:
> > Hi,
> >
> > Thanks for the KIP, I have a few questions:
> >
> > MM1: Can we have the definition for the new Exception classes? I'm
> > particularly interested in the exception for INVALID_CONFIG, how are
> > the failures returned to the client? From PushConfigResponse it seems
> > it's just a string? Should it be nullable collection with a specific
> > message for each issue?
>
> I've added the definition for the ConfigTooLargeException which is now the
> only exception added.
>
> > MM2: The new broker class is presented as a policy but if I understand
> > correctly in case the process method throws and PushConfigResponse has
> > a non-NONE error code, the client still continues. This is different
> > from the other policy classes AlterConfigPolicy, CreateTopicPolicy
> > which prevent the action in case of a violation. Are we aiming for
> > validation, observability or both?
>
> We are focused on observability in this KIP, but as you point out, the
> policy moniker doesn't make sense in this context.
>
> > MM3: It seems you're treating Streams as a separate client instead of
> > an aggregation of Producers and Consumers (I see StreamsConfig in
> > Configuration Payload Size Enforcement). What about Connect?
>
> Clients such as Kafka Streams, Connect, etc. are tricky because it's
> important to provide context to the embedded client. For example, a
> consumer used in a Connect sink would benefit from having that context so
> that downstream filtering, aggregation, etc. can take that into account.
>
> LMK if that answers your question or not.
>
> > MM4: Regarding sensitive configurations, the current criteria would
> > treat custom configurations as non sensitive. For example if my
> > producer has a custom serializer it may have its own custom
> > configurations too. Is that behavior what you wanted? If so let's me
> > it clear. By precaution, I'd lean towards considering custom configs
> > as sensitive as the client has no idea what it is.
>
> The intention of the KIP is for any custom configuration (custom meaning
> not known by the client implementation) to be omitted by default. If the
> user overrides config.push.allowed.keys with a custom configuration,
> however, the KIP would allow that to be included in the configuration that
> is pushed to the broker. Does that seem sensible, or no?
>
> Thanks for the feedback!
>
> Kirk
>
> >
> > Thanks,
> > Mickael
> >
> > On Sun, May 3, 2026 at 5:24 PM Muralidhar Basani via dev
> > <[email protected]> wrote:
> > >
> > > Hi Kirk,
> > >
> > > Thanks for this thoughtful kip.
> > >
> > > I have a few additional points.
> > >
> > > mb-1 : Would there be any recommendation for reference implementation
> to
> > > store the client configs ? may be in a compacted topic etc. Otherwise,
> > > everyone will come up with their impl, and could be re-invented again
> and
> > > again?
> > >
> > > mb-2 : When a client reconnects, there might be duplicates of
> clientconfigs
> > > pushed. Should this be handled by plugin may be based on
> uuid/clientid? (I
> > > am not sure, if client restarts and there is a new uuid, would there be
> > > stale entries indefinitely)
> > >
> > > mb-3 : In your reply to Hector, you noted "it's difficult to ensure
> that
> > > client is marked as 'in violation' across all brokers in a cluster."
> Does
> > > the same challenge apply to the observability goal? If the PushConfig
> lands
> > > on a single randomly chosen broker, only that broker's
> ClientConfigPolicy
> > > sees it, so the plugin effectively must fan out to shared storage for
> the
> > > feature to be useful cluster-wide ?
> > >
> > > mb-4 : If AdminClient configs are not handled, how about any dynamic
> config
> > > changes like AdminClient.alterClientConfigs would also work like hot
> reload
> > > ? Or any explicit re-push mechanism be possible?
> > >
> > > mb-5 : Regarding configs.push.allowed.keys, would it help if there is a
> > > deny list which can override allow list? or to avoid any sensitive keys
> > > just in case.
> > >
> > > mb-6 : In general, if the rpc fails for one random node, will it retry
> to a
> > > different node?
> > >
> > >
> > > Thanks,
> > > Murali
> > >
> > > On Sun, May 3, 2026 at 1:37 PM Kirk True <[email protected]> wrote:
> > >
> > > > Hi Andrew,
> > > >
> > > > On Tue, Apr 28, 2026, at 6:28 PM, Andrew Schofield wrote:
> > > > > Hi Kirk,
> > > > > Thanks for the KIP. Inevitably, I have some comments.
> > > > >
> > > > > AS1: I see a few mysterious mentions of "profile". I suppose that
> these
> > > > are evidence of a concept which did not eventually see the light of
> day in
> > > > the KIP.
> > > >
> > > > That's a vestige from a previous design. I will remove them ASAP.
> > > >
> > > > > AS2: It seems that the default configurations pushed for a share
> > > > consumer are incorrect (because some of them are group configs not
> client
> > > > configs). I suggest:
> > > > >
> > > > > * client.id
> > > > > * group.id
> > > > > * share.acknowledgement.mode
> > > > > * share.acquire.mode
> > > > > * max.poll.records
> > > > > * max.poll.interval.ms
> > > > > * fetch.min.bytes
> > > > > * fetch.max.wait.ms
> > > >
> > > > Thanks for the feedback!
> > > >
> > > > > AS3: Is there any reason why you need to send the configuration
> type
> > > > information to the broker? I wonder if it would be simpler just to
> leave
> > > > everything as strings. I see that you've got a enum for
> ClientConfigType
> > > > which is awfully similar to Config.Type, and you're going to end up
> mapping
> > > > between these enums.
> > > >
> > > > Most of the configuration we're collecting by default end up being
> > > > numeric, so defaulting to strings is less efficient. Also, the
> thought
> > > > process was that implementors of the ClientConfigPolicy plugin may
> benefit
> > > > from/need the type information to achieve their goal.
> > > >
> > > > Agreed that the mapping between ClientConfigType and Config.Type
> seems
> > > > unnecessary. ClientConfigType is a strict subset of Config.Type,
> > > > specifically no PASSWORD type, so I wanted to provide some compile
> time
> > > > support to prevent their usage. Maybe I was being too zealous in this
> > > > regard?
> > > >
> > > > > AS4: ClientConfigUnknownProfileException has a couple of problems.
> We
> > > > tend to use Unknown at the start of the error codes and exception
> class
> > > > names, not the middle. Profile is not a thing. I suggest it should be
> > > > UnknownClientConfigSomethingException, but I am not qualified to say
> what
> > > > the Something is.
> > > >
> > > > I will propose a change in the next revision of the KIP. Thanks for
> > > > catching that.
> > > >
> > > > > AS5: You should include a list of the exceptions and error codes
> you are
> > > > introducing to the protocol. I've seen a few new exceptions and they
> > > > generally have error codes which correspond 1:1.
> > > > >
> > > > > * ClientConfigUnknownProfileException - Should this just be
> > > > InvalidConfigurationException (exists) which maps to INVALID_CONFIG
> error
> > > > code?
> > > > > * ClientConfigTooLargeException - I would expect this to map to the
> > > > CLIENT_CONFIG_TOO_LARGE error code (new)
> > > > > * ClientConfigPolicyException - See AS4. Policy exceptions are
> usually
> > > > PolicyViolationException, but this policy doesn't validate, it just
> > > > processes.
> > > >
> > > > I wasn't certain where on the specificity vs. generality spectrum to
> land
> > > > on creating or reusing existing errors. In my next KIP revision I'll
> look
> > > > to reuse existing error codes where possible.
> > > >
> > > > > AS6: You've tended to use Config (singular) not Configs (plural).
> > > > However, in the configurations for the configurations, you've used
> plural,
> > > > such as client.configs.policy.class.name. I would err on the side of
> > > > consistency.
> > > >
> > > > Agreed. I'll make this more consistent.
> > > >
> > > > > AS7: We are going to have to resolve the relationship between this
> KIP
> > > > and KIP-1313. The latter introduces client instance ID on all RPCs,
> and
> > > > will simplify your KIP if it is accepted first.
> > > >
> > > > Yes, KIP-1313 are racing in this regard. I see KIP-1313 has entered
> the
> > > > voting phase, so I will likely end up removing most of the client ID
> > > > instance references from my KIP, leaving on those that are more
> pertinent
> > > > to the KIP's specific needs.
> > > >
> > > > > AS8: Please confirm whether there are any timing considerations
> between
> > > > PushConfigs and GetTelemetrySubscriptions/PushTelemetry RPCs. I
> suppose
> > > > that a client could overlap these RPCs, or even send them to
> different
> > > > randomly selected brokers, as part of its initial connection setup.
> > > >
> > > > The intention was for the PushConfig RPC to execute before the
> telemetry
> > > > RPCs. However, this will likely change based on other feedback,
> moving
> > > > toward more of an "overlap" sequencing. From the perspective of this
> KIP,
> > > > it shouldn't matter if they're sent to different brokers. I wonder
> if the
> > > > ClientConfigPolicy plugin implementors would see that differently?
> > > >
> > > > > AS9: Please add some broker metrics for this new feature. I suggest
> > > > looking at KIP-714 for inspiration.
> > > >
> > > > Will do. I intentionally omitted metrics on the broker, leaving them
> for
> > > > the specific implementations. But I will look at the KIP-714
> metrics, as
> > > > suggested.
> > > >
> > > > > AS10: Why does the client block while the config handshake is being
> > > > performed? The handshake is not validating the configurations and the
> > > > client doesn't throw an exception even if the PushConfigs response
> contains
> > > > an error code. Doesn't this unnecessarily slow down the initial
> connection,
> > > > and that's arguably too long with Kafka clients already.
> > > >
> > > > The approach will likely move away from a blocking approach to a
> > > > background, best effort approach. I will update the KIP to reflect
> this.
> > > >
> > > > > AS11: Which are the default configurations sent for the Apache
> Kafka
> > > > Java admin client?
> > > >
> > > > In initial discussions, the admin client was deemed as too
> uninteresting
> > > > to warrant sending configuration since it doesn't effect correctness
> or
> > > > performance of the critical produce/consume path. Do you disagree?
> > > >
> > > > Thanks for the thorough feedback!
> > > >
> > > > Kirk
> > > >
> > > > > Thanks,
> > > > > Andrew
> > > > >
> > > > > On 2026/04/23 17:59:11 Kirk True wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > I would like to start a discussion on KIP-1324: Support client
> > > > configuration observability:
> > > > > >
> > > > > >
> > > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-1324%3A+Support+client+configuration+observability
> > > > > >
> > > > > > Thanks,
> > > > > > Kirk
> > > > > >
> > > > >
> > > >
> >
>

Reply via email to