Proposal: Support for Custom Authentication Type in Kafka Clients #148
Conversation
Signed-off-by: Shisong Yuan (Accenture International Limited) <[email protected]>
YuanShisong
force-pushed
the
custom-authentication
branch
from
d23f219 to
da17166
Compare
There was a problem hiding this comment.
Thanks for the proposal. Some quick comments:
Make sure reviewing the PR is as easy as possible. For example by:
- Using some empty lines:
- Before and after headers
- Before and after lists
- Before and after examples etc.
- Having one sentence per line will make it easier to comment on a specific sentence and not on a whole paragraph
From the technical side of things:
- Please check the other
type: customAPIs and how they work. Specifically the KAfka brokerstype: customauthentication should give you an insight on what are the things you are likely missing (SASL mechanism? More different options?). There is also no Kafka otpion namedcustomCallbackClass. So it is not clear what will you do with it. - Clarify which operands are (not) affected. I would expect this to be supported in Connect, MM2 and Bridge.
- Provide more technical details about how the implementation will work for each of the above ^^^. (It might make sense to wait for Move Connect / MM2 configuration preparation from the shell scripts to the operator strimzi-kafka-operator#10668 to be done as that will significantly change it for Connect and MM2)
- Provide test / system test strategy (likely using this to configure something simple like SCRAM-SHA-512 or maybe OAuth what we can test on our own against Strimzi powered brokers)
- The key request here from our users is support for Amazon MSK and Microsoft Azure IAM authentication. So the proposal has to clarify either how will those work / be supported or how will this be extended to support these in the future.
| 2. **Volume and Secret Handling:** | ||
| - No volumes or secrets will be automatically mounted for the custom authentication type. | ||
| - Users are responsible for managing any required resources. |
There was a problem hiding this comment.
This is fine, but you should still explain how will the users do it -> the answer here is using the additional volumes through the spec.template section. You should expain it here. You should also explain how the users will add their own custom plugins you expect to use (likely by building a new container image and adding the required jar(s) there.
| customCallbackClass: com.example.kafka.auth.MyCustomAuthHandler | ||
| ``` | ||
| ### Key Changes | ||
| 1. **New Class:** `KafkaClientAuthenticationCustom` |
There was a problem hiding this comment.
you should be clear that it's going to be part of the api module
| ```yaml | ||
| authentication: | ||
| type: custom | ||
| customCallbackClass: com.example.kafka.auth.MyCustomAuthHandler |
There was a problem hiding this comment.
other than just configuring, you should explain that a user has to bake an image by adding the jar including the handler class, right?
| - No volumes or secrets will be automatically mounted for the custom authentication type. | ||
| - Users are responsible for managing any required resources. | ||
| 3. **Validation:** | ||
| - Strimzi will ensure the `customCallbackClass` property is provided and correctly formatted. |
There was a problem hiding this comment.
what does exactly mean "correctly formatted"?
| - **API:** New class and CRD validation for `KafkaClientAuthenticationCustom`. | ||
| - **Documentation:** Updated guidance for configuring and using custom authentication. | ||
| ### Not affected | ||
| - Kafka Brokers (custom logic is client-side). |
There was a problem hiding this comment.
I don't think this is relevant here. Brokers are anyway part of Apache Kafka project so not within Strimzi and a Strimzi proposal can't have any effect on Apache Kafka.
| - **Documentation:** Updated guidance for configuring and using custom authentication. | ||
| ### Not affected | ||
| - Kafka Brokers (custom logic is client-side). | ||
| - User Operator. |
There was a problem hiding this comment.
I would also remove this one, otherwise the Topic Operator would be here as well.
Let's leave the affected part only.
|
@YuanShisong are you still planning to work on this proposal? After more than one month it seems there are no new changes after the first feedback from the community. |
Yes, I intend to continue with this proposal within the next week. Apologize for the lack of updates — I've been occupied with other work commitments. Thank you for your understanding. |
No description provided.