Skip to content

Commit

Permalink
fix: parseConfig stumbling over globalThis.Netlify usage in global …
Browse files Browse the repository at this point in the history
…scope (#427)

* chore: implement regression test

* fix: fix!

* fix: read globals from bootstrap

* fix: read Netlify from index-combined.ts

* feat: allow bootstrapURL to be injected from the outside
  • Loading branch information
Skn0tt committed Jul 27, 2023
1 parent a9e1e11 commit d829e70
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 26 deletions.
5 changes: 4 additions & 1 deletion deno/config.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
const [functionURL, collectorURL, rawExitCodes] = Deno.args
const [functionURL, collectorURL, bootstrapURL, rawExitCodes] = Deno.args
const exitCodes = JSON.parse(rawExitCodes)

const { Netlify } = await import(bootstrapURL)
globalThis.Netlify = Netlify

let func

try {
Expand Down
6 changes: 4 additions & 2 deletions node/bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ interface BundleOptions {
onBeforeDownload?: OnBeforeDownloadHook
systemLogger?: LogFunction
internalSrcFolder?: string
bootstrapURL?: string
}

const bundle = async (
Expand All @@ -49,6 +50,7 @@ const bundle = async (
onBeforeDownload,
systemLogger,
internalSrcFolder,
bootstrapURL = 'https://edge.netlify.com/bootstrap/index-combined.ts',
}: BundleOptions = {},
) => {
const logger = getLogger(systemLogger, debug)
Expand Down Expand Up @@ -113,11 +115,11 @@ const bundle = async (
// Retrieving a configuration object for each function.
// Run `getFunctionConfig` in parallel as it is a non-trivial operation and spins up deno
const internalConfigPromises = internalFunctions.map(
async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger)] as const,
async (func) => [func.name, await getFunctionConfig({ func, importMap, deno, log: logger, bootstrapURL })] as const,
)

const userConfigPromises = userFunctions.map(
async (func) => [func.name, await getFunctionConfig(func, importMap, deno, logger)] as const,
async (func) => [func.name, await getFunctionConfig({ func, importMap, deno, log: logger, bootstrapURL })] as const,
)

// Creating a hash of function names to configuration objects.
Expand Down
46 changes: 26 additions & 20 deletions node/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { FunctionConfig, getFunctionConfig } from './config.js'
import type { Declaration } from './declaration.js'
import { ImportMap } from './import_map.js'

const bootstrapURL = 'https://edge.netlify.com/bootstrap/index-combined.ts'

const importMapFile = {
baseURL: new URL('file:///some/path/import-map.json'),
imports: {
Expand Down Expand Up @@ -136,15 +138,16 @@ describe('`getFunctionConfig` extracts configuration properties from function fi
await fs.writeFile(path, func.source)

const funcCall = () =>
getFunctionConfig(
{
getFunctionConfig({
func: {
name: func.name,
path,
},
new ImportMap([importMapFile]),
importMap: new ImportMap([importMapFile]),
deno,
logger,
)
log: logger,
bootstrapURL,
})

if (func.error) {
await expect(funcCall()).rejects.toThrowError(func.error)
Expand Down Expand Up @@ -235,15 +238,16 @@ test('Passes validation if default export exists and is a function', async () =>
await fs.writeFile(path, func.source)

await expect(
getFunctionConfig(
{
getFunctionConfig({
func: {
name: func.name,
path,
},
new ImportMap([importMapFile]),
importMap: new ImportMap([importMapFile]),
deno,
logger,
),
log: logger,
bootstrapURL,
}),
).resolves.not.toThrow()

await rm(tmpDir, { force: true, recursive: true, maxRetries: 10 })
Expand Down Expand Up @@ -271,15 +275,16 @@ test('Fails validation if default export is not function', async () => {

await fs.writeFile(path, func.source)

const config = getFunctionConfig(
{
const config = getFunctionConfig({
func: {
name: func.name,
path,
},
new ImportMap([importMapFile]),
importMap: new ImportMap([importMapFile]),
deno,
logger,
)
log: logger,
bootstrapURL,
})

await expect(config).rejects.toThrowError(invalidDefaultExportErr(path))

Expand Down Expand Up @@ -307,15 +312,16 @@ test('Fails validation if default export is not present', async () => {

await fs.writeFile(path, func.source)

const config = getFunctionConfig(
{
const config = getFunctionConfig({
func: {
name: func.name,
path,
},
new ImportMap([importMapFile]),
importMap: new ImportMap([importMapFile]),
deno,
logger,
)
log: logger,
bootstrapURL,
})

await expect(config).rejects.toThrowError(invalidDefaultExportErr(path))

Expand Down
15 changes: 14 additions & 1 deletion node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,19 @@ const getConfigExtractor = () => {
return configExtractorPath
}

export const getFunctionConfig = async (func: EdgeFunction, importMap: ImportMap, deno: DenoBridge, log: Logger) => {
export const getFunctionConfig = async ({
func,
importMap,
deno,
bootstrapURL,
log,
}: {
func: EdgeFunction
importMap: ImportMap
deno: DenoBridge
bootstrapURL: string
log: Logger
}) => {
// The extractor is a Deno script that will import the function and run its
// `config` export, if one exists.
const extractorPath = getConfigExtractor()
Expand All @@ -79,6 +91,7 @@ export const getFunctionConfig = async (func: EdgeFunction, importMap: ImportMap
extractorPath,
pathToFileURL(func.path).href,
pathToFileURL(collector.path).href,
bootstrapURL,
JSON.stringify(ConfigExitCode),
],
{ rejectOnExitCode: false },
Expand Down
18 changes: 17 additions & 1 deletion node/server/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ test('Starts a server and serves requests for edge functions', async () => {
name: 'greet',
path: join(paths.internal, 'greet.ts'),
},
{
name: 'global_netlify',
path: join(paths.user, 'global_netlify.ts'),
},
]
const options = {
getFunctionsConfig: true,
Expand All @@ -44,7 +48,7 @@ test('Starts a server and serves requests for edge functions', async () => {
options,
)
expect(success).toBe(true)
expect(functionsConfig).toEqual([{ path: '/my-function' }, {}])
expect(functionsConfig).toEqual([{ path: '/my-function' }, {}, { path: '/global-netlify' }])

for (const key in functions) {
const graphEntry = graph?.modules.some(
Expand Down Expand Up @@ -74,4 +78,16 @@ test('Starts a server and serves requests for edge functions', async () => {
})
expect(response2.status).toBe(200)
expect(await response2.text()).toBe('HELLO!')

const response3 = await fetch(`http://0.0.0.0:${port}/global-netlify`, {
headers: {
'x-nf-edge-functions': 'global_netlify',
'x-ef-passthrough': 'passthrough',
'X-NF-Request-ID': uuidv4(),
},
})
expect(await response3.json()).toEqual({
global: 'i love netlify',
local: 'i love netlify',
})
})
4 changes: 3 additions & 1 deletion node/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ const prepareServer = ({
let functionsConfig: FunctionConfig[] = []

if (options.getFunctionsConfig) {
functionsConfig = await Promise.all(functions.map((func) => getFunctionConfig(func, importMap, deno, logger)))
functionsConfig = await Promise.all(
functions.map((func) => getFunctionConfig({ func, importMap, deno, bootstrapURL, log: logger })),
)
}

const success = await waitForServer(port, processRef.ps)
Expand Down
14 changes: 14 additions & 0 deletions test/fixtures/serve_test/netlify/edge-functions/global_netlify.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { Config } from 'https://edge.netlify.com/'

const global = globalThis.Netlify.env.get('very_secret_secret')

export default () => {
return Response.json({
global,
local: globalThis.Netlify.env.get('very_secret_secret'),
})
}

export const config: Config = {
path: '/global-netlify',
}

0 comments on commit d829e70

Please sign in to comment.