fix(functions): resolve default regions for parameterized event triggers

[ajperel]

Fixes #11020.

Description

When 2nd-gen Cloud Functions declare parameterized event trigger filters (such as database: defineString("DATABASE") or bucket: defineString("BUCKET")), default region resolution failed because parameter evaluation occurred after region resolution. As a result, the CLI passed raw CEL expressions (e.g., "{{ params.DATABASE }}") to GCP Firestore and Storage REST APIs. The APIs rejected these with invalid ID errors, causing the CLI to silently fall back to us-central1 for functions whose event sources lived in other regions (e.g., eur3 -> europe-west1).

This regression was introduced by PR #10471, which moved region resolution before build.resolveBackend to ensure VPC connectors could be built with concrete region paths. However, because build.resolveBackend bundled both parameter evaluation (params.resolveParams) and backend conversion (build.toBackend), parameter resolution was inadvertently moved after region resolution.

Fix

We unbundle parameter evaluation from backend conversion so region resolution can sit between them:

1. Evaluate parameters first: Parameter values are resolved before default region resolution runs.
2. Ephemeral trigger lookup: When querying GCP APIs to discover an event source's region, we ephemerally substitute trigger filter expressions for the API call without mutating the Build endpoint in-place.
3. Pristine AST preservation: The Build endpoint retains its original parameter expressions until build.toBackend performs the definitive transformation into a Backend object.
4. Test additions: Added unit tests in src/deploy/functions/prepare.spec.ts covering parameterized Firestore and Storage triggers, asserting that GCP API stubs receive the concrete resolved resource names, the region resolves to the mapped location, and the Build endpoint's trigger filters remain completely unmutated.

Alternative

An alternative design is to introduce a dedicated build.resolveEventFilters(wantBuild, paramValues) step that mutates eventFilters directly on the Build object before region resolution runs.

• Tradeoffs: While this would allow resolveDefaultRegionsForBuild to read plain strings without accepting paramValues, it introduces an asymmetry where trigger filters are partially resolved on Build ahead of time while all other expression fields (serviceAccount, cpu, memory, vpc, schedule) remain unresolved until toBackend. Additionally, toBackend must retain filter resolution logic anyway for standalone callers like the emulator.
• We opted for ephemeral resolution to avoid mutating Build state and keep toBackend as the sole authority for expression evaluation.

Test Scenarios

• Parameterized Firestore triggers: Tested that database: "{{ params.DATABASE }}" with DATABASE=custom-db in eur3 resolves the endpoint region to europe-west1, passes "custom-db" to getDatabase(), and verifies want.endpoints["onDocumentCreate"].eventTrigger.eventFilters remains unmutated ({ database: "{{ params.DATABASE }}" }).
• Parameterized Storage triggers: Tested that bucket: "{{ params.BUCKET }}" with BUCKET=custom-bucket in eu resolves the endpoint region to europe-west1, passes "custom-bucket" to getBucket(), and verifies want.endpoints["onArchive"].eventTrigger.eventFilters remains unmutated ({ bucket: "{{ params.BUCKET }}" }).
• Error fallback path: Tested that parameterized triggers with unresolvable/missing parameters gracefully fall back to us-central1 and leave the trigger filter expressions intact.
• npm run test

Sample Commands

• firebase deploy --only functions

[gemini-code-assist]

Code Review

This pull request fixes an issue where second-generation functions with parameterized trigger event filters failed default region resolution. It unbundles parameter resolution and backend generation in prepare.ts so that parameter values can be ephemerally resolved in resolveRegionForTrigger before querying GCP APIs. The feedback suggests refactoring resolveRegionForTrigger to use an early return if the endpoint is not event-triggered, which simplifies the logic and aligns with the repository's style guide on reducing nesting.

[gemini-code-assist]

To adhere to the repository style guide on reducing nesting and keeping the main logic flat, we can refactor resolveRegionForTrigger to use an early return if the endpoint is not event-triggered. This also allows us to remove the redundant paramValues check (since it defaults to {} and is always truthy) and the let re-assignment.

async function resolveRegionForTrigger(
  endpoint: build.Endpoint,
  paramValues: Record<string, params.ParamValue> = {},
): Promise<string> {
  if (!build.isEventTriggered(endpoint)) {
    const service = serviceForEndpoint(endpoint);
    return service.getDefaultRegion(endpoint);
  }

  const targetEndpoint = {
    ...endpoint,
    eventTrigger: {
      ...endpoint.eventTrigger,
      ...(endpoint.eventTrigger.eventFilters && {
        eventFilters: mapObject(endpoint.eventTrigger.eventFilters, (v) =>
          params.resolveString(v, paramValues),
        ),
      }),
      ...(endpoint.eventTrigger.eventFilterPathPatterns && {
        eventFilterPathPatterns: mapObject(endpoint.eventTrigger.eventFilterPathPatterns, (v) =>
          params.resolveString(v, paramValues),
        ),
      }),
    },
  };
  const service = serviceForEndpoint(targetEndpoint);
  return service.getDefaultRegion(targetEndpoint);
}

References

1. Reduce nesting as much as possible: Code should avoid unnecessarily deep nesting or long periods of nesting. Use early returns, continue, and break statements in functions and loops to handle edge cases early and keep main logic flat. ^(link)

[joehan]

I like this approach more than the alternative you suggested - the alternative of partially resolving some fields before others seems complex and ripe for future bugs , and I think it is reasonable for 'resolveDefaultRegions' to require param values.

As a slight twist - Should we keep resolveBackend and just move resolveDefaultRegionForBuild inside of it? Or is there some other call site that needs resolveBackend to not include resolveDefaultRegionForBuild?

[ajperel]

I like this approach more than the alternative you suggested - the alternative of partially resolving some fields before others seems complex and ripe for future bugs , and I think it is reasonable for 'resolveDefaultRegions' to require param values.

As a slight twist - Should we keep resolveBackend and just move resolveDefaultRegionForBuild inside of it? Or is there some other call site that needs resolveBackend to not include resolveDefaultRegionForBuild?

Yeah. I investigated this path with Gemini cause it also sounded nice to me and I didn't like recreating resolveBackend. But with a little more proding maybe we can make it work. The two issues to solve:

1. resolveBackend is also called by the Functions emulator (src/emulator/functionsEmulator.ts:630). Putting resolveDefaultRegionsForBuild inside resolveBackend would cause firebase emulators:start to make live GCP REST calls (getDatabase, getBucket), breaking offline emulation and local/demo projects without GCP credentials. But there's already an isEmulator field we could check and skip all this stuff.

2. Additionally, resolveDefaultRegionsForBuild depends on have (the live deployed functions on GCP) to match existing regions, which resolveBackend does not have access to. Keeping build.ts hermetic and keeping cloud region discovery in prepare.ts preserves the clean boundary between AST conversion and deployment orchestration." But we could work around this in one of two ways:

   • resolveBackend could take an optional method that already had bound have and we avoid a circular dependency.
   • We just move resolveBackend to another file like prepare.ts or a new file we create, e.g., resolve.ts

I do think that could be a bit better?