diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..345f59e35c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Enforce webframeworks enablement only on webframeworks sites (#8168) diff --git a/src/deploy/hosting/deploy.spec.ts b/src/deploy/hosting/deploy.spec.ts new file mode 100644 index 00000000000..7770af2e96c --- /dev/null +++ b/src/deploy/hosting/deploy.spec.ts @@ -0,0 +1,122 @@ +import { expect } from "chai"; +import * as sinon from "sinon"; + +import { setEnabled } from "../../experiments"; +import { FirebaseConfig } from "../../firebaseConfig"; +import * as frameworks from "../../frameworks"; +import * as config from "../../hosting/config"; +import { HostingOptions } from "../../hosting/options"; +import { Options } from "../../options"; +import { matchesHostingTarget, prepareFrameworksIfNeeded } from "../index"; + +describe("hosting prepare", () => { + let frameworksStub: sinon.SinonStubbedInstance; + let classicSiteConfig: config.HostingResolved; + let webFrameworkSiteConfig: config.HostingResolved; + let firebaseJson: FirebaseConfig; + let options: HostingOptions & Options; + + beforeEach(() => { + frameworksStub = sinon.stub(frameworks); + + // We're intentionally using pointer references so that editing site + // edits the results of hostingConfig() and changes firebase.json + classicSiteConfig = { + site: "classic", + target: "classic", + public: ".", + }; + webFrameworkSiteConfig = { + site: "webframework", + target: "webframework", + source: "src", + }; + firebaseJson = { + hosting: [classicSiteConfig, webFrameworkSiteConfig], + }; + options = { + cwd: ".", + configPath: ".", + only: "hosting", + except: "", + filteredTargets: ["HOSTING"], + force: false, + json: false, + nonInteractive: false, + interactive: true, + debug: false, + projectId: "project", + config: { + src: firebaseJson, + get: (key: string) => { + if (key === "hosting") { + return firebaseJson.hosting; + } + return null; + }, + } as any, + rc: null as any, + + // Forces caching behavior of hostingConfig call + normalizedHostingConfig: [classicSiteConfig, webFrameworkSiteConfig], + }; + }); + + afterEach(() => { + sinon.verifyAndRestore(); + }); + + describe("matchesHostingTarget", () => { + it("should correctly identify if a hosting target should be included in deployment", () => { + const cases = [ + { only: "hosting:site1", target: "site1", expected: true }, + { only: "hosting:site12", target: "site1", expected: false }, + { only: "hosting:", target: "", expected: true }, + { only: "functions:fn1", target: "site1", expected: true }, + { only: "hosting:site1,hosting:site2", target: "site1", expected: true }, + { only: undefined, target: "site1", expected: true }, + { only: "hosting:site1,functions:fn1", target: "site1", expected: true }, + ]; + + cases.forEach(({ only, target, expected }) => { + expect(matchesHostingTarget(only, target)).to.equal(expected); + }); + }); + }); + + it("deploys classic site without webframeworks disabled", async () => { + setEnabled("webframeworks", false); + options.only = "hosting:classic"; + await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.not.be.rejected; + }); + + it("fails webframework deploy with webframeworks disabled", async () => { + setEnabled("webframeworks", false); + options.only = "hosting:webframework"; + await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.be.rejectedWith( + /Cannot deploy a web framework from source because the experiment.+webframeworks.+is not enabled/, + ); + }); + + it("deploys webframework site with webframeworks enabled", async () => { + setEnabled("webframeworks", true); + options.only = "hosting:webframework"; + await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.not.be.rejected; + expect(frameworksStub.prepareFrameworks).to.have.been.calledOnceWith("deploy", ["hosting"]); + }); + + it("deploys classic site with webframeworks enabled", async () => { + setEnabled("webframeworks", true); + options.only = "hosting:classic"; + await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.not.be.rejected; + expect(frameworksStub.prepareFrameworks).to.not.have.been.called; + }); + + it("fails when at least one site has webframeworks enabled and the experiment is disabled", async () => { + setEnabled("webframeworks", false); + options.only = "hosting"; + await expect(prepareFrameworksIfNeeded(["hosting"], options, {})).to.be.rejectedWith( + /Cannot deploy a web framework from source because the experiment.+webframeworks.+is not enabled/, + ); + }); +}); diff --git a/src/deploy/index.ts b/src/deploy/index.ts index d422327b443..ab2eb6681b7 100644 --- a/src/deploy/index.ts +++ b/src/deploy/index.ts @@ -17,13 +17,15 @@ import * as StorageTarget from "./storage"; import * as RemoteConfigTarget from "./remoteconfig"; import * as ExtensionsTarget from "./extensions"; import * as DataConnectTarget from "./dataconnect"; +import { HostingConfig } from "../firebaseConfig"; import { prepareFrameworks } from "../frameworks"; -import { HostingDeploy } from "./hosting/context"; +import { FrameworkContext } from "../frameworks/interfaces"; import { addPinnedFunctionsToOnlyString, hasPinnedFunctions } from "./hosting/prepare"; import { isRunningInGithubAction } from "../init/features/hosting/github"; import { TARGET_PERMISSIONS } from "../commands/deploy"; import { requirePermissions } from "../requirePermissions"; import { Options } from "../options"; +import { Context } from "./hosting/context"; const TARGETS = { hosting: HostingTarget, @@ -46,6 +48,29 @@ const chain = async function (fns: Chain, context: any, options: any, payload: a } }; +export const matchesHostingTarget = (only: string | undefined, target?: string): boolean => { + if (!only) return true; + if (!only.includes("hosting:")) return true; + const targetStr = `hosting:${target ?? ""}`; + return only.split(",").some((t) => t === targetStr); +}; + +export const prepareFrameworksIfNeeded = async function ( + targetNames: (keyof typeof TARGETS)[], + options: DeployOptions, + context: FrameworkContext, +): Promise { + const config = options.config.get("hosting") as HostingConfig; + if ( + Array.isArray(config) + ? config.some((it) => it.source && matchesHostingTarget(options.only, it.target)) + : config.source + ) { + experiments.assertEnabled("webframeworks", "deploy a web framework from source"); + await prepareFrameworks("deploy", targetNames, context, options); + } +}; + /** * The `deploy()` function runs through a three step deploy process for a listed * number of deploy targets. This allows deploys to be done all together or @@ -59,7 +84,7 @@ export const deploy = async function ( const projectId = needProjectId(options); const payload = {}; // a shared context object for deploy targets to decorate as needed - const context: any = Object.assign({ projectId }, customContext); + const context: Context = Object.assign({ projectId }, customContext); const predeploys: Chain = []; const prepares: Chain = []; const deploys: Chain = []; @@ -68,11 +93,7 @@ export const deploy = async function ( const startTime = Date.now(); if (targetNames.includes("hosting")) { - const config = options.config.get("hosting"); - if (Array.isArray(config) ? config.some((it) => it.source) : config.source) { - experiments.assertEnabled("webframeworks", "deploy a web framework from source"); - await prepareFrameworks("deploy", targetNames, context, options); - } + await prepareFrameworksIfNeeded(targetNames, options, context); } if (targetNames.includes("hosting") && hasPinnedFunctions(options)) { @@ -148,11 +169,11 @@ export const deploy = async function ( const deployedHosting = includes(targetNames, "hosting"); logger.info(bold("Project Console:"), consoleUrl(options.project ?? "_", "/overview")); if (deployedHosting) { - each(context.hosting.deploys as HostingDeploy[], (deploy) => { + each(context.hosting?.deploys, (deploy) => { logger.info(bold("Hosting URL:"), addSubdomain(hostingOrigin(), deploy.config.site)); }); - const versionNames = context.hosting.deploys.map((deploy: any) => deploy.version); - return { hosting: versionNames.length === 1 ? versionNames[0] : versionNames }; + const versionNames = context.hosting?.deploys.map((deploy: any) => deploy.version); + return { hosting: versionNames?.length === 1 ? versionNames[0] : versionNames }; } else { return { hosting: undefined }; }