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.

Sign up for GitHub

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

Jump to bottom

feat: allow certificate to be supplied to serve #7

Merged
merged 4 commits into from
May 3, 2022
Merged

feat: allow certificate to be supplied to serve #7

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4e60d5
feat: allow certificate to be supplied to `serve`
eduardoboucas Apr 27, 2022
d56bbec
feat: add support for server certificate
eduardoboucas Apr 28, 2022
b739569
refactor: remove `--origin-protocol` flag
eduardoboucas Apr 28, 2022
d9a8924
Merge branch 'main' into feat/server-cert
eduardoboucas May 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/server/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ interface PrepareServerOptions {
const prepareServer = ({
deno,
distDirectory,
flags,
flags: denoFlags,
formatExportTypeError,
formatImportError,
port,
Expand Down Expand Up @@ -56,7 +56,9 @@ const prepareServer = ({
// no-op
}

await deno.runInBackground(['run', ...flags, stage2Path, port.toString()], true, processRef)
const bootstrapFlags = ['--port', port.toString()]
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like we didn't supply the --port flag before - assuming this is unrelated to the certificate path, was broken before, and you drive-by-fixed this.

Copy link
Member Author

Choose a reason for hiding this comment

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

We did supply it, but as a positional parameter and not a flag (i.e. deno run script.js <PORT> as opposed to deno run script.js --port <PORT>). Using flags will allow us to supply additional parameters in the future in a more sustainable way.

You're right that it's not necessarily related to the certificate path, and I should've clarified that. Thanks for calling that out!

Copy link
Member Author

Choose a reason for hiding this comment

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

See https://github.com/netlify/edge-functions-bootstrap/pull/30/files#r860951538.


await deno.runInBackground(['run', ...denoFlags, stage2Path, ...bootstrapFlags], true, processRef)

const success = await waitForServer(port, processRef.ps)

Expand All @@ -70,6 +72,7 @@ const prepareServer = ({
}

interface ServeOptions {
certificatePath?: string
debug?: boolean
distImportMapPath?: string
importMaps?: ImportMapFile[]
Expand All @@ -81,6 +84,7 @@ interface ServeOptions {
}

const serve = async ({
certificatePath,
debug,
distImportMapPath,
formatExportTypeError,
Expand Down Expand Up @@ -108,13 +112,24 @@ const serve = async ({
const importMap = new ImportMap(importMaps)
const flags = ['--allow-all', '--unstable', `--import-map=${importMap.toDataURL()}`]

if (certificatePath) {
flags.push(`--cert=${certificatePath}`)
}

if (debug) {
flags.push('--log-level=debug')
} else {
flags.push('--quiet')
}

const server = await prepareServer({ deno, distDirectory, flags, formatExportTypeError, formatImportError, port })
const server = await prepareServer({
deno,
distDirectory,
flags,
formatExportTypeError,
formatImportError,
port,
})

if (distImportMapPath) {
await importMap.writeToFile(distImportMapPath)
Expand Down