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.

Sign up for GitHub

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

Jump to bottom

Add SSL Support to Kafka Provider Configuration #66

Merged
merged 29 commits into from
Nov 13, 2023
Merged

Add SSL Support to Kafka Provider Configuration #66

merged 29 commits into from
Nov 13, 2023

Conversation

walzph
Copy link
Contributor

@walzph walzph commented Sep 13, 2023 •
edited by KerstenBreuer
Loading

This PR introduces support for SSL communication between our service and Kafka brokers:

  • Added new fields security_protocol and ssl_context to the KafkaConfig class. This allows users to specify the desired communication protocol (PLAINTEXT or SSL) and provide an SSL context if required.

  • Updated the Kafka producer and consumer initialization code to utilize the new security_protocol and ssl_context configuration properties.

These changes ensure that we can establish secure connections to Kafka when necessary, enhancing the security posture of our system.

Bumps version to 0.11.0.

@walzph walzph changed the title Enable SSL for kafka provider Add SSL Support to Kafka Provider Configuration Sep 13, 2023
@github-actions
Copy link

github-actions bot commented Sep 13, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 6826089061

  • 110 of 117 (94.02%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 89.859%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/hexkit/providers/akafka/testcontainer.py 96 103 93.2%
Totals Coverage Status
Change from base Build 6729769151: 0.3%
Covered Lines: 1338
Relevant Lines: 1489
💛 - Coveralls

Cito
Cito requested changes Sep 14, 2023
View reviewed changes
hexkit/providers/akafka/provider.py Outdated Show resolved Hide resolved
hexkit/providers/akafka/provider.py Outdated Show resolved Hide resolved
Cito
Cito approved these changes Sep 20, 2023
View reviewed changes
Copy link
Member

@Cito Cito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but see comments.

hexkit/providers/akafka/provider.py Outdated Show resolved Hide resolved
KerstenBreuer
KerstenBreuer previously approved these changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@KerstenBreuer KerstenBreuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice. Thanks a lot. But as Christoph mentioned, tests would be nice either in this PR or a separate one.

@KerstenBreuer
Copy link
Contributor

Ah just saw that mypy and bandit are failing in the static checks.

Cito
Cito previously requested changes Sep 20, 2023
View reviewed changes
hexkit/providers/akafka/provider.py Outdated Show resolved Hide resolved
@KerstenBreuer
Copy link
Contributor

Hi @walzph, I will help you migrating to the latest template.

KerstenBreuer
KerstenBreuer previously approved these changes Oct 7, 2023
View reviewed changes
@KerstenBreuer
Copy link
Contributor

Hi @walzph, it is now compatible with the main branch again. Would you like to add tests to this PR or in a separate one?

@KerstenBreuer KerstenBreuer requested a review from Cito October 7, 2023 12:58
@walzph walzph force-pushed the enable-ssl-for-kafka branch from 9d3ee69 to 15fa49e Compare November 2, 2023 10:33
@dontseyit dontseyit marked this pull request as draft November 2, 2023 11:49
@walzph walzph force-pushed the enable-ssl-for-kafka branch from 9c56c5d to 77da1d5 Compare November 2, 2023 15:37
@Cito Cito marked this pull request as ready for review November 8, 2023 09:41
@Cito Cito removed their request for review November 8, 2023 09:42
@Cito Cito dismissed their stale review November 8, 2023 09:44

Rewrote the tests

mephenor
mephenor previously approved these changes Nov 8, 2023
View reviewed changes
.devcontainer/kafka_secrets/create_secrets.sh Outdated Show resolved Hide resolved
.devcontainer/kafka_secrets/create_secrets.sh Outdated Show resolved Hide resolved
@Cito Cito requested a review from mephenor November 8, 2023 11:40
@Cito Cito requested a review from KerstenBreuer November 9, 2023 17:52
KerstenBreuer
KerstenBreuer approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@KerstenBreuer KerstenBreuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good, thanks for going the extra mile.

KerstenBreuer
KerstenBreuer previously approved these changes Nov 10, 2023
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
Cito and others added 2 commits November 10, 2023 15:27
@Cito @KerstenBreuer
Update pyproject.toml
af44944 
Co-authored-by: Kersten Breuer <kersten-breuer@outlook.com>
@Cito
Recreate lock files
b3e09d5
@KerstenBreuer KerstenBreuer self-requested a review November 12, 2023 08:52
@Cito Cito merged commit 4c05b00 into main Nov 13, 2023
4 checks passed
@Cito Cito deleted the enable-ssl-for-kafka branch November 13, 2023 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cito Cito Cito left review comments

@KerstenBreuer KerstenBreuer KerstenBreuer approved these changes

@mephenor mephenor Awaiting requested review from mephenor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@walzph @KerstenBreuer @Cito @mephenor @dontseyit