[Feat] Env from secret #641
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @redno2, 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 feature that allows users to inject environment variables into containers directly from an existing Kubernetes Secret. This enhancement simplifies configuration by enabling the mounting of all key/value pairs from a specified Secret, eliminating the need for manual, individual environment variable definitions. The change involves modifying the deployment template to conditionally include the "envFrom" block and updating the Helm values documentation to reflect this new capability.
Highlights
- Kubernetes Environment Variables: Implemented support for "envFromSecret" in Helm charts, enabling the injection of all key/value pairs from a Kubernetes Secret directly into container environment variables.
- Helm Chart Configuration: Updated "values.yaml" with documentation and an example for the new "envFromSecret" configuration option, detailing how to reference an existing Kubernetes Secret.
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.
Code Review
This pull request adds a useful feature to load environment variables from a Kubernetes secret using envFromSecret. The implementation in the Helm template has a critical issue where it can generate an invalid Kubernetes manifest due to a duplicate envFrom key. Additionally, the new configuration option should be added to values.schema.json for validation and better developer experience. I've provided comments with details on how to address these points.
| {{- if $modelSpec.envFromSecret }} | ||
| envFrom: | ||
| - secretRef: | ||
| name: {{ $modelSpec.envFromSecret.name }} | ||
| {{- end }} |
There was a problem hiding this comment.
This new envFrom section will conflict with an existing envFrom on line 342 if both are enabled, leading to an invalid Kubernetes manifest due to a duplicate key. The envFrom sources should be consolidated under a single envFrom key. Please move this logic and merge it with the existing envFrom block around line 342.
| # - envFromSecret: (optional) Reference to an existing Kubernetes Secret from which | ||
| # all key/value pairs will be loaded as environment variables into the container. | ||
| # Example: | ||
| # envFromSecret: | ||
| # name: s3-registry | ||
| # | ||
| # The referenced Secret could look like: | ||
| # apiVersion: v1 | ||
| # kind: Secret | ||
| # metadata: | ||
| # name: s3-registry | ||
| # namespace: vllm | ||
| # data: | ||
| # AWS_ACCESS_KEY_ID: <base64-encoded-value> | ||
| # AWS_SECRET_ACCESS_KEY: <base64-encoded-value> | ||
| # AWS_ENDPOINT_URL: <base64-encoded-value> |
There was a problem hiding this comment.
The new envFromSecret option is a great addition. To ensure validation and provide better editor support (like autocompletion), it should also be added to the helm/values.schema.json file under servingEngineSpec.modelSpec.items.properties.
Here's a suggested schema definition:
"envFromSecret": {
"type": "object",
"properties": {
"name": {
"type": "string"
}
},
"required": ["name"]
}
|
@redno2 can you fix the DCO issue? |
Signed-off-by: Redno2 <redno2@users.noreply.github.com>
Signed-off-by: Redno2 <redno2@users.noreply.github.com>
Signed-off-by: Redno2 <redno2@users.noreply.github.com>
Signed-off-by: Redno2 <redno2@users.noreply.github.com>
[Feat] Add support for loading environment variables from an existing Kubernetes Secret via envFromSecret in values.yaml.
This allows users to mount all key/value pairs from a Secret as container environment variables without manually specifying each one.
Example usage in values.yaml: