KAFKA-7185: Allow empty resource name when matching ACLs #5400
Conversation
dhruvilshah3
changed the title
|
retest this please |
| def testAuthorizeWithEmptyResourceName(): Unit = { | ||
| assertFalse(simpleAclAuthorizer.authorize(session, Read, Resource(Group, "", LITERAL))) | ||
| simpleAclAuthorizer.addAcls(Set[Acl](allowReadAcl), Resource(Group, WildCardResource, LITERAL)) | ||
| assertTrue(simpleAclAuthorizer.authorize(session, Read, Resource(Group, "", LITERAL))) |
There was a problem hiding this comment.
Is it possible to authorize the empty resource only? It would be good to have a test for that case too.
There was a problem hiding this comment.
I don't think we can create ACLs for empty string since ACLs are stored in a ZK node with the resource name (it is an odd default to use, but I suppose we can't get of it now).
There was a problem hiding this comment.
Right, I actually tried testing this but the create failed with an exception because of the reason you mention.
There was a problem hiding this comment.
Maybe we should just have a test for that case to assert the current behaviour (even if not ideal)
| @@ -238,7 +238,7 @@ class SimpleAclAuthorizer extends Authorizer with Logging { | |||
|
|
|||
| val prefixed = aclCache.range( | |||
| Resource(resourceType, resourceName, PatternType.PREFIXED), | |||
| Resource(resourceType, resourceName.substring(0, 1), PatternType.PREFIXED) | |||
| Resource(resourceType, resourceName.substring(0, Math.min(1, resourceName.length)), PatternType.PREFIXED) | |||
There was a problem hiding this comment.
Can you simply use take instead of substring?
|
Retest this please |
|
@dhruvilshah3 Thanks for the PR, merging to trunk and 2.0. |
Reviewers: Ismael Juma <ismael@juma.me.uk>, Rajini Sivaram <rajinisivaram@googlemail.com>
No description provided.