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

deps: replace zeebe-node with @camunda8/sdk #4233

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Conversation

jwulf
Copy link
Member

@jwulf jwulf commented Apr 10, 2024

fixes #4109

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Apr 10, 2024
@barmac barmac self-requested a review April 10, 2024 11:06
@barmac
Copy link
Contributor

barmac commented Apr 11, 2024

We just had a session together. The mocks need to be fixed. @jwulf will continue on the source code migration.

Discovered #4236.

@barmac
Copy link
Contributor

barmac commented Apr 11, 2024

I rebased this on top of develop so that you can build it locally.

As a side note, you can also run the application via npm start which is significantly faster than waiting for the app to be built. You may need to restart the script once you change code in app directory.

@barmac barmac changed the title refactor: replace zeebe-node with @camunda8/sdk deps: replace zeebe-node with @camunda8/sdk Apr 11, 2024
@barmac
Copy link
Contributor

barmac commented Apr 11, 2024

I pushed changes of the test files which fix some of the failing cases. To be continued.

@barmac
Copy link
Contributor

barmac commented Apr 15, 2024

Let's make it a draft until it's ready for merge.

@barmac barmac marked this pull request as draft April 15, 2024 13:01
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Apr 15, 2024
@barmac
Copy link
Contributor

barmac commented Apr 15, 2024

@jwulf zeebe-node uses property port to later construct the URL, but I can't see a relevant property in the SDK config. Is it smart enough to figure out defaults for HTTP(s) or do we need to add them to the Zeebe address?

@barmac
Copy link
Contributor

barmac commented Apr 15, 2024

Also it looks like the SDK expects us to pass a path to the custom root certificates (e.g. CAMUNDA_CUSTOM_ROOT_CERT_PATH) while in zeebe-node we would pass a Buffer with certificates.

@barmac
Copy link
Contributor

barmac commented Apr 15, 2024

For the OS certificates, we cannot really pass a path.

@barmac
Copy link
Contributor

barmac commented Apr 15, 2024

I've migrated most of the tests to the new way of creating the client. @jwulf please take a look at the comments above so that we can proceed with this PR: #4233 (comment), #4233 (comment)

@nikku
Copy link
Member

nikku commented Apr 23, 2024

@barmac Let's ensure that it is clear what else we'd need to get this merged (potentially from the SDK side).

@barmac
Copy link
Contributor

barmac commented Apr 25, 2024

To get this merged, we need to:

@jwulf Please let me know if you need any support with this.

@jwulf
Copy link
Member Author

jwulf commented May 1, 2024

To get this merged, we need to:

@jwulf Please let me know if you need any support with this.

OK. I have moved the custom certificate management code into the SDK itself (see here). At the moment it can take a file path to a custom certificate.

I've added tests for the feature, as well as tests on Windows in CI (didn't have any of those previously - see here).

I have opened an issue to support passing the certificate itself as a Buffer or String, as zeebe-node used to.

camunda/camunda-8-js-sdk#142

I'll get to that at the end of this week or beginning of next week.

For the port number, this is now just part of the ZEEBE_ADDRESS parameter.

@barmac
Copy link
Contributor

barmac commented May 7, 2024

Thanks for the update. I will look into the changes this week.

@jwulf
Copy link
Member Author

jwulf commented May 7, 2024

Version 8.5.2 of the Camunda 8 JS SDK supports CAMUNDA_CUSTOM_ROOT_CERT_STRING as a configuration parameter. This allows you to supply a custom TLS certificate as a string.

It will be aggregated with the system certificates automatically, and used to verify the TLS certificate used to secure the Zeebe Gateway.

@barmac
Copy link
Contributor

barmac commented May 10, 2024

Unfortunately, I need to postpone this to the week after CamundaCon.

@barmac barmac self-assigned this May 21, 2024
@barmac
Copy link
Contributor

barmac commented May 21, 2024

I am looking into this today.

@barmac
Copy link
Contributor

barmac commented May 21, 2024

In order to check the SDK configuration, I implemented a connection test in camunda/zeebe-connection-test#12

@barmac
Copy link
Contributor

barmac commented May 22, 2024

@jwulf Is the basic auth supported in the SDK? I couldn't find a configuration option for this even though it was supported in zeebe-node.

@barmac
Copy link
Contributor

barmac commented May 23, 2024

This is blocked by camunda/camunda-8-js-sdk#165

@barmac
Copy link
Contributor

barmac commented May 24, 2024

This is blocked by camunda/camunda-8-js-sdk#165

We are unblocked now 🚀

@nikku
Copy link
Member

nikku commented May 24, 2024

@barmac You can verify this works via camunda/zeebe-connection-test#11

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

Successfully merging this pull request may close these issues.

Migrate from zeebe-node to @camunda8/sdk
3 participants