Cloud Run Functions data model #8767

inlined · 2025-06-18T16:20:13Z

Thanks for your patience. I've got part one of 2-3 on getting some PoCs ready for CRF. This just extends the data model to support the RunV2 API (needed for the submit build stuff), the service<->endpoint stuff, and the changes that popped up with assertExhaustive once a new type has been added.

This PR exposed two bugs (not fixed):

The Eventarc source is not converting function IDs to service IDs (which can use a utility function where there is a TODO in the runv2 code)
The billing calculator was created when GCFv2 had a different SKU and that's no longer the case. But the billing change will also go out, so we need to figure out which way to fix it.

taeold · 2025-07-02T02:01:26Z

/gemini review

gemini-code-assist

Code Review

This pull request extends the data model to support Cloud Run Functions, introducing a new 'run' platform type. The changes include refactoring shared utilities into a new k8s.ts file and adding a comprehensive runv2.ts module with tests for interacting with the Cloud Run v2 API.

My review focuses on ensuring consistency and correctness in the new code. I've identified areas for improvement, including an inconsistency in the AllFunctionsPlatforms array, a potential bug in how CPU values are handled, and critical bugs related to function ID conversion and billing calculations.

gemini-code-assist · 2025-07-02T02:03:39Z

src/deploy/functions/prepare.ts

        // N.B. If GCF starts allowing v1's allowable characters in IDs they're
        // going to need to have a transform to create a service ID (which has a
        // more restrictive character set). We'll need to reimplement that here.
+        // BUG BUG BUG. This has happened and we need to fix it.


This comment indicates a known issue where the function ID to service ID conversion is not happening. It's marked as a BUG BUG BUG, suggesting it's critical and needs to be addressed.

Suggested change

// BUG BUG BUG. This has happened and we need to fix it.

// TODO: Implement function ID to service ID conversion

IMO the severity of this comment is accurate. This is a WIP and a BUG should be resolved before releasing to the public whereas a TODO can be ignored for years.

src/deploy/functions/backend.ts

gemini-code-assist · 2025-07-02T02:03:40Z

src/gcp/runv2.ts

