-
Notifications
You must be signed in to change notification settings - Fork 217
Add defineJsonSecret API for structured secret configuration #1745
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
Conversation
Implements defineJsonSecret() to store JSON objects in Cloud Secret Manager. Useful for consolidating related secrets (e.g., API keys, webhooks, client IDs) into a single secret, reducing costs and improving organization. Features: - Automatic JSON parsing with error handling - Supports object destructuring - Throws on missing or invalid JSON Wire protocol changes (backward compatible): - Added optional format field to ParamSpec/WireParamSpec - JsonSecretParam.toSpec() returns format: "json" as CLI hint - Old CLIs ignore unknown fields, new CLIs can enhance UX - Format is NOT stored in Secret Manager (just in param spec)
Summary of ChangesHello @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 significant enhancement to secret management by providing a dedicated API, 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 introduces a new defineJsonSecret
API for handling structured JSON secrets, which is a valuable addition. The implementation is well-tested and follows existing patterns in the codebase. My review includes suggestions to improve the new API's type safety by making it generic, which would enhance the developer experience. I've also recommended clarifying the documentation regarding error handling.
aae7dd4
to
6e5d68b
Compare
/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 new defineJsonSecret
API for handling structured JSON secrets, which is a great addition for managing complex configurations. The implementation is solid, with good documentation and comprehensive tests covering various scenarios. I have a couple of minor suggestions to improve debuggability and documentation consistency.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
it("handles empty object JSON", () => { | ||
const jsonSecret = params.defineJsonSecret("EMPTY_OBJECT"); | ||
const value = jsonSecret.value(); | ||
expect(value).to.deep.equal({}); | ||
}); |
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.
is this the behavior we want? maybe we should error on empty JSON
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.
Given that user would've have to store {}
in SCM, I feel kinda okay not throwing.
Implements
defineJsonSecret()
to retrieve JSON objects in Cloud Secret Manager.Useful for consolidating related secrets (e.g., API keys, webhooks, client IDs) into a single secret, reducing costs and improving organization.
defineJsonSecret
is really just a string that the SDK will parse as JSON at runtime. If the value isundefined
or not a valid JSON, the call to.value()
will immediately throw.This change also introduces a backward compatible wire protocol changes (backward compatible). A new annotation
format
will be set tojson
for json secrets, and CLI can use it to validate the user input as valid JSON at deploy time.