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

Config apis ACLs #1010

Merged
merged 4 commits into from
Apr 2, 2021
Merged

Conversation

mmaslankaprv
Copy link
Member

Cover letter

Added missing authorization to describe, alter and incremental alter config Kafka APIs

Both `IncrementalAlterConfigs` and `AlterConfigs` Kafka APIs are very
similar in their structure. Added function template that perform
authorization for both API types.

Signed-off-by: Michal Maslanka <michal@vectorized.io>
Signed-off-by: Michal Maslanka <michal@vectorized.io>
Signed-off-by: Michal Maslanka <michal@vectorized.io>
Signed-off-by: Michal Maslanka <michal@vectorized.io>
Copy link
Contributor

@Lazin Lazin left a comment

Choose a reason for hiding this comment

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

LGTM

* responsens and modifies passed in group_resources<T>
*/
template<typename T, typename R>
std::vector<R> authorize_alter_config_resources(
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want a quick test for this partitioning function ? how are we thinking of testing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we will create a whole suite of tests for authorization

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good, please follow up w/ @dotnwat for the ducktape tests for this.

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.

3 participants