squah-confluent commented on code in PR #23121:
URL: https://github.com/apache/kafka/pull/23121#discussion_r3858660840
##########
core/src/main/scala/kafka/server/ConfigAdminManager.scala:
##########
@@ -64,15 +65,19 @@ import scala.jdk.CollectionConverters._
*
* Configuration processing is split into two parts.
* - The first step, called "preprocessing," handles setting KIP-412 log
levels, validating
- * BROKER configurations. We also filter out some other things here like
UNKNOWN resource
- * types, etc.
+ * BROKER configurations, and performing the full GROUP configuration
validation (see
+ * [[org.apache.kafka.coordinator.group.GroupConfig#validateOnBroker]]). We
also filter out
+ * some other things here like UNKNOWN resource types, etc.
* - The second step is "persistence," and handles storing the configurations
durably to our
* metadata store.
*
* The active controller performs its own configuration validation step in
* [[kafka.server.ControllerConfigurationValidator]]. This is mainly important
for
* TOPIC resources, since we already validated changes to BROKER resources on
the
- * forwarding broker. The controller is also responsible for enforcing the
configured
+ * forwarding broker. GROUP resources are validated on the controller too, so
that the
+ * cluster stays protected while it may still contain brokers that predate the
broker-side
+ * check; see [[kafka.server.ControllerConfigurationValidator]] for the
upgrade-safety
+ * gating. The controller is also responsible for enforcing the configured
* [[org.apache.kafka.server.policy.AlterConfigPolicy]].
Review Comment:
nit: I'm assuming these javadoc updates are Claude-written and will push to
cut them down since Claude loves to cram information that's true but not
immediately helpful to the reader into them.
* "protected" is not quite the right concept.
* We already point to `ControllerConfigurationValidator` for controller
validation above.
```
* TOPIC resources, since we already validated changes to BROKER and GROUP
resources on the
* forwarding broker. When the cluster may contain brokers that predate the
broker-side GROUP
* validation, GROUP resources are validated on the controller too. The
controller is also
* responsible for enforcing the configured
[[org.apache.kafka.server.policy.AlterConfigPolicy]].
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
