Skip to content

Commit

Permalink
Add log lines to clarify what pinning is doing (#5787)
Browse files Browse the repository at this point in the history
Co-authored-by: joehan <joehanley@google.com>
  • Loading branch information
inlined and joehan authored May 3, 2023
1 parent f44b1da commit 25613a6
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 7 deletions.
35 changes: 30 additions & 5 deletions src/deploy/hosting/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { FirebaseError } from "../../error";
import * as api from "../../hosting/api";
import * as config from "../../hosting/config";
import * as deploymentTool from "../../deploymentTool";
import * as clc from "colorette";
import { Context } from "./context";
import { Options } from "../../options";
import { HostingOptions } from "../../hosting/options";
import { assertExhaustive, zipIn } from "../../functional";
import { track } from "../../track";
import * as utils from "../../utils";
import { HostingSource } from "../../firebaseConfig";
import { HostingSource, RunRewrite } from "../../firebaseConfig";
import * as backend from "../functions/backend";
import { ensureTargeted } from "../../functions/ensureTargeted";

Expand Down Expand Up @@ -60,8 +61,9 @@ export async function addPinnedFunctionsToOnlyString(
// a scalar to an array now
handlePublicDirectoryFlag(options);

let addedFunctions = false;
const addedFunctions: string[] = [];
for (const c of config.hostingConfig(options)) {
const addedFunctionsPerSite: string[] = [];
for (const r of c.rewrites || []) {
if (!("function" in r) || typeof r.function !== "object" || !r.function.pinTag) {
continue;
Expand All @@ -76,10 +78,19 @@ export async function addPinnedFunctionsToOnlyString(
// This endpoint is just being added in this push. We don't know what codebase it is.
options.only = ensureTargeted(options.only, r.function.functionId);
}
addedFunctions = true;
addedFunctionsPerSite.push(r.function.functionId);
}
if (addedFunctionsPerSite.length) {
utils.logLabeledBullet(
"hosting",
"The following function(s) are pinned to site " +
`${clc.bold(c.site)} and will be deployed as well: ` +
addedFunctionsPerSite.map(clc.bold).join(",")
);
addedFunctions.push(...addedFunctionsPerSite);
}
}
return addedFunctions;
return addedFunctions.length !== 0;
}

/**
Expand All @@ -103,10 +114,24 @@ export async function prepare(context: Context, options: HostingOptions & Option
}
const unsafe = await unsafePins(context, config);
if (unsafe.length) {
const msg = `Cannot deploy site ${config.site} to channel ${context.hostingChannel} because it would modify one or more rewrites in "live" that are not pinned, breaking production. Please pin "live" before pinning other channels.`;
const msg =
`Cannot deploy site ${clc.bold(config.site)} to channel ` +
`${clc.bold(context.hostingChannel!)} because it would modify one or ` +
`more rewrites in "live" that are not pinned, breaking production. ` +
`Please pin "live" before pinning other channels.`;
utils.logLabeledError("Hosting", msg);
throw new Error(msg);
}
const runPins = config.rewrites
?.filter((r) => "run" in r && r.run.pinTag)
?.map((r) => (r as RunRewrite).run.serviceId);
if (runPins?.length) {
utils.logLabeledBullet(
"hosting",
`The site ${clc.bold(config.site)} will pin rewrites to the current ` +
`latest revision of service(s) ${runPins.map(clc.bold).join(",")}`
);
}
const version: Omit<api.Version, api.VERSION_OUTPUT_FIELDS> = {
status: "CREATED",
labels,
Expand Down
26 changes: 24 additions & 2 deletions src/test/deploy/hosting/prepare.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expect } from "chai";
import * as sinon from "sinon";
import * as clc from "colorette";

import { FirebaseConfig } from "../../../firebaseConfig";
import { HostingOptions } from "../../../hosting/options";
Expand All @@ -9,6 +10,7 @@ import * as hostingApi from "../../../hosting/api";
import * as tracking from "../../../track";
import * as deploymentTool from "../../../deploymentTool";
import * as config from "../../../hosting/config";
import * as utils from "../../../utils";
import {
addPinnedFunctionsToOnlyString,
hasPinnedFunctions,
Expand All @@ -22,6 +24,7 @@ describe("hosting prepare", () => {
let hostingStub: sinon.SinonStubbedInstance<typeof hostingApi>;
let trackingStub: sinon.SinonStubbedInstance<typeof tracking>;
let backendStub: sinon.SinonStubbedInstance<typeof backend>;
let loggerStub: sinon.SinonStub;
let siteConfig: config.HostingResolved;
let firebaseJson: FirebaseConfig;
let options: HostingOptions & Options;
Expand All @@ -30,13 +33,21 @@ describe("hosting prepare", () => {
hostingStub = sinon.stub(hostingApi);
trackingStub = sinon.stub(tracking);
backendStub = sinon.stub(backend);
loggerStub = sinon.stub(utils, "logLabeledBullet");

// We're intentionally using pointer references so that editing site
// edits the results of hostingConfig() and changes firebase.json
siteConfig = {
site: "site",
public: ".",
rewrites: [
{
glob: "run",
run: {
serviceId: "service",
pinTag: true,
},
},
{
glob: "**",
function: {
Expand Down Expand Up @@ -103,6 +114,11 @@ describe("hosting prepare", () => {
},
],
});
expect(loggerStub).to.have.been.calledWith(
"hosting",
`The site ${clc.bold("site")} will pin rewrites to the current latest ` +
`revision of service(s) ${clc.bold("service")}`
);
});

it("passes a smoke test without web framework", async () => {
Expand Down Expand Up @@ -254,10 +270,11 @@ describe("hosting prepare", () => {

it("detects a lack of function tags", () => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
delete (options as any).config.src.hosting?.rewrites?.[0]?.function?.pinTag;
delete (options as any).config.src.hosting?.rewrites?.[1]?.function?.pinTag;
expect(hasPinnedFunctions(options)).to.be.false;
});
});

describe("addPinnedFunctionsToOnlyString", () => {
it("adds functions to deploy targets w/ codebases", async () => {
backendStub.existingBackend.resolves({
Expand All @@ -276,6 +293,11 @@ describe("hosting prepare", () => {

await expect(addPinnedFunctionsToOnlyString({} as any, options)).to.eventually.be.true;
expect(options.only).to.equal("hosting,functions:backend:function");
expect(loggerStub).to.have.been.calledWith(
"hosting",
`The following function(s) are pinned to site ${clc.bold("site")} ` +
`and will be deployed as well: ${clc.bold("function")}`
);
});

it("adds functions to deploy targets w/o codebases", async () => {
Expand All @@ -298,7 +320,7 @@ describe("hosting prepare", () => {

it("doesn't add untagged functions", async () => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
delete (siteConfig as any).rewrites[0].function.pinTag;
delete (siteConfig as any).rewrites[1].function.pinTag;

await expect(addPinnedFunctionsToOnlyString({} as any, options)).to.eventually.be.false;
expect(options.only).to.equal("hosting");
Expand Down

0 comments on commit 25613a6

Please sign in to comment.