-
Notifications
You must be signed in to change notification settings - Fork 750
Conversation
Adding an example configuration for SSL based connectivity for Kafka Triggers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the example! I have a few suggestions
@@ -171,3 +171,38 @@ When using SSL to secure kafka communication, you must set `KAFKA_ENABLE_TLS`, a | |||
* `KAFKA_CACERTS` to check server certificate | |||
* `KAFKA_CERT` and `KAFKA_KEY` to check client certificate | |||
* `KAFKA_INSECURE` to skip TLS verfication | |||
```yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind adding some paragraph before doing what you are setting here? Something like "Below you can find an example of a Kafka controller deployment using TLS"
docs/use-existing-kafka.md
Outdated
@@ -171,3 +171,38 @@ When using SSL to secure kafka communication, you must set `KAFKA_ENABLE_TLS`, a | |||
* `KAFKA_CACERTS` to check server certificate | |||
* `KAFKA_CERT` and `KAFKA_KEY` to check client certificate | |||
* `KAFKA_INSECURE` to skip TLS verfication | |||
```yaml | |||
$ echo ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove the $echo '
part (I know it's like that in the other example but it's not clarifying anything
docs/use-existing-kafka.md
Outdated
env: | ||
... | ||
- name: KAFKA_ENABLE_TLS | ||
value: "true" # CHANGE THIS! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are using TLS this should be "true"
right? Why are you setting the comment CHANGE THIS
?
value: "/path/to/cert.pem" # CHANGE THIS! (MAKE SURE PATH ARE VOLUME MOUNTED SAY FROM SECRETS) | ||
- name: KAFKA_KEY | ||
value: "/path/to/key.pem" # CHANGE THIS! (MAKE SURE PATH ARE VOLUME MOUNTED SAY FROM SECRETS) | ||
... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind adding the volumeMounts
and volumes
section? So people know these files come from a Secret.
Done Changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update!
Adding an example configuration for SSL based connectivity for Kafka Triggers
Issue Ref: [Issue number related to this PR or None]
Description:
[PR Description]
TODOs: