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.

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

fix: set Blobs context on the process #6284

Merged
merged 7 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
16 changes: 9 additions & 7 deletions src/commands/dev/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import process from 'process'

import { OptionValues, Option } from 'commander'

import { getBlobsContext } from '../../lib/blobs/blobs.js'
import { BLOBS_CONTEXT_VARIABLE, encodeBlobsContext, getBlobsContext } from '../../lib/blobs/blobs.js'
import { promptEditorHelper } from '../../lib/edge-functions/editor-helper.js'
import { startFunctionsServer } from '../../lib/functions/server.js'
import { printBanner } from '../../utils/banner.js'
Expand Down Expand Up @@ -105,6 +105,14 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {

env.NETLIFY_DEV = { sources: ['internal'], value: 'true' }

const blobsContext = await getBlobsContext({
debug: options.debug,
projectRoot: command.workingDir,
siteID: site.id ?? 'unknown-site-id',
})

env[BLOBS_CONTEXT_VARIABLE] = { sources: ['internal'], value: encodeBlobsContext(blobsContext) }

if (!options.offline && siteInfo.use_envelope) {
env = await getEnvelopeEnv({ api, context: options.context, env, siteInfo })
log(`${NETLIFYDEVLOG} Injecting environment variable values for ${chalk.yellow('all scopes')}`)
Expand Down Expand Up @@ -155,12 +163,6 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
},
})

const blobsContext = await getBlobsContext({
debug: options.debug,
projectRoot: command.workingDir,
siteID: site.id ?? 'unknown-site-id',
})

const functionsRegistry = await startFunctionsServer({
api,
blobsContext,
Expand Down
4 changes: 3 additions & 1 deletion src/commands/serve/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import process from 'process'

import { OptionValues } from 'commander'

import { getBlobsContext } from '../../lib/blobs/blobs.js'
import { BLOBS_CONTEXT_VARIABLE, encodeBlobsContext, getBlobsContext } from '../../lib/blobs/blobs.js'
import { promptEditorHelper } from '../../lib/edge-functions/editor-helper.js'
import { startFunctionsServer } from '../../lib/functions/server.js'
import { printBanner } from '../../utils/banner.js'
Expand Down Expand Up @@ -100,6 +100,8 @@ export const serve = async (options: OptionValues, command: BaseCommand) => {
siteID: site.id ?? 'unknown-site-id',
})

process.env[BLOBS_CONTEXT_VARIABLE] = encodeBlobsContext(blobsContext)

const functionsRegistry = await startFunctionsServer({
api,
blobsContext,
Expand Down
9 changes: 9 additions & 0 deletions src/lib/blobs/blobs.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Buffer } from 'buffer'
import path from 'path'

import { BlobsServer } from '@netlify/blobs'
Expand All @@ -8,6 +9,8 @@ import { getPathInProject } from '../settings.js'

let hasPrintedLocalBlobsNotice = false

export const BLOBS_CONTEXT_VARIABLE = 'NETLIFY_BLOBS_CONTEXT'
Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally @netlify/blobs should export this.


export interface BlobsContext {
deployID: string
edgeURL: string
Expand Down Expand Up @@ -64,3 +67,9 @@ export const getBlobsContext = async ({ debug, projectRoot, siteID }: GetBlobsCo

return context
}

/**
* Returns a Base-64, JSON-encoded representation of the Blobs context. This is
* the format that the `@netlify/blobs` package expects to find the context in.
*/
export const encodeBlobsContext = (context: BlobsContext) => Buffer.from(JSON.stringify(context)).toString('base64')
9 changes: 9 additions & 0 deletions src/lib/functions/runtimes/js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { Worker } from 'worker_threads'

import lambdaLocal from 'lambda-local'

import { BLOBS_CONTEXT_VARIABLE } from '../../../blobs/blobs.js'

import detectNetlifyLambdaBuilder from './builders/netlify-lambda.js'
import detectZisiBuilder, { parseFunctionForMetadata } from './builders/zisi.js'
import { SECONDS_TO_MILLISECONDS } from './constants.js'
Expand Down Expand Up @@ -104,6 +106,13 @@ export const invokeFunctionDirectly = async ({ context, event, func, timeout })
const lambdaPath = func.buildData?.buildPath ?? func.mainFile
const result = await lambdaLocal.execute({
clientContext: JSON.stringify(context),
environment: {
// We've set the Blobs context on the parent process, which means it will
// be available to the Lambda. This would be inconsistent with production
// where only V2 functions get the context injected. To fix it, unset the
// context variable before invoking the function.
[BLOBS_CONTEXT_VARIABLE]: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the implementation of this, it seems like it mutates process.env:

https://github.com/ashiina/lambda-local/blob/95fb3dfd18ec51014d3cc5d9f405e21a5d784a50/src/lambdalocal.ts#L250-L252

This leads to weird side-effects, like the env variable vanishing upon the first V1 Function execution. In this specific instance, it's probably alright because the framework child process was started earlier, and because V2 Functions don't rely on this env variable. But might be worth adding a comment.

},
event,
lambdaPath,
timeoutMs: timeout * SECONDS_TO_MILLISECONDS,
Expand Down
17 changes: 14 additions & 3 deletions tests/integration/commands/dev/dev.config.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Handlers are meant to be async outside tests
import { Buffer } from 'buffer'
import { version } from 'process'

import FormData from 'form-data'
Expand Down Expand Up @@ -142,7 +142,7 @@ describe.concurrent('commands/dev/config', () => {
})
})

test('should provide CLI version in env var', async (t) => {
test('should provide environment variables to framework server', async (t) => {
await withSiteBuilder('site-with-netlify-version-env-var', async (builder) => {
await builder
.withContentFile({
Expand All @@ -151,6 +151,7 @@ describe.concurrent('commands/dev/config', () => {

http.createServer((req, res) => {
res.write(JSON.stringify({
NETLIFY_BLOBS_CONTEXT: process.env.NETLIFY_BLOBS_CONTEXT,
NETLIFY_CLI_VERSION: process.env.NETLIFY_CLI_VERSION,
}))
res.end()
Expand All @@ -171,7 +172,17 @@ describe.concurrent('commands/dev/config', () => {

await withDevServer({ cwd: builder.directory }, async (server) => {
const resp = await fetch(server.url)
const { NETLIFY_CLI_VERSION } = await resp.json()
const { NETLIFY_BLOBS_CONTEXT, NETLIFY_CLI_VERSION } = await resp.json()

t.expect(NETLIFY_BLOBS_CONTEXT).toBeTypeOf('string')

const { deployID, edgeURL, siteID, token } = JSON.parse(Buffer.from(NETLIFY_BLOBS_CONTEXT, 'base64').toString())

t.expect(deployID).toBe('0')
t.expect(edgeURL.startsWith('http://localhost:')).toBeTruthy()
t.expect(siteID).toBeTypeOf('string')
t.expect(token).toBeTypeOf('string')

t.expect(NETLIFY_CLI_VERSION).toMatch(/\d+\.\d+\.\d+/)
})
})
Expand Down
Loading