Skip to content
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

Remove the client secrets file for Google provider #452

Closed
wants to merge 7 commits into from

Conversation

stuntgoat
Copy link
Contributor

Terraform seems to work without the client secrets json file. I haven't run all the acceptance tests but I'm able to interact with the API.

@stuntgoat stuntgoat changed the title remove the client secrets file for Google provider providers/google: remove the client secrets file for Google provider Oct 19, 2014
@mitchellh
Copy link
Contributor

This is pretty interesting. I'm going to hold off for 0.3.1 to test it a bit more. Will get it in after that.

@stuntgoat
Copy link
Contributor Author

Sounds good. I've just removed the authURL and tokenURL.

@sparkprime
Copy link
Contributor

I don't even have an appengine service account on my newer GCP project. This could be because it has been discontinued, or it could be because I haven't set up appengine within that project. I could find out, but either way, it'd be confusing for users to be told by Terraform (and Packer) documentation to download a key from a non-existent service account. So it's a good idea to remove this. One service account is all you need to authenticate

@sethvargo sethvargo changed the title providers/google: remove the client secrets file for Google provider Remove the client secrets file for Google provider Nov 19, 2014
@sparkprime
Copy link
Contributor

FYI hashicorp/packer#1669
hashicorp/packer#1679

As far as I can tell, this PR and proposed changes to packer (by my colleagues) are consistent, which is good.

secrets file_.

4. Create a new OAuth client ID and select "Service account" as the type
3. Create a new OAuth client ID and select "Service account" as the type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small detail here: the packer documentation changes are slightly different (and more correct). This description should be changed to note that after creating the service account, the user is prompted to download a .p12 file first. After this, you need to generate a new JSON key. This key is that should be used for the account file.

It took me a few minutes to figure out just what I needed to do here. It would be nice to make the documentation clear that the first file that is downloaded is not what is needed. In fact, you can delete the p12 key since that won't be used by terraform.

@sparkprime
Copy link
Contributor

Hi again, any update on this? I'd like to work on #606 which is related.

svend pushed a commit to whitepages/terraform-provider-stingray that referenced this pull request Dec 11, 2014
svend pushed a commit to whitepages/terraform-provider-stingray that referenced this pull request Dec 11, 2014
@stuntgoat
Copy link
Contributor Author

I'm confused as to why tests are failing; they're passing locally.

@sparkprime
Copy link
Contributor

Looks like the test run got killed, maybe due to some transient problem with the travis hosting (out of RAM or whatever).

@grosskur
Copy link

I'd love to see this get merged. Could someone re-run the Travis build?

@phinze
Copy link
Contributor

phinze commented Jan 28, 2015

Retriggered! 🎸

@phinze
Copy link
Contributor

phinze commented Jan 28, 2015

Looks like the build was unsalvageable in its current state. I'm rebasing and testing this branch now - will push as a separate PR and link here. 🐫

@phinze phinze self-assigned this Jan 28, 2015
phinze pushed a commit that referenced this pull request Jan 28, 2015
phinze added a commit that referenced this pull request Jan 28, 2015
with this commit, the google compute instance acceptance tests are
passing

 - remove GOOGLE_CLIENT_FILE requirement from provider tests to finish
   out #452
 - skip extra "#" key that shows up in metadata maps, fixes #757 and
   sprouts #883 to figure out core issue
 - more verbose variablenames in metadata parsing, since it took me
   awhile to grok and i thought there might have been a shadowing bug in
   there for a minute. maybe someday when i'm a golang master i'll be
   smart enough to be comfortable with one-char varnames. :)
@phinze
Copy link
Contributor

phinze commented Jan 28, 2015

Closing this in favor of #884, (which squashes but retains commit authorship) - thanks for the contribution @stuntgoat! 🍬

@phinze phinze closed this Jan 28, 2015
phinze added a commit that referenced this pull request Jan 28, 2015
[REPACK] #452 providers/google: remove deprecated client secrets file
yahyapo pushed a commit to yahyapo/terraform that referenced this pull request Mar 13, 2015
yahyapo pushed a commit to yahyapo/terraform that referenced this pull request Mar 13, 2015
with this commit, the google compute instance acceptance tests are
passing

 - remove GOOGLE_CLIENT_FILE requirement from provider tests to finish
   out hashicorp#452
 - skip extra "#" key that shows up in metadata maps, fixes hashicorp#757 and
   sprouts hashicorp#883 to figure out core issue
 - more verbose variablenames in metadata parsing, since it took me
   awhile to grok and i thought there might have been a shadowing bug in
   there for a minute. maybe someday when i'm a golang master i'll be
   smart enough to be comfortable with one-char varnames. :)
@ghost
Copy link

ghost commented May 4, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants