You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2016/11/10 14:56:10 UTC
[jclouds/jclouds-karaf] Take into account the credentials file in GCE
(#87)
When configuring the credential for GCE, you can pass the key in PEM format as a String, or you can pass (for conevnience) a JSON file with the credentials. This was [enabled in all compute commands](https://github.com/jclouds/jclouds-karaf/blob/master/commands/src/main/java/org/jclouds/karaf/commands/compute/ComputeCommandWithOptions.java#L90-L92) but was not taken into account in reusable compute services.
This change allows reusable compute services to use the credentials file in addition to the PEM string.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-karaf/pull/87
-- Commit Summary --
* Also take into account the credentials file in GCE
-- File Changes --
M commands/src/main/java/org/jclouds/karaf/commands/compute/ComputeServiceCreateCommand.java (5)
-- Patch Links --
https://github.com/jclouds/jclouds-karaf/pull/87.patch
https://github.com/jclouds/jclouds-karaf/pull/87.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Ignasi Barrera <no...@github.com>.
@nacx pushed 1 commit.
0e7b7e8 Explicit method name to read the Google Cloud credentials
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87/files/bfe265677f5f072008d6f4825b056ae8914673e4..0e7b7e85af789ce8055d0175e2e6aa3feb6d9b30
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> String endpointValue = EnvHelper.getComputeEndpoint(endpoint);
+ String credentialValue = EnvHelper.getComputeCredential(credential);
+ if (providerValue != null && credentialValue != null && providerValue.equals("google-compute-engine")) {
+ credentialValue = EnvHelper.getGoogleCredentialFromJsonFile(credentialValue);
There is no property in the provider. There is a property used in the tests, to denote the json file, but when instantiating the google provider, if you cant to use the json file you need to pass the [GoogleCredentialsFromJson](https://github.com/jclouds/jclouds/blob/master/common/googlecloud/src/main/java/org/jclouds/googlecloud/GoogleCredentialsFromJson.java) credential supplier yourself instead of the credential when creating the context.
>Also, as a minor change, check credentialValue != null first?
Done!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Andrew Phillips <no...@github.com>.
Thanks for the update, @nacx! +1 - looks good to go to me.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87#issuecomment-262151770
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Ignasi Barrera <no...@github.com>.
Squashed and pushed to master as [e22b0613](http://git-wip-us.apache.org/repos/asf/jclouds-karaf/commit/e22b0613).
@andrewgaul I've now understood your comment :) and probably we need to add the file logic [here](https://github.com/jclouds/jclouds-karaf/blob/master/commands/src/main/java/org/jclouds/karaf/commands/blobstore/BlobStoreServiceCreateCommand.java#L149) too. I've just seen it after merging so I'll open another PR to add it there.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87#issuecomment-262231747
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Ignasi Barrera <no...@github.com>.
> Does google-cloud-storage need a similar fix?
Changed the condition to cover GCS too.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87#issuecomment-261916055
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Andrew Phillips <no...@github.com>.
demobox commented on this pull request.
> String endpointValue = EnvHelper.getComputeEndpoint(endpoint);
+ String credentialValue = EnvHelper.getComputeCredential(credential);
+ if (providerValue != null && credentialValue != null && providerValue.equals("google-compute-engine")) {
+ credentialValue = EnvHelper.getGoogleCredentialFromJsonFile(credentialValue);
If I understand this correctly, we're effectively adding fallback logic - "try normal first, if that doesn't work, try the JSON file". Is that the same sequence as in the provide? Is there some property that we could use to determine up front which of the two options is being used?
Also, as a minor change, check `credentialValue != null` first?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87#pullrequestreview-8251044
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Andrew Gaul <no...@github.com>.
Does google-cloud-storage need a similar fix?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87#issuecomment-259763915
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Ignasi Barrera <no...@github.com>.
Closed #87.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87#event-868121344
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Andrew Phillips <no...@github.com>.
demobox commented on this pull request.
> * @return
*/
- public static String getGoogleCredentialFromJsonFile(String jsonFile) {
- try {
- String fileContents = Files.toString(new File(jsonFile), Charsets.UTF_8);
+ public static String getGoogleCredentialFromJsonFile(String credentialValue) {
A minor point: the way the method name reads now, I think it's not clear that the method only _tries_ to check if `credentialValue` is a file name, and returns the input value if not. I would assume from the name that it would _always_ read from a file, and e.g. fail if the file is not present.
How about something like:
```
public static String getGoogleCredentialFromJsonFileIfPath(String credentialValueOrPath) { // or "filePathOrCredentialValue"?
...
}
```
Then, where it's used, I think it's a little clearer what is actually happening.
Alternatively, add a comment in places where the method is called? What do you think, @nacx?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87#pullrequestreview-9454899
Re: [jclouds/jclouds-karaf] Take into account the credentials file in
GCE (#87)
Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.
> * @return
*/
- public static String getGoogleCredentialFromJsonFile(String jsonFile) {
- try {
- String fileContents = Files.toString(new File(jsonFile), Charsets.UTF_8);
+ public static String getGoogleCredentialFromJsonFile(String credentialValue) {
Much better :) Done!
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-karaf/pull/87