-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Support env vars for local/zip deploys #10163
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
Changes from all commits
65e0c59
ecbbe5e
60adf94
bcf0cd3
4603750
eb89f6b
9470c3d
269da87
92a3f49
76ef53a
340020f
d5343bd
e17f55c
8341bc4
4ed3b41
b76fa54
31fed5d
2f653ca
b19b8bf
611f258
37bb714
1df73d7
7d61148
0ec1b92
2ea831d
d20b46c
25262ac
2f63f7a
de4f95b
7f122c2
a4476a1
4158d52
826754b
89d1b2f
6829e31
004e0e7
613bdcc
84a34b6
6349afc
2f040c9
2d7c228
ae833ee
e6b011f
8564e52
cc78270
3027dad
b5e285b
2b73285
90fb82b
29fbd6e
5db54e2
f12056a
b571019
7f19002
13ed461
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||
| import * as path from "path"; | ||||||
| import { BuildConfig, Env } from "../gcp/apphosting"; | ||||||
| import { localBuild as localAppHostingBuild } from "@apphosting/build"; | ||||||
| import { EnvMap } from "./yaml"; | ||||||
|
|
||||||
| /** | ||||||
| * Triggers a local build of your App Hosting codebase. | ||||||
|
|
@@ -17,31 +19,72 @@ import { localBuild as localAppHostingBuild } from "@apphosting/build"; | |||||
| export async function localBuild( | ||||||
| projectRoot: string, | ||||||
| framework: string, | ||||||
| env: EnvMap = {}, | ||||||
| ): Promise<{ | ||||||
| outputFiles: string[]; | ||||||
| annotations: Record<string, string>; | ||||||
| buildConfig: BuildConfig; | ||||||
| }> { | ||||||
| const apphostingBuildOutput = await localAppHostingBuild(projectRoot, framework); | ||||||
| // We need to inject the environment variables into the process.env | ||||||
| // because the build adapter uses them to build the app. | ||||||
| // We'll restore the original process.env after the build is done. | ||||||
| const originalEnv = { ...process.env }; | ||||||
| const projectNodeModules = path.join(projectRoot, "node_modules"); | ||||||
| const newNodePath = process.env.NODE_PATH | ||||||
| ? `${process.env.NODE_PATH}${path.delimiter}${projectNodeModules}` | ||||||
| : projectNodeModules; | ||||||
|
|
||||||
| const addedEnv = toProcessEnv(env); | ||||||
| for (const [key, value] of Object.entries(addedEnv)) { | ||||||
| process.env[key] = value; | ||||||
| } | ||||||
| const originalNodePath = process.env.NODE_PATH; | ||||||
| process.env.NODE_PATH = newNodePath; | ||||||
|
|
||||||
| let apphostingBuildOutput; | ||||||
| try { | ||||||
| apphostingBuildOutput = await localAppHostingBuild(projectRoot, framework); | ||||||
| } finally { | ||||||
| for (const key in process.env) { | ||||||
| if (!(key in originalEnv)) { | ||||||
| delete process.env[key]; | ||||||
| } | ||||||
| } | ||||||
| for (const [key, value] of Object.entries(originalEnv)) { | ||||||
| process.env[key] = value; | ||||||
| } | ||||||
| if (originalNodePath !== undefined) { | ||||||
| process.env.NODE_PATH = originalNodePath; | ||||||
| } else { | ||||||
| delete process.env.NODE_PATH; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| const annotations: Record<string, string> = Object.fromEntries( | ||||||
| Object.entries(apphostingBuildOutput.metadata).map(([key, value]) => [key, String(value)]), | ||||||
| ); | ||||||
|
|
||||||
| const env: Env[] | undefined = apphostingBuildOutput.runConfig.environmentVariables?.map( | ||||||
| ({ variable, value, availability }) => ({ | ||||||
| variable, | ||||||
| value, | ||||||
| availability, | ||||||
| }), | ||||||
| ); | ||||||
| const discoveredEnv: Env[] | undefined = | ||||||
| apphostingBuildOutput.runConfig.environmentVariables?.map( | ||||||
| ({ variable, value, availability }) => ({ | ||||||
| variable, | ||||||
| value, | ||||||
| availability, | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| return { | ||||||
| outputFiles: apphostingBuildOutput.outputFiles?.serverApp.include ?? [], | ||||||
| annotations, | ||||||
| buildConfig: { | ||||||
| runCommand: apphostingBuildOutput.runConfig.runCommand, | ||||||
| env: env ?? [], | ||||||
| env: discoveredEnv ?? [], | ||||||
| }, | ||||||
| }; | ||||||
| } | ||||||
|
|
||||||
| function toProcessEnv(env: EnvMap): NodeJS.ProcessEnv { | ||||||
| return Object.fromEntries( | ||||||
| Object.entries(env).map(([key, value]) => [key, value.value || ""]), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The expression
Suggested change
|
||||||
| ) as NodeJS.ProcessEnv; | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,9 @@ import { | |
| parseBackendName, | ||
| serviceAgentEmail, | ||
| } from "../../gcp/apphosting"; | ||
| import { AppHostingYamlConfig, EnvMap } from "../../apphosting/yaml"; | ||
| import { WebConfig } from "../../fetchWebSetup"; | ||
| import { Env, getAppHostingConfiguration, splitEnvVars } from "../../apphosting/config"; | ||
| import { getGitRepositoryLink, parseGitRepositoryLinkName } from "../../gcp/devConnect"; | ||
| import { addServiceAccountToRoles } from "../../gcp/resourceManager"; | ||
|
|
||
|
|
@@ -22,6 +25,8 @@ import { logLabeledBullet, logLabeledWarning } from "../../utils"; | |
| import { localBuild } from "../../apphosting/localbuilds"; | ||
| import { Context } from "./args"; | ||
| import { FirebaseError } from "../../error"; | ||
| import * as managementApps from "../../management/apps"; | ||
| import { getAutoinitEnvVars } from "../../apphosting/utils"; | ||
| import * as experiments from "../../experiments"; | ||
| import { logger } from "../../logger"; | ||
|
|
||
|
|
@@ -46,6 +51,27 @@ export default async function (context: Context, options: Options): Promise<void | |
| await ensureAppHostingServiceAgentRoles(projectId, projectNumber); | ||
| } | ||
|
|
||
| const buildEnv: Record<string, EnvMap> = {}; | ||
| const runtimeEnv: Record<string, Env[]> = {}; | ||
|
|
||
| for (const cfg of configs) { | ||
| const rootDir = options.projectRoot || process.cwd(); | ||
| const appDir = path.join(rootDir, cfg.rootDir || ""); | ||
| let yamlConfig = AppHostingYamlConfig.empty(); | ||
| try { | ||
| yamlConfig = await getAppHostingConfiguration(appDir); | ||
| } catch (e: unknown) { | ||
| if (e instanceof Error && e.message && !e.message.includes("doesn't exist")) { | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| const { build, runtime } = splitEnvVars(yamlConfig.env); | ||
|
|
||
| buildEnv[cfg.backendId] = build; | ||
| runtimeEnv[cfg.backendId] = runtime; | ||
| } | ||
|
|
||
| const { backends } = await listBackends(projectId, "-"); | ||
|
|
||
| const foundBackends: AppHostingSingle[] = []; | ||
|
|
@@ -169,20 +195,46 @@ export default async function (context: Context, options: Options): Promise<void | |
| } | ||
| experiments.assertEnabled("apphostinglocalbuilds", "locally build App Hosting backends"); | ||
| logLabeledBullet("apphosting", `Starting local build for backend ${cfg.backendId}`); | ||
| const backend = backends.find((b) => parseBackendName(b.name).id === cfg.backendId); | ||
| if (backend?.appId) { | ||
| try { | ||
| const webappConfig = (await managementApps.getAppConfig( | ||
| backend.appId, | ||
| managementApps.AppPlatform.WEB, | ||
| )) as WebConfig; | ||
| const autoinitVars = getAutoinitEnvVars(webappConfig); | ||
| for (const [key, value] of Object.entries(autoinitVars)) { | ||
| if (!(key in buildEnv[cfg.backendId])) { | ||
| buildEnv[cfg.backendId][key] = { value }; | ||
| } | ||
| if (!runtimeEnv[cfg.backendId].some((e) => e.variable === key)) { | ||
| runtimeEnv[cfg.backendId].push({ variable: key, value }); | ||
| } | ||
| } | ||
| } catch (e) { | ||
| logLabeledWarning( | ||
| "apphosting", | ||
| `Unable to lookup details for backend ${cfg.backendId}. Firebase SDK autoinit will not be available.`, | ||
| ); | ||
| } | ||
| } | ||
| try { | ||
| const { outputFiles, annotations, buildConfig } = await localBuild( | ||
| options.projectRoot || "./", | ||
| "nextjs", | ||
| buildEnv[cfg.backendId] || {}, | ||
| ); | ||
| if (outputFiles.length !== 1) { | ||
| throw new FirebaseError( | ||
| `Local build for backend ${cfg.backendId} failed: No output files found.`, | ||
| ); | ||
| } | ||
| context.backendLocalBuilds[cfg.backendId] = { | ||
| // TODO(9114): This only works for nextjs. | ||
| buildDir: outputFiles[0], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| buildConfig, | ||
| buildConfig: { | ||
| ...buildConfig, | ||
| env: mergeEnvVars(buildConfig.env || [], runtimeEnv[cfg.backendId] || []), | ||
| }, | ||
| annotations, | ||
| }; | ||
| } catch (e: unknown) { | ||
|
|
@@ -227,6 +279,24 @@ export function getBackendConfigs(options: Options): AppHostingMultiple { | |
| return backendConfigs.filter((cfg) => backendIds.includes(cfg.backendId)); | ||
| } | ||
|
|
||
| /** | ||
| * Merges two lists of environment variables, giving precedence to the values in overrides. | ||
| */ | ||
| function mergeEnvVars(base: Env[], overrides: Env[]): Env[] { | ||
| const merged = new Map<string, Env>(); | ||
| for (const env of base) { | ||
| if (env.variable) { | ||
| merged.set(env.variable, env); | ||
| } | ||
| } | ||
| for (const env of overrides) { | ||
| if (env.variable) { | ||
| merged.set(env.variable, env); | ||
| } | ||
| } | ||
| return Array.from(merged.values()); | ||
| } | ||
|
|
||
| /** | ||
| * Ensures that the App Hosting service agent has the necessary roles to access | ||
| * project resources (e.g. storage) for a given project. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,8 @@ export default async function (context: Context, options: Options): Promise<void | |
| backendId, | ||
| location: context.backendLocations[backendId], | ||
| buildInput: { | ||
| config: context.backendLocalBuilds[backendId]?.buildConfig, | ||
| // TODO: Confirm this config behavior | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| config: context.backendLocalBuilds[backendId]?.buildConfig || {}, | ||
| source: { | ||
| archive: { | ||
| userStorageUri: context.backendStorageUris[backendId], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
as anyhere for numeric values, while functional due to explicit string conversion insplitEnvVars, could be made more type-safe. Consider defining theEnvMaptype to explicitly allownumberforvalueif that's the intended input, or provide string literals if the input is always expected to be stringified before use.