KAFKA-8760; New Java Authorizer API (KIP-504) #7268
Conversation
rajinisivaram
force-pushed
the
KAFKA-8760-authorizer
branch
from
322e6fb
to
a3901b6
Compare
| def authorizeTopicDescribe(partition: TopicPartition) = | ||
| authorize(request.session, Describe, Resource(Topic, partition.topic, LITERAL)) | ||
| def partitionAuthorized[T](elements: List[T], topic: T => String): (Seq[T], Seq[T]) = { | ||
| val authorizedTopics = filterAuthorized(request, DESCRIBE, TOPIC, elements.toList.map(topic)) |
There was a problem hiding this comment.
nit: elements.toList.map(topic) => elements.map(topic)
|
|
||
| @Test | ||
| def testNoAclFound(): Unit = { | ||
| assertFalse("when acls = [], authorizer should deny op.",authorize(aclAuthorizer, requestContext, READ, resource)) |
There was a problem hiding this comment.
nit: missing space between arguments
| val ZkUrlProp = AclAuthorizer.ZkUrlProp | ||
| val ZkConnectionTimeOutProp = AclAuthorizer.ZkConnectionTimeOutProp | ||
| val ZkSessionTimeOutProp = AclAuthorizer.ZkSessionTimeOutProp | ||
| val ZkMaxInFlightRequests = AclAuthorizer.ZkSessionTimeOutProp |
There was a problem hiding this comment.
AclAuthorizer.ZkSessionTimeOutProp => AclAuthorizer.ZkMaxInFlightRequests
| } | ||
|
|
||
| @Test(expected = classOf[IllegalArgumentException]) | ||
| def testAuthorizeThrowsOnNoneLiteralResource(): Unit = { |
There was a problem hiding this comment.
nit: testAuthorizeThrowsOnNoneLiteralResource => testAuthorizeThrowsOnNonLiteralResource
|
@omkreddy Thanks for the review, updated the PR. |
|
@rajinisivaram Thanks for the PR. LGTM. |
|
@omkreddy Thanks for the review, merging to trunk. |
| * Returns any exception during create. If exception is null, the request has succeeded. | ||
| */ | ||
| public ApiException exception() { | ||
| return exception; |
There was a problem hiding this comment.
Did we consider using Optional here?
There was a problem hiding this comment.
Good point, opened another PR for this: #7294
|
|
||
| private final ApiException exception; | ||
|
|
||
| public AclCreateResult() { |
There was a problem hiding this comment.
Should this be private given SUCCESS?
New Java Authorizer API and a new out-of-the-box authorizer that implements the new interface.
Committer Checklist (excluded from commit message)