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

Get project ID from JSON credentials file #1813

Merged

Conversation

daspecster
Copy link
Contributor

This is a possible resolution to #1792, making it easier to follow along with the documentation.

I looked for a way to clarify the documentation for this but it looks like this should make it work in the way that the documentation shows already.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 23, 2016
@jgeewax
Copy link
Contributor

jgeewax commented May 23, 2016

Do we have tests that make sure the gcloud auth login case works as expected?

In other words, we need to follow the same policy of grabbing project IDs, trying the following in order to grab project ID from...

  1. explicitly in code
  2. the environment variable
  3. gcloud's environment (gcloud config list project)
  4. the service account credentials
    • hopefully this covers the GCE / GAE scenario

(I think that's the right order at least...)

import os
import tempfile
with tempfile.NamedTemporaryFile() as credential_file:
credential_file.write('{"project_id": "test-project-id"}')

This comment was marked as spam.

This comment was marked as spam.

@daspecster
Copy link
Contributor Author

To get this to work with the happy path that @jgeewax mentioned(gcloud auth login), what would be the best way to programatically access gcloud config list project which I believe is reading the project key from ~/.config/gcloud/configurations/config_default?

@dhermes
Copy link
Contributor

dhermes commented May 23, 2016

I'm not sure. @jgeewax do we have a gcloud CLI point person that can tell us? @daspecster we could assume that it is correct, or we could just use a system call to get the project?

As for putting it with auth, we can at least agree that this is configuration and not auth? So we should work to find a place. We usually discuss config in each usage doc, but you could start a new gcloud-config doc and then refer to it from all the usage docs.

@daspecster
Copy link
Contributor Author

I can do the gcloud config list project system call for now and if there's a better way we can find it.

Ok cool, thanks @dhermes! I'll make a configuration doc.

@daspecster daspecster force-pushed the project-id-from-credentials-file branch 3 times, most recently from 170dc66 to 5125599 Compare May 25, 2016 16:02
:rtype: str or ``NoneType``
:returns: Project-ID from ``gcloud info`` else ``None``
"""
command = subprocess.Popen(['gcloud info'],

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@daspecster
Copy link
Contributor Author

@dhermes @tseaver @jonparrott
Do we want to try and use something like this or drop it?
I'm not sure who to ask about properly accessing gcloud environment properties programmatically?

@dhermes
Copy link
Contributor

dhermes commented May 27, 2016

Not sure either.

  1. You should just use ${HOME}/.config/gcloud/configurations/config_default if it exists. You could use oauth2client.client._get_well_known_file. If the file isn't there or you can't parse it, don't fail for the user and just silently move on.
  2. For a service account, the project ID is in the JSON key file, so you can just use the GOOGLE_APPLICATION_CREDENTIALS env. var. (this is what "Project was not passed" error, even though ran  #1792 is actually asking for)
$ cat keyfile.json
{
  "type": "service_account",
  "project_id": "PROJECT",
  "private_key_id": "...",
  "private_key": "-----BEGIN PRIVATE KEY-----...-----END PRIVATE KEY-----\n",
  "client_email": "foo@PROJECT.iam.gserviceaccount.com",
  "client_id": "...",
  "auth_uri": "https://accounts.google.com/o/oauth2/auth",
  "token_uri": "https://accounts.google.com/o/oauth2/token",
  "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
  "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/foo%40PROJECT.iam.gserviceaccount.com"
}

@daspecster
Copy link
Contributor Author

@dhermes, so the only issue with grabbing it from GOOGLE_APPLICATION_CREDENTIALS, is that the GOOGLE_APPLICATION_CREDENTIALS environment variable isn't set when you just login with gcloud auth login.

Also, is that default file location the same for windows and linux both?

I saw oauth2client.client._get_well_known_file, but I also noticed this comment in that method.

TODO(orestica): Revisit this method once gcloud provides a better way

of pinpointing the exact location of the file.

So I wasn't sure what the story was there.

But I think using _get_well_known_file might be the best option if it works after gcloud auth login.

@dhermes
Copy link
Contributor

dhermes commented May 27, 2016

@daspecster There should be a cascade of checks here just like there is for credentials. (i.e. so the absence of GOOGLE_APPLICATION_CREDENTIALS just means you move on to the next check)

@daspecster daspecster force-pushed the project-id-from-credentials-file branch from 43157db to 4c686be Compare May 27, 2016 19:07
@dhermes
Copy link
Contributor

dhermes commented May 27, 2016

Also using _get_well_known_file (${HOME}/.config/gcloud/application_default_credentials.json) won't help get the file you want (${HOME}/.config/gcloud/configurations/config_default)

@daspecster
Copy link
Contributor Author

Also, if I look back at #1792, the flow that happened there, GOOGLE_APPLICATION_CREDENTIALS wasn't set.

I can add that to the list of places to check but I think we're back to pulling it from the gcloud config?

@dhermes
Copy link
Contributor

dhermes commented May 27, 2016

The current project-check cascade is:

  1. DATASTORE_DATASET environment variable, set by gcd tool (only sometimes)
  2. GCLOUD_PROJECT environment variable
  3. App Engine App ID
  4. GCE project ID

I'm suggesting you inject two more checks in there (not sure about the order yet):

  • Check for a project ID in a JSON svc. acct. keyfile specified by GOOGLE_APPLICATION_CREDENTIALS
  • Check in the ${HOME}/.config/gcloud/configurations/config_default file

For reference the cascade for credentials is:

  1. GAE application ID service
  2. filename via GOOGLE_APPLICATION_CREDENTIALS
  3. the "well-known" file set by the gcloud CLI (${HOME}/.config/gcloud/application_default_credentials.json)
  4. GCE instance creds

@daspecster
Copy link
Contributor Author

Ok I will do that! Thanks again @dhermes!

@daspecster
Copy link
Contributor Author

@dhermes @tseaver Can you guys let me know if this is good to go?

>>> from gcloud import bigquery
>>> client = bigquery.Client()

- :class:`Client <gcloud.client.Client>` objects hold both a ``project``

This comment was marked as spam.

@daspecster daspecster force-pushed the project-id-from-credentials-file branch 2 times, most recently from be7e034 to 8c3b234 Compare June 13, 2016 14:35
@daspecster daspecster force-pushed the project-id-from-credentials-file branch from 8c3b234 to d7b1951 Compare June 13, 2016 22:25
@daspecster daspecster mentioned this pull request Jun 13, 2016
7 tasks
@daspecster
Copy link
Contributor Author

@tseaver could you take a look at this real quick?
I think this is good to go but I we kind of left things hanging about raising an exception if the file is missing but I believe per @dhermes last comment that we're ok.

I'll merge if it looks good to you.

@tseaver
Copy link
Contributor

tseaver commented Jun 14, 2016

Sorry, I was AFK waiting on a contractor. Looking this evening.

@@ -48,6 +51,7 @@
(?P<nanos>\d{1,9}) # nanoseconds, maybe truncated
Z # Zulu
""", re.VERBOSE)
DEFAULT_CONFIGURATION_PATH = '~/.config/gcloud/configurations/config_default'

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Jun 15, 2016

