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

feat: inject environment variables from .env files into edge functions locally #5620

Merged
merged 6 commits into from
Apr 17, 2023
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
5 changes: 3 additions & 2 deletions src/commands/dev/dev-exec.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import execa from 'execa'

import { injectEnvVariables } from '../../utils/dev.mjs'
import { getDotEnvVariables, injectEnvVariables } from '../../utils/dev.mjs'
import { getEnvelopeEnv, normalizeContext } from '../../utils/env/index.mjs'

/**
Expand All @@ -16,7 +16,8 @@ const devExec = async (cmd, options, command) => {
env = await getEnvelopeEnv({ api, context: options.context, env, siteInfo })
}

await injectEnvVariables({ devConfig: { ...config.dev }, env, site })
env = await getDotEnvVariables({ devConfig: { ...config.dev }, env, site })
injectEnvVariables(env)

await execa(cmd, command.args.slice(1), {
stdio: 'inherit',
Expand Down
5 changes: 3 additions & 2 deletions src/commands/dev/dev.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
normalizeConfig,
} from '../../utils/command-helpers.mjs'
import detectServerSettings, { getConfigWithPlugins } from '../../utils/detect-server-settings.mjs'
import { getSiteInformation, injectEnvVariables } from '../../utils/dev.mjs'
import { getDotEnvVariables, getSiteInformation, injectEnvVariables } from '../../utils/dev.mjs'
import { getEnvelopeEnv, normalizeContext } from '../../utils/env/index.mjs'
import { ensureNetlifyIgnore } from '../../utils/gitignore.mjs'
import { startNetlifyGraph, startPollingForAPIAuthentication } from '../../utils/graph.mjs'
Expand Down Expand Up @@ -96,7 +96,8 @@ const dev = async (options, command) => {
log(`${NETLIFYDEVLOG} Injecting environment variable values for ${chalk.yellow('all scopes')}`)
}

await injectEnvVariables({ devConfig, env, site })
env = await getDotEnvVariables({ devConfig, env, site })
injectEnvVariables(env)
Copy link
Contributor Author

@danez danez Apr 12, 2023

Choose a reason for hiding this comment

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

addDotEnvVariables now mutates env here and adds the variables from the dot-env files to it, so that later one we can use them in edge-functions. Previously dotenv variables were only added to process.env directly.

await promptEditorHelper({ chalk, config, log, NETLIFYDEVLOG, repositoryRoot, state })

const { addonsUrls, capabilities, siteUrl, timeouts } = await getSiteInformation({
Expand Down
5 changes: 3 additions & 2 deletions src/commands/functions/functions-create.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import ora from 'ora'
import { fileExistsAsync } from '../../lib/fs.mjs'
import { getAddons, getCurrentAddon, getSiteData } from '../../utils/addons/prepare.mjs'
import { NETLIFYDEVERR, NETLIFYDEVLOG, NETLIFYDEVWARN, chalk, error, log } from '../../utils/command-helpers.mjs'
import { injectEnvVariables } from '../../utils/dev.mjs'
import { getDotEnvVariables, injectEnvVariables } from '../../utils/dev.mjs'
import execa from '../../utils/execa.mjs'
import { readRepoURL, validateRepoURL } from '../../utils/read-repo-url.mjs'

Expand Down Expand Up @@ -549,11 +549,12 @@ const handleOnComplete = async ({ command, onComplete }) => {
const { config } = command.netlify

if (onComplete) {
await injectEnvVariables({
const env = await getDotEnvVariables({
devConfig: { ...config.dev },
env: command.netlify.cachedConfig.env,
site: command.netlify.site,
})
injectEnvVariables(env)
await onComplete.call(command)
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/commands/functions/functions-serve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import { join } from 'path'

import { startFunctionsServer } from '../../lib/functions/server.mjs'
import { acquirePort, getSiteInformation, injectEnvVariables } from '../../utils/dev.mjs'
import { acquirePort, getDotEnvVariables, getSiteInformation, injectEnvVariables } from '../../utils/dev.mjs'
import { getFunctionsDir } from '../../utils/functions/index.mjs'

const DEFAULT_PORT = 9999
Expand All @@ -16,11 +16,12 @@ const functionsServe = async (options, command) => {
const { api, config, site, siteInfo } = command.netlify

const functionsDir = getFunctionsDir({ options, config }, join('netlify', 'functions'))
const { env } = command.netlify.cachedConfig
let { env } = command.netlify.cachedConfig

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

await injectEnvVariables({ devConfig: { ...config.dev }, env, site })
env = await getDotEnvVariables({ devConfig: { ...config.dev }, env, site })
injectEnvVariables(env)

const { capabilities, siteUrl, timeouts } = await getSiteInformation({
offline: options.offline,
Expand Down
5 changes: 3 additions & 2 deletions src/commands/serve/serve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
normalizeConfig,
} from '../../utils/command-helpers.mjs'
import detectServerSettings, { getConfigWithPlugins } from '../../utils/detect-server-settings.mjs'
import { getSiteInformation, injectEnvVariables } from '../../utils/dev.mjs'
import { getDotEnvVariables, getSiteInformation, injectEnvVariables } from '../../utils/dev.mjs'
import { getEnvelopeEnv, normalizeContext } from '../../utils/env/index.mjs'
import { getInternalFunctionsDir } from '../../utils/functions/functions.mjs'
import { ensureNetlifyIgnore } from '../../utils/gitignore.mjs'
Expand Down Expand Up @@ -52,7 +52,8 @@ const serve = async (options, command) => {
log(`${NETLIFYDEVLOG} Injecting environment variable values for ${chalk.yellow('all scopes')}`)
}

await injectEnvVariables({ devConfig, env, site })
env = await getDotEnvVariables({ devConfig, env, site })
injectEnvVariables(env)
await promptEditorHelper({ chalk, config, log, NETLIFYDEVLOG, repositoryRoot, state })

const { addonsUrls, capabilities, siteUrl, timeouts } = await getSiteInformation({
Expand Down
10 changes: 8 additions & 2 deletions src/lib/edge-functions/registry.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export class EdgeFunctionsRegistry {
* @param {object} opts.config
* @param {string} opts.configPath
* @param {string[]} opts.directories
* @param {Record<string, string>} opts.env
* @param {Record<string, { sources: string[], value: string}>} opts.env
* @param {() => Promise<object>} opts.getUpdatedConfig
* @param {Declaration[]} opts.internalFunctions
* @param {string} opts.projectDir
Expand Down Expand Up @@ -178,14 +178,20 @@ export class EdgeFunctionsRegistry {
return edgeFunctions
}

/**
*
* @param {Record<string, { sources: string[], value: string}>} envConfig
* @returns {Record<string, string>}
*/
static getEnvironmentVariables(envConfig) {
const env = Object.create(null)
Object.entries(envConfig).forEach(([key, variable]) => {
if (
variable.sources.includes('ui') ||
variable.sources.includes('account') ||
variable.sources.includes('addons') ||
variable.sources.includes('internal')
variable.sources.includes('internal') ||
variable.sources.some((source) => source.startsWith('.env'))
danez marked this conversation as resolved.
Show resolved Hide resolved
) {
env[key] = variable.value
}
Expand Down
30 changes: 20 additions & 10 deletions src/utils/dev.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -137,30 +137,40 @@ const getEnvSourceName = (source) => {
return printFn(name)
}

// Takes a set of environment variables in the format provided by @netlify/config, augments it with variables from both
// dot-env files and the process itself, and injects into `process.env`.
export const injectEnvVariables = async ({ devConfig, env, site }) => {
const environment = new Map(Object.entries(env))
/**
* @param {{devConfig: any, env: Record<string, { sources: string[], value: string}>, site: any}} param0
* @returns {Promise<Record<string, { sources: string[], value: string}>>}
*/
export const getDotEnvVariables = async ({ devConfig, env, site }) => {
const dotEnvFiles = await loadDotEnvFiles({ envFiles: devConfig.envFiles, projectDir: site.root })

dotEnvFiles.forEach(({ env: fileEnv, file }) => {
const newSourceName = `${file} file`

Object.keys(fileEnv).forEach((key) => {
const newSourceName = `${file} file`
const sources = environment.has(key) ? [newSourceName, ...environment.get(key).sources] : [newSourceName]
const sources = key in env ? [newSourceName, ...env[key].sources] : [newSourceName]

if (sources.includes('internal')) {
return
}

environment.set(key, {
env[key] = {
sources,
value: fileEnv[key],
})
}
})
})

return env
}

/**
* Takes a set of environment variables in the format provided by @netlify/config and injects them into `process.env`
* @param {Record<string, { sources: string[], value: string}>} env
* @return {void}
*/
export const injectEnvVariables = (env) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this function to only inject variables into process.env and split out all the dotenv stuff into the function above.

// eslint-disable-next-line fp/no-loops
for (const [key, variable] of environment) {
for (const [key, variable] of Object.entries(env)) {
const existsInProcess = process.env[key] !== undefined
const [usedSource, ...overriddenSources] = existsInProcess ? ['process', ...variable.sources] : variable.sources
const usedSourceName = getEnvSourceName(usedSource)
Expand Down
16 changes: 12 additions & 4 deletions tests/integration/100.command.dev.test.cjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Handlers are meant to be async outside tests
const path = require('path')

// eslint-disable-next-line ava/use-test
Expand Down Expand Up @@ -1020,6 +1019,10 @@ test('should have only allowed environment variables set', async (t) => {
handler: () => new Response(`${JSON.stringify(Deno.env.toObject())}`),
name: 'env',
})
.withContentFile({
content: 'FROM_ENV="YAS"',
path: '.env',
})

await builder.buildAsync()

Expand All @@ -1040,16 +1043,21 @@ test('should have only allowed environment variables set', async (t) => {
)
const envKeys = Object.keys(response)

t.false(envKeys.includes('DENO_DEPLOYMENT_ID'))
// t.true(envKeys.includes('DENO_DEPLOYMENT_ID'))
// t.is(response.DENO_DEPLOYMENT_ID, 'xxx=')
t.true(envKeys.includes('PATH'))

t.true(envKeys.includes('DENO_REGION'))
t.is(response.DENO_REGION, 'local')

t.true(envKeys.includes('NETLIFY_DEV'))
t.is(response.NETLIFY_DEV, 'true')

t.true(envKeys.includes('SECRET_ENV'))
t.is(response.SECRET_ENV, 'true')

t.true(envKeys.includes('FROM_ENV'))
t.is(response.FROM_ENV, 'YAS')

t.false(envKeys.includes('DENO_DEPLOYMENT_ID'))
t.false(envKeys.includes('NODE_ENV'))
t.false(envKeys.includes('DEPLOY_URL'))
t.false(envKeys.includes('URL'))
Expand Down