From 2206898b141e1ce54ae979b21bfb7d1c999b8cde Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Mon, 1 Aug 2022 18:50:03 +0200 Subject: [PATCH 1/9] feat: allow setting environment variables and disable extending in DenoServer --- src/bridge.ts | 18 ++++++++++-------- src/server/server.ts | 8 ++++++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/bridge.ts b/src/bridge.ts index 9e2f44c5..c5049e28 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -31,6 +31,8 @@ interface ProcessRef { interface RunOptions { pipeOutput?: boolean + env?: NodeJS.ProcessEnv + extendEnv?: boolean } class DenoBridge { @@ -186,8 +188,8 @@ class DenoBridge { return { global: false, path: downloadedPath } } - getEnvironmentVariables() { - const env: Record = {} + getEnvironmentVariables(inputEnv: NodeJS.ProcessEnv = {}) { + const env: NodeJS.ProcessEnv = { ...inputEnv } if (this.denoDir !== undefined) { env.DENO_DIR = this.denoDir @@ -206,20 +208,20 @@ class DenoBridge { // Runs the Deno CLI in the background and returns a reference to the child // process, awaiting its execution. - async run(args: string[], { pipeOutput }: RunOptions = {}) { + async run(args: string[], { pipeOutput, env: inputEnv, extendEnv }: RunOptions = {}) { const { path: binaryPath } = await this.getBinaryPath() - const env = this.getEnvironmentVariables() - const options = { env } + const env = this.getEnvironmentVariables(inputEnv) + const options: Options = { env, extendEnv } return DenoBridge.runWithBinary(binaryPath, args, options, pipeOutput) } // Runs the Deno CLI in the background, assigning a reference of the child // process to a `ps` property in the `ref` argument, if one is supplied. - async runInBackground(args: string[], pipeOutput?: boolean, ref?: ProcessRef) { + async runInBackground(args: string[], ref?: ProcessRef, { pipeOutput, env: inputEnv, extendEnv }: RunOptions = {}) { const { path: binaryPath } = await this.getBinaryPath() - const env = this.getEnvironmentVariables() - const options = { env } + const env = this.getEnvironmentVariables(inputEnv) + const options: Options = { env, extendEnv } const ps = DenoBridge.runWithBinary(binaryPath, args, options, pipeOutput) if (ref !== undefined) { diff --git a/src/server/server.ts b/src/server/server.ts index 45321d83..19135303 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -29,7 +29,7 @@ const prepareServer = ({ port, }: PrepareServerOptions) => { const processRef: ProcessRef = {} - const startIsolate = async (newFunctions: EdgeFunction[]) => { + const startIsolate = async (newFunctions: EdgeFunction[], env: NodeJS.ProcessEnv = {}) => { if (processRef?.ps !== undefined) { await killProcess(processRef.ps) } @@ -59,7 +59,11 @@ const prepareServer = ({ const bootstrapFlags = ['--port', port.toString()] - await deno.runInBackground(['run', ...denoFlags, stage2Path, ...bootstrapFlags], true, processRef) + await deno.runInBackground(['run', ...denoFlags, stage2Path, ...bootstrapFlags], processRef, { + pipeOutput: true, + env, + extendEnv: false, + }) const success = await waitForServer(port, processRef.ps) From a7aab678dd12e52e873cb2f98fb06a029c5b5a3c Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Mon, 1 Aug 2022 19:01:37 +0200 Subject: [PATCH 2/9] chore: fix test --- test/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/main.ts b/test/main.ts index 25303729..6591b269 100644 --- a/test/main.ts +++ b/test/main.ts @@ -17,7 +17,7 @@ const archiver = require('archiver') test('Downloads the Deno CLI on demand and caches it for subsequent calls', async (t) => { const latestVersion = '1.20.3' - const mockBinaryOutput = `#!/usr/bin/env node\n\nconsole.log("deno ${latestVersion}")` + const mockBinaryOutput = `#!/usr/bin/env sh\n\necho "deno ${latestVersion}"` const data = new PassThrough() const archive = archiver('zip', { zlib: { level: 9 } }) From 80b52fd4e5ba59b61fa80cf68de1bf619af87838 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Mon, 1 Aug 2022 19:05:31 +0200 Subject: [PATCH 3/9] chore: fix tests --- src/bridge.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/bridge.ts b/src/bridge.ts index c5049e28..a27c4937 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -208,7 +208,7 @@ class DenoBridge { // Runs the Deno CLI in the background and returns a reference to the child // process, awaiting its execution. - async run(args: string[], { pipeOutput, env: inputEnv, extendEnv }: RunOptions = {}) { + async run(args: string[], { pipeOutput, env: inputEnv, extendEnv = true }: RunOptions = {}) { const { path: binaryPath } = await this.getBinaryPath() const env = this.getEnvironmentVariables(inputEnv) const options: Options = { env, extendEnv } @@ -218,7 +218,11 @@ class DenoBridge { // Runs the Deno CLI in the background, assigning a reference of the child // process to a `ps` property in the `ref` argument, if one is supplied. - async runInBackground(args: string[], ref?: ProcessRef, { pipeOutput, env: inputEnv, extendEnv }: RunOptions = {}) { + async runInBackground( + args: string[], + ref?: ProcessRef, + { pipeOutput, env: inputEnv, extendEnv = true }: RunOptions = {}, + ) { const { path: binaryPath } = await this.getBinaryPath() const env = this.getEnvironmentVariables(inputEnv) const options: Options = { env, extendEnv } From 4037410f7838d455c7cc59b16cbd187768582572 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Thu, 4 Aug 2022 12:03:05 +0200 Subject: [PATCH 4/9] chore: add test --- test/bridge.ts | 141 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 test/bridge.ts diff --git a/test/bridge.ts b/test/bridge.ts new file mode 100644 index 00000000..6b744cad --- /dev/null +++ b/test/bridge.ts @@ -0,0 +1,141 @@ +import { Buffer } from 'buffer' +import fs from 'fs' +import { createRequire } from 'module' +import { platform, env } from 'process' +import { PassThrough } from 'stream' + +import test from 'ava' +import nock from 'nock' +import { spy } from 'sinon' +import tmp, { DirectoryResult } from 'tmp-promise' + +import { DenoBridge } from '../src/bridge.js' +import { getPlatformTarget } from '../src/platform.js' + +const require = createRequire(import.meta.url) +const archiver = require('archiver') + +const getMockDenoBridge = function (tmpDir: DirectoryResult, mockBinaryOutput: string) { + const latestVersion = '1.20.3' + const data = new PassThrough() + const archive = archiver('zip', { zlib: { level: 9 } }) + + archive.pipe(data) + archive.append(Buffer.from(mockBinaryOutput.replace(/@@@latestVersion@@@/g, latestVersion)), { + name: platform === 'win32' ? 'deno.exe' : 'deno', + }) + archive.finalize() + + const target = getPlatformTarget() + + nock('https://dl.deno.land').get('/release-latest.txt').reply(200, `v${latestVersion}`) + nock('https://dl.deno.land') + .get(`/release/v${latestVersion}/deno-${target}.zip`) + .reply(200, () => data) + + const beforeDownload = spy() + const afterDownload = spy() + + return new DenoBridge({ + cacheDirectory: tmpDir.path, + onBeforeDownload: beforeDownload, + onAfterDownload: afterDownload, + useGlobal: false, + }) +} + +test('Does not inherit environment variables if `extendEnv` is false', async (t) => { + const tmpDir = await tmp.dir() + const deno = getMockDenoBridge( + tmpDir, + `#!/usr/bin/env sh + + if [ "$1" == "test" ] + then + env + else + echo "deno @@@latestVersion@@@" + fi`, + ) + + // The environment sets some varaibles so let us see what they are and remove them from the result + const referenceOutput = await deno.run(['test'], { env: {}, extendEnv: false }) + env.TADA = 'TUDU' + const result = await deno.run(['test'], { env: { LULU: 'LALA' }, extendEnv: false }) + let output = result?.stdout ?? '' + + delete env.TADA + + referenceOutput?.stdout.split('\n').forEach((line) => { + output = output.replace(line.trim(), '') + }) + output = output.trim().replace(/\n+/, '\n') + + t.is(output, 'LULU=LALA') + + await fs.promises.rmdir(tmpDir.path, { recursive: true }) +}) + +test('Does inherit environment variables if `extendEnv` is true', async (t) => { + const tmpDir = await tmp.dir() + const deno = getMockDenoBridge( + tmpDir, + `#!/usr/bin/env sh + + if [ "$1" == "test" ] + then + env + else + echo "deno @@@latestVersion@@@" + fi`, + ) + + // The environment sets some variables so let us see what they are and remove them from the result + const referenceOutput = await deno.run(['test'], { env: {}, extendEnv: true }) + env.TADA = 'TUDU' + const result = await deno.run(['test'], { env: { LULU: 'LALA' }, extendEnv: true }) + let output = result?.stdout ?? '' + + delete env.TADA + + referenceOutput?.stdout.split('\n').forEach((line) => { + output = output.replace(line.trim(), '') + }) + output = output.trim().replace(/\n+/, '\n') + + t.is(output, 'LULU=LALA\nTADA=TUDU') + + await fs.promises.rmdir(tmpDir.path, { recursive: true }) +}) + +test('Does inherit environment variables if `extendEnv` is not set', async (t) => { + const tmpDir = await tmp.dir() + const deno = getMockDenoBridge( + tmpDir, + `#!/usr/bin/env sh + + if [ "$1" == "test" ] + then + env + else + echo "deno @@@latestVersion@@@" + fi`, + ) + + // The environment sets some variables so let us see what they are and remove them from the result + const referenceOutput = await deno.run(['test'], { env: {}, extendEnv: true }) + env.TADA = 'TUDU' + const result = await deno.run(['test'], { env: { LULU: 'LALA' } }) + let output = result?.stdout ?? '' + + delete env.TADA + + referenceOutput?.stdout.split('\n').forEach((line) => { + output = output.replace(line.trim(), '') + }) + output = output.trim().replace(/\n+/, '\n') + + t.is(output, 'LULU=LALA\nTADA=TUDU') + + await fs.promises.rmdir(tmpDir.path, { recursive: true }) +}) From 889334434881944a52e2aba69a1c55d1e23ca9c3 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Thu, 4 Aug 2022 12:22:44 +0200 Subject: [PATCH 5/9] chore: debug --- test/bridge.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/bridge.ts b/test/bridge.ts index 6b744cad..6f972667 100644 --- a/test/bridge.ts +++ b/test/bridge.ts @@ -124,6 +124,7 @@ test('Does inherit environment variables if `extendEnv` is not set', async (t) = // The environment sets some variables so let us see what they are and remove them from the result const referenceOutput = await deno.run(['test'], { env: {}, extendEnv: true }) + console.log(referenceOutput?.stdout) env.TADA = 'TUDU' const result = await deno.run(['test'], { env: { LULU: 'LALA' } }) let output = result?.stdout ?? '' From 231d3a4732ad28433b17d529fdbe4ff791a75dcc Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Thu, 4 Aug 2022 12:27:24 +0200 Subject: [PATCH 6/9] chore: fix tests --- test/bridge.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/bridge.ts b/test/bridge.ts index 6f972667..67b56872 100644 --- a/test/bridge.ts +++ b/test/bridge.ts @@ -50,7 +50,7 @@ test('Does not inherit environment variables if `extendEnv` is false', async (t) tmpDir, `#!/usr/bin/env sh - if [ "$1" == "test" ] + if [ "$1" = "test" ] then env else @@ -82,7 +82,7 @@ test('Does inherit environment variables if `extendEnv` is true', async (t) => { tmpDir, `#!/usr/bin/env sh - if [ "$1" == "test" ] + if [ "$1" = "test" ] then env else @@ -114,7 +114,7 @@ test('Does inherit environment variables if `extendEnv` is not set', async (t) = tmpDir, `#!/usr/bin/env sh - if [ "$1" == "test" ] + if [ "$1" = "test" ] then env else @@ -124,7 +124,6 @@ test('Does inherit environment variables if `extendEnv` is not set', async (t) = // The environment sets some variables so let us see what they are and remove them from the result const referenceOutput = await deno.run(['test'], { env: {}, extendEnv: true }) - console.log(referenceOutput?.stdout) env.TADA = 'TUDU' const result = await deno.run(['test'], { env: { LULU: 'LALA' } }) let output = result?.stdout ?? '' From c7a391135d54543167970b85e667213c0e859fe0 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Thu, 4 Aug 2022 12:45:44 +0200 Subject: [PATCH 7/9] chore: fix tests? --- test/bridge.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/bridge.ts b/test/bridge.ts index 67b56872..bb276ddc 100644 --- a/test/bridge.ts +++ b/test/bridge.ts @@ -44,7 +44,7 @@ const getMockDenoBridge = function (tmpDir: DirectoryResult, mockBinaryOutput: s }) } -test('Does not inherit environment variables if `extendEnv` is false', async (t) => { +test.serial('Does not inherit environment variables if `extendEnv` is false', async (t) => { const tmpDir = await tmp.dir() const deno = getMockDenoBridge( tmpDir, @@ -76,7 +76,7 @@ test('Does not inherit environment variables if `extendEnv` is false', async (t) await fs.promises.rmdir(tmpDir.path, { recursive: true }) }) -test('Does inherit environment variables if `extendEnv` is true', async (t) => { +test.serial('Does inherit environment variables if `extendEnv` is true', async (t) => { const tmpDir = await tmp.dir() const deno = getMockDenoBridge( tmpDir, @@ -108,7 +108,7 @@ test('Does inherit environment variables if `extendEnv` is true', async (t) => { await fs.promises.rmdir(tmpDir.path, { recursive: true }) }) -test('Does inherit environment variables if `extendEnv` is not set', async (t) => { +test.serial('Does inherit environment variables if `extendEnv` is not set', async (t) => { const tmpDir = await tmp.dir() const deno = getMockDenoBridge( tmpDir, From ea134f35bedebd350e77480f0d9be714743313f0 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Mon, 8 Aug 2022 15:13:26 +0200 Subject: [PATCH 8/9] Update test/bridge.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eduardo Bouças --- test/bridge.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bridge.ts b/test/bridge.ts index bb276ddc..05d97d48 100644 --- a/test/bridge.ts +++ b/test/bridge.ts @@ -58,7 +58,7 @@ test.serial('Does not inherit environment variables if `extendEnv` is false', as fi`, ) - // The environment sets some varaibles so let us see what they are and remove them from the result + // The environment sets some variables so let us see what they are and remove them from the result const referenceOutput = await deno.run(['test'], { env: {}, extendEnv: false }) env.TADA = 'TUDU' const result = await deno.run(['test'], { env: { LULU: 'LALA' }, extendEnv: false }) From f6fe66a1f0051bc7aa723365d5f36d0eca1eaf7d Mon Sep 17 00:00:00 2001 From: Daniel Tschinder <231804+danez@users.noreply.github.com> Date: Mon, 8 Aug 2022 15:33:36 +0200 Subject: [PATCH 9/9] Update src/server/server.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eduardo Bouças --- src/server/server.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/server/server.ts b/src/server/server.ts index ec3c17cb..fc27a7f0 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -60,6 +60,9 @@ const prepareServer = ({ const bootstrapFlags = ['--port', port.toString()] + // We set `extendEnv: false` to avoid polluting the edge function context + // with variables from the user's system, since those will not be available + // in the production environment. await deno.runInBackground(['run', ...denoFlags, stage2Path, ...bootstrapFlags], processRef, { pipeOutput: true, env,