LGTM whichever way you answer my question about passing the Windows config path in addition to the Unix-y one.

@daspecster
Copy link
Contributor Author

@tseaver I added it!

    win32_config_path = os.path.join(os.getenv('APPDATA', ''),
                                     'gcloud\configurations\config_default')

I don't have a windows machine to test on though. Is there a way that we test gcloud-python on windows in general?

@tseaver
Copy link
Contributor

tseaver commented Jun 15, 2016

@daspecster pylint doesn't like your backsplashes. I believe you could use forward-slashes there.

Is there a way that we test gcloud-python on windows in general?

We could look (again) into adding Appveyor CI.

@daspecster daspecster force-pushed the project-id-from-credentials-file branch from 63444c2 to 3f408c1 Compare June 15, 2016 22:38
@daspecster
Copy link
Contributor Author

daspecster commented Jun 15, 2016

@tseaver I think I got it this time!

I just let python pick the slashes.

    win32_config_path = os.path.join(os.getenv('APPDATA', ''),
                                     'gcloud', 'configurations',
                                     'config_default')

@daspecster
Copy link
Contributor Author

@tseaver if this looks good to you, I'll merge.
I appreciate you guys helping with this. I think it'll make the end user experience for beginners a lot easier, at least I really hope it does.

@tseaver
Copy link
Contributor

tseaver commented Jun 16, 2016

LGTM

@daspecster daspecster merged commit 4cf311b into googleapis:master Jun 16, 2016
@daspecster daspecster deleted the project-id-from-credentials-file branch June 16, 2016 14:37
@dhermes
Copy link
Contributor

dhermes commented Jun 16, 2016

@tseaver RE: AppVeyor, we've had a build running for 4+ months. I can add you guys as admins if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: core auth cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants