Skip to content
Open
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Add `MCP-Protocol-Version`, `Mcp-Method`, and `Mcp-Name` HTTP headers to `OneMcpServer` requests per the MCP 0728 standard release candidate (https://blog.modelcontextprotocol.io/posts/2026-07-28-release-candidate/ and https://modelcontextprotocol.io/seps/2243-http-standardization).
- Fixes Storage Emulator to support JSON uploads larger than 100KB without hanging or throwing 413 error (#8355)
- `firebase deploy --only functions` now fails fast with an actionable error if a 1st gen function relies on a missing or disabled default App Engine service account, instead of surfacing a generic "internal error" after several retries (#4524)
- Add `extdeprecationwarnings` experiment to display phased deprecation notices and guidance across `ext:*` CLI commands.
164 changes: 164 additions & 0 deletions src/deploy/functions/checkIam.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import * as sinon from "sinon";
import * as checkIam from "./checkIam";
import * as storage from "../../gcp/storage";
import * as rm from "../../gcp/resourceManager";
import * as iam from "../../gcp/iam";
import * as backend from "./backend";
import * as args from "./args";
import { Options } from "../../options";
import { FirebaseError } from "../../error";

const projectId = "my-project";
const projectNumber = "123456789";
Expand Down Expand Up @@ -75,6 +79,166 @@ describe("checkIam", () => {
});
});

describe("checkServiceAccountIam", () => {
let getServiceAccountStub: sinon.SinonStub;
let testResourceIamPermissionsStub: sinon.SinonStub;

beforeEach(() => {
getServiceAccountStub = sinon
.stub(iam, "getServiceAccount")
.throws("unexpected call to iam.getServiceAccount");
testResourceIamPermissionsStub = sinon.stub(iam, "testResourceIamPermissions").resolves({
allowed: ["iam.serviceAccounts.actAs"],
missing: [],
passed: true,
});
});

afterEach(() => {
getServiceAccountStub.restore();
testResourceIamPermissionsStub.restore();
});

it("should pass when actAs is granted", async () => {
await expect(checkIam.checkServiceAccountIam(projectId)).to.not.be.rejected;
});

it("should throw if actAs permission is missing", async () => {
testResourceIamPermissionsStub.resolves({
allowed: [],
missing: ["iam.serviceAccounts.actAs"],
passed: false,
});

await expect(checkIam.checkServiceAccountIam(projectId)).to.be.rejectedWith(
FirebaseError,
/Missing permissions required for functions deploy/,
);
});
});

describe("checkDefaultServiceAccountEnabled", () => {
let getServiceAccountStub: sinon.SinonStub;
const saEmail = `${projectId}@appspot.gserviceaccount.com`;
const context: args.Context = { projectId };
const options = {} as Options;

const gcfv1EndpointWithDefaultSA: backend.Endpoint = {
id: "gcfv1fn",
entryPoint: "gcfv1fn",
platform: "gcfv1",
httpsTrigger: {},
...SPEC,
};
const gcfv1EndpointWithCustomSA: backend.Endpoint = {
id: "gcfv1fnCustomSA",
entryPoint: "gcfv1fnCustomSA",
platform: "gcfv1",
serviceAccount: "custom@my-project.iam.gserviceaccount.com",
httpsTrigger: {},
...SPEC,
};
const gcfv2Endpoint: backend.Endpoint = {
id: "gcfv2fn",
entryPoint: "gcfv2fn",
platform: "gcfv2",
httpsTrigger: {},
...SPEC,
};

function payloadFor(...endpoints: backend.Endpoint[]): args.Payload {
return {
functions: {
codebase: {
wantBackend: backend.of(...endpoints),
haveBackend: backend.empty(),
},
},
};
}

beforeEach(() => {
getServiceAccountStub = sinon.stub(iam, "getServiceAccount");
});

afterEach(() => {
getServiceAccountStub.restore();
});

it("should not look up the service account when there are no 1st gen endpoints", async () => {
await expect(
checkIam.checkDefaultServiceAccountEnabled(context, options, payloadFor(gcfv2Endpoint)),
).to.not.be.rejected;
expect(getServiceAccountStub).to.not.have.been.called;
});

it("should not look up the service account when the 1st gen endpoint has a custom service account", async () => {
await expect(
checkIam.checkDefaultServiceAccountEnabled(
context,
options,
payloadFor(gcfv1EndpointWithCustomSA),
),
).to.not.be.rejected;
expect(getServiceAccountStub).to.not.have.been.called;
});

it("should pass if the default service account exists and is enabled", async () => {
getServiceAccountStub.resolves({ disabled: false } as iam.ServiceAccount);

await expect(
checkIam.checkDefaultServiceAccountEnabled(
context,
options,
payloadFor(gcfv1EndpointWithDefaultSA),
),
).to.not.be.rejected;
expect(getServiceAccountStub).to.have.been.calledWith(projectId, saEmail);
});

it("should throw a helpful error if the default service account is disabled", async () => {
getServiceAccountStub.resolves({ disabled: true } as iam.ServiceAccount);

await expect(
checkIam.checkDefaultServiceAccountEnabled(
context,
options,
payloadFor(gcfv1EndpointWithDefaultSA),
),
).to.be.rejectedWith(
FirebaseError,
new RegExp(
`${saEmail}.*is disabled[\\s\\S]*` +
`https://console.cloud.google.com/iam-admin/serviceaccounts\\?project=${projectId}`,
),
);
});

it("should throw a helpful error if the default service account does not exist", async () => {
getServiceAccountStub.rejects({ status: 404 });

await expect(
checkIam.checkDefaultServiceAccountEnabled(
context,
options,
payloadFor(gcfv1EndpointWithDefaultSA),
),
).to.be.rejectedWith(FirebaseError, new RegExp(`${saEmail}.*does not exist`));
});

it("should fail open if the lookup errors for a reason other than a missing account", async () => {
getServiceAccountStub.rejects({ status: 403 });

await expect(
checkIam.checkDefaultServiceAccountEnabled(
context,
options,
payloadFor(gcfv1EndpointWithDefaultSA),
),
).to.not.be.rejected;
});
});

describe("ensureServiceAgentRoles", () => {
it("should return early if we do not have new services", async () => {
const v1EventFn: backend.Endpoint = {
Expand Down
66 changes: 65 additions & 1 deletion src/deploy/functions/checkIam.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { logger } from "../../logger";
import { getEndpointFilters, endpointMatchesAnyFilter } from "./functionsDeployHelper";
import { FirebaseError } from "../../error";
import { FirebaseError, getErrStatus } from "../../error";
import { Options } from "../../options";
import { flattenArray } from "../../functional";
import * as iam from "../../gcp/iam";
Expand Down Expand Up @@ -41,7 +41,7 @@
["iam.serviceAccounts.actAs"],
);
passed = iamResult.passed;
} catch (err: any) {

Check warning on line 44 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
logger.debug("[functions] service account IAM check errored, deploy may fail:", err);
// we want to fail this check open and not rethrow since it's informational only
return;
Expand Down Expand Up @@ -73,7 +73,7 @@
if (!payload.functions) {
return;
}
const filters = context.filters || getEndpointFilters(options, context.config!);

Check warning on line 76 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Forbidden non-null assertion
const wantBackends = Object.values(payload.functions).map(({ wantBackend }) => wantBackend);
const httpEndpoints = [...flattenArray(wantBackends.map((b) => backend.allEndpoints(b)))]
.filter((f) => backend.isHttpsTriggered(f) || backend.isDataConnectGraphqlTriggered(f))
Expand All @@ -99,7 +99,7 @@
try {
const iamResult = await iam.testIamPermissions(context.projectId, [PERMISSION]);
passed = iamResult.passed;
} catch (e: any) {

Check warning on line 102 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
logger.debug(
"[functions] failed http create setIamPolicy permission check. deploy may fail:",
e,
Expand All @@ -126,13 +126,77 @@
logger.debug("[functions] found setIamPolicy permission, proceeding with deploy");
}

/**
* Checks that the default App Engine service account exists and is enabled, for codebases
* that will rely on it. Only 1st gen functions without an explicit `serviceAccount` fall back
* to the App Engine default service account (`<projectId>@appspot.gserviceaccount.com`) as
* their runtime identity; 2nd gen functions use the Compute Engine default service account
* instead, so this check is a no-op unless such a 1st gen endpoint is present.
*
* A missing or disabled service account otherwise causes the deploy to fail deep in the
* build/run pipeline with an opaque, generic error, so we surface an actionable error here
* instead.
* @param context The deploy context.
* @param options The command-wide options object.
* @param payload The deploy payload.
*/
export async function checkDefaultServiceAccountEnabled(
context: args.Context,
options: Options,
payload: args.Payload,
): Promise<void> {
if (!payload.functions) {
return;
}
const filters = context.filters || getEndpointFilters(options, context.config!);

Check warning on line 151 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Forbidden non-null assertion
const wantBackends = Object.values(payload.functions).map(({ wantBackend }) => wantBackend);
const usesDefaultServiceAccount = [
...flattenArray(wantBackends.map((b) => backend.allEndpoints(b))),
]
Comment thread
IzaakGough marked this conversation as resolved.
.filter((f) => endpointMatchesAnyFilter(f, filters))
.some((f) => f.platform === "gcfv1" && !f.serviceAccount);

if (!usesDefaultServiceAccount) {
return;
}

const projectId = context.projectId;
const saEmail = `${projectId}@appspot.gserviceaccount.com`;
let serviceAccount: iam.ServiceAccount;
try {
serviceAccount = await iam.getServiceAccount(projectId, saEmail);
} catch (err: unknown) {
if (getErrStatus(err) === 404) {
throw new FirebaseError(
`The default App Engine service account ${bold(saEmail)} does not exist.\n\n` +
`To successfully deploy 1st gen functions, this service account is required. You can create it by enabling the App Engine Admin API here:\n\n` +
`https://console.cloud.google.com/apis/library/appengine.googleapis.com?project=${projectId}`,
);
}
logger.debug(
"[functions] failed to look up the default App Engine service account, deploy may fail:",
err,
);
// we want to fail this check open and not rethrow since it's informational only
return;
}

if (serviceAccount.disabled) {
throw new FirebaseError(
`The default App Engine service account ${bold(saEmail)} is disabled.\n\n` +
`To successfully deploy 1st gen functions, this service account must be enabled. Please enable it here:\n\n` +
`https://console.cloud.google.com/iam-admin/serviceaccounts?project=${projectId}`,
);
}
}

/** obtain the pubsub service agent */
function getPubsubServiceAgent(projectNumber: string): string {
return `service-${projectNumber}@gcp-sa-pubsub.iam.gserviceaccount.com`;
}

/** Callback reducer function */
function reduceEventsToServices(services: Array<Service>, endpoint: backend.Endpoint) {

Check warning on line 199 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Missing return type on function
const service = serviceForEndpoint(endpoint);
if (service.requiredProjectBindings && !services.find((s) => s.name === service.name)) {
services.push(service);
Expand All @@ -151,7 +215,7 @@
* Finds the required project level IAM bindings for the Pub/Sub service agent.
* If the user enabled Pub/Sub on or before April 8, 2021, then we must enable the token creator role.
* @param projectNumber project number
* @param existingPolicy the project level IAM policy

Check warning on line 218 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

@param "existingPolicy" does not match an existing function parameter
*/
export function obtainPubSubServiceAgentBindings(projectNumber: string): iam.Binding[] {
const serviceAccountTokenCreatorBinding: iam.Binding = {
Expand All @@ -165,7 +229,7 @@
* Finds the required project level IAM bindings for the default compute service agent.
* Before a user creates an EventArc trigger, this agent must be granted the invoker and event receiver roles.
* @param projectNumber project number
* @param existingPolicy the project level IAM policy

Check warning on line 232 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

@param "existingPolicy" does not match an existing function parameter
*/
export async function obtainDefaultComputeServiceAgentBindings(
projectNumber: string,
Expand Down Expand Up @@ -256,7 +320,7 @@
// obtain all the bindings we need to have active in the project
const requiredBindingsPromises: Array<Promise<Array<iam.Binding>>> = [];
for (const service of newServices) {
requiredBindingsPromises.push(service.requiredProjectBindings!(projectNumber));

Check warning on line 323 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Forbidden non-null assertion
}
const nestedRequiredBindings = await Promise.all(requiredBindingsPromises);
const requiredBindings = [...flattenArray(nestedRequiredBindings)];
Expand Down Expand Up @@ -287,7 +351,7 @@
let policy: iam.Policy;
try {
policy = await getIamPolicy(projectNumber);
} catch (err: any) {

Check warning on line 354 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
iam.printManualIamConfig(requiredBindings, projectId, "functions");
utils.logLabeledBullet(
"functions",
Expand All @@ -307,7 +371,7 @@
try {
if (dryRun) {
logger.info(
`On your next deploy, the following required roles will be granted: ${requiredBindings.map(

Check warning on line 374 in src/deploy/functions/checkIam.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Invalid type "string[]" of template literal expression
(b) => `${b.members.join(", ")}: ${bold(b.role)}`,
)}`,
);
Expand Down
7 changes: 5 additions & 2 deletions src/deploy/functions/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { setGracefulCleanup } from "tmp";
import * as clc from "colorette";
import * as fs from "fs";

import { checkHttpIam } from "./checkIam";
import { checkHttpIam, checkDefaultServiceAccountEnabled } from "./checkIam";
import { logLabeledWarning, logLabeledSuccess, logWarning } from "../../utils";
import { Options } from "../../options";
import { configForCodebase } from "../../functions/projectConfig";
Expand Down Expand Up @@ -188,7 +188,10 @@ export async function deploy(

// Deploy functions
if (payload.functions && context.config) {
await checkHttpIam(context, options, payload);
await Promise.all([
checkDefaultServiceAccountEnabled(context, options, payload),
checkHttpIam(context, options, payload),
]);
const uploads: Promise<void>[] = [];
for (const [codebase, { wantBackend, haveBackend }] of Object.entries(payload.functions)) {
if (shouldUploadBeSkipped(context, wantBackend, haveBackend)) {
Expand Down
Loading