[jira] [Commented] (KAFKA-3328) SimpleAclAuthorizer can lose ACLs with frequent add/remove calls

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on KAFKA-3328:
---

Github user asfgit closed the pull request at:

https://github.com/apache/kafka/pull/1006


> SimpleAclAuthorizer can lose ACLs with frequent add/remove calls
> 
>
> Key: KAFKA-3328
> URL: https://issues.apache.org/jira/browse/KAFKA-3328
> Project: Kafka
>  Issue Type: Bug
>Reporter: Grant Henke
>Assignee: Grant Henke
>Priority: Critical
> Fix For: 0.10.0.0
>
>
> Currently when adding or removing an ACL with the SimpleAclAuthorizer the 
> following high level steps happen:
> # read acls from cache
> # merge with the changes acls
> # update zookeeper
> # add a change notification
> Then the Authorizers listening for the change notification know to invalidate 
> their cache and get the latest value. However that takes some time. In the 
> time between the ACL change and the cache update, a new add or remove request 
> could be made. This will follow the steps listed above, and if the cache is 
> not correct all changes from the previous request are lost.
> This can be solved on a single node, by updating the cache at the same time 
> you update zookeeper any time a change is made. However, because there can be 
> multiple instances of the Authorizer, a request could come to a separate 
> authorizer and overwrite the Zookeeper state again loosing changes from 
> earlier requests.
> To solve this on multiple instances. The authorizer could always read/write 
> state from zookeeper (instead of the cache) for add/remove requests and only 
> leverage the cache for get/authorize requests. Or it could block until all 
> the live instances have updated their cache. 
> Below is a log from a failed test in the WIP [pull 
> request|https://github.com/apache/kafka/pull/1005] for KAFKA-3266 that shows 
> this behavior:
> {noformat}
> [2016-03-03 11:09:20,714] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,726] DEBUG updatedAcls: Set(User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,738] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,739] DEBUG updatedAcls: Set(User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,752] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,755] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,762] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,768] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,773] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,777] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3328) SimpleAclAuthorizer can lose ACLs with frequent add/remove calls

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on KAFKA-3328:
---

GitHub user granthenke opened a pull request:

https://github.com/apache/kafka/pull/1006

KAFKA-3328: SimpleAclAuthorizer can lose ACLs with frequent add/remov…

…e calls

Changes the SimpleAclAuthorizer to:
- Always read state from Zookeeper before updating acls
- Update local cache when modifying acls

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/granthenke/kafka simple-authorizer-fix

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/1006.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1006


commit 3483b759e1cf5e5a42ad3206eca41e8e75906b41
Author: Grant Henke 
Date:   2016-03-03T20:59:03Z

KAFKA-3328: SimpleAclAuthorizer can lose ACLs with frequent add/remove calls

Changes the SimpleAclAuthorizer to:
- Always read state from Zookeeper before updating acls
- Update local cache when modifying acls




> SimpleAclAuthorizer can lose ACLs with frequent add/remove calls
> 
>
> Key: KAFKA-3328
> URL: https://issues.apache.org/jira/browse/KAFKA-3328
> Project: Kafka
>  Issue Type: Bug
>Reporter: Grant Henke
>Assignee: Grant Henke
>
> Currently when adding or removing an ACL with the SimpleAclAuthorizer the 
> following high level steps happen:
> # read acls from cache
> # merge with the changes acls
> # update zookeeper
> # add a change notification
> Then the Authorizers listening for the change notification know to invalidate 
> their cache and get the latest value. However that takes some time. In the 
> time between the ACL change and the cache update, a new add or remove request 
> could be made. This will follow the steps listed above, and if the cache is 
> not correct all changes from the previous request are lost.
> This can be solved on a single node, by updating the cache at the same time 
> you update zookeeper any time a change is made. However, because there can be 
> multiple instances of the Authorizer, a request could come to a separate 
> authorizer and overwrite the Zookeeper state again loosing changes from 
> earlier requests.
> To solve this on multiple instances. The authorizer could always read/write 
> state from zookeeper (instead of the cache) for add/remove requests and only 
> leverage the cache for get/authorize requests. Or it could block until all 
> the live instances have updated their cache. 
> Below is a log from a failed test in the WIP [pull 
> request|https://github.com/apache/kafka/pull/1005] for KAFKA-3266 that shows 
> this behavior:
> {noformat}
> [2016-03-03 11:09:20,714] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,726] DEBUG updatedAcls: Set(User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,738] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,739] DEBUG updatedAcls: Set(User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,752] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,755] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,762] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,768] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,773] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)

[jira] [Commented] (KAFKA-3328) SimpleAclAuthorizer can lose ACLs with frequent add/remove calls

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-3328:
---

[~parth.brahmbhatt] might have some thoughts as well.

