Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
- Fix demo projects + web frameworks with emulators (#6737)
- Fix Next.js static routes with server actions (#6664)
- Fixed an issue where `GOOGLE_CLOUD_QUOTA_PROJECT` was not correctly respected. (#6801)
- Make VPC egress settings in functions parameterizeable (#6843)
7 changes: 7 additions & 0 deletions src/deploy/functions/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,13 @@ export function isValidMemoryOption(mem: unknown): mem is MemoryOptions {
return allMemoryOptions.includes(mem as MemoryOptions);
}

/**
* Is a given string a valid VpcEgressSettings?
*/
export function isValidEgressSetting(egress: unknown): egress is VpcEgressSettings {
return egress === "PRIVATE_RANGES_ONLY" || egress === "ALL_TRAFFIC";
}

/** Returns a human-readable name with MB or GB suffix for a MemoryOption (MB). */
export function memoryOptionDisplayName(option: MemoryOptions): string {
return {
Expand Down
19 changes: 11 additions & 8 deletions src/deploy/functions/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export function isBlockingTriggered(triggered: Triggered): triggered is Blocking

export interface VpcSettings {
connector: string | Expression<string>;
egressSettings?: "PRIVATE_RANGES_ONLY" | "ALL_TRAFFIC" | null;
egressSettings?: "PRIVATE_RANGES_ONLY" | "ALL_TRAFFIC" | Expression<string> | null;
}

export interface SecretEnvVar {
Expand All @@ -204,12 +204,6 @@ export interface SecretEnvVar {

export type MemoryOption = 128 | 256 | 512 | 1024 | 2048 | 4096 | 8192 | 16384 | 32768;
const allMemoryOptions: MemoryOption[] = [128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768];
/**
* Is a given number a valid MemoryOption?
*/
export function isValidMemoryOption(mem: unknown): mem is MemoryOption {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird we had two of these...

return allMemoryOptions.includes(mem as MemoryOption);
}

export type FunctionsPlatform = backend.FunctionsPlatform;
export const AllFunctionsPlatforms: FunctionsPlatform[] = ["gcfv1", "gcfv2"];
Expand Down Expand Up @@ -514,7 +508,16 @@ export function toBackend(
}

bkEndpoint.vpc = { connector: bdEndpoint.vpc.connector };
proto.copyIfPresent(bkEndpoint.vpc, bdEndpoint.vpc, "egressSettings");
if (bdEndpoint.vpc.egressSettings) {
const egressSettings = r.resolveString(bdEndpoint.vpc.egressSettings);
if (!backend.isValidEgressSetting(egressSettings)) {
throw new FirebaseError(
`Value "${egressSettings}" is an invalid ` +
"egress setting. Valid values are PRIVATE_RANGES_ONLY and ALL_TRAFFIC",
);
}
bkEndpoint.vpc.egressSettings = egressSettings;
}
} else if (bdEndpoint.vpc === null) {
bkEndpoint.vpc = null;
}
Expand Down
3 changes: 2 additions & 1 deletion src/deploy/functions/runtimes/discovery/v1alpha1.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as build from "../../build";
import * as backend from "../../backend";
import * as params from "../../params";
import * as runtimes from "..";

Expand Down Expand Up @@ -126,7 +127,7 @@ function assertBuildEndpoint(ep: WireEndpoint, id: string): void {
platform: (platform) => build.AllFunctionsPlatforms.includes(platform),
entryPoint: "string",
omit: "Field<boolean>?",
availableMemoryMb: (mem) => mem === null || isCEL(mem) || build.isValidMemoryOption(mem),
availableMemoryMb: (mem) => mem === null || isCEL(mem) || backend.isValidMemoryOption(mem),
maxInstances: "Field<number>?",
minInstances: "Field<number>?",
concurrency: "Field<number>?",
Expand Down
33 changes: 31 additions & 2 deletions src/test/deploy/functions/build.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { expect } from "chai";
import * as build from "../../../deploy/functions/build";
import { ParamValue } from "../../../deploy/functions/params";
import { FirebaseError } from "../../../error";

describe("toBackend", () => {
it("populates backend info from Build", () => {
Expand Down Expand Up @@ -124,8 +125,8 @@ describe("toBackend", () => {
minInstances: "{{ params.mininstances }}",
serviceAccount: "{{ params.serviceaccount }}",
vpc: {
connector: "projects/project/locations/region/connectors/connector",
egressSettings: "PRIVATE_RANGES_ONLY",
connector: "{{ params.connector }}",
egressSettings: "{{ params.egressSettings }}",
},
ingressSettings: "ALLOW_ALL",
labels: {
Expand All @@ -140,6 +141,8 @@ describe("toBackend", () => {
maxinstances: new ParamValue("42", false, { number: true }),
mininstances: new ParamValue("1", false, { number: true }),
serviceaccount: new ParamValue("service-account-1@", false, { string: true }),
connector: new ParamValue("connector", false, { string: true }),
egressSettings: new ParamValue("ALL_TRAFFIC", false, { string: true }),
});
expect(Object.keys(backend.endpoints).length).to.equal(1);
const endpointDef = Object.values(backend.endpoints)[0];
Expand All @@ -154,6 +157,32 @@ describe("toBackend", () => {
expect(
"httpsTrigger" in endpointDef.func ? endpointDef.func.httpsTrigger.invoker : [],
).to.have.members(["service-account-2@", "service-account-3@"]);
expect(endpointDef.func.vpc?.connector).to.equal(
"projects/project/locations/us-central1/connectors/connector",
);
expect(endpointDef.func.vpc?.egressSettings).to.equal("ALL_TRAFFIC");
}
});

it("enforces enum correctness for VPC egress settings", () => {
const desiredBuild: build.Build = build.of({
func: {
platform: "gcfv2",
region: ["us-central1"],
project: "project",
runtime: "nodejs16",
entryPoint: "func",
vpc: {
connector: "connector",
egressSettings: "{{ params.egressSettings }}",
},
httpsTrigger: {},
},
});
expect(() => {
build.toBackend(desiredBuild, {
egressSettings: new ParamValue("INVALID", false, { string: true }),
});
}).to.throw(FirebaseError, /Value "INVALID" is an invalid egress setting./);
});
});