-
Notifications
You must be signed in to change notification settings - Fork 96
[cryocloud] Add opencode and claude configuration to prod.values.yaml #8905
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
Merged
+56
−5
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a5ffe7e
Add opencode configuration to prod.values.yaml
tsnow03 64f40d3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] c7253fc
Add OPENCODE_CONFIG and update Claude settings
tsnow03 24d9fe3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 02162fc
Merge branch 'main' into patch-7
tsnow03 200ec14
Merge branch '2i2c-org:main' into patch-7
tsnow03 b332163
Delete claude setting from prod.values.yaml
tsnow03 1d4683f
Add encrypted key for cryocloud openai
choldgraf 61e30a7
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
I'm worried that putting the key to an env variable would make the API visible to all users of a hub (not on github, but in an authenticated user session), which feels like a security risk, but it seems like something we already do for other communities so I'm assuming it's fine - eg JupyterHealth has the same pattern:
https://github.com/search?q=repo%3A2i2c-org%2Finfrastructure+openai&type=code
@tsnow03 should probably know about this though! I think any user of the cryocloud hub will be able to view and potentially exfiltrate this key
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.
Yes, that is a risk. I'd like to find better ways to handle API keys, but it would likely require us to support something like using a Hub as an OIDC provider (which isn't yet supported).
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.
Yes, there is a general purpose application that I think will benefit the JupyterHub community that is an auth proxy that allows JupyterHub tokens to be used to access a REST API via a proxy which uses a credential to access the upstream API.
But in the absence of that, these shared credential approaches are okay in that you are sharing a single managed credential with all users. I talked to @tsnow03 about this a bit ago, and it's a question of trust and mitigation: to what extent do you trust the users of this hub with that credential (they can absolutely take this and use NRP LLM resources from their home machine), and mitigating that by rotating this key. An NRP LLM token is not the most precious resource because it is not billed and probably most cryocloud users would have access to it themselves if they bothered to go through the process of asking for an account, so there is little benefit to exfiltration in this particular case. If the resource protected by this token were either not likely to be available to most users of the hub and/or if there was a cost risk associated with abuse
Key rotation is not easily facilitated by storing the key in SOPS and requiring 2i2c folks to set the secret value, which Tasha has to retrieve from NRP, so this is likely to be infrequent, but can be done on a time scale Tasha feels is appropriate for mitigating that risk. For example, the hub this is copied from runs with a relatively small number of reasonably trusted users, but when we use it to host a workshop, for example, I rotate the keys immediately afterward.