-
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 50 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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -473,6 +473,27 @@ | |||||||||
| }); | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| describe("splitEnvVars", () => { | ||||||||||
| it("should stringify numeric values", () => { | ||||||||||
| const env: AppHostingYamlConfig["env"] = { | ||||||||||
| STR: { value: "string" }, | ||||||||||
| NUM: { value: 12345 as any }, | ||||||||||
|
Check warning on line 480 in src/apphosting/config.spec.ts
|
||||||||||
| BUILD_AND_RUNTIME_NUM: { value: 67890 as any, availability: ["BUILD", "RUNTIME"] }, | ||||||||||
|
Check warning on line 481 in src/apphosting/config.spec.ts
|
||||||||||
|
Comment on lines
+480
to
+481
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. Using
Suggested change
|
||||||||||
| }; | ||||||||||
|
|
||||||||||
| const { build, runtime } = config.splitEnvVars(env); | ||||||||||
|
|
||||||||||
| expect(build["BUILD_AND_RUNTIME_NUM"].value).to.equal("67890"); | ||||||||||
| expect(runtime).to.deep.include({ variable: "STR", value: "string" }); | ||||||||||
| expect(runtime).to.deep.include({ variable: "NUM", value: "12345" }); | ||||||||||
| expect(runtime).to.deep.include({ | ||||||||||
| variable: "BUILD_AND_RUNTIME_NUM", | ||||||||||
| value: "67890", | ||||||||||
| availability: ["BUILD", "RUNTIME"], | ||||||||||
| }); | ||||||||||
| }); | ||||||||||
| }); | ||||||||||
|
|
||||||||||
| describe("getAppHostingConfiguration", () => { | ||||||||||
| let loadAppHostingYamlStub: sinon.SinonStub; | ||||||||||
| let listAppHostingFilesInPathStub: sinon.SinonStub; | ||||||||||
|
|
||||||||||
| 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; | ||||||
| } | ||||||
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.
The
errvariable is of typeunknown. It's safer to explicitly convert it to a string usingString(err)or use a utility likegetErrMsgfrom../errorto ensure proper logging, especially iferrmight not always be anErrorinstance.