-
Notifications
You must be signed in to change notification settings - Fork 1k
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
SDKConfig doesn't work past gcloud v179.0.0 #300
Comments
Looking at the current interface for the That only gives the access token and expiry, so it seems like the SDKConfig should be changed to implement a custom token source that invokes that gcloud command to get the access token/expiry, and then pass that token source in when constructing the transport for the HTTP client. The This would follow a similar pattern used for getting tokens from a metadata server, so I think there is a reasonable precedent for it. |
That seems possible, shouldn't be too hard to implement either. I'm a bit reluctant though given this:
At least according to the help of this command, we can't actually trust its output won't change or for it to stick around, even though the description seems to suggest otherwise. Given that apparently anything can change/disappear and without notice, should |
@daenney I think whether or not this support should be included at all is a valid question. Until that is decided, however, I think changing to use |
The existing implementation of SDKConfig assumes that the Cloud SDK writes a config file named `credentials`, which contains JSON-encoded OAuth2 credentials for the user. Since version 161.0.0 of the Cloud SDK, that has not been true, as the Cloud SDK has switched to storing credentials in a SQLite database instead of a JSON-encoded file. This change switches from trying to directly read the underlying credential store (which it is now clear can change without notice), to instead invoking the `gcloud` command and asking it for its credentials. This is done using the subcommand ```sh gcloud config config-helper ``` That subcommand is meant to provide auth data to external tools, so it seems to be a more appropriate choice than reading the underlying storage directly. This fixes golang#300
Agree with @ojarjur here that #301 is a clear improvement over the current state, where I'd argue for either fixing it, or dropping the functionality altogether. The current state is subpar— I just spent time implementing functionality around @daenney Were you ever able to find a long-term fix? |
I'm afraid not. I got away from the problem in that I never ended up completing the tool I was working on. I got stuck on this and then other priorities took over. Not having a stable interface to the credentials is really annoying though from a developer point of view. I either have to implement a full oauth2 flow in my CLI app and put the users through it, or use SDKConfig and hope the teams at Google won't change it up again anytime soon. I'm kinda torn here, I prefer not having to do the work of implementing the oauth2 flow, and I like the idea of having an updated SDKConfig, but I dread the day a team at Google decides to change this again b/c then we're right back where we started. It feels better to just drop SDKConfig in my opinion instead of having it be this moving target, and especially one that's not getting the maintenance it needs. Looking at #301 and it's associated https://go-review.googlesource.com/c/oauth2/+/121939#message-a9faec17665d2fab7e31a69d24a5295c05f1dc0e it seems nobody from the Go team ended up reviewing this either, so it's still stuck in limbo. |
Yeah, the status quo is a bit of a bummer. I'm working an internal CLI, and it would be nice to not force a user to login again if they've already run That said, I just found this project and perhaps it's an option: Apparently it implements the oauth2 flow. |
Nice find! That seems like a better way to go about it indeed. I suppose once that stabilizes it might make sense to have SDKConfig leverage that instead. |
In newer versions of gcloud SDK the credentials are no longer stored in a flat file, but in a sqlite database. However,
(New)SDKConfig
is completely unaware of this change and breaks on newer versions of the gcloud SDK even though you've managed to correctlygcloud auth login
and the account is properly shown as active.The current approach appears to be to downgrade to 179.0.0, execute
gcloud config set auth/use_sqlite_store false
and then do agcloud auth login
again. Now upgrading past that version appears to work and maintain the setting.Though this makes it work currently this is not a viable long-term solution and not something I can implement as part of a CLI tool (or continuously direct people to do). SDKConfig should be able to handle the new sqlite based storage format that gcloud SDK has started to use.
The text was updated successfully, but these errors were encountered: