Skip to content

Conversation

jack2012aa
Copy link

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.

@github-actions github-actions bot added triage PRs from the community tools small Small PRs labels Sep 4, 2025
Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jack2012aa Could you please add tests?

@jack2012aa
Copy link
Author

jack2012aa commented Sep 4, 2025

@jack2012aa Could you please add tests?

Thanks for the review. The test was added.

@github-actions github-actions bot removed the triage PRs from the community label Sep 5, 2025
Copy link
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jack2012aa thanks for this patch. Two small comments remains. PTAL

}

if (!aclBindings.isEmpty()) {
System.out.println("Adding ACLs for resource `" + resource + "`: " + NL + " " + aclsToAdd.stream().map(a -> "\t" + a).collect(Collectors.joining(NL)) + NL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aclsToAdd.stream() could be replaced by aclBindings.stream().map(AclBinding::entry), right?

admin.createAcls(aclBindings).all().get();

AclBindingFilter filter = new AclBindingFilter(resource.toFilter(), AccessControlEntryFilter.ANY);
Collection<AclBinding> existingBindings = admin.describeAcls(filter).values().get();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var existingBindingsSet = Set.copyOf(admin.describeAcls(filter).values().get());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants