[GitHub] [kafka] hachikuji commented on a change in pull request #10129: KAFKA-10817; Add clusterId validation to Fetch handling

[GitBox <gi...@apache.org>]

hachikuji commented on a change in pull request #10129:
URL: https://github.com/apache/kafka/pull/10129#discussion_r577993815



##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/Errors.java
##########
@@ -354,7 +355,8 @@
         "Requested position is not greater than or equal to zero, and less than the size of the snapshot.",
         PositionOutOfRangeException::new),
     UNKNOWN_TOPIC_ID(100, "This server does not host this topic ID.", UnknownTopicIdException::new),
-    DUPLICATE_BROKER_REGISTRATION(101, "This broker ID is already in use.", DuplicateBrokerRegistrationException::new);
+    DUPLICATE_BROKER_REGISTRATION(101, "This broker ID is already in use.", DuplicateBrokerRegistrationException::new),
+    INVALID_CLUSTER_ID(102, "The supplied cluster id is not valid.", InvalidClusterIdException::new);

Review comment:
       I am considering if we should have a more explicit name here. Specifically we are checking that the clusterId matches. Maybe `INCONSISTENT_CLUSTER_ID` would be clearer?

##########
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##########
@@ -925,6 +931,18 @@ private FetchResponseData buildEmptyFetchResponse(
         );
     }
 
+    private boolean hasValidClusterId(FetchRequestData request) {
+        // We don't enforce the cluster id if it is not provided.
+        if (request.clusterId() == null) {
+            return true;
+        }
+        try {
+            return Uuid.fromString(request.clusterId()).equals(clusterId);
+        } catch (IllegalArgumentException e) {
+            return false;

Review comment:
       Is it worth logging something here?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org