Skip to content

Commit

Permalink
Allow specifying the region for Hosting functions rewrites, defaultin…
Browse files Browse the repository at this point in the history 
…g to us-central1 if not provided (#3388)

* Add region option to Hosting functions rewrites, defaulting to us-central1 if not provided

Co-authored-by: Bryan Kendall <[email protected]>
  • Loading branch information
@kmcnellis @bkendall
kmcnellis and bkendall authored Feb 16, 2022
1 parent 0a10d10 commit 02bcc04
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 27 deletions.
27 changes: 27 additions & 0 deletions schema/firebase-config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,9 @@
},
"glob": {
"type": "string"
},
"region": {
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -696,6 +699,9 @@
"function": {
"type": "string"
},
"region": {
"type": "string"
},
"source": {
"type": "string"
}
Expand Down Expand Up @@ -774,6 +780,9 @@
},
"regex": {
"type": "string"
},
"region": {
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -1095,6 +1104,9 @@
},
"glob": {
"type": "string"
},
"region": {
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -1169,6 +1181,9 @@
"function": {
"type": "string"
},
"region": {
"type": "string"
},
"source": {
"type": "string"
}
Expand Down Expand Up @@ -1247,6 +1262,9 @@
},
"regex": {
"type": "string"
},
"region": {
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -1568,6 +1586,9 @@
},
"glob": {
"type": "string"
},
"region": {
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -1642,6 +1663,9 @@
"function": {
"type": "string"
},
"region": {
"type": "string"
},
"source": {
"type": "string"
}
Expand Down Expand Up @@ -1720,6 +1744,9 @@
},
"regex": {
"type": "string"
},
"region": {
"type": "string"
}
},
"required": [
Expand Down
5 changes: 5 additions & 0 deletions src/deploy/hosting/convertConfig.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export function convertConfig(config?: HostingConfig): { [k: string]: any } {
vRewrite.path = rewrite.destination;
} else if ("function" in rewrite) {
vRewrite.function = rewrite.function;
if (rewrite.region) {
vRewrite.functionRegion = rewrite.region;
} else {
vRewrite.functionRegion = "us-central1";
}
} else if ("dynamicLinks" in rewrite) {
vRewrite.dynamicLinks = rewrite.dynamicLinks;
} else if ("run" in rewrite) {
Expand Down
2 changes: 1 addition & 1 deletion src/firebaseConfig.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type HostingRedirects = HostingSource & {
export type HostingRewrites = HostingSource &
(
| { destination: string }
| { function: string }
| { function: string; region?: string }
| {
run: {
serviceId: string;
Expand Down
30 changes: 18 additions & 12 deletions src/hosting/functionsProxy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,35 @@ import { needProjectId } from "../projectUtils";
import { EmulatorRegistry } from "../emulator/registry";
import { Emulators } from "../emulator/types";
import { FunctionsEmulator } from "../emulator/functionsEmulator";
import { HostingRewrites } from "../firebaseConfig";
import { FirebaseError } from "../error";

export interface FunctionsProxyOptions {
port: number;
project?: string;
targets: string[];
}

export interface FunctionProxyRewrite {
function: string;
}

/**
* Returns a function which, given a FunctionProxyRewrite, returns a Promise
* that resolves with a middleware-like function that proxies the request to a
* hosted or live function.
*/
export default function (
export function functionsProxy(
options: FunctionsProxyOptions
): (r: FunctionProxyRewrite) => Promise<RequestHandler> {
return (rewrite: FunctionProxyRewrite) => {
): (r: HostingRewrites) => Promise<RequestHandler> {
return (rewrite: HostingRewrites) => {
return new Promise((resolve) => {
// TODO(samstern): This proxy assumes all functions are in the default region, but this is
// not a safe assumption.
const projectId = needProjectId(options);
let url = `https://us-central1-${projectId}.cloudfunctions.net/${rewrite.function}`;
if (!("function" in rewrite)) {
throw new FirebaseError(`A non-function rewrite cannot be used in functionsProxy`, {
exit: 2,
});
}
if (!rewrite.region) {
rewrite.region = "us-central1";
}
let url = `https://${rewrite.region}-${projectId}.cloudfunctions.net/${rewrite.function}`;
let destLabel = "live";

if (includes(options.targets, "functions")) {
Expand All @@ -45,12 +49,14 @@ export default function (
functionsEmu.getInfo().port,
projectId,
rewrite.function,
"us-central1"
rewrite.region
);
}
}

resolve(proxyRequestHandler(url, `${destLabel} Function ${rewrite.function}`));
resolve(
proxyRequestHandler(url, `${destLabel} Function ${rewrite.region}/${rewrite.function}`)
);
});
};
}
2 changes: 1 addition & 1 deletion src/serve/hosting.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { implicitInit, TemplateServerResponse } from "../hosting/implicitInit";
import { initMiddleware } from "../hosting/initMiddleware";
import { normalizedHostingConfigs } from "../hosting/normalizedHostingConfigs";
import cloudRunProxy from "../hosting/cloudRunProxy";
import functionsProxy from "../hosting/functionsProxy";
import { functionsProxy } from "../hosting/functionsProxy";
import { NextFunction, Request, Response } from "express";
import { Writable } from "stream";
import { EmulatorLogger } from "../emulator/emulatorLogger";
Expand Down
13 changes: 9 additions & 4 deletions src/test/deploy/hosting/convertConfig.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,19 @@ describe("convertConfig", () => {
want: { rewrites: [{ glob: "/foo$", path: "https://example.com" }] },
},
{
name: "returns rewrites for glob CF3",
name: "defaults to us-central1 for CF3",
input: { rewrites: [{ glob: "/foo", function: "foofn" }] },
want: { rewrites: [{ glob: "/foo", function: "foofn" }] },
want: { rewrites: [{ glob: "/foo", function: "foofn", functionRegion: "us-central1" }] },
},
{
name: "returns rewrites for glob CF3",
input: { rewrites: [{ glob: "/foo", function: "foofn", region: "europe-west2" }] },
want: { rewrites: [{ glob: "/foo", function: "foofn", functionRegion: "europe-west2" }] },
},
{
name: "returns rewrites for regex CF3",
input: { rewrites: [{ regex: "/foo$", function: "foofn" }] },
want: { rewrites: [{ regex: "/foo$", function: "foofn" }] },
input: { rewrites: [{ regex: "/foo$", function: "foofn", region: "us-central1" }] },
want: { rewrites: [{ regex: "/foo$", function: "foofn", functionRegion: "us-central1" }] },
},
{
name: "returns rewrites for glob Run",
Expand Down
4 changes: 3 additions & 1 deletion src/test/emulators/emulatorServer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import { EmulatorRegistry } from "../../emulator/registry";
import { expect } from "chai";
import { FakeEmulator } from "./fakeEmulator";
import { EmulatorServer } from "../../emulator/emulatorServer";
import { findAvailablePort } from "../../emulator/portUtils";

describe("EmulatorServer", () => {
it("should correctly start and stop an emulator", async () => {
const name = Emulators.FUNCTIONS;
const emulator = new FakeEmulator(name, "localhost", 5000);
const port = await findAvailablePort("localhost", 5000);
const emulator = new FakeEmulator(name, "localhost", port);
const server = new EmulatorServer(emulator);

await server.start();
Expand Down
9 changes: 6 additions & 3 deletions src/test/emulators/registry.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ALL_EMULATORS, Emulators } from "../../emulator/types";
import { EmulatorRegistry } from "../../emulator/registry";
import { expect } from "chai";
import { FakeEmulator } from "./fakeEmulator";
import { findAvailablePort } from "../../emulator/portUtils";

describe("EmulatorRegistry", () => {
afterEach(async () => {
Expand All @@ -18,7 +19,8 @@ describe("EmulatorRegistry", () => {

it("should correctly return information about a running emulator", async () => {
const name = Emulators.FUNCTIONS;
const emu = new FakeEmulator(name, "localhost", 5000);
const port = await findAvailablePort("localhost", 5000);
const emu = new FakeEmulator(name, "localhost", port);

expect(EmulatorRegistry.isRunning(name)).to.be.false;

Expand All @@ -27,12 +29,13 @@ describe("EmulatorRegistry", () => {
expect(EmulatorRegistry.isRunning(name)).to.be.true;
expect(EmulatorRegistry.listRunning()).to.eql([name]);
expect(EmulatorRegistry.get(name)).to.eql(emu);
expect(EmulatorRegistry.getPort(name)).to.eql(5000);
expect(EmulatorRegistry.getPort(name)).to.eql(port);
});

it("once stopped, an emulator is no longer running", async () => {
const name = Emulators.FUNCTIONS;
const emu = new FakeEmulator(name, "localhost", 5000);
const port = await findAvailablePort("localhost", 5000);
const emu = new FakeEmulator(name, "localhost", port);

expect(EmulatorRegistry.isRunning(name)).to.be.false;
await EmulatorRegistry.start(emu);
Expand Down
7 changes: 7 additions & 0 deletions src/test/fixtures/valid-config/firebase.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
"type" : 302
} ],
"rewrites": [ {
"source": "/region",
"function": "/foobar",
"region": "us-central1"
}, {
"source": "/default",
"function": "/foobar"
}, {
"source": "**",
"destination": "/index.html"
} ],
Expand Down
47 changes: 42 additions & 5 deletions src/test/hosting/functionsProxy.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ import * as nock from "nock";
import * as sinon from "sinon";
import * as supertest from "supertest";

import functionsProxy, {
FunctionProxyRewrite,
FunctionsProxyOptions,
} from "../../hosting/functionsProxy";
import { functionsProxy, FunctionsProxyOptions } from "../../hosting/functionsProxy";
import { EmulatorRegistry } from "../../emulator/registry";
import { Emulators } from "../../emulator/types";
import { FakeEmulator } from "../emulators/fakeEmulator";
import { HostingRewrites } from "../../firebaseConfig";

describe("functionsProxy", () => {
const fakeOptions: FunctionsProxyOptions = {
Expand All @@ -20,7 +18,11 @@ describe("functionsProxy", () => {
targets: [],
};

const fakeRewrite: FunctionProxyRewrite = { function: "bar" };
const fakeRewrite = { function: "bar", region: "us-central1" } as HostingRewrites;
const fakeRewriteEurope = {
function: "bar",
region: "europe-west3",
} as HostingRewrites;

beforeEach(async () => {
const fakeFunctionsEmulator = new FakeEmulator(Emulators.FUNCTIONS, "localhost", 7778);
Expand Down Expand Up @@ -49,6 +51,23 @@ describe("functionsProxy", () => {
});
});

it("should resolve a function returns middleware that proxies to the live version in another region", async () => {
nock("https://europe-west3-project-foo.cloudfunctions.net")
.get("/bar/")
.reply(200, "live version");

const mwGenerator = functionsProxy(fakeOptions);
const mw = await mwGenerator(fakeRewriteEurope);
const spyMw = sinon.spy(mw);

return supertest(spyMw)
.get("/")
.expect(200, "live version")
.then(() => {
expect(spyMw.calledOnce).to.be.true;
});
});

it("should resolve a function that returns middleware that proxies to a local version", async () => {
nock("http://localhost:7778").get("/project-foo/us-central1/bar/").reply(200, "local version");

Expand All @@ -67,6 +86,24 @@ describe("functionsProxy", () => {
});
});

it("should resolve a function that returns middleware that proxies to a local version in another region", async () => {
nock("http://localhost:7778").get("/project-foo/europe-west3/bar/").reply(200, "local version");

const options = cloneDeep(fakeOptions);
options.targets = ["functions"];

const mwGenerator = functionsProxy(options);
const mw = await mwGenerator(fakeRewriteEurope);
const spyMw = sinon.spy(mw);

return supertest(spyMw)
.get("/")
.expect(200, "local version")
.then(() => {
expect(spyMw.calledOnce).to.be.true;
});
});

it("should maintain the location header as returned by the function", async () => {
nock("http://localhost:7778")
.get("/project-foo/us-central1/bar/")
Expand Down

0 comments on commit 02bcc04

Please sign in to comment.