viktorsomogyi commented on code in PR #14083:
URL: https://github.com/apache/kafka/pull/14083#discussion_r1295660941


##########
core/src/main/scala/kafka/server/DelegationTokenManager.scala:
##########
@@ -257,6 +153,13 @@ class DelegationTokenManager(val config: KafkaConfig,
     scramCredentialMap.toMap
   }
 
+  /**
+   * @param token
+   */
+  def updateToken(token: DelegationToken): Unit = {

Review Comment:
   Should we protect this as well with the lock? The ZK variant is protected 
through the create/renew/expire methods but this isn't protected. (As I see we 
only use it through `DelegationTokenPublisher.onMetadataUpdate` but I don't 
know if calls to that come from multiple threads or just a single one.)



##########
core/src/main/scala/kafka/server/ControllerApis.scala:
##########
@@ -842,6 +847,75 @@ class ControllerApis(val requestChannel: RequestChannel,
       }
   }
 
+  def handleCreateDelegationTokenRequest(request: RequestChannel.Request): 
CompletableFuture[Unit] = {

Review Comment:
   Ok, then it's fine. I was asking because whether it would make sense to move 
all the validation to one place? Was there a particular reason you left some 
validation in ControllerApis and moved some other to 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.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

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

Reply via email to