GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Added extraCACerts config by gautamsi · Pull Request #1214 · triggerdotdev/trigger.dev

Conversation

@gautamsi

@gautamsi gautamsi commented Jul 13, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Closes #1210

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

  1. Generate a self signed cert and use this to protect trigger.dev deployment
  2. create a task which tries to access a self hosted api with self signed certificate
  3. try to deploy the project to self-hosted instance, see it fail
  4. create a ca.crt file containing signing certificate public key in pem format
  5. copy this file to project dir
  6. add extraCACerts config ./ca.crt
  7. deploy the task to self-hosted instance, this time it should deploy and run successfully

Changelog

Added config option extraCACerts to ProjectConfig type. This copies the ca file along with additionalFiles and sets NODE_EXTRA_CA_CERTS environment variable in built image as well as running the task.

Screenshots

[Screenshots]

💯

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gautamsi
Added extraCACerts config to help with self signed/private ca cert chain
9611fa8
@changeset-bot

changeset-bot Bot commented Jul 13, 2024

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 9611fa8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gautamsi
changeset
e787a80
gautamsi
gautamsi commented Jul 13, 2024
View reviewed changes
Comment thread packages/cli-v3/src/commands/deploy.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
nicktrn
nicktrn requested changes Jul 16, 2024
View reviewed changes

@nicktrn nicktrn left a comment

Copy link
Copy Markdown
Collaborator

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.

Thanks for this @gautamsi!

Good call with resolveConfig(). I think it will be a lot cleaner doing everything in there, that is:

  1. Add the raw extraCACerts path to additionalFiles
  2. Tranform it so resolvedConfig.extraCACerts now points at the deployed destination path -> /app/...

Re the last point: We could even just strip .. and keep the path relative to /app if NODE_EXTRA_CA_CERTS works fine with relative paths. /app is the work dir in the resulting image so that should be fine.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gautamsi reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread packages/cli-v3/src/commands/deploy.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread packages/cli-v3/src/commands/deploy.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread packages/cli-v3/src/commands/deploy.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread packages/cli-v3/src/commands/deploy.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread packages/cli-v3/src/commands/deploy.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
@nicktrn
Update changeset
8ebbf09
@gautamsi

gautamsi commented Jul 16, 2024

Copy link
Copy Markdown
Contributor Author

I will update this soon
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gautamsi and others added 2 commits July 16, 2024 21:14
@gautamsi
resolved pr comments
f606d3e
@nicktrn
Merge branch 'main' into fixes-triggerdotdev#1210
e69a20f
nicktrn
nicktrn approved these changes Jul 17, 2024
View reviewed changes
@nicktrn

nicktrn commented Jul 17, 2024

Copy link
Copy Markdown
Collaborator

Thanks for the changes! Will get this merged and tested. Should be in the next release.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nicktrn
nicktrn merged commit e417aca into triggerdotdev:main Jul 17, 2024
@gautamsi
gautamsi deleted the fixes-#1210 branch July 17, 2024 11:02
@gautamsi

gautamsi commented Jul 18, 2024

Copy link
Copy Markdown
Contributor Author

verified beta-49 works with this feature :)
nicktrn reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nicktrn

nicktrn commented Jul 18, 2024

Copy link
Copy Markdown
Collaborator

I did test this locally briefly and relative paths work as well 👍
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gautamsi gautamsi mentioned this pull request Sep 21, 2024
v3: new build system #1265
Merged
3 of 6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicktrn nicktrn nicktrn approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: Allow injecting ca cert in project

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@gautamsi @nicktrn
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.