Skip to content

Commit d6688f8

Browse files
authored
KAFKA-15983 Kafka-acls should return authorization already done if repeating work is issued (apache#20482)
# Description `kafka-acls.sh` doesn't print message about duplicate authorization. # Changes Now the cli searches for existing AclBinding, prints duplicate bindings, and removes them from the adding list. Reviewers: Chia-Ping Tsai <[email protected]>
1 parent 350577d commit d6688f8

File tree

2 files changed

+38
-3
lines changed

2 files changed

+38
-3
lines changed

tools/src/main/java/org/apache/kafka/tools/AclCommand.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,24 @@ private static void addAcls(Admin admin, AclCommandOptions opts) throws Executio
106106
for (Map.Entry<ResourcePattern, Set<AccessControlEntry>> entry : resourceToAcl.entrySet()) {
107107
ResourcePattern resource = entry.getKey();
108108
Set<AccessControlEntry> acls = entry.getValue();
109-
System.out.println("Adding ACLs for resource `" + resource + "`: " + NL + " " + acls.stream().map(a -> "\t" + a).collect(Collectors.joining(NL)) + NL);
110-
Collection<AclBinding> aclBindings = acls.stream().map(acl -> new AclBinding(resource, acl)).collect(Collectors.toList());
111-
admin.createAcls(aclBindings).all().get();
109+
110+
AclBindingFilter filter = new AclBindingFilter(resource.toFilter(), AccessControlEntryFilter.ANY);
111+
Set<AclBinding> existingBindingsSet = Set.copyOf(admin.describeAcls(filter).values().get());
112+
113+
List<AclBinding> aclBindings = new ArrayList<>();
114+
for (AccessControlEntry acl : acls) {
115+
AclBinding binding = new AclBinding(resource, acl);
116+
if (existingBindingsSet.contains(binding)) {
117+
System.out.println("Acl " + binding + " already exists.");
118+
} else {
119+
aclBindings.add(binding);
120+
}
121+
}
122+
123+
if (!aclBindings.isEmpty()) {
124+
System.out.println("Adding ACLs for resource `" + resource + "`: " + NL + " " + aclBindings.stream().map(AclBinding::entry).map(a -> "\t" + a).collect(Collectors.joining(NL)) + NL);
125+
admin.createAcls(aclBindings).all().get();
126+
}
112127
}
113128
}
114129

tools/src/test/java/org/apache/kafka/tools/AclCommandTest.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.apache.kafka.tools;
1818

1919
import org.apache.kafka.common.acl.AccessControlEntry;
20+
import org.apache.kafka.common.acl.AclBinding;
2021
import org.apache.kafka.common.acl.AclBindingFilter;
2122
import org.apache.kafka.common.acl.AclOperation;
2223
import org.apache.kafka.common.acl.AclPermissionType;
@@ -79,6 +80,7 @@
7980
import static org.apache.kafka.server.config.ServerConfigs.AUTHORIZER_CLASS_NAME_CONFIG;
8081
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
8182
import static org.junit.jupiter.api.Assertions.assertEquals;
83+
import static org.junit.jupiter.api.Assertions.assertFalse;
8284
import static org.junit.jupiter.api.Assertions.assertNull;
8385
import static org.junit.jupiter.api.Assertions.assertThrows;
8486
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -275,6 +277,24 @@ public void testPatternTypesWithAdminAPIAndBootstrapController(ClusterInstance c
275277
testPatternTypes(adminArgsWithBootstrapController(cluster.bootstrapControllers(), Optional.empty()));
276278
}
277279

280+
@ClusterTest
281+
public void testDuplicateAdd(ClusterInstance cluster) {
282+
final String topicName = "test-topic";
283+
final String principal = "User:Alice";
284+
ResourcePattern resource = new ResourcePattern(ResourceType.TOPIC, topicName, PatternType.LITERAL);
285+
AccessControlEntry ace = new AccessControlEntry(principal, WILDCARD_HOST, READ, ALLOW);
286+
AclBinding binding = new AclBinding(resource, ace);
287+
List<String> cmdArgs = adminArgs(cluster.bootstrapServers(), Optional.empty());
288+
List<String> initialAddArgs = new ArrayList<>(cmdArgs);
289+
initialAddArgs.addAll(List.of(ADD, TOPIC, topicName, "--allow-principal", principal, OPERATION, "Read"));
290+
291+
callMain(initialAddArgs);
292+
String out = callMain(initialAddArgs).getKey();
293+
294+
assertTrue(out.contains("Acl " + binding + " already exists."));
295+
assertFalse(out.contains("Adding ACLs for resource"));
296+
}
297+
278298
@Test
279299
public void testUseBootstrapServerOptWithBootstrapControllerOpt() {
280300
assertInitializeInvalidOptionsExitCodeAndMsg(

0 commit comments

Comments
 (0)