Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functions SDK v4 #1161

Merged
merged 72 commits into from
Oct 13, 2022
Merged

Functions SDK v4 #1161

merged 72 commits into from
Oct 13, 2022

Conversation

taeold
Copy link
Contributor

@taeold taeold commented Jun 30, 2022

Breaking Changes

  • Deprecated allowInvalidAppCheckToken option. Instead useenforceAppCheck.

  • App Check enforcement on callable functions is disabled by default in v4.

  • Requests containing invalid App Check tokens won't be denied unless you

  • explicitly enable App Check enforcement using the new enforceAppCheck option.

  • Furthermore, when enforcement is enabled, callable functions will deny

  • all requests without App Check tokens.

  • Dropped support for Node.js versions 8, 10, and 12.

  • Dropped support for Admin SDK versions 8 and 9.

  • Removed the functions.handler namespace.

  • DataSnapshot passed to the Firebase Realtime Database trigger now
    matches the DataSnapshot returned by the Admin SDK, with null values
    removed.

  • Removed __trigger object on function handlers.

  • Reorganized source code location. This affects only apps that directly import files instead of using the recommend entry points specified in the

  • Reworked the apps library and removed lodash as a runtime dependency.

  • Unspecified function configuration value will be reset to platform default. Use preserveExternalChanges to prevent this behavior.

Enhancements

  • Logs created with the functions.logger package in v2 functions
    are now annotated with each request's trace ID, making it easy to correlate
    log entries with the incoming request. Trace IDs are especially useful for
    cases where 2nd gen's concurrency feature permits a function
    to handle multiple requests at any given time. See
    Correlate log entries to learn more.
  • functions.logger.error now always outputs an error object and is included in Google Cloud Error Reporting.
  • The logging severity of Auth/App Check token validation has changed from info to debug level.
  • Event parameters for 2nd generation functions are now strongly typed, permitting stronger TypeScript types for matched parameters.
  • Add new params package to support parameterized environment configuration. See https://firebase.google.com/docs/functions/config-env for more information.
  • Add new functions.RESET_VALUE and functions.v2.options.RESET_VALUE sentinel value for explicitly resetting function configuration to platform default.
  • Add new preserveExternalChanges option to prevent Firebase CLI from resetting unspecified configuration option to platform default

inlined and others added 12 commits June 15, 2022 13:59
* Bump admin SDK version. Remove vendored APIs + sniffing

* Bump node version; remove tests for old node versions

* Package lock

