-
Notifications
You must be signed in to change notification settings - Fork 94
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
fix: Also build rdkafka with ssl if ssl feature is enabled #881
Conversation
I tried to build this version, but got this:
Removing
It's interesting how SSL works on the HTTP side, but not the Kafka side. |
@ajacques I think I figured it out, can you try again |
Not sure what I'm doing wrong, but it's still failing with the same error:
|
no sorry you're right, this is a problem inside of the docker image. I am not sure what's going on though, why rust-rdkafka does not link properly against openssl while clearly all the other stuff can. It even uses the same crate (openssl-sys) to find the openssl installation |
@ajacques can you try again. This time it actually builds for me. |
We have success! I was able to connect to my broker over SSL using this latest commit. |
@@ -63,7 +63,9 @@ RUN echo "Building OpenSSL" \ | |||
FROM getsentry/sentry-cli:1 AS sentry-cli | |||
FROM relay-deps AS relay-builder | |||
|
|||
ARG RELAY_FEATURES=ssl,processing | |||
# ssl and processing are required for basic functionality in onprem |
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.
This is not exactly true, our onprem setup works well without kafka-ssl
. I would even go as far and not build Relay with kafka-ssl as I see this as a severe edge case; however, if you are confident that there are no side effects, we can leave it on.
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.
yeah by the comment I mean processing
and ssl
are required, kafka-ssl
is no extra cost to add
Co-authored-by: Jan Michael Auer <[email protected]>
* master: feat: Enable use of multiple value types per field (#882) fix(user-report): Make all fields but event-id optional (#886) release: 20.12.1 release: 20.12.0 ci(release): Move to getsentry/publish for releases (#885) meta: Fix CODEOWNERS (#884) fix: Also build rdkafka with ssl if ssl feature is enabled (#881)
)" This reverts commit 5f83948.
…889) Co-authored-by: Jan Michael Auer <[email protected]> This PR reverts #881 as the feature-flagging setup broke our development workflow Reverts #888 Reverts #881 We should find a different solution that does not prevent us from using --all-features. #skip-changelog
Fix #879
librdkafka can link exclusively against openssl, while relay's http stack actually uses native-tls abstraction which does not require openssl outside of linux (see relay's release page, we also offer standalone .exe outside of docker). This poses a problem for our local dev workflow, as all our workstations are OS X, have no OpenSSL, but we still want to compile Relay in processing mode. The solution is a separate featureflag
kafka-ssl
.