[jira] [Commented] (KAFKA-13889) Broker can't handle ACCESS_CONTROL_ENTRY_RECORD quickly followed by REMOVE_ACCESS_CONTROL_ENTRY_RECORD for same ACL

["Colin McCabe (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-13889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17534001#comment-17534001 ] 

Colin McCabe commented on KAFKA-13889:
--------------------------------------

Thanks for picking this up, [~andyg2]!

I would really prefer that we not relax the invariants here. We don't want an actual bug to go undetected because we decided to ignore deletions of ACLs we couldn't locate.

I think the best way to handle this is to check if the ACL being deleted exists in the image when adding a new deletion. If the ACL exists in the Delta but not in the underlying image, we can just remove it entirely from the Delta, so calling code never sees it.

> Broker can't handle ACCESS_CONTROL_ENTRY_RECORD quickly followed by REMOVE_ACCESS_CONTROL_ENTRY_RECORD for same ACL
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: KAFKA-13889
>                 URL: https://issues.apache.org/jira/browse/KAFKA-13889
>             Project: Kafka
>          Issue Type: Bug
>            Reporter: Andrew Grant
>            Priority: Major
>
> In [https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/image/AclsDelta.java#L64] we store the pending deletion in the changes map. This could override a creation that might have just happened. This is an issue because in BrokerMetadataPublisher this results in us making a removeAcl call which finally results in [https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/metadata/authorizer/StandardAuthorizerData.java#L203] being executed and this code throws an exception if the ACL isnt in the Map yet. If the ACCESS_CONTROL_ENTRY_RECORD event never got processed by BrokerMetadataPublisher then the ACL wont be in the Map yet.
> My feeling is we might want to make removeAcl idempotent in that it returns success if the ACL doesn't exist: no matter how many times removeAcl is called it returns success if the ACL is deleted. Maybe we’d just log a warning or something?
> Note, I dont think the AclControlManager has this issue because it doesn't batch the events like AclsDelta does. However, we still do throw a RuntimeException here [https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/controller/AclControlManager.java#L197] - maybe we should still follow the same logic (if we make the fix suggested above) and just log a warning if the ACL doesnt exist in the Map?



--
This message was sent by Atlassian Jira
(v8.20.7#820007)