> SimpleAclAuthorizer can lose ACLs with frequent add/remove calls
> 
>
> Key: KAFKA-3328
> URL: https://issues.apache.org/jira/browse/KAFKA-3328
> Project: Kafka
>  Issue Type: Bug
>Reporter: Grant Henke
>Assignee: Grant Henke
>
> Currently when adding or removing an ACL with the SimpleAclAuthorizer the 
> following high level steps happen:
> # read acls from cache
> # merge with the changes acls
> # update zookeeper
> # add a change notification
> Then the Authorizers listening for the change notification know to invalidate 
> their cache and get the latest value. However that takes some time. In the 
> time between the ACL change and the cache update, a new add or remove request 
> could be made. This will follow the steps listed above, and if the cache is 
> not correct all changes from the previous request are lost.
> This can be solved on a single node, by updating the cache at the same time 
> you update zookeeper any time a change is made. However, because there can be 
> multiple instances of the Authorizer, a request could come to a separate 
> authorizer and overwrite the Zookeeper state again loosing changes from 
> earlier requests.
> To solve this on multiple instances. The authorizer could always read/write 
> state from zookeeper (instead of the cache) for add/remove requests and only 
> leverage the cache for get/authorize requests. Or it could block until all 
> the live instances have updated their cache. 
> Below is a log from a failed test in the WIP [pull 
> request|https://github.com/apache/kafka/pull/1005] for KAFKA-3266 that shows 
> this behavior:
> {noformat}
> [2016-03-03 11:09:20,714] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,726] DEBUG updatedAcls: Set(User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,738] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,739] DEBUG updatedAcls: Set(User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,752] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,755] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,762] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,768] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,773] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,777] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3328) SimpleAclAuthorizer can lose ACLs with frequent add/remove calls

[Ashish K Singh (JIRA)]

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

Ashish K Singh commented on KAFKA-3328:
---

Sounds good, however can we do lazy writes to ZK? Basically, batch multiple 
acls CRUD in a single ZK write? Depending on use-case, it will be helpful.

> SimpleAclAuthorizer can lose ACLs with frequent add/remove calls
> 
>
> Key: KAFKA-3328
> URL: https://issues.apache.org/jira/browse/KAFKA-3328
> Project: Kafka
>  Issue Type: Bug
>Reporter: Grant Henke
>Assignee: Grant Henke
>
> Currently when adding or removing an ACL with the SimpleAclAuthorizer the 
> following high level steps happen:
> # read acls from cache
> # merge with the changes acls
> # update zookeeper
> # add a change notification
> Then the Authorizers listening for the change notification know to invalidate 
> their cache and get the latest value. However that takes some time. In the 
> time between the ACL change and the cache update, a new add or remove request 
> could be made. This will follow the steps listed above, and if the cache is 
> not correct all changes from the previous request are lost.
> This can be solved on a single node, by updating the cache at the same time 
> you update zookeeper any time a change is made. However, because there can be 
> multiple instances of the Authorizer, a request could come to a separate 
> authorizer and overwrite the Zookeeper state again loosing changes from 
> earlier requests.
> To solve this on multiple instances. The authorizer could always read/write 
> state from zookeeper (instead of the cache) for add/remove requests and only 
> leverage the cache for get/authorize requests. Or it could block until all 
> the live instances have updated their cache. 
> Below is a log from a failed test in the WIP [pull 
> request|https://github.com/apache/kafka/pull/1005] for KAFKA-3266 that shows 
> this behavior:
> {noformat}
> [2016-03-03 11:09:20,714] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,726] DEBUG updatedAcls: Set(User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,738] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,739] DEBUG updatedAcls: Set(User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,752] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,755] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,762] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,768] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,773] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,777] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3328) SimpleAclAuthorizer can lose ACLs with frequent add/remove calls

[Grant Henke (JIRA)]

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

Grant Henke commented on KAFKA-3328:


I think I will work on this as it is affecting my KIP-4 work. Any thoughts on 
only using the cache for read operations and always communicating via Zookeeper 
for write operations? 

> SimpleAclAuthorizer can lose ACLs with frequent add/remove calls
> 
>
> Key: KAFKA-3328
> URL: https://issues.apache.org/jira/browse/KAFKA-3328
> Project: Kafka
>  Issue Type: Bug
>Reporter: Grant Henke
>Assignee: Grant Henke
>
> Currently when adding or removing an ACL with the SimpleAclAuthorizer the 
> following high level steps happen:
> # read acls from cache
> # merge with the changes acls
> # update zookeeper
> # add a change notification
> Then the Authorizers listening for the change notification know to invalidate 
> their cache and get the latest value. However that takes some time. In the 
> time between the ACL change and the cache update, a new add or remove request 
> could be made. This will follow the steps listed above, and if the cache is 
> not correct all changes from the previous request are lost.
> This can be solved on a single node, by updating the cache at the same time 
> you update zookeeper any time a change is made. However, because there can be 
> multiple instances of the Authorizer, a request could come to a separate 
> authorizer and overwrite the Zookeeper state again loosing changes from 
> earlier requests.
> To solve this on multiple instances. The authorizer could always read/write 
> state from zookeeper (instead of the cache) for add/remove requests and only 
> leverage the cache for get/authorize requests. Or it could block until all 
> the live instances have updated their cache. 
> Below is a log from a failed test in the WIP [pull 
> request|https://github.com/apache/kafka/pull/1005] for KAFKA-3266 that shows 
> this behavior:
> {noformat}
> [2016-03-03 11:09:20,714] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,726] DEBUG updatedAcls: Set(User:ANONYMOUS has Allow 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,738] DEBUG [KafkaApi-0] adding User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: * for Cluster:kafka-cluster 
> (kafka.server.KafkaApis:52)
> [2016-03-03 11:09:20,739] DEBUG updatedAcls: Set(User:ANONYMOUS has Deny 
> permission for operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,752] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,755] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,762] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,768] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,773] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> [2016-03-03 11:09:20,777] DEBUG Processing ACL change notification for 
> Cluster:kafka-cluster and Set(User:ANONYMOUS has Deny permission for 
> operations: Describe from hosts: *) 
> (kafka.security.auth.SimpleAclAuthorizer:52)
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)