Repository navigation
Add firebase deploy --only run for remote Cloud Build source deploys - #11205
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for deploying Cloud Run services directly via the Firebase CLI under the 'direct_cloud_run' experiment, adding the 'run' deploy target, integration with 'firebase init', and comprehensive tests. The review feedback suggests several key improvements: handling expected user-facing errors by importing and throwing 'FirebaseError', avoiding non-null assertions on the main container template to prevent runtime errors, ensuring temporary local archives are cleaned up in a 'finally' block to avoid disk leaks, parallelizing service preparation using 'Promise.all' for better performance, and adding defensive checks for potentially undefined configuration properties.
Contributor
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for deploying Cloud Run services directly from local source using the Firebase CLI. It adds the new run deploy target, implements its prepare, deploy, and release phases, integrates it into the initialization flow, and adds comprehensive unit and end-to-end tests. The review feedback highlights several key improvements: adhering to strict null checks by avoiding the non-null assertion operator on the main container, cleaning up temporary local archive files after upload to prevent disk space leaks, verifying the existence of the deployed service URI before logging, and using npx mocha in the test script for better environment robustness.
falahat
force-pushed
the
bapi_deploy_run
branch
from
September 30, 2026 21:37
3ba264a to
1086ebe
Compare
Re-running init on a service that's already in firebase.json filled in the default ignore list when the entry didn't have one. Init doesn't ask about ignore, so it shouldn't change it: an existing entry now only gets the rootDir and region the user just chose. New services still get the default ignore list.
…ce ID and region Pressing Enter now keeps the root directory saved in firebase.json instead of resetting it to /. Cloud Run service IDs are only unique within a region, so entries now match on both; the same ID in another region gets its own entry.
…gions Init deploys with --only run:<id>, which matches that ID in every region. Init now also passes the service's region to deploy, which skips services in other regions. firebase deploy and --only run:<id> are unchanged.
…actuate Init already writes firebase.json after every feature is set up, so the write in actuate only printed "Wrote configuration info to firebase.json" a second time. Leaving the write to init also means a failed init leaves firebase.json untouched.
# Conflicts: # src/init/features/run.spec.ts # src/init/features/run.ts
scripts/run-deploy-tests ran the real CLI against a real project. Nothing ran it automatically (npm test only covers src/, and CI doesn't list it), and each run left Cloud Build images behind in Artifact Registry. The unit tests cover the logic, and live testing is done by hand.
# Conflicts: # src/deploy/run/util.spec.ts # src/deploy/run/util.ts # src/init/features/run.spec.ts # src/init/features/run.ts
falahat
commented
Oct 8, 2026
Service IDs are only unique within a region, so firebase.json can list the same ID in more than one region. - run:<serviceId> still deploys that ID in every region it's listed in, and now logs which services it matched when there's more than one. - run:<serviceId>:<region> deploys exactly one service. Init deploys the service it set up this way, which replaces the region it used to pass in the deploy context. - Per-service messages (uploading, building, deployed) include the region. - firebase.json can't list the same service ID and region twice.
- deploy.ts reads as four steps: upload the source, build it, prepare the revision, then create the service or roll out a new revision. The revision rules are a pure function, revisionTemplate, with one small test per rule. - Helpers that only deploy.ts uses (deployRevision, toAppHostingConfig, the rollout timeout) moved there from util.ts. util.ts now only picks services from firebase.json and checks whether a service exists. - Stop deleting the local source zip. It's a tmp file, which firebase deploy deletes when it exits; App Hosting relies on the same thing. - Check every service's region once, up front, and stop --only filtering at the first service that isn't in firebase.json. - Tests use realistic names, check only the fields they're about, and drop duplicate coverage. The release test has its own file. - The help text no longer mentions local builds or run:services:update, which later PRs add. - Clearer docstrings and tests for init run's actuate and upsertRunConfig.
falahat
commented
Oct 12, 2026
- Add mainContainer() so one place decides which container is the main one. - Resolve the sticky base image with an if statement instead of a ternary. - Build the buildpack request with an explicit if instead of a conditional spread. - Explain that svc.baseImage is already resolved by prepare. - Rename createService to createNewService.
Yuangwang
reviewed
Oct 12, 2026
|
|
||
| const cfg = toAppHostingConfig(config); | ||
| // The zip is a tmp file, so it's deleted when the CLI exits. | ||
| const archive = await createSourceDeployArchive( |
Contributor
There was a problem hiding this comment.
Non-blocking note: zipping path.join(options.config.projectDir, cfg.rootDir) works nicely when rootDir is a standalone app folder, though for Nx/Turborepo monorepos (nx.json / turbo.json at projectDir with root lockfiles / shared packages) Cloud Build won't have the workspace root files unless we archive projectDir + set GOOGLE_BUILDABLE. Fine to leave as a follow-up!
Contributor
Author
There was a problem hiding this comment.
oh good call, I'm going to fix this in a follow-up PR
Yuangwang
approved these changes
Oct 12, 2026
Yuangwang
left a comment
Yuangwang
left a comment
Contributor
There was a problem hiding this comment.
Just one comment but otherwise lgtm
falahat
added a commit
that referenced
this pull request
Oct 13, 2026
* Create a basic, initial schema for firebase.json * improve testing * Require region, remove serviceAccount, and register run in Config.KNOWN_PROPS * Add direct_cloud_run experiment flag * Update shared GCP clients (runv2, run, artifactregistry) * Add firebase init run interactive setup * Add firebase deploy --only run for remote Cloud Build source deploys * Add firebase run:services:update command * feat(run): add local build support and build environment variables * chore(apphosting): add TODO for bundle.yaml env vars in local builds * Rename direct_cloud_run experiment to directcloudrun * Use directcloudrun experiment and validate rootDir is a directory * Clean up temporary source archive, guard release URI, and use directcloudrun * Validate service region and check all matching configs in updateService * Trim baseImage input in init run * Avoid empty template.annotations when deploy message is unset * Clear localBuild in upsertRunConfig without unused destructuring * Simplify template.annotations cleanup when deploy message is unset * Add defensive optional chaining in run.ts and runv2.ts * Remove unused Deployable predeploy/postdeploy from RunSingle * Make local build scratch dir cleanup best-effort Route both scratch dir deletions through removeTempPath so a cleanup failure can't fail a successful deploy or replace the real deploy/build error. * Default init run's build location to the saved localBuild setting Pressing Enter no longer switches a service that builds locally back to remote builds. * Move build env to its own PR Build-time env vars and secrets (the buildConfig.environmentVariablesAndSecrets annotation) move to bapi_build_env, so this PR only adds local builds. * Make local builds easier to read - deploy() creates the scratch folder and deletes it in one finally. - Explain the App Hosting Backend that the Node.js version check needs. - Init's base image validation is one expression. * Keep localBuild's options on one line * Make the local build tests easier to read - deploy: split the end-to-end local build test, stub the scratch folder instead of touching the filesystem, and keep #11205's upload stub. - prepare: drop a test of an implementation detail. - init: answer prompts with one helper, one behavior per test, and split the upsertRunConfig localBuild test into on and off. * Split Cloud Run deploys into remote and local build paths deploy() now calls deployRemoteBuild or deployLocalBuild. Each path has its own upload function, and local builds get their own revision template, so there are no more RevisionCode unions or repeated localBuild ternaries. The local build's output is passed between functions instead of being stored on ServiceDeploy. The local build guard's fix now uses run:services:update's --region flag. * Let a local build's env vars replace the service's values The env vars a build reports describe that build's output, like its start command, so they now replace any env var of the same name on the service instead of being skipped. The service's other env vars are kept.
falahat
added a commit
that referenced
this pull request
Oct 16, 2026
* Use Object.prototype.hasOwnProperty.call when checking autoinitVars keys * Ensure runtime service account has roles/firebase.sdkAdminServiceAgent for SDK autoinit * Use ternary instead of numeric length in object spreads * Add defensive optional chaining in run.ts and runv2.ts * Remove unused Deployable predeploy/postdeploy from RunSingle * Create a basic, initial schema for firebase.json * improve testing * Require region, remove serviceAccount, and register run in Config.KNOWN_PROPS * Remove unused Deployable predeploy/postdeploy from RunSingle * Update shared GCP clients (runv2, run, artifactregistry) * Add direct_cloud_run experiment flag * Add defensive optional chaining in run.ts and runv2.ts * Rename direct_cloud_run experiment to directcloudrun * Add firebase init run interactive setup * Use directcloudrun experiment and validate rootDir is a directory * Trim baseImage input in init run * Add firebase deploy --only run for remote Cloud Build source deploys * Clean up temporary source archive, guard release URI, and use directcloudrun * Avoid empty template.annotations when deploy message is unset * Simplify template.annotations cleanup when deploy message is unset * Add firebase run:services:update command * Validate service region and check all matching configs in updateService * Rename masterTimeout option to pollTimeoutMs in runv2 * Use pollTimeoutMs option when creating and updating Cloud Run services * Make updateMask fallback in updateService more readable * Add comment explaining default updateMask calculation in updateService * Type Service.ingress as IngressTraffic union in runv2 * Format updateMask call with Prettier and guard template literal in runv2 * Fix Policy stub type in prepare.spec.ts * Improve readability of firebase init run and remove mainContainer * Remove sidecar container from deploy.spec.ts * Use dirExistsSync from fsutils in init run promptRootDir * Make Cloud Run temp-file cleanup best-effort A failure to delete the temporary source archive is now logged at debug level instead of thrown, so it can't fail a deploy that succeeded or replace the real upload error. Mirrors the cleanup in deploy/apphosting/deploy.ts. * Make local build scratch dir cleanup best-effort Route both scratch dir deletions through removeTempPath so a cleanup failure can't fail a successful deploy or replace the real deploy/build error. * Fail clearly when SDK autoinit can't be set up, and simplify it - A Firebase Web App that can't be looked up now fails the deploy whether it's newly or already linked, instead of deploying without autoinit. The error includes the cause and how to switch or unlink the app. - Split resolveAutoInitEnv into flat, single-purpose helpers and use getError for the original error. - run:services:update --app no longer rebuilds when the service already uses that app. - Suggest asking an admin when granting the Admin SDK role fails. - Document that FIREBASE_CONFIG is the Admin SDK's runtime config and why prepare resolves it. * Always refresh FIREBASE_CONFIG from the linked app Deploy writes FIREBASE_CONFIG onto the container, and the next deploy treated any value on the container as a user override. So the CLI's own first value won forever: linking an app and later enabling Storage never gave the service a storageBucket. While an app is linked, deploy now always writes the app's current config, replacing whatever is on the container, including secret-backed values. Build-time overrides still go through the build env annotation. * Always rebuild in run:services:update, even if nothing changes Clearing a base image that isn't set no longer skips the deploy. run:services:update always rebuilds and deploys, the same as firebase deploy, instead of trying to detect no-op updates. * Always rebuild for --app and --clear-app too run:services:update no longer skips the deploy when the app is already linked or there's no app to clear, so every setting behaves the same. Clearing the app now only removes FIREBASE_CONFIG from a service that was linked, since the CLI set it there; on a service that was never linked, it's the user's own value. * Don't default to a base image in init run A base image turns on automatic base image updates, which are off by default. So init no longer suggests nodejs22. An existing service still defaults to its own base image. * Add firebase init run interactive setup * Use directcloudrun experiment and validate rootDir is a directory * Trim baseImage input in init run * Improve readability of firebase init run and remove mainContainer * Use dirExistsSync from fsutils in init run promptRootDir * Don't default to a base image in init run A base image turns on automatic base image updates, which are off by default. So init no longer suggests nodejs22. An existing service still defaults to its own base image. * Make upsertRunConfig easier to read Spell out what init changes on an existing service (rootDir and region) instead of merging three spreads. upsertRunConfig now adds the default ignore list itself, so actuate no longer passes it. Behavior is unchanged; added a test for the ignore fallback on existing services. * Make upsertRunConfig easier to read Spell out what init changes on an existing service (rootDir and region) instead of merging three spreads. upsertRunConfig now adds the default ignore list itself, so actuate no longer passes it. Behavior is unchanged; added a test for the ignore fallback on existing services. * Don't add an ignore list to an existing service in init run Re-running init on a service that's already in firebase.json filled in the default ignore list when the entry didn't have one. Init doesn't ask about ignore, so it shouldn't change it: an existing entry now only gets the rootDir and region the user just chose. New services still get the default ignore list. * Default init run's root directory to the saved one, matching by service ID and region Pressing Enter now keeps the root directory saved in firebase.json instead of resetting it to /. Cloud Run service IDs are only unique within a region, so entries now match on both; the same ID in another region gets its own entry. * Default init run's build location to the saved localBuild setting Pressing Enter no longer switches a service that builds locally back to remote builds. * Only deploy the service init set up, not same-ID services in other regions Init deploys with --only run:<id>, which matches that ID in every region. Init now also passes the service's region to deploy, which skips services in other regions. firebase deploy and --only run:<id> are unchanged. * Write firebase.json once, from init, instead of also from init run's actuate Init already writes firebase.json after every feature is set up, so the write in actuate only printed "Wrote configuration info to firebase.json" a second time. Leaving the write to init also means a failed init leaves firebase.json untouched. * Remove the Cloud Run deploy E2E suite scripts/run-deploy-tests ran the real CLI against a real project. Nothing ran it automatically (npm test only covers src/, and CI doesn't list it), and each run left Cloud Build images behind in Artifact Registry. The unit tests cover the logic, and live testing is done by hand. * Let --only run:<serviceId>:<region> pick one Cloud Run service Service IDs are only unique within a region, so firebase.json can list the same ID in more than one region. - run:<serviceId> still deploys that ID in every region it's listed in, and now logs which services it matched when there's more than one. - run:<serviceId>:<region> deploys exactly one service. Init deploys the service it set up this way, which replaces the region it used to pass in the deploy context. - Per-service messages (uploading, building, deployed) include the region. - firebase.json can't list the same service ID and region twice. * Make run:services:update change exactly one service firebase run:services:update <serviceId> now also accepts <serviceId>:<region>. - If firebase.json lists the service ID once, the region is optional. - If it lists the ID in more than one region, the command fails and lists the <serviceId>:<region> names to choose from. - It deploys only run:<serviceId>:<region>, so it never redeploys the same ID in another region. The "doesn't exist yet" message also suggests the command that deploys just that service and region. * Make the Cloud Run deploy code and tests easier to read - deploy.ts reads as four steps: upload the source, build it, prepare the revision, then create the service or roll out a new revision. The revision rules are a pure function, revisionTemplate, with one small test per rule. - Helpers that only deploy.ts uses (deployRevision, toAppHostingConfig, the rollout timeout) moved there from util.ts. util.ts now only picks services from firebase.json and checks whether a service exists. - Stop deleting the local source zip. It's a tmp file, which firebase deploy deletes when it exits; App Hosting relies on the same thing. - Check every service's region once, up front, and stop --only filtering at the first service that isn't in firebase.json. - Tests use realistic names, check only the fields they're about, and drop duplicate coverage. The release test has its own file. - The help text no longer mentions local builds or run:services:update, which later PRs add. - Clearer docstrings and tests for init run's actuate and upsertRunConfig. * Make run:services:update easier to read - Read the flags into a settings object and pass it straight to deploy. - Move the missing-service message into update.ts, its only caller. - Tests: realistic names, an update() helper that reads like the CLI call, one expectDeployed() helper, and one test for the always-rebuild decision. * Move build env to its own PR Build-time env vars and secrets (the buildConfig.environmentVariablesAndSecrets annotation) move to bapi_build_env, so this PR only adds local builds. * Make local builds easier to read - deploy() creates the scratch folder and deletes it in one finally. - Explain the App Hosting Backend that the Node.js version check needs. - Init's base image validation is one expression. * Keep localBuild's options on one line * Support build-time env vars and secrets for Cloud Run Reads the firebase.google.com/buildConfig.environmentVariablesAndSecrets annotation on the service. Plain values go to Cloud Build and local builds; secret references only work in local builds, which read them with the deployer's credentials. * Make the build env tests easier to read Realistic names (API_URL, NPM_TOKEN, npm-token) and shorter fixtures. * Make the local build tests easier to read - deploy: split the end-to-end local build test, stub the scratch folder instead of touching the filesystem, and keep #11205's upload stub. - prepare: drop a test of an implementation detail. - init: answer prompts with one helper, one behavior per test, and split the upsertRunConfig localBuild test into on and off. * Make the SDK auto-init tests easier to read - Realistic app IDs, small fixture helpers, and no prettier-mangled calls. - Drop two tests that repeated others, and test that prepare rejects a service it can't build before looking up the app or touching IAM. * Address review comments on the Cloud Run deploy - Add mainContainer() so one place decides which container is the main one. - Resolve the sticky base image with an if statement instead of a ternary. - Build the buildpack request with an explicit if instead of a conditional spread. - Explain that svc.baseImage is already resolved by prepare. - Rename createService to createNewService. * Split run:services:update's service name into serviceId and region updateService took one name that was either <serviceId> or <serviceId>:<region>. It now takes serviceId and region, and the command takes the region from --region. * Say which deploy step sets each ServiceDeploy field * Split Cloud Run deploys into remote and local build paths deploy() now calls deployRemoteBuild or deployLocalBuild. Each path has its own upload function, and local builds get their own revision template, so there are no more RevisionCode unions or repeated localBuild ternaries. The local build's output is passed between functions instead of being stored on ServiceDeploy. The local build guard's fix now uses run:services:update's --region flag. * Let a local build's env vars replace the service's values The env vars a build reports describe that build's output, like its start command, so they now replace any env var of the same name on the service instead of being skipped. The service's other env vars are kept. * Use App Hosting's secret format for build secrets A build secret is now written the way apphosting.yaml writes one ({"secret": "npm-token@3"}) and passed straight to loadSecret, so there are no extra regexes or format conversions. Cloud Build gets an explicit filter for plain values instead of a cast, and the comments and errors say why only local builds can use secrets. * Rename plainValues to nonSecretValues The name and doc comment now say that the result never includes secrets or their values. * Accept numbers and booleans in the build env annotation, like apphosting.yaml * Check for secrets with isSecretRef instead of typeof string * Make the FIREBASE_CONFIG and annotation logic in deploy easier to follow Name the case where a deploy unlinks the app. Keep an emptied env list instead of deleting it: to Cloud Run, [] and no env field mean the same thing. Always send the service's annotations, instead of only when the linked app changed, which removes the undefined return and the conditional update mask. * Rename serviceAnnotations to serviceAnnotationsToDeploy * Fail the deploy if the linked Firebase Web App doesn't exist For an app ID with no app, the web app config lookup returns the project's config without an appId, so a typo in --app deployed and was saved on the service. Check that the app exists first, with the same check App Hosting's --app uses, so the deploy fails before building or saving anything. * Group the linked app's ID and config into ServiceDeploy.autoInit * init run: ask which web app to link, and create a new one only in actuate * Stop granting roles/firebase.sdkAdminServiceAgent; print the grant command when linking an app * init run: fail instead of deploying unlinked when the web app can't be created getOrCreateWebApp only warns, and returns nothing, when the project is out of web apps (App Hosting backends work without one). Init then deployed the service without a linked app, silently skipping Firebase SDK auto-initialization. * Add a TODO for handing runtime FIREBASE_CONFIG to Cloud Run * Shorten the runtime FIREBASE_CONFIG TODO * Reword the FIREBASE_CONFIG TODO to match the agreed behavior The CLI always sets FIREBASE_CONFIG on linked services; users override auto-init in code, not with the env var. Once Cloud Run injects it, only the annotation is needed. * Always use the linked app's config at build time too The build env annotation could override FIREBASE_CONFIG and FIREBASE_WEBAPP_CONFIG for the build, while the container always got the app's FIREBASE_CONFIG. Now both come from the app: users override auto-init in code, not with these env vars.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
firebase deploy --only run, behind thedirectcloudrunexperiment. For each Cloud Run service in firebase.json, it builds the local source on Cloud Build, then creates the service or rolls out a new revision.firebase init runnow deploys the service it sets up.prepare.ts: picks the services, reads them from Cloud Run, and keeps their base image.deploy.ts: uploads the source, builds it, then creates the service or rolls out a new revision.revisionTemplatekeeps the live revision's settings.release.ts: logs each service's URL.util.ts: handles--only run,run:<serviceId>, andrun:<serviceId>:<region>.run:services:update(#11206), local builds (#11207), and SDK auto-init (#11208) come in follow-up PRs.Scenarios Tested
-m, and used--onlywith the same service ID in two regions.Sample Commands