From 5a947b370c28a67f99e41aa9754ef017000ee488 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Thu, 3 Jun 2021 14:37:14 -0700 Subject: [PATCH 1/2] Encapsulate runtime-dependent code Node JS code is now behind /deploy/functions/runtimes/node. The rest of the codebase now only interacts with the runtime through a RuntimeDelegate interface exposed in /deploy/functions/runtimes. All Runtime specific code from backend.ts (and functions API code + pacakge.json parser) has been deduplicated in runtimes/index.ts The following files/functions have been renamed: checkFirebaseSDKVersions.ts -> deploy/functions/runtimes/node/versioning.ts deploy/functions/discovery/jsexports/* -> deploy/functions/runtimes/node deploy/functions/checkRuntimeDependencies.ts -> deploy/functions/ensureCloudBuildEnabled.ts deploy/functions/validate.ts -> deploy/functions/runtimes/node/validate.ts (Node methods only) Some code has been refactored to be more coehsive or to remove redundancies: 1. Multiple places loaded the current firebase-functions SDK. This is now called by the RuntimeDelegate and passed where it is needed 2. Context no longer needs anything to do with runtimes. All Runtime-dependent code is handled in prepare 3. prepareFunctionsUplaod did a _lot_ more than just pacakaging source. It was previously getting the backend (and storing it in context so it could be reused), getting env vars, getting runtime config, etc. It now just uploads source and everything else is in prepare direclty. 4. The min-SDK version check has been moved from getRuntimeChoice() to a versioning.ts:checkFunctionsSDKVersion, which always did other checks. This saved an exec to npm. Some small fixes/improvements along the way: 1. The log line we print when a runtime is not specified now tells customers to use the latest nodejs runtime 2. We warn customers of breaking changes for any breaking change, not just 0.x to 1.x. --- src/checkFirebaseSDKVersion.ts | 111 ---------------- src/commands/deploy.js | 2 - src/deploy/functions/args.ts | 2 +- src/deploy/functions/backend.ts | 17 +-- src/deploy/functions/discovery/index.ts | 53 -------- ...ndencies.ts => ensureCloudBuildEnabled.ts} | 3 +- src/deploy/functions/prepare.ts | 47 ++++--- .../functions/prepareFunctionsUpload.ts | 21 +-- src/deploy/functions/release.ts | 1 - src/deploy/functions/runtimes/index.ts | 122 ++++++++++++++++++ .../node}/extractTriggers.js | 0 src/deploy/functions/runtimes/node/index.ts | 102 +++++++++++++++ .../node}/parseRuntimeAndValidateSDK.ts | 82 +++--------- .../node}/parseTriggers.ts | 14 +- .../node}/triggerParser.js | 0 .../functions/runtimes/node/validate.ts | 57 ++++++++ .../functions/runtimes/node/versioning.ts | 117 +++++++++++++++++ src/deploy/functions/tasks.ts | 7 +- src/deploy/functions/validate.ts | 67 ---------- src/gcp/cloudfunctions.ts | 8 +- src/gcp/cloudfunctionsv2.ts | 8 +- ...ies.spec.ts => ensureCloudBuildEnabled.ts} | 90 ++++++------- .../deploy/functions/runtimes/index.spec.ts | 9 ++ .../node}/extractTriggers.spec.js | 2 +- .../node}/parseRuntimeAndValidateSDK.spec.ts | 46 +------ .../node}/parseTriggers.spec.ts | 2 +- .../functions/runtimes/node/validate.spec.ts | 64 +++++++++ .../runtimes/node/versioning.spec.ts | 47 +++++++ src/test/deploy/functions/validate.spec.ts | 87 +------------ 29 files changed, 632 insertions(+), 556 deletions(-) delete mode 100644 src/checkFirebaseSDKVersion.ts delete mode 100644 src/deploy/functions/discovery/index.ts rename src/deploy/functions/{checkRuntimeDependencies.ts => ensureCloudBuildEnabled.ts} (91%) create mode 100644 src/deploy/functions/runtimes/index.ts rename src/deploy/functions/{discovery/jsexports => runtimes/node}/extractTriggers.js (100%) create mode 100644 src/deploy/functions/runtimes/node/index.ts rename src/deploy/functions/{ => runtimes/node}/parseRuntimeAndValidateSDK.ts (53%) rename src/deploy/functions/{discovery/jsexports => runtimes/node}/parseTriggers.ts (94%) rename src/deploy/functions/{discovery/jsexports => runtimes/node}/triggerParser.js (100%) create mode 100644 src/deploy/functions/runtimes/node/validate.ts create mode 100644 src/deploy/functions/runtimes/node/versioning.ts rename src/test/deploy/functions/{checkRuntimeDependencies.spec.ts => ensureCloudBuildEnabled.ts} (50%) create mode 100644 src/test/deploy/functions/runtimes/index.spec.ts rename src/test/deploy/functions/{discovery/jsexports => runtimes/node}/extractTriggers.spec.js (97%) rename src/test/deploy/functions/{ => runtimes/node}/parseRuntimeAndValidateSDK.spec.ts (69%) rename src/test/deploy/functions/{discovery/jsexports => runtimes/node}/parseTriggers.spec.ts (99%) create mode 100644 src/test/deploy/functions/runtimes/node/validate.spec.ts create mode 100644 src/test/deploy/functions/runtimes/node/versioning.spec.ts diff --git a/src/checkFirebaseSDKVersion.ts b/src/checkFirebaseSDKVersion.ts deleted file mode 100644 index f0c0eca428a..00000000000 --- a/src/checkFirebaseSDKVersion.ts +++ /dev/null @@ -1,111 +0,0 @@ -import * as _ from "lodash"; -import * as clc from "cli-color"; -import * as path from "path"; -import * as semver from "semver"; -import * as spawn from "cross-spawn"; - -import * as utils from "./utils"; -import { FirebaseError } from "./error"; -import { logger } from "./logger"; - -interface NpmListResult { - name: string; - dependencies: { - "firebase-functions": { - version: string; - from: string; - resolved: string; - }; - }; -} - -interface NpmShowResult { - "dist-tags": { - latest: string; - }; -} - -/** - * Returns the version of firebase-functions SDK specified by package.json and package-lock.json. - * @param sourceDir Source directory of functions code - * @return version string (e.g. "3.1.2"), or void if firebase-functions is not in package.json - * or if we had trouble getting the version. - */ -export function getFunctionsSDKVersion(sourceDir: string): string | void { - try { - const child = spawn.sync("npm", ["list", "firebase-functions", "--json=true"], { - cwd: sourceDir, - encoding: "utf8", - }); - if (child.error) { - logger.debug("getFunctionsSDKVersion encountered error:", child.error.stack); - return; - } - const output: NpmListResult = JSON.parse(child.stdout); - return _.get(output, ["dependencies", "firebase-functions", "version"]); - } catch (e) { - logger.debug("getFunctionsSDKVersion encountered error:", e); - return; - } -} - -/** - * Checks if firebase-functions SDK is not the latest version in NPM, and prints update notice if it is outdated. - * If it is unable to do the check, it does nothing. - * @param options Options object from "firebase deploy" command. - */ -export function checkFunctionsSDKVersion(options: any): void { - if (!options.config.has("functions")) { - return; - } - - const sourceDirName = options.config.get("functions.source"); - if (!sourceDirName) { - throw new FirebaseError( - `No functions code detected at default location ("./functions"), and no "functions.source" defined in "firebase.json"` - ); - } - const sourceDir = path.join(options.config.projectDir, sourceDirName); - const currentVersion = getFunctionsSDKVersion(sourceDir); - if (!currentVersion) { - logger.debug("getFunctionsSDKVersion was unable to retrieve 'firebase-functions' version"); - return; - } - try { - const child = spawn.sync("npm", ["show", "firebase-functions", "--json=true"], { - encoding: "utf8", - }); - if (child.error) { - logger.debug( - "checkFunctionsSDKVersion was unable to fetch information from NPM", - child.error.stack - ); - return; - } - const output: NpmShowResult = JSON.parse(child.stdout); - if (_.isEmpty(output)) { - return; - } - const latest = _.get(output, ["dist-tags", "latest"]); - - if (semver.lt(currentVersion, latest)) { - utils.logWarning( - clc.bold.yellow("functions: ") + - "package.json indicates an outdated version of firebase-functions.\nPlease upgrade using " + - clc.bold("npm install --save firebase-functions@latest") + - " in your functions directory." - ); - if (semver.satisfies(currentVersion, "0.x") && semver.satisfies(latest, "1.x")) { - utils.logWarning( - clc.bold.yellow("functions: ") + - "Please note that there will be breaking changes when you upgrade.\n Go to " + - clc.bold("https://firebase.google.com/docs/functions/beta-v1-diff") + - " to learn more." - ); - } - } - } catch (e) { - logger.debug("checkFunctionsSDKVersion encountered error:", e); - return; - } -} diff --git a/src/commands/deploy.js b/src/commands/deploy.js index 6a96c62d12e..41ebb2d32dc 100644 --- a/src/commands/deploy.js +++ b/src/commands/deploy.js @@ -5,7 +5,6 @@ const { requireDatabaseInstance } = require("../requireDatabaseInstance"); const { requirePermissions } = require("../requirePermissions"); const { checkServiceAccountIam } = require("../deploy/functions/checkIam"); const checkValidTargetFilters = require("../checkValidTargetFilters"); -const checkFunctionsSDKVersion = require("../checkFirebaseSDKVersion").checkFunctionsSDKVersion; const { Command } = require("../command"); const deploy = require("../deploy"); const requireConfig = require("../requireConfig"); @@ -79,7 +78,6 @@ module.exports = new Command("deploy") } }) .before(checkValidTargetFilters) - .before(checkFunctionsSDKVersion) .action(function (options) { return deploy(options.filteredTargets, options); }); diff --git a/src/deploy/functions/args.ts b/src/deploy/functions/args.ts index 2940eaab3db..f7e5503acb7 100644 --- a/src/deploy/functions/args.ts +++ b/src/deploy/functions/args.ts @@ -2,6 +2,7 @@ import { ReadStream } from "fs"; import * as backend from "./backend"; import * as gcfV2 from "../../gcp/cloudfunctionsv2"; +import * as runtimes from "./runtimes"; // These types should proably be in a root deploy.ts, but we can only boil the ocean one bit at a time. @@ -21,7 +22,6 @@ export interface Context { // Filled in the "prepare" phase. functionsSource?: string; - runtimeChoice?: backend.Runtime; runtimeConfigEnabled?: boolean; firebaseConfig?: FirebaseConfig; diff --git a/src/deploy/functions/backend.ts b/src/deploy/functions/backend.ts index 6bd84078f36..217cdeb5708 100644 --- a/src/deploy/functions/backend.ts +++ b/src/deploy/functions/backend.ts @@ -1,11 +1,10 @@ import * as proto from "../../gcp/proto"; import * as gcf from "../../gcp/cloudfunctions"; import * as gcfV2 from "../../gcp/cloudfunctionsv2"; -import * as cloudscheduler from "../../gcp/cloudscheduler"; import * as utils from "../../utils"; +import * as runtimes from "./runtimes"; import { FirebaseError } from "../../error"; import { Context } from "./args"; -import { logger } from "../../logger"; import { previews } from "../../previews"; /** Retry settings for a ScheduleSpec. */ @@ -116,18 +115,6 @@ export function memoryOptionDisplayName(option: MemoryOptions): string { export const SCHEDULED_FUNCTION_LABEL = Object.freeze({ deployment: "firebase-schedule" }); -/** Supported runtimes for new Cloud Functions. */ -export type Runtime = "nodejs10" | "nodejs12" | "nodejs14"; - -/** Runtimes that can be found in existing backends but not used for new functions. */ -export type DeprecatedRuntime = "nodejs6" | "nodejs8"; -const RUNTIMES: string[] = ["nodejs10", "nodejs12", "nodejs14"]; - -/** Type deduction helper for a runtime string. */ -export function isValidRuntime(runtime: string): runtime is Runtime { - return RUNTIMES.includes(runtime); -} - /** * IDs used to identify a regional resource. * This type exists so we can have lightweight references from a Pub/Sub topic @@ -151,7 +138,7 @@ export interface FunctionSpec extends TargetIds { apiVersion: FunctionsApiVersion; entryPoint: string; trigger: HttpsTrigger | EventTrigger; - runtime: Runtime | DeprecatedRuntime; + runtime: runtimes.Runtime | runtimes.DeprecatedRuntime; labels?: Record; environmentVariables?: Record; diff --git a/src/deploy/functions/discovery/index.ts b/src/deploy/functions/discovery/index.ts deleted file mode 100644 index 7d24d7fe8b5..00000000000 --- a/src/deploy/functions/discovery/index.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { FirebaseError } from "../../../error"; -import { logger } from "../../../logger"; -import * as backend from "../backend"; -import * as args from "../args"; -import * as jsTriggerParsing from "./jsexports/parseTriggers"; -import { Options } from "../../../options"; - -type BackendDiscoveryStrategy = ( - context: args.Context, - options: Options, - runtimeConfig: backend.RuntimeConfigValues, - env: backend.EnvironmentVariables -) => Promise; - -type UseBackendDiscoveryStrategy = (context: args.Context) => Promise; - -type Strategy = { - name: string; - useStrategy: UseBackendDiscoveryStrategy; - discoverBackend: BackendDiscoveryStrategy; -}; - -const STRATEGIES: Strategy[] = [ - { - name: "parseJSExports", - useStrategy: jsTriggerParsing.useStrategy, - discoverBackend: jsTriggerParsing.discoverBackend, - }, -]; - -// TODO(inlined): Replace runtimeConfigValues with ENV variables. -// TODO(inlined): Parse the Runtime within this method instead of before it. We need this to support other languages. -export async function discoverBackendSpec( - context: args.Context, - options: Options, - runtimeConfigValues: backend.RuntimeConfigValues, - envs: backend.EnvironmentVariables -): Promise { - let strategy: Strategy | undefined = undefined; - for (const testStrategy of STRATEGIES) { - if (await testStrategy.useStrategy(context)) { - strategy = testStrategy; - break; - } - } - - if (strategy) { - logger.debug("Analyizing backend with strategy", strategy.name); - } else { - throw new FirebaseError("Cannot determine how to analyze backend"); - } - return strategy.discoverBackend(context, options, runtimeConfigValues, envs); -} diff --git a/src/deploy/functions/checkRuntimeDependencies.ts b/src/deploy/functions/ensureCloudBuildEnabled.ts similarity index 91% rename from src/deploy/functions/checkRuntimeDependencies.ts rename to src/deploy/functions/ensureCloudBuildEnabled.ts index 3cff2a6c0c7..bda6d8cd083 100644 --- a/src/deploy/functions/checkRuntimeDependencies.ts +++ b/src/deploy/functions/ensureCloudBuildEnabled.ts @@ -45,9 +45,8 @@ function isPermissionError(e: { context?: { body?: { error?: { status?: string } * of the deployed functions. * * @param projectId Project ID upon which to check enablement. - * @param runtime The runtime as declared in package.json, e.g. `nodejs10`. */ -export async function checkRuntimeDependencies(projectId: string, runtime: string): Promise { +export async function ensureCloudBuildEnabled(projectId: string): Promise { try { await ensure(projectId, CLOUD_BUILD_API, "functions"); } catch (e) { diff --git a/src/deploy/functions/prepare.ts b/src/deploy/functions/prepare.ts index a9f059b690d..b9d19e863c0 100644 --- a/src/deploy/functions/prepare.ts +++ b/src/deploy/functions/prepare.ts @@ -1,19 +1,20 @@ import * as clc from "cli-color"; -import { checkRuntimeDependencies } from "./checkRuntimeDependencies"; import { FirebaseError } from "../../error"; +import { Options } from "../../options"; +import { ensureCloudBuildEnabled } from "./ensureCloudBuildEnabled"; import { functionMatchesAnyGroup, getFilterGroups } from "./functionsDeployHelper"; -import { getRuntimeChoice } from "./parseRuntimeAndValidateSDK"; -import { prepareFunctionsUpload } from "./prepareFunctionsUpload"; -import { promptForFailurePolicies, promptForMinInstances } from "./prompts"; import { logBullet } from "../../utils"; +import { getFunctionsConfig, getEnvs, prepareFunctionsUpload } from "./prepareFunctionsUpload"; +import { promptForFailurePolicies, promptForMinInstances } from "./prompts"; import * as args from "./args"; import * as backend from "./backend"; import * as ensureApiEnabled from "../../ensureApiEnabled"; import * as functionsConfig from "../../functionsConfig"; import * as getProjectId from "../../getProjectId"; +import * as runtimes from "./runtimes"; import * as validate from "./validate"; -import { Options } from "../../options"; +import { logger } from "../../logger"; export async function prepare( context: args.Context, @@ -24,19 +25,13 @@ export async function prepare( return; } - const sourceDirName = options.config.get("functions.source") as string; - if (!sourceDirName) { - throw new FirebaseError( - `No functions code detected at default location (./functions), and no functions.source defined in firebase.json` - ); - } - const sourceDir = options.config.path(sourceDirName); - const projectDir = options.config.projectDir; - const projectId = getProjectId(options); + const runtimeDelegate = await runtimes.getRuntimeDelegate(context, options); + logger.debug(`Validating ${runtimeDelegate.name} source`); + await runtimeDelegate.validate(); + logger.debug(`Building ${runtimeDelegate.name} source`); + await runtimeDelegate.build(); - // Check what runtime to use, first in firebase.json, then in 'engines' field. - const runtimeFromConfig = (options.config.get("functions.runtime") as backend.Runtime) || ""; - context.runtimeChoice = getRuntimeChoice(sourceDir, runtimeFromConfig); + const projectId = getProjectId(options); // Check that all necessary APIs are enabled. const checkAPIsEnabled = await Promise.all([ @@ -47,13 +42,22 @@ export async function prepare( "runtimeconfig", /* silent=*/ true ), - checkRuntimeDependencies(projectId, context.runtimeChoice!), + ensureCloudBuildEnabled(projectId), ]); context.runtimeConfigEnabled = checkAPIsEnabled[1]; // Get the Firebase Config, and set it on each function in the deployment. const firebaseConfig = await functionsConfig.getFirebaseConfig(options); context.firebaseConfig = firebaseConfig; + const runtimeConfig = await getFunctionsConfig(context); + const env = await getEnvs(context); + + logger.debug(`Analyzing ${runtimeDelegate.name} backend spec`); + const wantBackend = await runtimeDelegate.discoverSpec(runtimeConfig, env); + options.config.set("functions.backend", wantBackend); + if (backend.isEmptyBackend(wantBackend)) { + return; + } // Prepare the functions directory for upload, and set context.triggers. logBullet( @@ -62,10 +66,8 @@ export async function prepare( clc.bold(options.config.get("functions.source")) + " directory for uploading..." ); - context.functionsSource = await prepareFunctionsUpload(context, options); + context.functionsSource = await prepareFunctionsUpload(runtimeConfig, options); - // Get a list of CloudFunctionTriggers. - const wantBackend = options.config.get("functions.backend") as backend.Backend; // Setup default environment variables on each function. wantBackend.cloudFunctions.forEach((fn: backend.FunctionSpec) => { fn.environmentVariables = wantBackend.environmentVariables; @@ -91,9 +93,7 @@ export async function prepare( }; // Validate the function code that is being deployed. - validate.functionsDirectoryExists(options, sourceDirName); validate.functionIdsAreValid(wantBackend.cloudFunctions); - validate.packageJsonIsValid(sourceDirName, sourceDir, projectDir, !!runtimeFromConfig); // Check what --only filters have been passed in. context.filters = getFilterGroups(options); @@ -105,6 +105,5 @@ export async function prepare( const haveFunctions = (await backend.existingBackend(context)).cloudFunctions; await promptForFailurePolicies(options, wantFunctions, haveFunctions); await promptForMinInstances(options, wantFunctions, haveFunctions); - await backend.checkAvailability(context, wantBackend); } diff --git a/src/deploy/functions/prepareFunctionsUpload.ts b/src/deploy/functions/prepareFunctionsUpload.ts index a3e275dbf1e..f200ec9a302 100644 --- a/src/deploy/functions/prepareFunctionsUpload.ts +++ b/src/deploy/functions/prepareFunctionsUpload.ts @@ -8,8 +8,7 @@ import * as tmp from "tmp"; import { FirebaseError } from "../../error"; import { logger } from "../../logger"; -import { discoverBackendSpec } from "./discovery"; -import { isEmptyBackend } from "./backend"; +import * as backend from "./backend"; import * as functionsConfig from "../../functionsConfig"; import * as utils from "../../utils"; import * as fsAsync from "../../fsAsync"; @@ -18,7 +17,8 @@ import { Options } from "../../options"; const CONFIG_DEST_FILE = ".runtimeconfig.json"; -async function getFunctionsConfig(context: args.Context): Promise<{ [key: string]: any }> { +// TODO(inlined): move to a file that's not about uploading source code +export async function getFunctionsConfig(context: args.Context): Promise<{ [key: string]: any }> { let config: Record = {}; if (context.runtimeConfigEnabled) { try { @@ -46,7 +46,8 @@ async function getFunctionsConfig(context: args.Context): Promise<{ [key: string return config; } -async function getEnvs(context: args.Context): Promise<{ [key: string]: string }> { +// TODO(inlined): move to a file that's not about uploading source code +export async function getEnvs(context: args.Context): Promise<{ [key: string]: string }> { const envs = { FIREBASE_CONFIG: JSON.stringify(context.firebaseConfig), }; @@ -114,17 +115,9 @@ async function packageSource(options: Options, sourceDir: string, configValues: } export async function prepareFunctionsUpload( - context: args.Context, + runtimeConfig: backend.RuntimeConfigValues, options: Options ): Promise { const sourceDir = options.config.path(options.config.get("functions.source") as string); - const configValues = await getFunctionsConfig(context); - const envs = await getEnvs(context); - const backend = await discoverBackendSpec(context, options, configValues, envs); - options.config.set("functions.backend", backend); - if (isEmptyBackend(backend)) { - // No need to package if there are 0 functions to deploy. - return; - } - return packageSource(options, sourceDir, configValues); + return packageSource(options, sourceDir, runtimeConfig); } diff --git a/src/deploy/functions/release.ts b/src/deploy/functions/release.ts index 2b132f2702b..e53745d4e65 100644 --- a/src/deploy/functions/release.ts +++ b/src/deploy/functions/release.ts @@ -55,7 +55,6 @@ export async function release(context: args.Context, options: Options, payload: projectId, sourceUrl, storageSource: context.storageSource, - runtime: context.runtimeChoice, errorHandler, }; diff --git a/src/deploy/functions/runtimes/index.ts b/src/deploy/functions/runtimes/index.ts new file mode 100644 index 00000000000..1c8523ab5d3 --- /dev/null +++ b/src/deploy/functions/runtimes/index.ts @@ -0,0 +1,122 @@ +import { Options } from "../../../options"; +import * as backend from "../backend"; +import * as args from "../args"; +import * as node from "./node"; +import * as validate from "../validate"; +import { FirebaseError } from "../../../error"; + +/** Supported runtimes for new Cloud Functions. */ +const RUNTIMES: string[] = ["nodejs10", "nodejs12", "nodejs14"]; +export type Runtime = typeof RUNTIMES[number]; + +/** Runtimes that can be found in existing backends but not used for new functions. */ +const DEPRECATED_RUNTIMES = ["nodejs6", "nodejs8"]; +export type DeprecatedRuntime = typeof DEPRECATED_RUNTIMES[number]; + +/** Type deduction helper for a runtime string */ +export function isDeprecatedRuntime(runtime: string): runtime is DeprecatedRuntime { + return DEPRECATED_RUNTIMES.includes(runtime); +} + +/** Type deduction helper for a runtime string. */ +export function isValidRuntime(runtime: string): runtime is Runtime { + return RUNTIMES.includes(runtime); +} + +const MESSAGE_FRIENDLY_RUNTIMES: Record = { + nodejs6: "Node.js 6 (Deprecated)", + nodejs8: "Node.js 8 (Deprecated)", + nodejs10: "Node.js 10", + nodejs12: "Node.js 12", + nodejs14: "Node.js 14", +}; + +/** + * Returns a friendly string denoting the chosen runtime: Node.js 8 for nodejs 8 + * for example. If no friendly name for runtime is found, returns back the raw runtime. + * @param runtime name of runtime in raw format, ie, "nodejs8" or "nodejs10" + * @return A human-friendly string describing the runtime. + */ +export function getHumanFriendlyRuntimeName(runtime: Runtime | DeprecatedRuntime): string { + return MESSAGE_FRIENDLY_RUNTIMES[runtime] || runtime; +} + +/** + * RuntimeDelegate is a language-agnostic strategy for managing + * customer source. + */ +export interface RuntimeDelegate { + /** A friendly name for the runtime; used for debug purposes */ + name: string; + + /** + * The name of the specific runtime of this source code. + * This will often differ from `name` because `name` will be + * version-free but this will include a specific runtime for + * the GCF API. + */ + runtime: Runtime; + + /** + * Validate makes sure the customers' code is actually viable. + * This includes checks like making sure a package.json file is + * well formed. + * This is a first line of defense for static analysis and does + * not include any build or runtime errors in the customer's code. + */ + validate(): Promise; + + /** + * Perform any steps necessary to build a customer's code. This can + * include transpiling TypeScript, calling a Go compiler, or running + * docker build. This step will be run before a function is deployed. + */ + build(): Promise; + + /** + * Perform any steps necessary to continuously build a customer's code. + * This is for languages like TypeScript which have a "watch" feature. + * Returns a cancel function. + */ + watch(): Promise<() => Promise>; + + /** + * Inspect the customer's source for the backend spec it describes. + */ + // TODO: Once discoverSpec supports/is all an HTTP contract, we should find a way + // for this to reuse or keep alive an HTTP server. This will speed up the emulator + // by only loading customer code once. This part of the interface will be easier + // to figure out as we go. + discoverSpec( + configValues: backend.RuntimeConfigValues, + envs: backend.EnvironmentVariables + ): Promise; +} + +type Factory = (context: args.Context, options: Options) => Promise; +const factories: Factory[] = [node.tryCreateDelegate]; + +export async function getRuntimeDelegate( + context: args.Context, + options: Options +): Promise { + const sourceDirName = options.config.get("functions.source") as string; + if (!sourceDirName) { + throw new FirebaseError( + `No functions code detected at default location (./functions), and no functions.source defined in firebase.json` + ); + } + validate.functionsDirectoryExists(options, sourceDirName); + + for (const factory of factories) { + const delegate = await factory(context, options); + if (delegate) { + return delegate; + } + } + + throw new FirebaseError( + "Could not detect language for functions at", + options.config.get("functions.source") + ); +} diff --git a/src/deploy/functions/discovery/jsexports/extractTriggers.js b/src/deploy/functions/runtimes/node/extractTriggers.js similarity index 100% rename from src/deploy/functions/discovery/jsexports/extractTriggers.js rename to src/deploy/functions/runtimes/node/extractTriggers.js diff --git a/src/deploy/functions/runtimes/node/index.ts b/src/deploy/functions/runtimes/node/index.ts new file mode 100644 index 00000000000..f40d5d7da65 --- /dev/null +++ b/src/deploy/functions/runtimes/node/index.ts @@ -0,0 +1,102 @@ +import { promisify } from "util"; +import * as fs from "fs"; +import * as path from "path"; + +import { FirebaseError } from "../../../../error"; +import { Options } from "../../../../options"; +import { getRuntimeChoice } from "./parseRuntimeAndValidateSDK"; +import * as args from "../../args"; +import * as backend from "../../backend"; +import * as getProjectId from "../../../../getProjectId"; +import * as runtimes from ".."; +import * as validate from "./validate"; +import { logger } from "../../../../logger"; +import * as versioning from "./versioning"; +import * as parseTriggers from "./parseTriggers"; + +export async function tryCreateDelegate( + context: args.Context, + options: Options +): Promise { + const sourceDirName = options.config.get("functions.source") as string; + const sourceDir = options.config.path(sourceDirName); + const packageJsonPath = path.join(sourceDir, "package.json"); + + if (!(await promisify(fs.exists)(packageJsonPath))) { + logger.debug("Customer code is not Node"); + return undefined; + } + + // Check what runtime to use, first in firebase.json, then in 'engines' field. + let runtime = (options.config.get("functions.runtime") as runtimes.Runtime) || ""; + // TODO: This method loads the Functions SDK version which is then manually loaded elsewhere. + // We should find a way to refactor this code so we're not repeatedly invoking node. + runtime = getRuntimeChoice(sourceDir, runtime); + + if (!runtime.startsWith("nodejs")) { + logger.debug( + "Customer has a package.json but did not get a nodejs runtime. This should not happen" + ); + throw new FirebaseError(`Unexpected runtime ${runtime}`); + } + + return new Delegate( + getProjectId(options), + options.config.projectDir, + sourceDirName, + sourceDir, + runtime + ); +} + +// TODO(inlined): Consider moving contents in parseRuntimeAndValidateSDK and validate around. +// Those two files are currently pretty coupled (e.g. they borrow error messages from each other) +// and both files load package.json. Maybe the delegate should be constructed with a package.json and +// that can be passed to both methods. +export class Delegate { + public readonly name = "nodejs"; + + constructor( + private readonly projectId: string, + private readonly projectDir: string, + private readonly sourceDirName: string, + private readonly sourceDir: string, + public readonly runtime: runtimes.Runtime + ) {} + + // Using a caching interface because we (may/will) eventually depend on the SDK version + // to decide whether to use the JS export method of discovery or the HTTP container contract + // method of discovery. + _sdkVersion: string = ""; + get sdkVersion() { + if (!this._sdkVersion) { + this._sdkVersion = versioning.getFunctionsSDKVersion(this.sourceDir) || ""; + } + return this._sdkVersion; + } + + validate(): Promise { + versioning.checkFunctionsSDKVersion(this.sdkVersion); + + validate.packageJsonIsValid(this.sourceDirName, this.sourceDir, this.projectDir); + + return Promise.resolve(); + } + + async build(): Promise { + // TODO: consider running npm build or tsc. This is currently redundant with predeploy hooks, + // so we would need to detect and notify users that they can just use idiomatic options instead. + } + + watch(): Promise<() => Promise> { + // TODO: consider running npm run watch if it is defined or tsc watch when tsconfig.json is present. + return Promise.resolve(() => Promise.resolve()); + } + + async discoverSpec( + config: backend.RuntimeConfigValues, + env: backend.EnvironmentVariables + ): Promise { + return parseTriggers.discoverBackend(this.projectId, this.sourceDir, this.runtime, config, env); + } +} diff --git a/src/deploy/functions/parseRuntimeAndValidateSDK.ts b/src/deploy/functions/runtimes/node/parseRuntimeAndValidateSDK.ts similarity index 53% rename from src/deploy/functions/parseRuntimeAndValidateSDK.ts rename to src/deploy/functions/runtimes/node/parseRuntimeAndValidateSDK.ts index df5b67a2fc8..cc318d2f600 100644 --- a/src/deploy/functions/parseRuntimeAndValidateSDK.ts +++ b/src/deploy/functions/runtimes/node/parseRuntimeAndValidateSDK.ts @@ -1,32 +1,16 @@ import * as _ from "lodash"; import * as path from "path"; import * as clc from "cli-color"; -import * as semver from "semver"; -import { FirebaseError } from "../../error"; -import { getFunctionsSDKVersion } from "../../checkFirebaseSDKVersion"; -import { logger } from "../../logger"; -import * as backend from "./backend"; -import * as utils from "../../utils"; -import * as track from "../../track"; +import { FirebaseError } from "../../../../error"; +import * as track from "../../../../track"; +import * as runtimes from "../../runtimes"; // have to require this because no @types/cjson available // eslint-disable-next-line @typescript-eslint/no-var-requires const cjson = require("cjson"); -const MESSAGE_FRIENDLY_RUNTIMES: Record = { - nodejs6: "Node.js 6 (Deprecated)", - nodejs8: "Node.js 8 (Deprecated)", - nodejs10: "Node.js 10", - nodejs12: "Node.js 12", - nodejs14: "Node.js 14", -}; - -const DEPRECATED_RUTNIMES = ["nodejs6", "nodejs8"]; - -type DeprecatedRuntime = typeof DEPRECATED_RUTNIMES[number]; - -const ENGINE_RUNTIMES: Record = { +const ENGINE_RUNTIMES: Record = { 6: "nodejs6", 8: "nodejs8", 10: "nodejs10", @@ -39,7 +23,7 @@ const ENGINE_RUNTIMES_NAMES = Object.values(ENGINE_RUNTIMES); export const RUNTIME_NOT_SET = "`runtime` field is required but was not found in firebase.json.\n" + "To fix this, add the following lines to the `functions` section of your firebase.json:\n" + - '"runtime": "nodejs12"\n'; + '"runtime": "nodejs14"\n'; export const UNSUPPORTED_NODE_VERSION_FIREBASE_JSON_MSG = clc.bold( `functions.runtime value is unsupported. ` + @@ -61,42 +45,9 @@ export const DEPRECATED_NODE_VERSION_INFO = `Existing Node.js 8 functions ${clc.underline("will stop executing on 2021-03-15")}` )}. Update existing functions to Node.js 10 or greater as soon as possible.`; -export const FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING = - clc.bold.yellow("functions: ") + - "You must have a " + - clc.bold("firebase-functions") + - " version that is at least 2.0.0. Please run " + - clc.bold("npm i --save firebase-functions@latest") + - " in the functions folder."; - -function functionsSDKTooOld(sourceDir: string, minRange: string): boolean { - const userVersion = getFunctionsSDKVersion(sourceDir); - if (!userVersion) { - logger.debug("getFunctionsSDKVersion was unable to retrieve 'firebase-functions' version"); - return false; - } - try { - if (!semver.intersects(userVersion, minRange)) { - return true; - } - } catch (e) { - // do nothing - } - - return false; -} - -/** - * Returns a friendly string denoting the chosen runtime: Node.js 8 for nodejs 8 - * for example. If no friendly name for runtime is found, returns back the raw runtime. - * @param runtime name of runtime in raw format, ie, "nodejs8" or "nodejs10" - * @return A human-friendly string describing the runtime. - */ -export function getHumanFriendlyRuntimeName(runtime: backend.Runtime | DeprecatedRuntime): string { - return _.get(MESSAGE_FRIENDLY_RUNTIMES, runtime, runtime); -} - -function getRuntimeChoiceFromPackageJson(sourceDir: string): backend.Runtime | DeprecatedRuntime { +function getRuntimeChoiceFromPackageJson( + sourceDir: string +): runtimes.Runtime | runtimes.DeprecatedRuntime { const packageJsonPath = path.join(sourceDir, "package.json"); let loaded; try { @@ -106,8 +57,10 @@ function getRuntimeChoiceFromPackageJson(sourceDir: string): backend.Runtime | D } const engines = loaded.engines; if (!engines || !engines.node) { - // We should really never hit this, since deploy/functions/prepare already checked that - // the runtime is defined in either firebase.json or the "engines" field of the package.json. + // It's a little strange, but we're throwing an error telling customers to put runtime in firebase.json + // if it isn't set in package.json. This is because we know through the order of function calls (note this + // method isn't exported) that this condition is only hit if we've checked both firebase.json and + // package.json. throw new FirebaseError(RUNTIME_NOT_SET); } @@ -121,7 +74,7 @@ function getRuntimeChoiceFromPackageJson(sourceDir: string): backend.Runtime | D * @param runtimeFromConfig runtime from the `functions` section of firebase.json file (may be empty). * @return The runtime, e.g. `nodejs12`. */ -export function getRuntimeChoice(sourceDir: string, runtimeFromConfig?: string): backend.Runtime { +export function getRuntimeChoice(sourceDir: string, runtimeFromConfig?: string): runtimes.Runtime { const runtime = runtimeFromConfig || getRuntimeChoiceFromPackageJson(sourceDir); const errorMessage = (runtimeFromConfig @@ -133,18 +86,13 @@ export function getRuntimeChoice(sourceDir: string, runtimeFromConfig?: string): throw new FirebaseError(errorMessage, { exit: 1 }); } - // Note: the backend.isValidRuntime should always be true because we've verified + // Note: the runtimes.isValidRuntime should always be true because we've verified // it's in ENGINE_RUNTIME_NAMES and not in DEPRECATED_RUNTIMES. This is still a // good defense in depth and also lets us upcast the response to Runtime safely. - if (DEPRECATED_RUTNIMES.includes(runtime) || !backend.isValidRuntime(runtime)) { + if (runtimes.isDeprecatedRuntime(runtime) || !runtimes.isValidRuntime(runtime)) { track("functions_runtime_notices", `${runtime}_deploy_prohibited`); throw new FirebaseError(errorMessage, { exit: 1 }); } - if (functionsSDKTooOld(sourceDir, ">=2")) { - track("functions_runtime_notices", "functions_sdk_too_old"); - utils.logWarning(FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING); - } - return runtime; } diff --git a/src/deploy/functions/discovery/jsexports/parseTriggers.ts b/src/deploy/functions/runtimes/node/parseTriggers.ts similarity index 94% rename from src/deploy/functions/discovery/jsexports/parseTriggers.ts rename to src/deploy/functions/runtimes/node/parseTriggers.ts index a1cbb85a40c..da4ecb9272d 100644 --- a/src/deploy/functions/discovery/jsexports/parseTriggers.ts +++ b/src/deploy/functions/runtimes/node/parseTriggers.ts @@ -8,7 +8,7 @@ import * as backend from "../../backend"; import * as api from "../../../../api"; import * as proto from "../../../../gcp/proto"; import * as args from "../../args"; -import { Options } from "../../../../options"; +import * as runtimes from "../../runtimes"; const TRIGGER_PARSER = path.resolve(__dirname, "./triggerParser.js"); @@ -118,23 +118,23 @@ export function useStrategy(context: args.Context): Promise { } export async function discoverBackend( - context: args.Context, - options: Options, + projectId: string, + sourceDir: string, + runtime: runtimes.Runtime, configValues: backend.RuntimeConfigValues, envs: backend.EnvironmentVariables ): Promise { - const sourceDir = options.config.path(options.config.get("functions.source") as string); - const triggerAnnotations = await parseTriggers(context.projectId, sourceDir, configValues, envs); + const triggerAnnotations = await parseTriggers(projectId, sourceDir, configValues, envs); const want: backend.Backend = { ...backend.empty(), environmentVariables: envs }; for (const annotation of triggerAnnotations) { - addResourcesToBackend(context.projectId, context.runtimeChoice!, annotation, want); + addResourcesToBackend(projectId, runtime, annotation, want); } return want; } export function addResourcesToBackend( projectId: string, - runtime: backend.Runtime, + runtime: runtimes.Runtime, annotation: TriggerAnnotation, want: backend.Backend ) { diff --git a/src/deploy/functions/discovery/jsexports/triggerParser.js b/src/deploy/functions/runtimes/node/triggerParser.js similarity index 100% rename from src/deploy/functions/discovery/jsexports/triggerParser.js rename to src/deploy/functions/runtimes/node/triggerParser.js diff --git a/src/deploy/functions/runtimes/node/validate.ts b/src/deploy/functions/runtimes/node/validate.ts new file mode 100644 index 00000000000..87e6105f503 --- /dev/null +++ b/src/deploy/functions/runtimes/node/validate.ts @@ -0,0 +1,57 @@ +import * as path from "path"; + +import { FirebaseError } from "../../../../error"; +import { logger } from "../../../../logger"; +import * as fsutils from "../../../../fsutils"; + +// have to require this because no @types/cjson available +// tslint:disable-next-line +const cjson = require("cjson"); + +/** + * Asserts that functions source directory exists and source file is present. + * @param data Object representing package.json file. + * @param sourceDir Directory for the functions source. + * @param projectDir Project directory. + * @throws { FirebaseError } Functions source directory and source file must exist. + */ +function assertFunctionsSourcePresent(data: any, sourceDir: string, projectDir: string): void { + const indexJsFile = path.join(sourceDir, data.main || "index.js"); + if (!fsutils.fileExistsSync(indexJsFile)) { + const msg = `${path.relative( + projectDir, + indexJsFile + )} does not exist, can't deploy Cloud Functions`; + throw new FirebaseError(msg); + } +} + +/** + * Validate contents of package.json to ensure main file is present. + * @param sourceDirName Name of source directory. + * @param sourceDir Relative path of source directory. + * @param projectDir Relative path of project directory. + * @param hasRuntimeConfigInConfig Whether the runtime was chosen in the `functions` section of firebase.json. + * @throws { FirebaseError } Package.json must be present and valid. + */ +export function packageJsonIsValid( + sourceDirName: string, + sourceDir: string, + projectDir: string +): void { + const packageJsonFile = path.join(sourceDir, "package.json"); + if (!fsutils.fileExistsSync(packageJsonFile)) { + const msg = `No npm package found in functions source directory. Please run 'npm init' inside ${sourceDirName}`; + throw new FirebaseError(msg); + } + + let data; + try { + data = cjson.load(packageJsonFile); + logger.debug("> [functions] package.json contents:", JSON.stringify(data, null, 2)); + assertFunctionsSourcePresent(data, sourceDir, projectDir); + } catch (e) { + const msg = `There was an error reading ${sourceDirName}${path.sep}package.json:\n\n ${e.message}`; + throw new FirebaseError(msg); + } +} diff --git a/src/deploy/functions/runtimes/node/versioning.ts b/src/deploy/functions/runtimes/node/versioning.ts new file mode 100644 index 00000000000..6ae7187bcf0 --- /dev/null +++ b/src/deploy/functions/runtimes/node/versioning.ts @@ -0,0 +1,117 @@ +import * as _ from "lodash"; +import * as clc from "cli-color"; +import * as path from "path"; +import * as semver from "semver"; +import * as spawn from "cross-spawn"; + +import * as utils from "../../../../utils"; +import { logger } from "../../../../logger"; +import * as track from "../../../../track"; + +interface NpmListResult { + name: string; + dependencies: { + "firebase-functions": { + version: string; + from: string; + resolved: string; + }; + }; +} + +interface NpmShowResult { + "dist-tags": { + latest: string; + }; +} + +const MIN_SDK_VERSION = "2.0.0"; + +export const FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING = + clc.bold.yellow("functions: ") + + "You must have a " + + clc.bold("firebase-functions") + + " version that is at least 2.0.0. Please run " + + clc.bold("npm i --save firebase-functions@latest") + + " in the functions folder."; + +/** + * Returns the version of firebase-functions SDK specified by package.json and package-lock.json. + * @param sourceDir Source directory of functions code + * @return version string (e.g. "3.1.2"), or void if firebase-functions is not in package.json + * or if we had trouble getting the version. + */ +export function getFunctionsSDKVersion(sourceDir: string): string | void { + try { + const child = spawn.sync("npm", ["list", "firebase-functions", "--json=true"], { + cwd: sourceDir, + encoding: "utf8", + }); + if (child.error) { + logger.debug("getFunctionsSDKVersion encountered error:", child.error.stack); + return; + } + const output: NpmListResult = JSON.parse(child.stdout); + return _.get(output, ["dependencies", "firebase-functions", "version"]); + } catch (e) { + logger.debug("getFunctionsSDKVersion encountered error:", e); + return; + } +} + +export function getLatestSDKVersion(): string | undefined { + const child = spawn.sync("npm", ["show", "firebase-functions", "--json=true"], { + encoding: "utf8", + }); + if (child.error) { + logger.debug( + "checkFunctionsSDKVersion was unable to fetch information from NPM", + child.error.stack + ); + return; + } + const output: NpmShowResult = JSON.parse(child.stdout); + if (_.isEmpty(output)) { + return; + } + return _.get(output, ["dist-tags", "latest"]); +} + +/** + * Checks if firebase-functions SDK is not the latest version in NPM, and prints update notice if it is outdated. + * If it is unable to do the check, it does nothing. + * @param sourceDir the location of the customer's source code. + */ +export function checkFunctionsSDKVersion(currentVersion: string): void { + try { + if (semver.lt(currentVersion, MIN_SDK_VERSION)) { + track("functions_runtime_notices", "functions_sdk_too_old"); + utils.logWarning(FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING); + } + + // N.B. We must use exports.getLatestSDKVersion so that the method dynamic and we can stub in tests. + const latest = exports.getLatestSDKVersion(); + if (!latest) { + return; + } + + if (semver.eq(currentVersion, latest)) { + return; + } + utils.logWarning( + clc.bold.yellow("functions: ") + + "package.json indicates an outdated version of firebase-functions.\nPlease upgrade using " + + clc.bold("npm install --save firebase-functions@latest") + + " in your functions directory." + ); + if (semver.major(currentVersion) < semver.major(latest)) { + utils.logWarning( + clc.bold.yellow("functions: ") + + "Please note that there will be breaking changes when you upgrade." + ); + } + } catch (e) { + logger.debug("checkFunctionsSDKVersion encountered error:", e); + return; + } +} diff --git a/src/deploy/functions/tasks.ts b/src/deploy/functions/tasks.ts index 2f0f5125c2e..f9c0de9cc0c 100644 --- a/src/deploy/functions/tasks.ts +++ b/src/deploy/functions/tasks.ts @@ -5,7 +5,7 @@ import { logger } from "../../logger"; import { RegionalFunctionChanges } from "./deploymentPlanner"; import { OperationResult, OperationPollerOptions, pollOperation } from "../../operation-poller"; import { functionsOrigin, functionsV2Origin } from "../../api"; -import { getHumanFriendlyRuntimeName } from "./parseRuntimeAndValidateSDK"; +import { getHumanFriendlyRuntimeName } from "./runtimes"; import { deleteTopic } from "../../gcp/pubsub"; import { DeploymentTimer } from "./deploymentTimer"; import { ErrorHandler } from "./errorHandler"; @@ -59,7 +59,6 @@ export interface DeploymentTask { export interface TaskParams { projectId: string; - runtime?: backend.Runtime; sourceUrl?: string; storageSource?: gcfV2.StorageSource; errorHandler: ErrorHandler; @@ -80,7 +79,7 @@ export function createFunctionTask( utils.logBullet( clc.bold.cyan("functions: ") + "creating " + - getHumanFriendlyRuntimeName(params.runtime!) + + getHumanFriendlyRuntimeName(fn.runtime) + " function " + clc.bold(helper.getFunctionLabel(fn)) + "..." @@ -136,7 +135,7 @@ export function updateFunctionTask( utils.logBullet( clc.bold.cyan("functions: ") + "updating " + - getHumanFriendlyRuntimeName(params.runtime!) + + getHumanFriendlyRuntimeName(fn.runtime) + " function " + clc.bold(helper.getFunctionLabel(fn)) + "..." diff --git a/src/deploy/functions/validate.ts b/src/deploy/functions/validate.ts index b7956c7e3b2..f1c91201b73 100644 --- a/src/deploy/functions/validate.ts +++ b/src/deploy/functions/validate.ts @@ -1,9 +1,6 @@ import * as clc from "cli-color"; -import * as path from "path"; import { FirebaseError } from "../../error"; -import { logger } from "../../logger"; -import { RUNTIME_NOT_SET } from "./parseRuntimeAndValidateSDK"; import { getFunctionLabel } from "./functionsDeployHelper"; import * as backend from "./backend"; import * as fsutils from "../../fsutils"; @@ -49,41 +46,6 @@ export function functionIdsAreValid(functions: { id: string }[]): void { } } -/** - * Validate contents of package.json to ensure main file is present. - * @param sourceDirName Name of source directory. - * @param sourceDir Relative path of source directory. - * @param projectDir Relative path of project directory. - * @param hasRuntimeConfigInConfig Whether the runtime was chosen in the `functions` section of firebase.json. - * @throws { FirebaseError } Package.json must be present and valid. - */ -export function packageJsonIsValid( - sourceDirName: string, - sourceDir: string, - projectDir: string, - hasRuntimeConfigInConfig: boolean -): void { - const packageJsonFile = path.join(sourceDir, "package.json"); - if (!fsutils.fileExistsSync(packageJsonFile)) { - const msg = `No npm package found in functions source directory. Please run 'npm init' inside ${sourceDirName}`; - throw new FirebaseError(msg); - } - - let data; - try { - data = cjson.load(packageJsonFile); - logger.debug("> [functions] package.json contents:", JSON.stringify(data, null, 2)); - assertFunctionsSourcePresent(data, sourceDir, projectDir); - } catch (e) { - const msg = `There was an error reading ${sourceDirName}${path.sep}package.json:\n\n ${e.message}`; - throw new FirebaseError(msg); - } - - if (!hasRuntimeConfigInConfig) { - assertEnginesFieldPresent(data); - } -} - export function checkForInvalidChangeOfTrigger( fn: backend.FunctionSpec, exFn: backend.FunctionSpec @@ -124,32 +86,3 @@ export function checkForInvalidChangeOfTrigger( ); } } - -/** - * Asserts that functions source directory exists and source file is present. - * @param data Object representing package.json file. - * @param sourceDir Directory for the functions source. - * @param projectDir Project directory. - * @throws { FirebaseError } Functions source directory and source file must exist. - */ -function assertFunctionsSourcePresent(data: any, sourceDir: string, projectDir: string): void { - const indexJsFile = path.join(sourceDir, data.main || "index.js"); - if (!fsutils.fileExistsSync(indexJsFile)) { - const msg = `${path.relative( - projectDir, - indexJsFile - )} does not exist, can't deploy Cloud Functions`; - throw new FirebaseError(msg); - } -} - -/** - * Asserts the engines field is present in package.json. - * @param data Object representing package.json file. - * @throws { FirebaseError } Engines field must be present in package.json. - */ -function assertEnginesFieldPresent(data: any): void { - if (!data.engines || !data.engines.node) { - throw new FirebaseError(RUNTIME_NOT_SET); - } -} diff --git a/src/gcp/cloudfunctions.ts b/src/gcp/cloudfunctions.ts index f46e458205d..a1ffb8a4342 100644 --- a/src/gcp/cloudfunctions.ts +++ b/src/gcp/cloudfunctions.ts @@ -6,6 +6,7 @@ import * as api from "../api"; import * as backend from "../deploy/functions/backend"; import * as utils from "../utils"; import * as proto from "./proto"; +import * as runtimes from "../deploy/functions/runtimes"; export const API_VERSION = "v1"; @@ -63,7 +64,6 @@ export interface SecretVolume { }[]; } -export type Runtime = "nodejs10" | "nodejs12" | "nodejs14"; export type CloudFunctionStatus = | "ACTIVE" | "OFFLINE" @@ -97,7 +97,7 @@ export interface CloudFunction { // end oneof trigger; entryPoint: string; - runtime: Runtime; + runtime: runtimes.Runtime; // Seconds. Default = 60 timeout?: proto.Duration; @@ -392,7 +392,7 @@ export function specFromFunction(gcfFunction: CloudFunction): backend.FunctionSp }; } - if (!backend.isValidRuntime(gcfFunction.runtime)) { + if (!runtimes.isValidRuntime(gcfFunction.runtime)) { logger.debug("GCFv1 function has a deprecated runtime:", JSON.stringify(gcfFunction, null, 2)); } @@ -440,7 +440,7 @@ export function functionFromSpec( ); } - if (!backend.isValidRuntime(cloudFunction.runtime)) { + if (!runtimes.isValidRuntime(cloudFunction.runtime)) { throw new FirebaseError( "Failed internal assertion. Trying to deploy a new function with a deprecated runtime." + " This should never happen" diff --git a/src/gcp/cloudfunctionsv2.ts b/src/gcp/cloudfunctionsv2.ts index a7c5d936c27..a694dc4295f 100644 --- a/src/gcp/cloudfunctionsv2.ts +++ b/src/gcp/cloudfunctionsv2.ts @@ -5,6 +5,7 @@ import { FirebaseError } from "../error"; import { functionsV2Origin } from "../api"; import { logger } from "../logger"; import * as backend from "../deploy/functions/backend"; +import * as runtimes from "../deploy/functions/runtimes"; import * as proto from "./proto"; import * as utils from "../utils"; @@ -21,7 +22,6 @@ export const PUBSUB_PUBLISH_EVENT = "google.cloud.pubsub.topic.v1.messagePublish export type VpcConnectorEgressSettings = "PRIVATE_RANGES_ONLY" | "ALL_TRAFFIC"; export type IngressSettings = "ALLOW_ALL" | "ALLOW_INTERNAL_ONLY" | "ALLOW_INTERNAL_AND_GCLB"; export type FunctionState = "ACTIVE" | "FAILED" | "DEPLOYING" | "DELETING" | "UNKONWN"; -export type Runtime = "nodejs10" | "nodejs12" | "nodejs14"; // The GCFv2 funtion type has many inner types which themselves have output-only fields: // eventTrigger.trigger @@ -36,7 +36,7 @@ export type OutputOnlyFields = "state" | "updateTime"; /** Settings for building a container out of the customer source. */ export interface BuildConfig { - runtime: Runtime; + runtime: runtimes.Runtime; entryPoint: string; source: Source; environmentVariables: Record; @@ -333,7 +333,7 @@ export function functionFromSpec(cloudFunction: backend.FunctionSpec, source: St ); } - if (!backend.isValidRuntime(cloudFunction.runtime)) { + if (!runtimes.isValidRuntime(cloudFunction.runtime)) { throw new FirebaseError( "Failed internal assertion. Trying to deploy a new function with a deprecated runtime." + " This should never happen" @@ -430,7 +430,7 @@ export function specFromFunction(gcfFunction: CloudFunction): backend.FunctionSp }; } - if (!backend.isValidRuntime(gcfFunction.buildConfig.runtime)) { + if (!runtimes.isValidRuntime(gcfFunction.buildConfig.runtime)) { logger.debug("GCFv2 function has a deprecated runtime:", JSON.stringify(gcfFunction, null, 2)); } diff --git a/src/test/deploy/functions/checkRuntimeDependencies.spec.ts b/src/test/deploy/functions/ensureCloudBuildEnabled.ts similarity index 50% rename from src/test/deploy/functions/checkRuntimeDependencies.spec.ts rename to src/test/deploy/functions/ensureCloudBuildEnabled.ts index 7d0584051c4..1f8234b4e78 100644 --- a/src/test/deploy/functions/checkRuntimeDependencies.spec.ts +++ b/src/test/deploy/functions/ensureCloudBuildEnabled.ts @@ -4,11 +4,11 @@ import * as nock from "nock"; import { logger } from "../../../logger"; import { configstore } from "../../../configstore"; -import { checkRuntimeDependencies } from "../../../deploy/functions/checkRuntimeDependencies"; +import { ensureCloudBuildEnabled } from "../../../deploy/functions/ensureCloudBuildEnabled"; import { POLL_SETTINGS } from "../../../ensureApiEnabled"; import * as api from "../../../api"; -describe("checkRuntimeDependencies()", () => { +describe("ensureCloudBuildEnabled()", () => { let restoreInterval: number; before(() => { restoreInterval = POLL_SETTINGS.pollInterval; @@ -71,63 +71,57 @@ describe("checkRuntimeDependencies()", () => { timeStub.withArgs("motd.cloudBuildErrorAfter").returns(errorAfter); } - ["nodejs10", "nodejs12", "nodejs14"].forEach((runtime) => { - describe(`with ${runtime}`, () => { - describe("with cloudbuild service enabled", () => { - beforeEach(() => { - mockServiceCheck(true); - }); + describe("with cloudbuild service enabled", () => { + beforeEach(() => { + mockServiceCheck(true); + }); - it("should succeed", async () => { - stubTimes(Date.now() - 10000, Date.now() - 5000); + it("should succeed", async () => { + stubTimes(Date.now() - 10000, Date.now() - 5000); - await expect(checkRuntimeDependencies("test-project", runtime)).to.eventually.be - .fulfilled; - expect(logStub?.callCount).to.eq(0); - }); - }); + await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.fulfilled; + expect(logStub?.callCount).to.eq(0); + }); + }); - describe("with cloudbuild service disabled, but enabling succeeds", () => { - beforeEach(() => { - mockServiceCheck(false); - mockServiceEnableSuccess(); - mockServiceCheck(true); - }); + describe("with cloudbuild service disabled, but enabling succeeds", () => { + beforeEach(() => { + mockServiceCheck(false); + mockServiceEnableSuccess(); + mockServiceCheck(true); + }); - it("should succeed", async () => { - stubTimes(Date.now() - 10000, Date.now() - 5000); + it("should succeed", async () => { + stubTimes(Date.now() - 10000, Date.now() - 5000); - await expect(checkRuntimeDependencies("test-project", runtime)).to.eventually.be - .fulfilled; - expect(logStub?.callCount).to.eq(1); // enabling an api logs a warning - }); - }); + await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.fulfilled; + expect(logStub?.callCount).to.eq(1); // enabling an api logs a warning + }); + }); - describe("with cloudbuild service disabled, but enabling fails with billing error", () => { - beforeEach(() => { - mockServiceCheck(false); - mockServiceEnableBillingError(); - }); + describe("with cloudbuild service disabled, but enabling fails with billing error", () => { + beforeEach(() => { + mockServiceCheck(false); + mockServiceEnableBillingError(); + }); - it("should error", async () => { - stubTimes(Date.now() - 10000, Date.now() - 5000); + it("should error", async () => { + stubTimes(Date.now() - 10000, Date.now() - 5000); - await expect(checkRuntimeDependencies("test-project", runtime)).to.eventually.be.rejected; - }); - }); + await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.rejected; + }); + }); - describe("with cloudbuild service disabled, but enabling fails with permission error", () => { - beforeEach(() => { - mockServiceCheck(false); - mockServiceEnablePermissionError(); - }); + describe("with cloudbuild service disabled, but enabling fails with permission error", () => { + beforeEach(() => { + mockServiceCheck(false); + mockServiceEnablePermissionError(); + }); - it("should error", async () => { - stubTimes(Date.now() - 10000, Date.now() - 5000); + it("should error", async () => { + stubTimes(Date.now() - 10000, Date.now() - 5000); - await expect(checkRuntimeDependencies("test-project", runtime)).to.eventually.be.rejected; - }); - }); + await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.rejected; }); }); }); diff --git a/src/test/deploy/functions/runtimes/index.spec.ts b/src/test/deploy/functions/runtimes/index.spec.ts new file mode 100644 index 00000000000..460783f733b --- /dev/null +++ b/src/test/deploy/functions/runtimes/index.spec.ts @@ -0,0 +1,9 @@ +import { expect } from "chai"; + +import * as runtimes from "../../../../deploy/functions/runtimes"; + +describe("getHumanFriendlyRuntimeName", () => { + it("should properly convert raw runtime to human friendly runtime", () => { + expect(runtimes.getHumanFriendlyRuntimeName("nodejs6")).to.contain("Node.js"); + }); +}); diff --git a/src/test/deploy/functions/discovery/jsexports/extractTriggers.spec.js b/src/test/deploy/functions/runtimes/node/extractTriggers.spec.js similarity index 97% rename from src/test/deploy/functions/discovery/jsexports/extractTriggers.spec.js rename to src/test/deploy/functions/runtimes/node/extractTriggers.spec.js index c94a396bb72..865acf7e947 100644 --- a/src/test/deploy/functions/discovery/jsexports/extractTriggers.spec.js +++ b/src/test/deploy/functions/runtimes/node/extractTriggers.spec.js @@ -3,7 +3,7 @@ const chai = require("chai"); const expect = chai.expect; -const extractTriggers = require("../../../../../deploy/functions/discovery/jsexports/extractTriggers"); +const extractTriggers = require("../../../../../deploy/functions/runtimes/node/extractTriggers"); describe("extractTriggers", function () { const fnWithTrigger = function () {}; diff --git a/src/test/deploy/functions/parseRuntimeAndValidateSDK.spec.ts b/src/test/deploy/functions/runtimes/node/parseRuntimeAndValidateSDK.spec.ts similarity index 69% rename from src/test/deploy/functions/parseRuntimeAndValidateSDK.spec.ts rename to src/test/deploy/functions/runtimes/node/parseRuntimeAndValidateSDK.spec.ts index 3355f993f79..4707fc62564 100644 --- a/src/test/deploy/functions/parseRuntimeAndValidateSDK.spec.ts +++ b/src/test/deploy/functions/runtimes/node/parseRuntimeAndValidateSDK.spec.ts @@ -5,27 +5,16 @@ import * as sinon from "sinon"; // eslint-disable-next-line const cjson = require("cjson"); -import { FirebaseError } from "../../../error"; -import * as checkFirebaseSDKVersion from "../../../checkFirebaseSDKVersion"; -import * as runtime from "../../../deploy/functions/parseRuntimeAndValidateSDK"; -import * as utils from "../../../utils"; - -describe("getHumanFriendlyRuntimeName", () => { - it("should properly convert raw runtime to human friendly runtime", () => { - expect(runtime.getHumanFriendlyRuntimeName("nodejs6")).to.contain("Node.js"); - }); -}); +import { FirebaseError } from "../../../../../error"; +import * as runtime from "../../../../../deploy/functions/runtimes/node/parseRuntimeAndValidateSDK"; describe("getRuntimeChoice", () => { const sandbox = sinon.createSandbox(); let cjsonStub: sinon.SinonStub; - let warningSpy: sinon.SinonSpy; let SDKVersionStub: sinon.SinonStub; beforeEach(() => { cjsonStub = sandbox.stub(cjson, "load"); - warningSpy = sandbox.spy(utils, "logWarning"); - SDKVersionStub = sandbox.stub(checkFirebaseSDKVersion, "getFunctionsSDKVersion"); }); afterEach(() => { @@ -34,47 +23,27 @@ describe("getRuntimeChoice", () => { context("when the runtime is set in firebase.json", () => { it("should error if runtime field is set to node 6", () => { - SDKVersionStub.returns("2.0.0"); - expect(() => { runtime.getRuntimeChoice("path/to/source", "nodejs6"); }).to.throw(runtime.UNSUPPORTED_NODE_VERSION_FIREBASE_JSON_MSG); }); it("should error if runtime field is set to node 8", () => { - SDKVersionStub.returns("2.0.0"); - expect(() => { runtime.getRuntimeChoice("path/to/source", "nodejs8"); }).to.throw(runtime.UNSUPPORTED_NODE_VERSION_FIREBASE_JSON_MSG); }); it("should return node 10 if runtime field is set to node 10", () => { - SDKVersionStub.returns("3.4.0"); - expect(runtime.getRuntimeChoice("path/to/source", "nodejs10")).to.equal("nodejs10"); - expect(warningSpy).not.called; }); it("should return node 12 if runtime field is set to node 12", () => { - SDKVersionStub.returns("3.4.0"); - expect(runtime.getRuntimeChoice("path/to/source", "nodejs12")).to.equal("nodejs12"); - expect(warningSpy).not.called; }); it("should return node 14 if runtime field is set to node 14", () => { - SDKVersionStub.returns("3.4.0"); - expect(runtime.getRuntimeChoice("path/to/source", "nodejs14")).to.equal("nodejs14"); - expect(warningSpy).not.called; - }); - - it("should print warning when firebase-functions version is below 2.0.0", () => { - SDKVersionStub.returns("0.5.0"); - - runtime.getRuntimeChoice("path/to/source", "nodejs10"); - expect(warningSpy).calledWith(runtime.FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING); }); it("should throw error if unsupported node version set", () => { @@ -88,7 +57,6 @@ describe("getRuntimeChoice", () => { context("when the runtime is not set in firebase.json", () => { it("should error if engines field is set to node 6", () => { cjsonStub.returns({ engines: { node: "6" } }); - SDKVersionStub.returns("2.0.0"); expect(() => { runtime.getRuntimeChoice("path/to/source", ""); @@ -97,7 +65,6 @@ describe("getRuntimeChoice", () => { it("should error if engines field is set to node 8", () => { cjsonStub.returns({ engines: { node: "8" } }); - SDKVersionStub.returns("2.0.0"); expect(() => { runtime.getRuntimeChoice("path/to/source", ""); @@ -106,41 +73,32 @@ describe("getRuntimeChoice", () => { it("should return node 10 if engines field is set to node 10", () => { cjsonStub.returns({ engines: { node: "10" } }); - SDKVersionStub.returns("3.4.0"); expect(runtime.getRuntimeChoice("path/to/source", "")).to.equal("nodejs10"); - expect(warningSpy).not.called; }); it("should return node 12 if engines field is set to node 12", () => { cjsonStub.returns({ engines: { node: "12" } }); - SDKVersionStub.returns("3.4.0"); expect(runtime.getRuntimeChoice("path/to/source", "")).to.equal("nodejs12"); - expect(warningSpy).not.called; }); it("should return node 14 if engines field is set to node 12", () => { cjsonStub.returns({ engines: { node: "14" } }); - SDKVersionStub.returns("3.4.0"); expect(runtime.getRuntimeChoice("path/to/source", "")).to.equal("nodejs14"); - expect(warningSpy).not.called; }); it("should print warning when firebase-functions version is below 2.0.0", () => { cjsonStub.returns({ engines: { node: "10" } }); - SDKVersionStub.returns("0.5.0"); runtime.getRuntimeChoice("path/to/source", ""); - expect(warningSpy).calledWith(runtime.FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING); }); it("should not throw error if user's SDK version fails to be fetched", () => { cjsonStub.returns({ engines: { node: "10" } }); // Intentionally not setting SDKVersionStub so it can fail to be fetched. expect(runtime.getRuntimeChoice("path/to/source", "")).to.equal("nodejs10"); - expect(warningSpy).not.called; }); it("should throw error if unsupported node version set", () => { diff --git a/src/test/deploy/functions/discovery/jsexports/parseTriggers.spec.ts b/src/test/deploy/functions/runtimes/node/parseTriggers.spec.ts similarity index 99% rename from src/test/deploy/functions/discovery/jsexports/parseTriggers.spec.ts rename to src/test/deploy/functions/runtimes/node/parseTriggers.spec.ts index 54705b44f2c..814688be889 100644 --- a/src/test/deploy/functions/discovery/jsexports/parseTriggers.spec.ts +++ b/src/test/deploy/functions/runtimes/node/parseTriggers.spec.ts @@ -2,7 +2,7 @@ import { expect } from "chai"; import { FirebaseError } from "../../../../../error"; import * as backend from "../../../../../deploy/functions/backend"; -import * as parseTriggers from "../../../../../deploy/functions/discovery/jsexports/parseTriggers"; +import * as parseTriggers from "../../../../../deploy/functions/runtimes/node/parseTriggers"; import * as api from "../../../../../api"; describe("addResourcesToBackend", () => { diff --git a/src/test/deploy/functions/runtimes/node/validate.spec.ts b/src/test/deploy/functions/runtimes/node/validate.spec.ts new file mode 100644 index 00000000000..dba30e61e5b --- /dev/null +++ b/src/test/deploy/functions/runtimes/node/validate.spec.ts @@ -0,0 +1,64 @@ +import { expect } from "chai"; +import * as sinon from "sinon"; + +import { FirebaseError } from "../../../../../error"; +import { RUNTIME_NOT_SET } from "../../../../../deploy/functions/runtimes/node/parseRuntimeAndValidateSDK"; +import * as validate from "../../../../../deploy/functions/runtimes/node/validate"; +import * as fsutils from "../../../../../fsutils"; + +const cjson = require("cjson"); + +describe("validate", () => { + describe("packageJsonIsValid", () => { + const sandbox: sinon.SinonSandbox = sinon.createSandbox(); + let cjsonLoadStub: sinon.SinonStub; + let fileExistsStub: sinon.SinonStub; + + beforeEach(() => { + fileExistsStub = sandbox.stub(fsutils, "fileExistsSync"); + cjsonLoadStub = sandbox.stub(cjson, "load"); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("should throw error if package.json file is missing", () => { + fileExistsStub.withArgs("sourceDir/package.json").returns(false); + + expect(() => { + validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); + }).to.throw(FirebaseError, "No npm package found"); + }); + + it("should throw error if functions source file is missing", () => { + cjsonLoadStub.returns({ name: "my-project", engines: { node: "8" } }); + fileExistsStub.withArgs("sourceDir/package.json").returns(true); + fileExistsStub.withArgs("sourceDir/index.js").returns(false); + + expect(() => { + validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); + }).to.throw(FirebaseError, "does not exist, can't deploy"); + }); + + it("should throw error if main is defined and that file is missing", () => { + cjsonLoadStub.returns({ name: "my-project", main: "src/main.js", engines: { node: "8" } }); + fileExistsStub.withArgs("sourceDir/package.json").returns(true); + fileExistsStub.withArgs("sourceDir/src/main.js").returns(false); + + expect(() => { + validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); + }).to.throw(FirebaseError, "does not exist, can't deploy"); + }); + + it("should not throw error if runtime is set in the config and the engines field is not set", () => { + cjsonLoadStub.returns({ name: "my-project" }); + fileExistsStub.withArgs("sourceDir/package.json").returns(true); + fileExistsStub.withArgs("sourceDir/index.js").returns(true); + + expect(() => { + validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir"); + }).to.not.throw(); + }); + }); +}); diff --git a/src/test/deploy/functions/runtimes/node/versioning.spec.ts b/src/test/deploy/functions/runtimes/node/versioning.spec.ts new file mode 100644 index 00000000000..f2f5802ceb4 --- /dev/null +++ b/src/test/deploy/functions/runtimes/node/versioning.spec.ts @@ -0,0 +1,47 @@ +import { expect } from "chai"; +import * as sinon from "sinon"; + +import * as utils from "../../../../../utils"; +import * as versioning from "../../../../../deploy/functions/runtimes/node/versioning"; + +describe("checkFunctionsSDKVersion", () => { + let warningSpy: sinon.SinonSpy; + let latestVersion: sinon.SinonStub; + + beforeEach(() => { + warningSpy = sinon.spy(utils, "logWarning"); + latestVersion = sinon.stub(versioning, "getLatestSDKVersion"); + }); + + afterEach(() => { + warningSpy.restore(); + latestVersion.restore(); + }); + + it("Should warn if the SDK version is too low", () => { + latestVersion.returns("1.9.9"); + versioning.checkFunctionsSDKVersion("1.9.9"); + expect(warningSpy).calledWith(versioning.FUNCTIONS_SDK_VERSION_TOO_OLD_WARNING); + }); + + it("Should not warn for the latest SDK version", () => { + latestVersion.returns("3.14.159"); + versioning.checkFunctionsSDKVersion("3.14.159"); + expect(warningSpy).not.called; + }); + + it("Should give an upgrade warning", () => { + latestVersion.returns("5.0.1"); + versioning.checkFunctionsSDKVersion("5.0.0"); + expect(warningSpy).to.have.been.calledWith(sinon.match("Please upgrade")); + }); + + it("Should give a breaking change warning", () => { + latestVersion.returns("6.0.0"); + versioning.checkFunctionsSDKVersion("5.9.9"); + expect(warningSpy).to.have.been.calledWith(sinon.match("Please upgrade")); + expect(warningSpy).to.have.been.calledWith( + sinon.match("Please note that there will be breaking changes") + ); + }); +}); diff --git a/src/test/deploy/functions/validate.spec.ts b/src/test/deploy/functions/validate.spec.ts index 0a65c28a012..12f87154507 100644 --- a/src/test/deploy/functions/validate.spec.ts +++ b/src/test/deploy/functions/validate.spec.ts @@ -2,7 +2,7 @@ import { expect } from "chai"; import * as sinon from "sinon"; import { FirebaseError } from "../../../error"; -import { RUNTIME_NOT_SET } from "../../../deploy/functions/parseRuntimeAndValidateSDK"; +import { RUNTIME_NOT_SET } from "../../../deploy/functions/runtimes/node/parseRuntimeAndValidateSDK"; import { FunctionSpec } from "../../../deploy/functions/backend"; import * as fsutils from "../../../fsutils"; import * as validate from "../../../deploy/functions/validate"; @@ -182,89 +182,4 @@ describe("validate", () => { }).not.to.throw(); }); }); - - describe("packageJsonIsValid", () => { - const sandbox: sinon.SinonSandbox = sinon.createSandbox(); - let cjsonLoadStub: sinon.SinonStub; - let fileExistsStub: sinon.SinonStub; - - beforeEach(() => { - fileExistsStub = sandbox.stub(fsutils, "fileExistsSync"); - cjsonLoadStub = sandbox.stub(cjson, "load"); - }); - - afterEach(() => { - sandbox.restore(); - }); - - it("should throw error if package.json file is missing", () => { - fileExistsStub.withArgs("sourceDir/package.json").returns(false); - - expect(() => { - validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir", false); - }).to.throw(FirebaseError, "No npm package found"); - }); - - it("should throw error if functions source file is missing", () => { - cjsonLoadStub.returns({ name: "my-project", engines: { node: "8" } }); - fileExistsStub.withArgs("sourceDir/package.json").returns(true); - fileExistsStub.withArgs("sourceDir/index.js").returns(false); - - expect(() => { - validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir", false); - }).to.throw(FirebaseError, "does not exist, can't deploy"); - }); - - it("should throw error if main is defined and that file is missing", () => { - cjsonLoadStub.returns({ name: "my-project", main: "src/main.js", engines: { node: "8" } }); - fileExistsStub.withArgs("sourceDir/package.json").returns(true); - fileExistsStub.withArgs("sourceDir/src/main.js").returns(false); - - expect(() => { - validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir", false); - }).to.throw(FirebaseError, "does not exist, can't deploy"); - }); - - it("should not throw error if runtime is set in the config and the engines field is not set", () => { - cjsonLoadStub.returns({ name: "my-project" }); - fileExistsStub.withArgs("sourceDir/package.json").returns(true); - fileExistsStub.withArgs("sourceDir/index.js").returns(true); - - expect(() => { - validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir", true); - }).to.not.throw(); - }); - - context("runtime is not set in the config", () => { - it("should throw error if runtime is not set in the config and the engines field is not set", () => { - cjsonLoadStub.returns({ name: "my-project" }); - fileExistsStub.withArgs("sourceDir/package.json").returns(true); - fileExistsStub.withArgs("sourceDir/index.js").returns(true); - - expect(() => { - validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir", false); - }).to.throw(FirebaseError, RUNTIME_NOT_SET); - }); - - it("should throw error if engines field is set but node field missing", () => { - cjsonLoadStub.returns({ name: "my-project", engines: {} }); - fileExistsStub.withArgs("sourceDir/package.json").returns(true); - fileExistsStub.withArgs("sourceDir/index.js").returns(true); - - expect(() => { - validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir", false); - }).to.throw(FirebaseError, RUNTIME_NOT_SET); - }); - - it("should not throw error if package.json, functions file exists and engines present", () => { - cjsonLoadStub.returns({ name: "my-project", engines: { node: "8" } }); - fileExistsStub.withArgs("sourceDir/package.json").returns(true); - fileExistsStub.withArgs("sourceDir/index.js").returns(true); - - expect(() => { - validate.packageJsonIsValid("sourceDirName", "sourceDir", "projectDir", false); - }).to.not.throw(); - }); - }); - }); }); From 8f3a8d0c73ebd21f3c27eb13274c7075811c1377 Mon Sep 17 00:00:00 2001 From: Thomas Bouldin Date: Mon, 7 Jun 2021 09:31:13 -0700 Subject: [PATCH 2/2] PR feedback --- src/deploy/functions/args.ts | 3 --- src/deploy/functions/runtimes/node/validate.ts | 2 +- src/test/deploy/functions/ensureCloudBuildEnabled.ts | 11 +++++++++-- .../deploy/functions/runtimes/node/versioning.spec.ts | 1 + 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/deploy/functions/args.ts b/src/deploy/functions/args.ts index f7e5503acb7..52bcd4a2854 100644 --- a/src/deploy/functions/args.ts +++ b/src/deploy/functions/args.ts @@ -1,8 +1,5 @@ -import { ReadStream } from "fs"; - import * as backend from "./backend"; import * as gcfV2 from "../../gcp/cloudfunctionsv2"; -import * as runtimes from "./runtimes"; // These types should proably be in a root deploy.ts, but we can only boil the ocean one bit at a time. diff --git a/src/deploy/functions/runtimes/node/validate.ts b/src/deploy/functions/runtimes/node/validate.ts index 87e6105f503..510d0aef2e5 100644 --- a/src/deploy/functions/runtimes/node/validate.ts +++ b/src/deploy/functions/runtimes/node/validate.ts @@ -41,7 +41,7 @@ export function packageJsonIsValid( ): void { const packageJsonFile = path.join(sourceDir, "package.json"); if (!fsutils.fileExistsSync(packageJsonFile)) { - const msg = `No npm package found in functions source directory. Please run 'npm init' inside ${sourceDirName}`; + const msg = `No npm package found in functions source directory ${sourceDirName}.`; throw new FirebaseError(msg); } diff --git a/src/test/deploy/functions/ensureCloudBuildEnabled.ts b/src/test/deploy/functions/ensureCloudBuildEnabled.ts index 1f8234b4e78..8f50ece8794 100644 --- a/src/test/deploy/functions/ensureCloudBuildEnabled.ts +++ b/src/test/deploy/functions/ensureCloudBuildEnabled.ts @@ -2,6 +2,7 @@ import { expect } from "chai"; import * as sinon from "sinon"; import * as nock from "nock"; +import { FirebaseError } from "../../../error"; import { logger } from "../../../logger"; import { configstore } from "../../../configstore"; import { ensureCloudBuildEnabled } from "../../../deploy/functions/ensureCloudBuildEnabled"; @@ -108,7 +109,10 @@ describe("ensureCloudBuildEnabled()", () => { it("should error", async () => { stubTimes(Date.now() - 10000, Date.now() - 5000); - await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.rejected; + await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.rejectedWith( + FirebaseError, + /must be on the Blaze \(pay-as-you-go\) plan to complete this command/ + ); }); }); @@ -121,7 +125,10 @@ describe("ensureCloudBuildEnabled()", () => { it("should error", async () => { stubTimes(Date.now() - 10000, Date.now() - 5000); - await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.rejected; + await expect(ensureCloudBuildEnabled("test-project")).to.eventually.be.rejectedWith( + FirebaseError, + /Please ask a project owner to visit the following URL to enable Cloud Build/ + ); }); }); }); diff --git a/src/test/deploy/functions/runtimes/node/versioning.spec.ts b/src/test/deploy/functions/runtimes/node/versioning.spec.ts index f2f5802ceb4..a6e10c07447 100644 --- a/src/test/deploy/functions/runtimes/node/versioning.spec.ts +++ b/src/test/deploy/functions/runtimes/node/versioning.spec.ts @@ -34,6 +34,7 @@ describe("checkFunctionsSDKVersion", () => { latestVersion.returns("5.0.1"); versioning.checkFunctionsSDKVersion("5.0.0"); expect(warningSpy).to.have.been.calledWith(sinon.match("Please upgrade")); + expect(warningSpy).to.not.have.been.calledWith(sinon.match("breaking change")); }); it("Should give a breaking change warning", () => {