-
Notifications
You must be signed in to change notification settings - Fork 1k
chore: refactor instance identity to be a SessionTokenProvider #19566
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
This stack of pull requests is managed by Graphite. Learn more about stacking. |
1b453cd
to
421347b
Compare
6f7bb8d
to
ad7200f
Compare
421347b
to
7b1affe
Compare
7789799
to
d9ee61f
Compare
7b1affe
to
a13a334
Compare
ad45ac9
to
ce4943c
Compare
53e1b76
to
7ea81d2
Compare
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.
For the most part, this looks like a good and a useful refactor 👍🏻.
I'd like to see something done about the root command logic/flag, however (see comment). Other than that just minor suggestions.
@@ -0,0 +1,97 @@ | |||
package agentsdk |
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.
Suggestion (optional): Consider moving into sub-package, assuming there aren't any blocking dependencies to this package that would cause a circular import.
ce4943c
to
608b392
Compare
7ea81d2
to
192c81e
Compare
59667e7
to
54f6878
Compare
54f6878
to
6cddf93
Compare
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.
This turned out nice, and love the explicit flag docs this gives us, thanks for implementing! Two minor suggestions but nothing blocking
@@ -220,7 +224,7 @@ func (r *RootCmd) Command(subcommands []*serpent.Command) (*serpent.Command, err | |||
// with a `gitaskpass` subcommand, we override the entrypoint | |||
// to check if the command was invoked. | |||
if gitauth.CheckCommand(i.Args, i.Environ.ToOS()) { | |||
return r.gitAskpass().Handler(i) | |||
return gitAskpass(hiddenAgentAuth).Handler(i) |
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.
Nice solution 👍🏻
6cddf93
to
68c9194
Compare
Merge activity
|
Refactors Agent instance identity to be a SessionTokenProvider.
Refactors the CLI to create Agent clients via a centralized function, rather than add-hoc via individual command handlers and their flags.
This allows commands besides
coder agent
, but which still use the agent identity, to support instance identity authentication.Fixes #19111 by unifying all API requests to go thru the SessionTokenProvider for auth credentials.