-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix non-interactive deployment failure when secrets exist in Secret Manager #9370
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?
Fix non-interactive deployment failure when secrets exist in Secret Manager #9370
Conversation
…anager PR firebase#9335 added a check that fails deployments in non-interactive mode when secrets are required. However, it didn't verify whether those secrets already exist in Secret Manager, causing deployments to fail even when all secrets were properly configured. This change queries Secret Manager before throwing the error to check if each required secret exists. Only truly missing secrets will cause the deployment to fail. Fixes firebase#9368
Summary of ChangesHello @TorbenWetter, 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 resolves an issue where non-interactive deployments, such as those in CI/CD pipelines, would erroneously fail when secrets were required, even if those secrets were already properly configured in Secret Manager. The fix introduces a pre-check that verifies the existence of required secrets in Secret Manager, ensuring that deployments only halt for truly missing secrets and providing accurate error messages when necessary. This significantly improves the reliability of automated deployment workflows involving secrets. 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
|
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 effectively fixes a bug in non-interactive deployments where existing secrets would cause a failure. The logic to check for secrets in Secret Manager before throwing an error is correct, and the accompanying unit tests are comprehensive and cover the relevant scenarios well. I have one suggestion to improve the performance of the secret checking logic.
Use Promise.all() instead of sequential await calls to check multiple secrets in parallel, improving performance when multiple secrets need to be validated.
Fixes #9368
Problem
PR #9335 added a check that fails deployments in non-interactive mode (e.g., GitHub Actions, CI/CD) when secrets are required. However, it didn't verify whether those secrets already exist in Secret Manager, causing deployments to fail even when all secrets were properly configured.
Solution
This PR modifies the non-interactive mode check to query Secret Manager before throwing an error. It checks if each required secret exists using
secretManager.getSecretMetadata(). Only truly missing secrets will cause the deployment to fail.Changes
src/deploy/functions/params.tsto check Secret Manager for existing secrets before throwing non-interactive errorTesting
src/deploy/functions/params.spec.tsImpact
This fix allows non-interactive deployments (CI/CD pipelines) to succeed when secrets are already configured in Secret Manager, while still providing helpful error messages when secrets are truly missing.