* Update v2 rtdb to expose RawRTDBEvent and RawRTDBCloudEventData (#1137)

* Update v2 rtdb to expose RawRTDBEvent and RawRTDBCloudEventData

This commit updates the v2 database provider to also expose the
RawRTDB Events.

This will be used in test-sdk when mocking returned DatabaseEvents.

* Adding @hidden instead of @internal

* Revert package lock

* Run formatter

Co-authored-by: Tyler Stark <[email protected]>
* Checkpoint

* Make logging.error show up in Cloud Error Reporting

* Make logging.error show up in Cloud Error Reporting

* Lint fixes
* Update v2 rtdb to expose RawRTDBEvent and RawRTDBCloudEventData (#1137)

* Update v2 rtdb to expose RawRTDBEvent and RawRTDBCloudEventData

This commit updates the v2 database provider to also expose the
RawRTDB Events.

This will be used in test-sdk when mocking returned DatabaseEvents.

* Adding @hidden instead of @internal

* Put tests in a codebase

* Revert changes to package-lock.json

* Format fix

Co-authored-by: Tyler Stark <[email protected]>
Co-authored-by: Daniel Young Lee <[email protected]>
* Update admin; remove lodash

* Adding missing file

* Uncomment code
* Divide everything into v1, v2, and common (unexported).

Did some refactoring, like putting `firebaseConfig` into common.
This means we have absolutely no dependencies on v1 in common or
v2. Some files are left at the root because they were modified
in other CLs that are outstanding and I want to linearize the
changes to minimize conflicts.

While working, I removed some legacy workarounds that should no
longer apply. For example, we no longer need to load fireaseConfig
from runtime config because all supported versions of the CLI
set the FIREBASE_CONFIG environment variable. This meant we
could remove all of setup.ts because all monkeypatches were for
outdated runtimes. Since there is no longer an environment
change between importing v1/index and a subpackage, I've gone
ahead and exported the providers for v1 to minimize customer
frustration with the v1 namespace.

* Fix dependency conflicts

* Lint fixes

* Remove __trigger (#1150)

* Remove __trigger

* Lint fixes

* Divide everything into v1, v2, and common (unexported).

Did some refactoring, like putting `firebaseConfig` into common.
This means we have absolutely no dependencies on v1 in common or
v2. Some files are left at the root because they were modified
in other CLs that are outstanding and I want to linearize the
changes to minimize conflicts.

While working, I removed some legacy workarounds that should no
longer apply. For example, we no longer need to load fireaseConfig
from runtime config because all supported versions of the CLI
set the FIREBASE_CONFIG environment variable. This meant we
could remove all of setup.ts because all monkeypatches were for
outdated runtimes. Since there is no longer an environment
change between importing v1/index and a subpackage, I've gone
ahead and exported the providers for v1 to minimize customer
frustration with the v1 namespace.

* Fix dependency conflicts

* Lint fixes

* Remove __trigger (#1150)

* Remove __trigger

* Lint fixes

* Move apps and utilities into common
# Conflicts:
#	package-lock.json
#	src/common/providers/database.ts
@taeold taeold added this to the v4 milestone Jun 30, 2022
inlined and others added 17 commits July 13, 2022 11:37
* Fix broken test.

* Fix package-lock.json
)

We have a weird edge case where an authorization in form we don't recognize will completely skip the auth check.

The fix here applies 2 changes:

1) We allow 'Bearer <TOKEN>' format to be case insensitive. 'bearer <TOKEN>' also works.

2) We reject other authorization header. e.g. 'Beaver <token>' is rejected.
…ing (#1194)

Let's bite the bullet - match styling of firebase/firebase-tools and start using eslint over prettier.

This change effectively touches almost all files in this repo, especially since eslint prefers double-quotes over single-quotes.

I've made couple of other changes to make eslint errors go away. Things like:

1) Eliminate dangling promises
2) Remove empty function definition (prefer `()=>undefined` over `()=>{}`)
3) Remove unused arguments from function definitions

In theory, these are just linter changes without changes to the runtime behavior. v4 launches will need good tests and bugbash anyway, so this feels like a great time to commit to this migration.
Implements go/cf3-typed-params.

WIP: currently only common and v2 are implemented.

Had to update `prettier` to the next major version so it could read TS 4.1 features without erroring out. I created  #1160 as a base for this change to make it easier to separate mechanical from hand-crafted changes. If this is approved, however, I'll commit both changes to `launch.next`
Turns out I missed most of the files for migration from prettier to eslint in #1194 due to poorly configured `.eslintignore` - it ignored all files under `src/v1` and `src/v2`!

Correctly configure eslint and run `npm run format`. Again, I had to manually fixup issues like:

1. Remove empty function definition (prefer ()=>undefined over ()=>{})
1. Remove unused arguments from function definitions
Integration test caught some bugs!

1. From #1155 - Some internal type must be exposed - otherwise compiling user projects fails w/ error message like below:

```
Error: node_modules/firebase-functions/lib/common/params.d.ts(10,13): error TS2314: Generic type 'Extract' requires 2 type argument(s).
Error: node_modules/firebase-functions/lib/common/params.d.ts(10,21): error TS2304: Cannot find name 'Split'.
Error: node_modules/firebase-functions/lib/common/params.d.ts(10,27): error TS2304: Cannot find name 'NullSafe'.
```

2. From #1181 - There is no way to call private callable functions since GCF requrie GCP identity token as authorization header while callable function requires Firebase auth tokens. Disabling those test cases in the integration test until we can mint a GCP project w/ permission to create public functions.
Few changes here:

1) Fix docgen script for v1 to work with the new file layout

2) Remove old apidoc scripts - no longer used!

3) Rewrite toc.ts script to fix the "lowercase" issue where every toc title was lowercased. While I fix the issue here, it also changes the overall toc.yaml structure to return to more flatten structure. I think the output looks fine to me, but we should discuss if we find this revert less desirable.

