Update functions CLI to support deploying onGraphRequest functions.
#9612
Conversation
Summary of ChangesHello @rosalyntan, 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 enhances the Firebase Functions CLI to properly support the deployment of Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for deploying onGraphRequest functions by introducing the necessary IAM bindings for Firebase Data Connect. The changes include adding logic to obtain the Data Connect service agent and grant it the Cloud Run Invoker role. My review includes suggestions to improve code clarity and maintainability, such as refactoring to avoid magic strings and using async functions appropriately.
| } else if (isDataConnectGraphqlTriggered(endpoint)) { | ||
| const dataConnectGraphqlTrigger: backend.DataConnectGraphqlTrigger = {}; | ||
| if (endpoint.dataConnectGraphqlTrigger.invoker === null) { | ||
| dataConnectGraphqlTrigger.invoker = null; |
There was a problem hiding this comment.
this would end up clearing up the invoker setting to "private", including getting rid of access for FDC P4SA 🤔 wondering if that's what we want
There was a problem hiding this comment.
I think this is addressed with my changes in fabricator.ts -- what is a test case that would trigger this? I've verified that removing the invoker from the options when redeploying does not remove access for the FDC P4SA.
| const httpsFunctions = backend.allEndpoints(results).filter(backend.isHttpsTriggered); | ||
| const httpsFunctions = backend | ||
| .allEndpoints(results) | ||
| .filter(backend.isHttpsTriggered || backend.isDataConnectGraphqlTriggered); |
There was a problem hiding this comment.
i don't think it's useful to print trigger url for FDC urls, esp. since it's not meant to be a public function.
There was a problem hiding this comment.
We need the URL printed so developers can copy-paste it into their dataconnect.yaml files. However, I realized that this logic wasn't actually working, so updated with a fix. In addition, it seems like only the non-deterministic run.app URL gets returned from the backend, so I manually construct the deterministic URL here (it's a little hacky).
2 participants
Description
This adds support to the Firebase Functions CLI to deploy
onGraphRequestfunctions. The deployed function will by default be IAM restricted to the Firebase Data Connect P4SA, with any explicitly specified invokers also granted access.In addition, if
schemaFilePathis specified in theonGraphRequestoptions, the CLI will pull in the schema GQL source files atschemaFilePathand bundle them with the functions deploy payload.Scenarios Tested
Sample Commands