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

Additional options for kafka clients #2281

Merged
merged 5 commits into from
Jun 1, 2022

Conversation

mgotz
Copy link
Contributor

@mgotz mgotz commented Jun 1, 2022

I wanted to enable ssl encryption and authentication for the kafka communications in the alarm-system. This reqires several additional settings for the kafka consumer and producers.
This PR adds a kafka_properties preference to the phoebus client and a corresponding cmd option to the alarm-server, alarm-logger and alarm-config-logger. This preference is a path to a properties file to load additional kafka-client properties from. The properties file format and content is the same as the one for the kafka-console clients. This should provide a lot of flexibility to configure the kafka clients without adding a lot of extra preferences to phoebus.
In addition, I tried to add this to the existing cmd parse logic in the alarm-server. However, I found it very complicated to add an additional argument in the existing structure, while also ensuring that all combinations and also orders of arguments work properly. So, I rewrote the cmd argument parsing making the processing independent of the order the arguments were given. Hopefully, this makes it a bit more flexible and it removed some of the repetitive code.

@shroffk shroffk requested a review from kasemir June 1, 2022 08:49
@kasemir
Copy link
Collaborator

kasemir commented Jun 1, 2022

While there are many little changes, they're all just about adding a new kafka_properties(_file) parameter to the various methods. By default it's "" and basically ignored, so that should be fine.

Can you add some example for how to actually use ssl encryption and authentication?
For example, add a section to the https://github.com/ControlSystemStudio/phoebus/blob/master/app/alarm/Readme.md.
Towards the end, after "Demos" but before the final "Issues" paragraph, add a section

Encryption and Authentication
=======================

The default setup as described so far connects to Kafka without encryption nor authentication.
While this may be acceptable for a closed control system network, you can enable encryption and
require a user/password for kafka connections as follows.

Configure Kafka
----------------

In kafka/config/whatever.yml, add this to enable encrypted connections and ...

Configure CS-Studio UI, Alarm Server, Alarm Logger
----------------------------------------------------

Create a kafka properties file like this:

xxx =yyy
user=
password=


@shroffk
Copy link
Member

shroffk commented Jun 1, 2022

Documentation would be great, @mgotz could you open a new PR with the documentation
I will merge this now since I want to commit changes to the alarm-logger in preparation for elastic 8 and want to avoid conflicts

@shroffk shroffk merged commit 5b9fc41 into ControlSystemStudio:master Jun 1, 2022
@mgotz
Copy link
Contributor Author

mgotz commented Jun 2, 2022

Wonderful, I will add some documentation and make a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants