google: Prevent GCE auth to hide S3 auth #921
Closed
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
GoogleAuthType._is_gce() is going to return True on any GCE instance, but if there are S3 credentials, they should be used.
|
seeking input from @crunk1 |
crunk1
approved these changes
Oct 24, 2016
|
LGTM. Sorry for the hassle! |
|
@crunk1 No worries! Thanks for your work on libcloud. |
|
@tonybaloney Anything else I should do? Thanks. |
|
@asfgit Anything I can do to help get this merged? Thanks. |
|
hey @pquentin back from travelling. let's get this merged. |
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.
Prevent GCE auth to hide S3 auth
Description
We currently authenticate to Google Cloud Storage using Amazon S3 compatibility auth. Our code runs in Kubernetes on Google Container Engine. We tried to upgrade libcloud recently but 3849f65 from @crunk1 prevented us to authenticate. (Interestingly, it's also the commit that made us want to upgrade, since we eventually want to use service accounts.)
The issue happened for two reasons:
GoogleAuthType._is_gce()always returns True when the code is run on the Google Container Engine, regardless of the authentication provided (which makes the issue impossible to reproduce in a local Docker environment)GoogleAuthType._is_gcs_s3()is always checked after_is_gce(), so it could not be used on Google Container EngineThis pull request simply changes the order to give S3 higher priority. Note that Installed Applications auth has lower priority still, because it's the default auth when everything else fails. That's OK because I guess it's not possible on GCE. Still, I think the documentation should recommend to always specify the auth type, because explicit is better than implicit and it helps to avoid unclear errors.
done, ready for review
Checklist (tick everything that applies)