Fix for KAFKA-4090: Add an SSL check before to create a client #136
Conversation
gurinderu
force-pushed
the
dt-2684-be-make-kafka-client-v331-cdk
branch
5 times, most recently
from
3394814
to
a816525
Compare
| @@ -0,0 +1,207 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
@gurinderu Did you implement this or did you copy it from somewhere?
There was a problem hiding this comment.
copied from https://github.com/confluentinc/confluent-platform-security-tools/blob/master/kafka-generate-ssl.sh
but yep, probably need to drop it here
There was a problem hiding this comment.
Please add a comment telling where this scripts comes from :)
There was a problem hiding this comment.
I'd prefer we keep this script and we just document its origin
There was a problem hiding this comment.
Not sure that is a great idea in case of licenses
zio-kafka-test-utils/src/main/scala/zio/kafka/KafkaTestUtils.scala
Outdated
Show resolved
Hide resolved
| val buf = ByteBuffer.allocate(5) | ||
| channel.write(buf) | ||
| buf.position(0) | ||
| channel.read(buf) | ||
| buf.position(0) | ||
| isTls(buf) |
There was a problem hiding this comment.
🌕 Can you please add a comment explaining what this is doing and you found this, please?
| private def isTls(buf: ByteBuffer): Boolean = { | ||
| val tlsMessageType = buf.get() | ||
| tlsMessageType match { | ||
| case 20 | 21 | 22 | 23 | 255 => | ||
| true | ||
| case _ => tlsMessageType >= 128 | ||
| } | ||
| } |
There was a problem hiding this comment.
🌕 Can you please add a comment telling where this piece of code comes from, please?
There was a problem hiding this comment.
from the downpage Notes here https://cwiki.apache.org/confluence/display/KAFKA/KIP-498%3A+Add+client-side+configuration+for+maximum+response+size+to+protect+against+OOM# , TLS version
gurinderu
force-pushed
the
dt-2684-be-make-kafka-client-v331-cdk
branch
from
a816525
to
325f0d1
Compare
| ZIO.attempt(SocketChannel.open(addr)) | ||
| )(channel => ZIO.attempt(channel.close()).orDie) | ||
| tls <- ZIO.attempt { | ||
| //make a simple request here and validate a server response |
There was a problem hiding this comment.
🌕 This comment is really not helping, sorry.
We know you're making a request and you validate the response.
The non obvious things are:
- why
5inByteBuffer.allocate(5)? - where is piece of code/algorithm comes from?
- is there any documentation that can explain this unusual piece of code?
guizmaii
changed the title
* Add ssl check * Review fixes * Review fixes * Review fixes * Review fixes * Review fixes * Fmt --------- Co-authored-by: Jules Ivanic <guizmaii@users.noreply.github.com>
* Add ssl check * Review fixes * Review fixes * Review fixes * Review fixes * Review fixes * Fmt --------- Co-authored-by: Jules Ivanic <guizmaii@users.noreply.github.com>
* Add ssl check * Review fixes * Review fixes * Review fixes * Review fixes * Review fixes * Fmt --------- Co-authored-by: Jules Ivanic <guizmaii@users.noreply.github.com>
* Add ssl check * Review fixes * Review fixes * Review fixes * Review fixes * Review fixes * Fmt --------- Co-authored-by: Jules Ivanic <guizmaii@users.noreply.github.com>
* Add ssl check * Review fixes * Review fixes * Review fixes * Review fixes * Review fixes * Fmt --------- Co-authored-by: Jules Ivanic <guizmaii@users.noreply.github.com>
* Add ssl check * Review fixes * Review fixes * Review fixes * Review fixes * Review fixes * Fmt --------- Co-authored-by: Jules Ivanic <guizmaii@users.noreply.github.com>
* Add ssl check * Review fixes * Review fixes * Review fixes * Review fixes * Review fixes * Fmt --------- Co-authored-by: Jules Ivanic <guizmaii@users.noreply.github.com>
No description provided.