+        ],
+        resources: {
+          limits: {
+            cpu: String((endpoint.cpu as Number) || 1),


Using the logical OR || for default values can lead to unexpected behavior if 0 is a valid value for endpoint.cpu. The nullish coalescing operator ?? is safer as it only provides the default for null or undefined values.

Suggested change

cpu: String((endpoint.cpu as Number) || 1),

cpu: String((endpoint.cpu as number) ?? 1),

0 is not a valid value AFAIK, so this shouldn't make a difference.

gemini-code-assist · 2025-07-02T02:03:40Z

src/functions/secrets.ts

+    // This may be tricky because the image has been deleted. How does this work
+    // with GCF?
+    throw new FirebaseError("Updating Cloud Run functions is not yet implemented.");


This error message is not very informative. Consider adding more context to the error message to help users understand why updating Cloud Run functions is not yet implemented.

Suggested change

// This may be tricky because the image has been deleted. How does this work

// with GCF?

throw new FirebaseError("Updating Cloud Run functions is not yet implemented.");

throw new FirebaseError("Updating secrets for Cloud Run functions is not yet implemented. Please use GCFv2.");

Bad suggestion

taeold · 2025-07-07T04:17:47Z

src/gcp/runv2.ts

+export const RUNTIME_LABEL = "goog-cloudfunctions-runtime";
+export const CLIENT_NAME_LABEL = "goog-managed-by";
+export const CLIENT_NAME_ANNOTATION = "run.googleapis.com/client-name";
+export const CPU_BOOST_ANNOTATION = "run.googleapis.com/startup-cpu-boost";
+export const TRIGGER_TYPE_ANNOTATION = "cloudfunctions.googleapis.com/trigger-type";
+export const FUNCTION_TARGET_ANNOTATION = "run.googleapis.com/build-function-target"; // e.g. '{"worker":"triggerTest"}'
+export const FUNCTION_ID_ANNOTATION = "cloudfunctions.googleapis.com/function-id"; // e.g. "triggerTest"
+export const BASE_IMAGE_ANNOTATION = "run.googleapis.com/base-images"; // : '{"worker":"us-central1-docker.pkg.dev/serverless-runtimes/google-22-full/runtimes/nodejs22"}'
+export const MAX_INSTANCES_ANNOTATION = "autoscaling.knative.dev/maxScale";
+export const MIN_INSTANCES_ANNOTATION = "autoscaling.knative.dev/minScale";
+export const DEFAULT_FUNCTION_CONTAINER_NAME = "worker";


In https://cloud.google.com/run/docs/reference/rest/v2/projects.locations.services#Service

Cloud Run API v2 does not support annotations with run.googleapis.com, cloud.googleapis.com, serving.knative.dev, or autoscaling.knative.dev namespaces, and they will be rejected in new resources. All system annotations in v1 now have a corresponding field in v2 Service.

Suspect we'll have to refactor many of these?

Uuuuuugh... I think the Cloud Console uses the V1 API and I need to start using resource explorer. Will go back to the drawing board.

Updated to read from these fields where there is no analog (or the analog is an env var) but write to fields we have access to

fuuuudge. Edited in the wrong branch (IIRC a parent to this one). Sifting through the Git nonsense.

inlined · 2025-07-30T22:10:52Z

/gemini review

gemini-code-assist · 2025-07-30T22:21:06Z

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

inlined · 2025-08-01T16:49:14Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces the data model for Cloud Run Functions (CRF) by adding a new run platform type. It extends various parts of the functions deployment logic, including backend definitions, argument types, preparation steps, and pricing calculations, to accommodate this new platform. A significant part of the change is the new runv2.ts module, which provides the API client and data models for Cloud Run v2, along with conversion functions to and from the internal Endpoint representation. The changes are well-structured, with new tests for the runv2 module and refactoring of shared utilities. My review found a couple of areas for improvement to ensure consistency and code quality.

gemini-code-assist · 2025-08-01T16:50:29Z

src/deploy/functions/backend.ts

@@ -300,7 +300,7 @@
  serviceAccount?: string | null;
 }

-export type FunctionsPlatform = "gcfv1" | "gcfv2";
+export type FunctionsPlatform = "gcfv1" | "gcfv2" | "run";


While the FunctionsPlatform type is correctly updated to include run, the related AllFunctionsPlatforms constant on the next line is not. This will cause parts of the system that iterate over AllFunctionsPlatforms (e.g., in release/planner.ts) to miss Cloud Run functions, which can lead to them not being planned for updates or deletion. Please update AllFunctionsPlatforms to include run to be consistent with the type definition.

gemini-code-assist · 2025-08-01T16:50:29Z

src/gcp/runv2.ts

+        ],
+        resources: {
+          limits: {
+            cpu: String((endpoint.cpu as Number) || 1),


The type assertion as Number uses the Number object wrapper type, which is not idiomatic in TypeScript. It's better to use the primitive type number. While this works due to auto-unboxing, it can be confusing. Using as number or even omitting the assertion if the type is already correctly inferred would be cleaner and improve readability.

Suggested change

cpu: String((endpoint.cpu as Number) || 1),

cpu: String(endpoint.cpu || 1),

codecov-commenter · 2025-08-15T00:14:25Z

Codecov Report

❌ Patch coverage is 69.78417% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.16%. Comparing base (ecbbc91) to head (c5bbba5).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
src/gcp/runv2.ts	82.02%	10 Missing and 6 partials ⚠️
src/deploy/functions/release/fabricator.ts	14.28%	9 Missing and 3 partials ⚠️
src/deploy/functions/functionsDeployHelper.ts	16.66%	5 Missing ⚠️
src/deploy/functions/backend.ts	33.33%	1 Missing and 1 partial ⚠️
src/functions/secrets.ts	0.00%	2 Missing ⚠️
src/gcp/k8s.ts	83.33%	1 Missing and 1 partial ⚠️
src/deploy/functions/ensure.ts	0.00%	1 Missing ⚠️
src/deploy/functions/prepare.ts	0.00%	1 Missing ⚠️
src/gcp/cloudscheduler.ts	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8767      +/-   ##
==========================================
- Coverage   50.93%   49.16%   -1.78%     
==========================================
  Files         510      670     +160     
  Lines       33396    38334    +4938     
  Branches     6934     7644     +710     
==========================================
+ Hits        17011    18846    +1835     
- Misses      14886    17968    +3082     
- Partials     1499     1520      +21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

* Checkpoint * Checkpoint * Run Functions experiment + model changes * Simplify; format * Checkpoint * Checkpoint * Run Functions experiment + model changes * Simplify; format * Checkpoint * Refactor scaling and tool * Fix tests. Somehow unrelated code is failing * Fix tests * Resotore runv2 from reflog * Format fix * PR feedback

inlined added 4 commits May 19, 2025 11:38

Checkpoint

7ef4f67

Checkpoint

4e61dbe

Run Functions experiment + model changes

3d43507

Simplify; format

ec92c97

inlined requested a review from taeold June 18, 2025 16:20

github-project-automation bot added this to [Cloud] Extensions + Functions Jun 18, 2025

Merge branch 'master' into inlined.run-functions-exp

2ae9410

gemini-code-assist bot reviewed Jul 2, 2025

View reviewed changes

taeold reviewed Jul 13, 2025

View reviewed changes

inlined added 9 commits July 28, 2025 17:13

Checkpoint

cd0d894

Checkpoint

eb95e4d

Run Functions experiment + model changes

05120c7

Simplify; format

6744e91

Checkpoint

a41f634

Refactor scaling and tool

f3ba1b2

Fix tests. Somehow unrelated code is failing

50c0806

Fix tests

0cb1545

Merge branch 'inlined.crf-deploy' into inlined.run-functions-exp

6ce821e

inlined force-pushed the inlined.run-functions-exp branch from 15e40f2 to 6ce821e Compare July 30, 2025 21:59

inlined and others added 3 commits July 30, 2025 15:00

Merge branch 'master' into inlined.run-functions-exp

3f906e7

Resotore runv2 from reflog

7847b16

Format fix

33efe3f

Merge branch 'master' into inlined.run-functions-exp

ca83ac3

gemini-code-assist bot reviewed Aug 1, 2025

View reviewed changes

taeold approved these changes Aug 5, 2025

View reviewed changes

github-project-automation bot moved this to Approved [PR] in [Cloud] Extensions + Functions Aug 5, 2025

taeold approved these changes Aug 5, 2025

View reviewed changes

inlined and others added 2 commits August 14, 2025 17:05

PR feedback

ef2e4a8

Merge branch 'master' into inlined.run-functions-exp

c5bbba5

inlined added 2 commits August 14, 2025 17:24

Merge branch 'master' into inlined.run-functions-exp

2c6a398

Merge branch 'master' into inlined.run-functions-exp

c3ce91f

inlined merged commit fd963fc into master Aug 15, 2025
48 checks passed

inlined deleted the inlined.run-functions-exp branch August 15, 2025 01:03

github-project-automation bot moved this from Approved [PR] to Done in [Cloud] Extensions + Functions Aug 15, 2025

	// BUG BUG BUG. This has happened and we need to fix it.
	// TODO: Implement function ID to service ID conversion

	cpu: String((endpoint.cpu as Number) \|\| 1),
	cpu: String((endpoint.cpu as number) ?? 1),

	cpu: String((endpoint.cpu as Number) \|\| 1),
	cpu: String(endpoint.cpu \|\| 1),

Cloud Run Functions data model #8767

Cloud Run Functions data model #8767

Uh oh!

Conversation

inlined commented Jun 18, 2025

Uh oh!

taeold commented Jul 2, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

inlined Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gemini-code-assist bot Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

inlined Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

inlined Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

taeold Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

inlined Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

inlined Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

inlined Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

inlined commented Jul 30, 2025

Uh oh!

gemini-code-assist bot commented Jul 30, 2025

Uh oh!

inlined commented Aug 1, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Aug 15, 2025

Codecov Report

Uh oh!

Uh oh!

Uh oh!