-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: add extension.yaml to Functions Build converter #8990
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
base: master
Are you sure you want to change the base?
Conversation
### Description Implements an adapter that converts Firebase Extensions (extension.yaml) to Functions Build format, enabling deployment of extensions using `firebase deploy --only functions`. ### Key Changes - Add extensionAdapter module to convert extension.yaml to Build objects - Support for all extension resource types (v1 and v2 functions) - Parameter reference conversion from ${param:X} to CEL {{ params.X }} - Comprehensive golden tests with 7 test fixtures - Fix typo in types: maxConcurrentDispatchs → maxConcurrentDispatches - Add resourceType field to Param interface for SELECT_RESOURCE params ### Scenarios Tested - Simple HTTPS functions - Firestore triggers with parameter references - Scheduled functions - Task queue functions with rate limits - v2 functions with event filters - Storage resize extension with complex parameters - Various parameter types (string, select, multiSelect, secret) ### Sample Commands ```bash # Run tests npx mocha lib/deploy/functions/extensionAdapter.golden.spec.js # Future usage (after integration): firebase deploy --only functions # Will auto-detect extension.yaml ``` This is the foundation for supporting extension.yaml files in Functions deployment. Next steps include integrating into Node.js runtime discovery.
- Rename extensionAdapter.golden.spec.ts to extensionAdapter.spec.ts - Remove unnecessary comments - Remove error silencing in getFixtures() for explicit test failures
- Remove unnecessary type guard for parameter references - Use existing k8s.mebibytes() utility instead of duplicating memory parsing - Remove duplicate parseMemory() function - Simplify parameter reference checks to inline conditions - Run formatter to clean up code style
- Extract trigger building into separate functions (buildV1Trigger, buildV2Trigger) - Create unified createEndpoint function that handles both v1 and v2 - Remove all 'any' types, use proper type narrowing instead - Simplify convertResources to use single endpoint creation path - Reduce code duplication while maintaining type safety
- Extract helper functions: buildOptions, buildTextInput, convertParam - Replace nested if-else chain with cleaner switch statement - Use functional approach with map instead of for loop - Separate concerns into smaller, focused functions - Make code more maintainable and easier to extend
- Extensions already specify runtime per resource in the YAML - Remove unnecessary runtime parameter from all function signatures - Use runtime directly from extension resource properties - Add validation to ensure runtime is specified for each function - Remove Build-level runtime since each endpoint has its own - Update tests and expected files accordingly
- Remove unused sourceDir parameter from detectAndAdaptExtension - Add deployment-specific validation (name, version, resources required) - Extract magic string to DEFAULT_RESOURCE_TYPE constant - Add comprehensive JSDoc for public API - Update tests to match new function signature Note: Using custom validation instead of validateSpec() since that's designed for publishing extensions (requires license, etc.) while we only need basic validation for deployment.
- Remove verbose displayIAMRoles function - Replace with simple log message for IAM roles - Add clear TODO comments for future IAM integration - Add TODO for lifecycle events support - Remove unused Role import These features aren't critical for initial implementation and can be added when needed for full extension compatibility.
Reduced extensionAdapter.ts from 534 to 346 lines (35% reduction) by: - Leveraging readExtensionYaml from specHelper for parsing - Using getResourceRuntime and DEFAULT_RUNTIME from existing utils - Removing duplicate runtime extraction logic - Cleaning up trigger handling without hacky delete operations - Fixing TypeScript types to avoid any casts - Consolidating v1/v2 logic into single createEndpoint function All tests pass and functionality remains identical.
- Use proto.copyIfPresent and proto.renameIfPresent instead of manual null checks - Remove unnecessary inline comments - Revise JSDoc comments to describe WHAT not WHY
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @taeold, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new converter that transforms Firebase Extensions defined in extension.yaml into the Functions Build format. This is a crucial step towards enabling the deployment of Firebase Extensions directly through the Firebase CLI, laying the groundwork for future integration with the deploy command.
Highlights
- Core Conversion Logic: A new file extensionAdapter.ts is added, responsible for converting extension.yaml specifications into the Functions Build format.
- Parameter Syntax Transformation: The converter handles the transformation of extension parameter syntax (e.g., ${param:NAME} or ${NAME}) into Common Expression Language (CEL) expressions ({{ params.NAME }}), which are used for later resolution.
- Comprehensive Trigger Support: The adapter supports all major Cloud Functions trigger types, including HTTPS, Event, Schedule, and Task Queue triggers, for both v1 and v2 functions.
- Code Reusability: The implementation leverages existing utility functions from specHelper and proto modules, ensuring a clean and efficient codebase.
- Extensive Test Coverage: A new test file extensionAdapter.spec.ts is introduced, along with 8 golden test fixtures, to thoroughly validate the conversion logic across various extension.yaml patterns.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a converter to transform extension.yaml
files into the Functions Build format, which is a great step towards supporting extension deployments. The implementation is comprehensive, covering various trigger types and parameter configurations for both v1 and v2 functions. The addition of a thorough golden test suite is excellent for ensuring correctness. My review includes a few suggestions to improve robustness and type safety in the new adapter logic.
…tests - Fix parameter substitution checks to verify both opening and closing braces - Add radix parameter to parseInt for memory parsing - Replace 'any' type with proper TextInput type from params module - Add support for parsing memory values in IEC notation (1GiB, 512MiB) - Extensions use IEC notation while k8s.mebibytes expects Kubernetes notation - Convert by stripping 'B' suffix (both represent same binary values) - Add test coverage for 1GiB memory edge case - Add test coverage for Firestore trigger with match-path-pattern operator
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a well-structured and thoroughly tested converter to transform extension.yaml
files into the Functions Build format. The implementation is comprehensive, covering various function triggers and parameter types for both v1 and v2 functions. My review includes two main points: one to enhance robustness by adding validation for a required parameter property to prevent silent misconfigurations, and another to improve code clarity by simplifying some conditional logic and removing redundant code. Overall, this is a solid contribution that lays important groundwork for future extension deployment capabilities.
Major improvements: - Refactored monster createEndpoint function into createV1Endpoint and createV2Endpoint - Extracted parseMemoryToMb and parseTimeout helper functions for better reusability - Fixed type issues: parseTimeout accepts string only, return types use Field<number> - Better use of proto.* helpers (convertIfPresent, copyIfPresent, renameIfPresent) - Removed DEFAULT_RUNTIME import from emulator, use supported.latest("nodejs") directly - Improved type safety throughout - removed all type assertions - Fixed lint warnings: removed unnecessary string assertions, proper error handling without any The code is now more maintainable, follows established patterns in the codebase, and has proper type safety without cheating the type system.
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a converter to transform extension.yaml
files into the Functions Build format, which is a significant step towards supporting extension deployments. The implementation is comprehensive, covering various trigger types, parameter types, and both v1 and v2 functions. The addition of a thorough golden test suite is excellent for ensuring correctness. My review includes a few suggestions for refactoring to improve code consistency and maintainability, and to address a potential issue with a fallback value.
- Use more robust regex for hasParamReference to avoid false positives - Simplify v1 endpoint to use proto.convertIfPresent for all field conversions - Simplify v2 endpoint min/max instances handling with proto.convertIfPresent - Fix parseTimeout function to be actually used (was previously unused) - Remove incorrect default for selectResource resourceType - it's a required field and should error if missing, not silently use a default All tests passing and no lint warnings.
- Remove redundant file system check - directly try reading extension.yaml and handle ENOENT gracefully (one I/O operation instead of two) - Remove path import as it's no longer needed - Properly wrap errors with context when extension.yaml read fails - Remove obvious/redundant comments throughout the code - Keep only meaningful comments (IEC notation explanation, TODOs) - Fix all lint and formatting issues This completes all the code review feedback. The code is now cleaner, more efficient, and follows best practices.
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a converter to transform extension.yaml
files into the Functions Build format, which is a great step towards supporting extension deployments. The implementation is comprehensive, covering various trigger types for both v1 and v2 functions, and correctly handles parameter substitutions. The accompanying test suite with golden fixtures is also well-designed.
I've identified a couple of areas for improvement to enhance the robustness of the tests and the clarity of the implementation. My comments focus on making a test case less brittle and improving the precision of a condition for parameter processing.
Add default database and namespace filters to Firestore v2 event triggers when not explicitly provided, matching the emulator's behavior.
Update the expected output to include the namespace field that's now automatically added for Firestore v2 triggers.
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a converter to transform Firebase Extensions (extension.yaml) into the Functions Build format, which is a crucial step towards supporting extension deployment. The implementation is well-designed, handling various function versions (v1/v2), trigger types, and parameter substitutions. The inclusion of a comprehensive suite of golden tests is particularly commendable as it ensures the correctness of this complex transformation logic. The code is clean and follows existing patterns well. I have one minor suggestion for improving code clarity.
The build.empty() function already initializes endpoints to an empty object, so the explicit assignment is unnecessary.
- Extension multiSelect params now properly map to list type instead of string type - Default values are converted from comma-separated strings to arrays - Updated test fixtures to reflect correct behavior - Fixes 'Non-list params cannot have multi selector inputs' error
- Added detection for select params with all numeric values (e.g., FUNCTION_MEMORY) - These are converted to IntParam to allow proper type coercion in numeric contexts - Fixes 'Illegal type coercion' error when using FUNCTION_MEMORY with availableMemoryMb - Legacy pattern used by ~4% of extensions, later replaced by system params - Added test case for numeric select param conversion
…n adapter - Documented how system params work (auto-generated, advanced, not env vars) - Listed common system params (location, memory, timeout, etc.) - Explained differences between v1 and v2 functions (memory units) - Noted that full system param support would require detecting function types and user configuration - For local testing, we rely on properties defaults or platform defaults
Summary
Adds a converter to transform Firebase Extensions (extension.yaml) into Functions Build format, laying groundwork for future extension deployment support.
Key Changes
extensionAdapter.ts
to convert extension.yaml to Functions Build format${param:NAME}
to CEL expressions{{ params.NAME }}
for later resolutionspecHelper
andproto
for clean implementation (~330 lines)Test Plan
Note
This PR adds the conversion capability only. Integration with the deploy command will come in a future PR.