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

Add TLS Support #406

Merged
merged 3 commits into from
Jul 24, 2024
Merged

Add TLS Support #406

merged 3 commits into from
Jul 24, 2024

Conversation

willhoney7
Copy link
Contributor

@willhoney7 willhoney7 commented Jul 18, 2024

Adds support for passing in tlsOptions to a faktory connection: faktory.connect({ tlsOptions: { ... } });

A few notes:

  • I didn't add any tests, couldn't get the test suite to run locally (before my changes). Docker would boot up fine, but I kept getting timeout errors in ~1/2 the tests.
  • "host" and "port" are available as tlsOptions that technically override the passed in host and port values. Probably not a big deal, just something to be aware of.
  • My initial approach was to just "upgrade" the socket to a TLSSocket in the constructor but that didn't work correctly. Instead I had to modify the "connect" method to create a "new" TLSSocket, re-setup listeners, etc., when in tls mode.
  constructor(
    port: string | number,
    host?: string,
    tlsOptions?: tlsConnectionOptions
  ) {
    ...
    this.socket = new Socket();
    // this approach didn't seem to work.
    if (this.tlsOptions !== undefined) {
        this.socket = new TLSSocket(this.socket, tlsOptions);
    }
    this.socket.setKeepAlive(true);
    ...
  }

Closes #405

@jbielick jbielick merged commit 7db9c5f into jbielick:main Jul 24, 2024
4 checks passed
@jbielick
Copy link
Owner

Thank you.

@jbielick
Copy link
Owner

released in 4.6.0

npm notice name: faktory-worker
npm notice version: 4.6.0
npm notice filename: faktory-worker-4.6.0.tgz
npm notice package size: 34.9 kB
npm notice unpacked size: 119.7 kB
npm notice shasum: d8940f4f72468322120af453586cda81edf472a2
npm notice integrity: sha512-uof2Ym3IysNWa[...]hlXBGzxjup4sA==
npm notice total files: 35

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.

Support TLS
2 participants