```
toc:
  - title: firebase-functions
    path: /docs/functions/beta/reference/firebase-functions.md
    section:
      - title: Alerts
        path: /docs/functions/beta/reference/firebase-functions.alerts.md
      - title: CloudEvent
        path: /docs/functions/beta/reference/firebase-functions.cloudevent.md
      - title: CloudFunction
        path: /docs/functions/beta/reference/firebase-functions.cloudfunction.md
      - title: Database
        path: /docs/functions/beta/reference/firebase-functions.database.md
      - title: Eventarc
        path: /docs/functions/beta/reference/firebase-functions.eventarc.md
      - title: EventHandlerOptions
        path: >-
          /docs/functions/beta/reference/firebase-functions.eventhandleroptions.md
      - title: GlobalOptions
        path: /docs/functions/beta/reference/firebase-functions.globaloptions.md
      - title: Https
        path: /docs/functions/beta/reference/firebase-functions.https.md
      - title: Identity
        path: /docs/functions/beta/reference/firebase-functions.identity.md
      - title: Logger
        path: /docs/functions/beta/reference/firebase-functions.logger.md
      - title: Pubsub
        path: /docs/functions/beta/reference/firebase-functions.pubsub.md
      - title: Storage
        path: /docs/functions/beta/reference/firebase-functions.storage.md
      - title: Tasks
        path: /docs/functions/beta/reference/firebase-functions.tasks.md
```
We will no longer support functions.handler namespace from v4 and onward.
Update documentation for src/common/change.ts

* Removing `@hidden`
* Make fn internal.

Co-authored-by: Daniel Lee <[email protected]>
* added missing info to analytics

* fixing https package

* fixing pubsub

* adding storage

* change return to returns

* Make it a link!

* Unnecessary internal tag.

Co-authored-by: Daniel Lee <[email protected]>
* Add docs on schedule configs.

* Fix typo.
* init

* adding in tests & fixing remote config tsdoc strings

* adding exports

* remove @Alpha from remote config since docs are published

* add wrapTraceContext
@taeold
Copy link
Contributor Author

taeold commented Oct 3, 2022

@inlined
Copy link
Member

inlined commented Oct 3, 2022

FYI: just submitted another dependent PR for making some Storage fields required.

Berlioz and others added 13 commits October 5, 2022 13:03
…s to env.FIREBASE_CONFIG (#1231)

These correspond to magic params defined in the CLI which can be accessed during config resolution but are not saved to .env
…ig option. (#1250)

Instead of allowing `null`s to reset a configuration value, we are introducing a new sentinel value, `RESET_VALUE` to make it clear the intention of resetting the config value to platform default:

```js
import * as functions from "firebase-functions/v1";
import { onRequest } from "firebase-functions/v2/https";
import { RESET_VALUE } from "firebase-functions/v2/options";

export const v1Req = functions
  .runWith({
    minInstances: functions.RESET_VALUE,
    memory: functions.RESET_VALUE,
    serviceAccount: RESET_VALUE
  }).https.onRequest(
    (req, res) => {
      res.sendStatus(200)
    },
);

export const v2Req = onRequest(
  {
   memory: RESET_VALUE,
   minInstances: RESET_VALUE,
  },
  (req, res) => {
    res.sendStatus(200)
  },
);
```

Majority of this PR concerns fixing/updating typings on various configuration options.

Recall that in v2, we inlined/repeat all function options in each respective providers to make reference doc more clear. This unfortunately bloated up this PR since I had to repeat the changes in each and every provider.
This PR make some sweeping changes!

1) We change the default behavior of the SDK so that any un-annotated configuration option that is resettable are given `RESET_VALUE`. Practically, this means that given a function configuration like this:

```js
export const v2Req = onRequest(
  (req, res) => {
    res.sendStatus(200)
  },
);
```

And you go ahead and change the memory configuration of the function using GCP Console or `gcloud` tool, we will revert the memory back to platform default on the next function deploy.

2) You may optionally opt-out of this behavior by setting `preserveExternalChanges` option:

```js
exports.v1httpPreserve = functions
  .runWith({ preserveExternalChanges: true })
  .https.onRequest((req, resp) => {
    resp.status(200).send("PASS");
  });

functionsv2.setGlobalOptions({ preserveExternalChanges: true });

exports.v2http = functionsv2.https.onRequest((req, resp) => {
  resp.status(200).send("PASS");
});
```

This change broke a lot of test. I broke up the change for fixing test cases in a separate PR #1254.
Internal packages like firebase-functions-test relies on export of functions config singleton object to mock/override its behavior. We should keep this setup (with an @internal mark so it isn't exposed in type or reference doc) to allow these packages to continue to work.
@taeold taeold requested a review from Berlioz October 13, 2022 18:45
@colerogers
Copy link
Contributor

🔥 🔥 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants