Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

grant-permissions should check if rules already exist #1209

Closed
rkpattnaik780 opened this issue Oct 12, 2021 · 9 comments
Closed

grant-permissions should check if rules already exist #1209

rkpattnaik780 opened this issue Oct 12, 2021 · 9 comments
Labels
enhancement New feature or request feature/kafka-acl priority/important-soon Must be staffed and worked on either currently or very soon—ideally in time for the next release. Im

Comments

@rkpattnaik780
Copy link
Contributor

Feature or problem description

Creating the same rules twice prints success message

Originally discussed here

@rkpattnaik780 rkpattnaik780 added enhancement New feature or request priority/important-soon Must be staffed and worked on either currently or very soon—ideally in time for the next release. Im feature/kafka-acl labels Oct 12, 2021
@rkpattnaik780
Copy link
Contributor Author

While it is possible to check if a rule exists before every creation using filters, it might slow down commands that create multiple rules using synchronoyus API requests. My take would be to - close it or keep it blocked till API supports batch creation and lookup. Wdyt @craicoverflow @wtrocki ?

@craicoverflow
Copy link
Contributor

I agree with you. Due to the idempotent nature of the command, nothing dangerous or unexpected can happen from running the command twice.

Another option: is there a different status code returned if an ACL already exists? If so, we could track the number of successful requests and diff it against the total number of requests.

@wtrocki
Copy link
Collaborator

wtrocki commented Nov 15, 2021

I like us to bounce those ideas with Kafka sounding board to ask kafka experts what they would typically expect. I'm happy to facilitate them with their group

I'm in no means expert but it feels like this will be hard to do as grant access commands have non binary results so it will be hard to check if access is there without false negatives

@wtrocki
Copy link
Collaborator

wtrocki commented Nov 15, 2021

Example. If first rule exist but others do not user experience is to delete it manually? Or cli delete it?

Sometimes easier solutions have better UX as users understand behaviour. Command creates set of acl always etc.

@craicoverflow
Copy link
Contributor

I'm in no means expert but it feels like this will be hard to do

Since a couple of approaches are mentioned in this issue, could you go into which option you think will be hard to do? Just so that there are no misunderstandings.

@rkpattnaik780
Copy link
Contributor Author

Another option: is there a different status code returned if an ACL already exists? If so, we could track the number of successful requests and diff it against the total number of requests.

Upon investigating, same status code is returned if the ACL already exists.

@wtrocki
Copy link
Collaborator

wtrocki commented Nov 15, 2021

Since a couple of approaches are mentioned in this issue, could you go into which option you think will be hard to do?

I really do not want to discuss any technical solutions without understanding if that requirement makes sense. Core issue here is that we seem to bring this new requirement in GitHub, so first for us would be to bring and validate that requirement with stakeholders etc. I'm happy to help with getting this sorted with UX/UI/PM/Kafka experts

@wtrocki
Copy link
Collaborator

wtrocki commented Nov 15, 2021

Upon validation it looks like there is no duplicate ACLs possible so IMHO we should ask backend to implement ability to detect existing acl/return different code

@craicoverflow
Copy link
Contributor

Yes, this is the approach suggested earlier. What has changed so that we should not bring this to the Kafka sounding board?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature/kafka-acl priority/important-soon Must be staffed and worked on either currently or very soon—ideally in time for the next release. Im
Projects
None yet
Development

No branches or pull requests

4 participants