Scram Config API in Admin Client [KIP-554] #4241
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mahajanadhitya
force-pushed
the
feature/userscram-AdminClient
branch
from
4ee6965
to
4bff8d0
Compare
milindl
requested changes
Apr 28, 2023
There was a problem hiding this comment.
Reviewed all the non-example code.
Great work @mahajanadhitya!
examples/user_scram.c
Outdated
| /* | ||
| * librdkafka - Apache Kafka C library | ||
| * | ||
| * Copyright (c) 2020, Magnus Edenhill |
There was a problem hiding this comment.
Nit: Change copyright notice :)
src/rdkafka.h
Outdated
| typedef enum rd_kafka_ScramMechanism_s { | ||
| UNKNOWN = 0, | ||
| SCRAM_SHA_256 = 1, | ||
| SCRAM_SHA_512 = 2 |
There was a problem hiding this comment.
Rename these to RD_KAFKA_SCRAM_MECHANISM_SHA_256 and same for 512, and add a enum value at the end of the enum, called RD_KAFKA_SCRAM_MECHANISM__CNT (for looping and marking the end, if required)
src/rdkafka.h
Outdated
| typedef enum rd_kafka_UserScramCredentialAlteration_type_s { | ||
| RD_KAFKA_USER_SCRAM_CREDENTIAL_ALTERATION_TYPE_UPSERT, | ||
| RD_KAFKA_USER_SCRAM_CREDENTIAL_ALTERATION_TYPE_DELETE, | ||
| RD_KAFKA_USER_SCRAM_CREDENTIAL_ALTERATION_TYPE_CNT |
There was a problem hiding this comment.
use RD_KAFKA_USER_SCRAM_CREDENTIAL_ALTERATION_TYPE__CNT
src/rdkafka.h
Outdated
|
|
||
| typedef struct rd_kafka_UserScramCredentialAlteration_s rd_kafka_UserScramCredentialAlteration_t; | ||
|
|
||
| RD_EXPORT |
There was a problem hiding this comment.
need comments for all the publicly exported functions.
src/rdkafka.h
Outdated
| RD_EXPORT | ||
| void rd_kafka_AlterUserScramCredentials(rd_kafka_t *rk, | ||
| rd_kafka_UserScramCredentialAlteration_t **alterations, | ||
| size_t num_alterations, |
There was a problem hiding this comment.
Looking at convention, call it alteration_cnt
src/rdkafka.h
Outdated
| RD_EXPORT | ||
| void rd_kafka_DescribeUserScramCredentials(rd_kafka_t *rk, | ||
| char **users, | ||
| size_t num_users, |
There was a problem hiding this comment.
Same as alter, call it user_cnt
mahajanadhitya
force-pushed
the
feature/userscram-AdminClient
branch
from
80daa12
to
0b9b79b
Compare
emasab
changed the title
* Remove editor configuration file * Remove example binary and add it to other places where it's listed * Change copyright for new files * Remove new error code * Doxygen documentation, remove return values from admin functions * Make alteration type an internal enum * Make returned types constant, always return from async functions through the queue * Use event functions for top level error * Return arrays when the array contains pointers. Renamed rd_kafka_UserScramCredentialAlterationResultElement to rd_kafka_AlterUserScramCredentials_result_response following admin api conventions * Test improvements and fix for memory leak * Use byte array for salt and password, create random salt if not provided, move hmac so sasl isn't required, allow multiple alterations for the same user as in Java, example taking parameters from command line * Remove fprintf from tests * Correct asserts when configured without SSL * Compact bytes implementation, use compact bytes for serialization * Style fix * Fix CMake * Value 0x20000 is used by IncrementalAlterConfigs * Documentation order * Documentation improvements * Read compact bytes implementation * Improve security, refactor validation * Changelog and support table * Api versions update * Address some comments * Fix when disabling ssl * Style fix and use rd_strcmp2 LICENCE auto fix * Address comments
milindl
approved these changes
Jul 6, 2023
|
Note: Please don't merge immediately after CI green, wait for bindings changes as well, as discussed with @emasab |
sending the request
last optional parameter as in J.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.