Skip to content

feat: add SSL support #575

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

Merged
merged 3 commits into from
Apr 22, 2025
Merged

Conversation

neilenns
Copy link
Contributor

In this PR:

  • New feature (non-breaking change that adds functionality)

This adds support for using SSL certificates and running the server at https://.

Key changes

  • Added SSL_KEY_PATH and SSL_CERT_PATH environment variables.
  • Updated environment variable documentation list the new variables, with a note that both must be provided to enable SSL.
  • Added the variables to .env.example
  • Added HTTP2 to .env.example since it was missed in feat: add HTTP/2 support via environment variable #565.

As mentioned in the issue, I'm unable to get the tests to run successfully, however that happens even without these changes. I didn't add any tests for the new feature, env tests only look for mandatory ones. The thought of trying to add test certs to the repo to test https creation scared me off of adding any tests like that 😅.

A Docker image with these changes applied is published at ghcr.io/neilenns/turborepo-remote-cache:ssl. I've been running an instance of this for a few days and it's great and accessible via Cloudflare, if someone wants to try that I can provide a TURBO_TOKEN somehow outside of GitHub (Discord?).

Issues reference:

Fixes #572

Checklist:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you linted your code locally with pnpm lint before submission?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully built locally with pnpm build?
  • Have you successfully tested locally with pnpm test?
  • Have you committed using Conventional Commits?

Copy link

gitstream-cm bot commented Apr 22, 2025

🚨 Warning: Approaching Monthly Automation Limit

Monthly PRs automated: 248/250

Your organization has used over 90% of its monthly quota for gitStream automations. Once the quota is reached, new pull requests for this month will not trigger gitStream automations and will be marked as “Skipped”.

To avoid interruptions, consider optimizing your automation usage or upgrading your plan by contacting LinearB.

@matteovivona matteovivona requested a review from fox1t April 22, 2025 15:11
@matteovivona matteovivona added this to the v2 milestone Apr 22, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces SSL support for the server by adding new environment variables and updating the Fastify server configuration to optionally enable HTTPS.

  • Added optional SSL_KEY_PATH and SSL_CERT_PATH to the environment schema
  • Updated Fastify options to include HTTPS configuration when SSL files are available
  • Updated documentation to reflect the new SSL-related environment variables

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

File Description
src/env.ts Added optional SSL environment variables to the schema
src/app.ts Updated Fastify configuration to conditionally enable HTTPS
docs/environment-variables.md Updated documentation with new SSL-related environment variables
Files not reviewed (1)
  • .env.example: Language not supported
Comments suppressed due to low confidence (1)

src/app.ts:26

  • Spreading a null value into an object literal can cause a runtime error; consider replacing 'null' with an empty object '{}'.
...(isHttps ? { https: { key: fs.readFileSync(SSL_KEY_PATH), cert: fs.readFileSync(SSL_CERT_PATH) } } : null),

@matteovivona
Copy link
Contributor

@all-contributors add @neilenns on code

Copy link
Contributor

@matteovivona

I've put up a pull request to add @neilenns! 🎉

@neilenns
Copy link
Contributor Author

Addressed Copilot feedback. It's way less picky than Coderabbit.ai 🥰

@neilenns
Copy link
Contributor Author

OMG AUTOFORMAT NOOOOO. Standby.

@neilenns neilenns requested a review from matteovivona April 22, 2025 15:26
@matteovivona matteovivona merged commit 5e3d582 into ducktors:main Apr 22, 2025
16 checks passed
@neilenns
Copy link
Contributor Author

🥳

@matteovivona
Copy link
Contributor

🎉 This PR is included in version 2.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Deploy using HTTPS/SSL
2 participants