This is an automated email from the ASF dual-hosted git repository.
chia7712 pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new 82db250f504 KAFKA-18998 Clean up AuthHelper following review feedback
(#22672)
82db250f504 is described below
commit 82db250f50452f1a0a99d66918278ba10f6243c4
Author: PoAn Yang <[email protected]>
AuthorDate: Fri Jun 26 04:32:43 2026 +0900
KAFKA-18998 Clean up AuthHelper following review feedback (#22672)
- Unwrap the authorizer plugin in the constructor so the field is
`Optional<Authorizer>`, removing the repeated `authorizer.get().get()`
calls.
- Change `filterByAuthorized` to take `Collection<T>` instead of
`Iterable<T>`.
- Use the shorter `filterByAuthorized` overload in
`partitionByAuthorized`.
Reviewers: Chia-Ping Tsai <[email protected]>
---
.../main/scala/kafka/server/ControllerApis.scala | 12 +++++------
.../java/org/apache/kafka/server/AuthHelper.java | 25 +++++++++++-----------
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/core/src/main/scala/kafka/server/ControllerApis.scala
b/core/src/main/scala/kafka/server/ControllerApis.scala
index 2957d5770fb..9f56906f822 100644
--- a/core/src/main/scala/kafka/server/ControllerApis.scala
+++ b/core/src/main/scala/kafka/server/ControllerApis.scala
@@ -224,8 +224,8 @@ class ControllerApis(
request: DeleteTopicsRequestData,
apiVersion: Int,
hasClusterAuth: Boolean,
- getDescribableTopics: lang.Iterable[String] => util.Set[String],
- getDeletableTopics: lang.Iterable[String] => util.Set[String]
+ getDescribableTopics: util.Collection[String] => util.Set[String],
+ getDeletableTopics: util.Collection[String] => util.Set[String]
): CompletableFuture[util.List[DeletableTopicResult]] = {
// Check if topic deletion is enabled at all.
if (!config.deleteTopicEnable) {
@@ -391,8 +391,8 @@ class ControllerApis(
context: ControllerRequestContext,
request: CreateTopicsRequestData,
hasClusterAuth: Boolean,
- getCreatableTopics: lang.Iterable[String] => util.Set[String],
- getDescribableTopics: lang.Iterable[String] => util.Set[String],
+ getCreatableTopics: util.Collection[String] => util.Set[String],
+ getDescribableTopics: util.Collection[String] => util.Set[String],
forwarded: Boolean
): CompletableFuture[CreateTopicsResponseData] = {
val topicNames = new util.HashSet[String]()
@@ -798,7 +798,7 @@ class ControllerApis(
}
private def handleCreatePartitions(request: Request):
CompletableFuture[Unit] = {
- def filterAlterAuthorizedTopics(topics: lang.Iterable[String]):
util.Set[String] = {
+ def filterAlterAuthorizedTopics(topics: util.Collection[String]):
util.Set[String] = {
authHelper.filterByAuthorized(request.context, ALTER, TOPIC, topics, (n:
String) => n)
}
val createPartitionsRequest =
request.body(classOf[CreatePartitionsRequest])
@@ -830,7 +830,7 @@ class ControllerApis(
def createPartitions(
context: ControllerRequestContext,
request: CreatePartitionsRequestData,
- getAlterAuthorizedTopics: lang.Iterable[String] => util.Set[String]
+ getAlterAuthorizedTopics: util.Collection[String] => util.Set[String]
): CompletableFuture[util.List[CreatePartitionsTopicResult]] = {
val responses = new util.ArrayList[CreatePartitionsTopicResult]()
val duplicateTopicNames = new util.HashSet[String]()
diff --git a/server/src/main/java/org/apache/kafka/server/AuthHelper.java
b/server/src/main/java/org/apache/kafka/server/AuthHelper.java
index 8a3d2d6983f..724aa334c69 100644
--- a/server/src/main/java/org/apache/kafka/server/AuthHelper.java
+++ b/server/src/main/java/org/apache/kafka/server/AuthHelper.java
@@ -38,6 +38,7 @@ import org.apache.kafka.server.authorizer.AuthorizationResult;
import org.apache.kafka.server.authorizer.Authorizer;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -50,10 +51,10 @@ import java.util.stream.Collectors;
public class AuthHelper {
- private final Optional<Plugin<Authorizer>> authorizer;
+ private final Optional<Authorizer> authorizer;
public AuthHelper(Optional<Plugin<Authorizer>> authorizer) {
- this.authorizer = authorizer;
+ this.authorizer = authorizer.map(Plugin::get);
}
public boolean authorize(
@@ -72,7 +73,7 @@ public class AuthHelper {
List<Action> actions = List.of(
new Action(operation, resource, refCount, logIfAllowed,
logIfDenied)
);
- return authorizer.get().get().authorize(requestContext,
actions).get(0) == AuthorizationResult.ALLOWED;
+ return authorizer.get().authorize(requestContext, actions).get(0) ==
AuthorizationResult.ALLOWED;
}
public boolean authorize(
@@ -98,7 +99,7 @@ public class AuthHelper {
List<Action> actions = supportedOps.stream()
.map(op -> new Action(op, resourcePattern, 1, false, false))
.toList();
- List<AuthorizationResult> results =
authorizer.get().get().authorize(request.context(), actions);
+ List<AuthorizationResult> results =
authorizer.get().authorize(request.context(), actions);
authorizedOps = new HashSet<>();
// Authorizer.authorize returns one result per action in the same
order, so the i-th
// result corresponds to the i-th supported operation. Iterate up
to the smaller size to
@@ -123,18 +124,21 @@ public class AuthHelper {
AclOperation operation,
ResourceType resourceType
) {
- return authorizer.map(authorizerPlugin ->
authorizerPlugin.get().authorizeByResourceType(requestContext, operation,
resourceType) == AuthorizationResult.ALLOWED).orElse(true);
+ return authorizer.map(authorizerInstance ->
authorizerInstance.authorizeByResourceType(requestContext, operation,
resourceType) == AuthorizationResult.ALLOWED).orElse(true);
}
public <T> Set<String> filterByAuthorized(
RequestContext requestContext,
AclOperation operation,
ResourceType resourceType,
- Iterable<T> resources,
+ Collection<T> resources,
boolean logIfAllowed,
boolean logIfDenied,
Function<T, String> resourceName
) {
+ if (resources.isEmpty()) {
+ return Set.of();
+ }
if (authorizer.isEmpty()) {
Set<String> result = new HashSet<>();
for (T resource : resources) {
@@ -148,9 +152,6 @@ public class AuthHelper {
String name = resourceName.apply(resource);
resourceNameToCount.merge(name, 1, Integer::sum);
}
- if (resourceNameToCount.isEmpty()) {
- return Set.of();
- }
List<String> names = new ArrayList<>(resourceNameToCount.keySet());
List<Action> actions = names.stream()
@@ -162,7 +163,7 @@ public class AuthHelper {
logIfDenied
))
.toList();
- List<AuthorizationResult> results =
authorizer.get().get().authorize(requestContext, actions);
+ List<AuthorizationResult> results =
authorizer.get().authorize(requestContext, actions);
Set<String> authorized = new HashSet<>();
// Authorizer.authorize returns one result per action in the same
order, so the i-th
// result corresponds to the i-th resource name. Iterate up to the
smaller size to
@@ -180,7 +181,7 @@ public class AuthHelper {
RequestContext requestContext,
AclOperation operation,
ResourceType resourceType,
- Iterable<T> resources,
+ Collection<T> resources,
Function<T, String> resourceName
) {
return filterByAuthorized(requestContext, operation, resourceType,
resources, true, true, resourceName);
@@ -200,7 +201,7 @@ public class AuthHelper {
return new PartitionResult<>(resources, List.of());
}
Set<String> authorizedResourceNames = filterByAuthorized(
- requestContext, operation, resourceType, resources, true, true,
resourceName
+ requestContext, operation, resourceType, resources, resourceName
);
List<T> authorized = new ArrayList<>();
List<T> unauthorized = new ArrayList<>();
