-
Notifications
You must be signed in to change notification settings - Fork 118
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] Use OAuth2 on macOX arm64 failed. #282
Conversation
@merlimat @BewareMyPower I'm not sure if this is a good solution, PTAL. |
if [ $ARCH = 'arm64' ]; then | ||
SSL_CONF="--with-secure-transport" | ||
else | ||
SSL_CONF="--without-secure-transport --with-ssl=$PREFIX" | ||
fi |
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 didn't get why the secure transport is disabled when building curl on x64 architectures.
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 parameter means disabled curl calls to Apple's native secure transport. Refer here
I tested and the parameter doesn't work if compiled on x86_64. So when build the x86_64, leave it as it is (using OpenSSL)
I think we need to wait for the feedback from @ericallam.
It does not make sense to me. Can you confirm it's a bug of OpenSSL? Maybe it requires some certificates like this issue: https://stackoverflow.com/questions/65442972/ssl-routinestls-process-server-certificatecertificate-verify-failed |
What's the best way for me to test this? |
@ericallam You can follow the guide here (make sure this PR is included): pkg/mac/build-cpp-deps-lib.sh
pkg/mac/build-cpp-lib.sh
npm install Then you can replace any file under the @shibd BTW, I'm not sure whether we can provide the |
I don't think we need to expose If the user build from the source, It can be used after global installation:
|
Okay I'm working on testing this now. I started with Building from source using the steps in the README on the const Pulsar = require("../");
(async () => {
Pulsar.Client.setLogHandler((level, file, line, message) => {
console.log("[%s][%s:%d] %s", level, file, line, message);
});
const auth = new Pulsar.AuthenticationOauth2({
type: "sn_service_account",
client_id: "...",
client_secret: "...",
issuer_url: "https://auth.streamnative.cloud/",
audience: "...",
});
// Create a client
const client = new Pulsar.Client({
serviceUrl: "pulsar+ssl://<cluster>.<orgId>.snio.cloud:6651",
authentication: auth,
});
// Create a consumer
const consumer = await client.subscribe({
topic: "persistent://public/default/my-topic",
subscription: "sub1",
subscriptionType: "Shared",
});
const producer = await client.createProducer({
topic: "persistent://public/default/my-topic",
sendTimeoutMs: 30000,
batchingEnabled: true,
});
for (let i = 0; i < 10; i += 1) {
const msg = `my-message-${i}`;
producer.send({
data: Buffer.from(msg),
});
console.log(`Sent message: ${msg}`);
}
await producer.flush();
// Receive messages
for (let i = 0; i < 10; i += 1) {
const msg = await consumer.receive();
console.log(msg.getData().toString());
consumer.acknowledge(msg);
}
await producer.close();
await consumer.close();
await client.close();
})(); And here's the result of running that:
Now after integrating the changes from the PR, I did the following: $ rm -rf ./pkg/mac/build
$ ./pkg/mac/build-cpp-deps-lib.sh
$ ./pkg/mac/build-cpp-lib.sh
$ npm install Then I tried running an example and I'm getting an error:
|
I don't think this is an macOS issue. I tried running 1.8.0 in a docker container (built and run on linux) and I'm getting a similar error:
Am I supposed to be setting an option that I am not? Any of these: |
Hi, @ericallam Thanks for your feedback. I just wanted to confirm. Before v1.7.0, the same program and environment could run normally? I haven't tested with a certified domain yet, and I can run it by manually importing the certificate. Refer repo: https://github.com/shibd/pulsar-node-test
If you can download the certificate, you can try pointing to it:
I still suspect that linking static OpenSSL is the cause, I will try linking dynamic libraries to try. |
I have 1.7.0 working just fine locally, but it's connecting to a local docker container running and I don't use any authentication. This has only started once I've tried to add authentication (because I'm trying to connect to StreamNative cloud hosted instance). I'm now also trying to deploy it to production and this is where I'm running into issues. According to the StreamNative nodejs docs, they suggest passing
|
Hi, @ericallam Thanks for your feedback. I also used the cloud environment test, and after compiling through the source code according to the README step, I got the same error as you. I will prioritize trying to fix this issue. |
I was able to connect to StreamNative cloud with OAuth2 credentials on 1.7.0 from macOS, using the following steps to install the pulsar-client: https://github.com/triggerdotdev/trigger.dev/blob/main/DEVELOPMENT.md#pulsar-requirements |
This issue is only happening on NodeJS 18.0+ (at least from my tests), and does not occur when we downgrade to Node 16.19.0 on the 1.7.0 version |
@ericallam Hi, I released a hotfix version in my repository using this PR change, and I can use Could you refer to this repository README.md to see if it can be run on your host? |
I pulled down your repository and followed your directions and I got this error (using Node.js v18.12.1)
I tried using Node.js v16.19.0 as well but got the same error. |
@ericallam Could this error be reproduced by running the following code? const Pulsar = require('./node_modules/shibaodi-pulsar-client/lib/binding/Pulsar.node') You can change the path in |
From #282 (comment) I see
Then I tried the following patch, which adds the same compile flags as this PR does for arm64 macOS. diff --git a/pkg/mac/build-cpp-deps-lib.sh b/pkg/mac/build-cpp-deps-lib.sh
index e2078d1..aa4e04d 100755
--- a/pkg/mac/build-cpp-deps-lib.sh
+++ b/pkg/mac/build-cpp-deps-lib.sh
@@ -167,12 +167,12 @@ if [ ! -f curl-${CURL_VERSION}.done ]; then
tar xfz curl-${CURL_VERSION}.tar.gz
pushd curl-${CURL_VERSION}
CFLAGS="-fPIC -arch ${ARCH} -mmacosx-version-min=${MACOSX_DEPLOYMENT_TARGET}" \
- ./configure --with-ssl=$PREFIX \
+ ./configure \
--without-nghttp2 \
--without-libidn2 \
--disable-ldap \
--without-brotli \
- --without-secure-transport \
+ --with-secure-transport \
--disable-ipv6 \
--prefix=$PREFIX \
--host=$ARCH-apple-darwin Then, build from source. pkg/mac/build-cpp-deps-lib.sh
pkg/mac/build-cpp-lib.sh
npm install After that, I reproduced the same issue with #282 (comment) successfully
It's caused by loading the
|
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 the root cause of the "symbol not found" error is that libcurl.a
is statically linked, but after adding the --with-secure-transport
option when compiling libcurl.a
, the libcurl.a
will depend on Apple's SSL/TLS implementation, which is the Security Framework like:
/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 60158.100.133)
(You can run otool -L libcurl.dylib
to see the dependencies.)
If libpulsarwithdeps.a
doesn't link the Security Framework, the symbol would not be found. The current build process works well because the TLS function of libcurl
depends on OpenSSL, which is statically linked by libpulsarwithdeps.a
as well.
Here is how I solved the similar error when linking statically to libcurl on macOS:
-DCMAKE_CXX_FLAGS="-framework SystemConfiguration -framework CoreFoundation"
The command above links to the SystemConfiguration and CoreFoundation frameworks of macOS.
The dependency tree of current master:
Pulsar.node
- libpulsarwithdeps.a
- libcurl.a (TLS related symbols are included from libssl.a)
- libssl.a
- ...
After this patch:
Pulsar.node
- libpulsarwithdeps.a
- libcurl.a (TLS related symbols are included from Security Framework)
- libssl.a
- ...
It's not a good idea, I'll close it first. |
Fixes #281
Motivation
This issue only happened on the arm64 macOS environment and when using SSL.
This can be caused by static linking cross-compiled OpenSSL.
I noticed that CURL could directly configure the
--with-secure-transport
parameter to use Apple's SSL/TLS implementation instead of OpenSSL:Original document:
Modifications
--with-secure-transport
parameter to use Apple's SSL/TLS implementation when build arm64 macOS.Verifying this change
OAuth2
related unit tests later.Documentation
doc-required
(Your PR needs to update docs and you will update later)
doc-not-needed
(Please explain why)
doc
(Your PR contains doc changes)
doc-complete
(Docs have been already added)