feat: add firebase.json schema support and validation for function kits - #10842
feat: add firebase.json schema support and validation for function kits#10842wandamora wants to merge 19 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for "functions kits" by adding the KitFunctionConfig schema, TypeScript types, validation logic, and corresponding unit tests. The review feedback identifies several critical issues and improvement opportunities: a missing runtime property in ValidatedKitSingle that causes TypeScript compilation errors, a regression in isLocalConfig that breaks kit deployment via requireLocal, a JSON schema generation issue when using Record<string, string> for instances, a validation bypass when kit is an empty string, and a lack of naming constraint validation for kit instance IDs.
a6f6df7 to
ad01fb4
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for 'Functions Kits' under a new experiment flag, updating the configuration schema, validation logic, and deployment helpers to handle kit configurations and instances. The reviewer feedback highlights a critical issue where excluding kits from isLocalConfig breaks deployment via requireLocal, and suggests stricter validation for kit properties and instance IDs. Additionally, the reviewer recommends replacing unsafe type assertions (as any and as unknown as) in the test files with safer alternatives to comply with the repository's style guide.
85bfd78 to
30285e0
Compare
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
5dd4284 to
f160329
Compare
c6824f9 to
3dc10f4
Compare
Adds support for Function Kits schema in `firebase.json` (`kit` stanza as a peer to `codebase`). This includes: - Added `KitSourcePackage` and `KitFunctionConfig` types to `src/firebaseConfig.ts`. - Updated `FunctionConfig` union to include `KitFunctionConfig`. - Added `ValidatedKitSingle` and validation logic in `src/functions/projectConfig.ts` to enforce: - Mutual exclusivity between `kit` and `codebase`/`remoteSource`/`prefix`. - Project-wide kit name and kit instance ID uniqueness across kit stanzas. - Mutual exclusivity between codebase names and kit instance IDs. - Updated `isLocalConfig` and added `isKitConfig` type guards. - Regenerated JSON schema (`schema/firebase-config.json`). - Validated parsing of valid Function Kit configurations. - Verified validation failures when `kit` is combined with `codebase`, `remoteSource`, or `prefix`. - Verified validation failure when `kit` is missing required `source`. - Verified duplicate `kit` name rejection across stanzas. - Verified duplicate kit instance ID rejection across stanzas. - Verified mutual exclusivity error when a codebase name matches a kit instance ID. - Verified type guard narrowing for `isLocalConfig`, `isRemoteConfig`, and `isKitConfig`. - `npm run generate:json-schema` - `npm run lint:changed-files` - `npx mocha src/functions/projectConfig.spec.ts`
This change sets instances as codebases in the backend, and adds a new "kit" identifier to further differentiate standard function codebases with function kits.
If a user specifies a target to deploy that a kit and instance share a name for, confirm with the user if they'd like to deploy the full kit or indiviual instance. The prompt will only appear for kits with more than one instance.
We decided that introducing "kits" filtering overcomplicates the parser.
55f5117 to
ae5380b
Compare
ajperel
left a comment
There was a problem hiding this comment.
Very nice! A few comments but all pretty minor.
ajperel
left a comment
There was a problem hiding this comment.
I don't think this is new to your PR, but Gemini also suggests we validate that configDir resolves to something in the project.
There's probably not a good reason to allow something like: "inst1": "../../../../some/other/repo/config"
d06f134 to
5e642a7
Compare
5e642a7 to
8203892
Compare
466ae98 to
cd1ee2a
Compare
this should not have been public in the first place
918926f to
b31a9e6
Compare
Description
Adds comprehensive support for Function Kits in
firebase.json(kitstanza as a peer tocodebase), including configuration parsing, validation, JSON schema updates, deployment handling, and emulator / local serving integration behind thekitsexperiment flag.Specifically, this PR:
src/firebaseConfig.ts,src/functions/projectConfig.ts):KitSourcePackageandKitFunctionConfigtypes, updating theFunctionConfigunion.ValidatedKitSingletype and validation logic insrc/functions/projectConfig.tsenforcing:kitandcodebase/remoteSource/prefix.instancesmap mapping instance IDs to configuration subdirectories (configDir).isLocalConfigandrequireLocal, and addsisKitConfigtype guards.configForCodebasehelper for resolving configurations by standard codebase name or kit instance ID.schema/firebase-config.json,src/experiments.ts):schema/firebase-config.json) to includekitstanzas with instance mapping.kitsexperiment flag (ALL_EXPERIMENTS.kits).src/deploy/functions/):functionsDeployHelper.tsandprepare.ts).kit-<instance-id>, automatically applyingkit-<instance-id>-to all function endpoint IDs during discovery inprepare.tsto isolate instances and prevent function name collisions.parseFunctionSelector) and target codebase resolution (targetCodebases) to recognize kit instance IDs alongside codebase names.src/emulator/controller.ts,src/serve/functions.ts):servecommands by instantiating anEmulatableBackendfor each kit instance with its assignedconfigDirandkit-<instance-id>prefix.Scenarios Tested
firebase deploy --only functionswith a mix of standardcodebasestanzas andkitstanzasfirebase deploy --only functions:<instance-id>firebase deploy --only functions:<instance-id>:<function-name>