Drop proactive scope validation in credentials_gce. #185
Conversation
This file contains 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
Why? Avoid confusion like r-lib#161. When adding support for fetching credentials on a GCE VM, I had originally added a "guard rail": if the user was requesting scope(s) which weren't included in the list of scopes the service account was granted, I'd return `NULL`, letting the credential fetch fall back to another method (eg user creds). However, this can lead to confusion: most Google API calls are willing to accept various scopes, and some scopes imply other scopes (eg an API asking for a BQ scope will happily accept the cloud-platform scope). This change removes any scope validation in credentials_gce. On the upside, it should make it easier for users to use gargle from a GCE VM that *has* the required scopes. Unfortunately, it does allow one new bit of confusion: a user invoking gargle on a GCE VM *without* the scopes needed for an API would previously fall through to fetching user creds; they'll now need to do so explicitly.
|
Thanks for providing the context of the original decision and why we're OK with removing these checks. Sounds good to me. A gargle release was in flight (actually completed) before this PR came in, so, for better or worse, this will get a chance to be road-tested in the dev version for a little while. Hopefully those who care, such as the participants in #161, will install a dev version and accumulate some usage with the guard rail removed. |
|
I think this will fix an issue I've had trying to use google drive with gce auth. Looking forward to the next release! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why? Avoid confusion like #161.
When adding support for fetching credentials on a GCE VM, I had originally
added a "guard rail": if the user was requesting scope(s) which weren't
included in the list of scopes the service account was granted, I'd return
NULL, letting the credential fetch fall back to another method (eg usercreds).
However, this can lead to confusion: most Google API calls are willing to
accept various scopes, and some scopes imply other scopes (eg an API asking
for a BQ scope will happily accept the cloud-platform scope).
This change removes any scope validation in credentials_gce. On the upside, it
should make it easier for users to use gargle from a GCE VM that has the
required scopes. Unfortunately, it does allow one new bit of confusion: a user
invoking gargle on a GCE VM without the scopes needed for an API would
previously fall through to fetching user creds; they'll now need to do so
explicitly.
Fixes #161.