-
Notifications
You must be signed in to change notification settings - Fork 29
workloadmanager: prefer ENV for Router public key with Secret fallback #171
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: main
Are you sure you want to change the base?
workloadmanager: prefer ENV for Router public key with Secret fallback #171
Conversation
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary of ChangesHello @aaradhychinche-alt, 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 significantly refactors the WorkloadManager's public key retrieval process, introducing a more efficient and flexible environment variable-first approach. This change addresses operational complexities and namespace coupling previously associated with relying solely on Kubernetes Secrets for the Router's public key. By providing a clear preference for environment variables while maintaining a robust fallback to Secret polling, the PR enhances system startup performance and simplifies deployment configurations, all while ensuring full backward compatibility for existing setups. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 introduces a new, preferred method for configuring the Router's public key via an environment variable, while maintaining backward compatibility by falling back to the existing Secret-based approach. The core logic changes in Go and the Helm chart updates are well-implemented. The new QUICK_REFERENCE.md is a great addition for usability.
My review focuses on a few key areas for improvement:
- Correctness: There's a bug in
loadPublicKeyFromEnvwhere an environment variable containing only whitespace is incorrectly treated as a valid key, which could lead to runtime failures. - Documentation Accuracy: The design document (
PicoD-Plain-Authentication-Design.md) has several inconsistencies with the actual implementation. It incorrectly refers toConfigMapinstead ofSecret, uses the wrong resource name, and misrepresents how the public key is injected into pods.
Addressing these points will improve the robustness and clarity of this new feature.
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Aaradhy Chinche <aaradhychinche@gmail.com>
f5b0322 to
14f8243
Compare
|
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages. The list of commits with invalid commit messages:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
=======================================
Coverage ? 34.19%
=======================================
Files ? 29
Lines ? 2544
Branches ? 0
=======================================
Hits ? 870
Misses ? 1551
Partials ? 123
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR completes the fix proposed in Issue #60 by making WorkloadManager prefer loading the Router public key from an environment variable, while keeping the existing Secret-based mechanism as a fallback for backward compatibility.
Previously, although PicoD already supported ENV-based public key loading and CodeInterpreter sandboxes received the key via environment variables, WorkloadManager still depended on polling the picod-router-identity Secret. This created namespace coupling and operational complexity in multi-controller setups.
Changes included
Core logic
Added AGENTCUBE_ROUTER_PUBLIC_KEY as the preferred source for Router’s public key.
Introduced ENV-first loading logic with fallback to Secret polling.
Added clear logging to indicate whether ENV or Secret was used.
Added workloadmanager.routerPublicKey to Helm values.
Updated WorkloadManager template to inject the ENV variable when configured.
Documentation :-
Updated PicoD authentication design docs to reflect the new ENV-first flow.
Added usage examples and migration guidance.
fixes #60