-
Notifications
You must be signed in to change notification settings - Fork 3
devboxes support (no API Key) #277
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
fixed devbox list added devbox delete adapted sandbox apply adapted local override adapted traffic record
| return dsm, nil | ||
| } | ||
|
|
||
| func createAPIClient(ciConfig *config.ConnectInvocationConfig, authInfo *auth.ResolvedAuth) (*client.SignadotAPI, error) { |
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.
Why can't we use the normal API client and we need a custom one? (note that we do auth refresh in the regular one)
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.
- The normal API client only refreshes tokens for KeyringAuthSource (line 94 in api.go), not PlainTextAuthSource, even
though refreshKeyringAuth() supports both. - The normal API client requires OrgName upfront (line 85-87), which may not be available initially.
- The normal API client doesn't support fallback to ciConfig.APIKey.
this was put in devbox pkg to isolate the changes without introducing a new refactoring.
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.
probably should refactor to fix the bug in the original in 1...
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.
If you want to do that later on a separate PR, I'm fine with it.
daniel-de-vera
left a comment
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.
LGTM
* add skeleton for devbox * devboxes support (no API Key) (#277) * checkpoint local connect with devboxes * plumbing status and released detection * add released detection in sbmgr fixed devbox list added devbox delete adapted sandbox apply adapted local override adapted traffic record * misc fixes to list, register * incorporate api key changes and merged metadata field * use ValidateSession * nilness-uniformity * CR: - local connect --devbox - nilness in sbmgr - using local status in override * use go-sdk w/ devbox support on its main branch --------- Co-authored-by: Scott Cotton <scott@signadot.com> Co-authored-by: Scott Cotton <scott@mindowl.com>
Depends on signadot/go-sdk#75
And depends on https://github.com/signadot/signadot/pull/6388 for sandbox apply, local override, traffic record
stacked on #276
Summary
Adds devbox support for user authentication via bearer token
Key Changes
New Devbox Commands
signadot devbox register- Register a new devbox with optional name and claim flagssignadot devbox list- List devboxes (with--allflag to show all devboxes)signadot devbox delete- Delete a devbox (immediate deletion, no wait flags)Devbox Session Management
internal/devbox/session_manager.go) - Manages devbox session lifecycle with automatic renewal~/.signadot/.devbox-idto avoid re-registrationRelease Detection in Sandbox Manager
signadot local statuscan report the released statesignadot local statusdisplays devbox session health, including when a session has been released, showing the error reason and timestampIntegration Updates
Protocol & Status Updates
DevboxSessionStatusproto message with health, release status, and error trackingConfiguration
Devboxconfig struct with sub-configs for list, register, and delete commandsConnectInvocationConfignow includesDevboxIDandDevboxSessionIDfieldsTechnical Details
Testing Notes
signadot local statusshows devbox session status including release detectionKnown TODOS
Aside from API Key support, this requires some additional cleanup of the grpc protos -- removing RegisterSandbox.
Another round of sdk update for labels vs idmeta to be ironed out in a subsequent iteration.