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

Create client from service account stored in string #8

Closed
frankyn opened this issue Apr 17, 2020 · 5 comments · Fixed by #54
Closed

Create client from service account stored in string #8

frankyn opened this issue Apr 17, 2020 · 5 comments · Fixed by #54
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@frankyn
Copy link
Member

frankyn commented Apr 17, 2020

Hi,

This was raised in python-storage: googleapis/python-storage#93

python-cloud-core library has support for from_service_account_json, which accepts a file path to a service account.

Feature request is to add support to accept a json string of the service account. User stores the service account file contents in an environment variable instead of in a file.

Open question: I'm not sure if this is a path that should be supported so I'm leaving that as an open question in this issue. If this feature is not supported then the workaround is to construct a credentials object and explicitly set a project id:

# Storage example but is a general issue AFAIU

import os
import json

from google.cloud import storage
from google.oauth2 import service_account

credentials_json_string = "{...service account....}"

credentials_json = json.loads(credentials_json_string)
credentials = service_account.Credentials.from_service_account_info(credentials_json)

# Credentials has project_id already so project should be optional but it's not.
client = storage.Client(project=credentials['project_id'], credentials=credentials)

print(list(client.list_buckets()))

Proposal:

    @classmethod
    def from_service_account_json_string(cls, json_credentials, *args, **kwargs):
        """Factory to create credentials with provided json_credentials.
        :type json_credentials: str
        :param json_credentials: The contents of a private key file. This string must contain
                                      a JSON object with a private key and
                                      other credentials information (downloaded
                                      from the Google APIs console).
        :type args: tuple
        :param args: Remaining positional arguments to pass to constructor.
        :param kwargs: Remaining keyword arguments to pass to constructor.
        :rtype: :class:`_ClientFactoryMixin`
        :returns: The client created with the provided JSON credentials.
        :raises TypeError: if there is a conflict with the kwargs
                 and the credentials created by the factory.
        """
        if "credentials" in kwargs:
            raise TypeError("credentials must not be in keyword arguments")
        credentials_info = json.load(json_credentials)
        credentials = service_account.Credentials.from_service_account_info(
            credentials_info
        )
        if cls._SET_PROJECT:
            if "project" not in kwargs:
                kwargs["project"] = credentials_info.get("project_id")

        kwargs["credentials"] = credentials
        return cls(*args, **kwargs)
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Apr 18, 2020
@frankyn
Copy link
Member Author

frankyn commented Apr 20, 2020

@busunkim96 are you an owner of Python-cloud-core? Not sure who I should ask if this is something that we'd like to support.

@busunkim96
Copy link
Contributor

@frankyn Yes!

If this is added to google-cloud-core it should also be added to the generated clients.

https://github.com/googleapis/python-asset/blob/15b60a349c93c928fe121dc47d44d812a0c14439/google/cloud/asset_v1p1beta1/gapic/asset_service_client.py#L53

Since this will impact all the clients, I'll bring it up tomorrow at the Python client weekly.

@busunkim96 busunkim96 added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed triage me I really want to be triaged. labels Apr 20, 2020
@rvandegrift
Copy link

I just want to point out, this issue contains a secondary feature request. The original request provides this example:

# Credentials has project_id already so project should be optional but it's not.
client = storage.Client(project=credentials['project_id'], credentials=credentials)

In fact project_id is optional today, but bad things happen if you don't specify it (see googleapis/python-storage#177 for an example issue)

So the secondary feature request here is: if credentials are explicitly provided, then the client should skip autoloading. This behavior would be good whether the credentials are loaded from a file or string.

@tseaver
Copy link
Contributor

tseaver commented Dec 10, 2020

@rvandegrift The secondary request is now tracked in #27, and should be resolved in PR #51.

@frankyn The spelling of the @classmethod factory in google-auth taking a dictionary, rather than a filename, is from_service_account_info. ISTM that we could add that factory, and have users responsible for decoding the serialized values from whatever envvar they like, stored in whatever format:

class _ClientFactoryMixin(object):
    ...
    @classmethod
    def from_service_account_info(cls, info, *args, **kwargs):
        """Factory to load JSON credentials while creating client.

        :type info: dict
        :param info:
            A JSON object with a private key and other credentials information
            (downloaded from the Google APIs console).

        :type args: tuple
        :param args: Remaining positional arguments to pass to constructor.

        :param kwargs: Remaining keyword arguments to pass to constructor.

        :rtype: :class:`_ClientFactoryMixin`
        :returns: The client created with the retrieved JSON credentials.
        :raises TypeError: if there is a conflict with the kwargs
                 and the credentials created by the factory.
        """
        if "credentials" in kwargs:
            raise TypeError("credentials must not be in keyword arguments")

        credentials = service_account.Credentials.from_service_account_info(info)

        if cls._SET_PROJECT:
            if "project" not in kwargs:
                kwargs["project"] = credentials_info.get("project_id")

        kwargs["credentials"] = credentials
        return cls(*args, **kwargs)

The existing from_service_account_json would then be just a wrapper around the new method, loading the JSON credentials file.

@busunkim96 I created googleapis/gapic-generator-python#705 to track adding this method for the microgenerator. Do we need to add support for it in the macrogenerator too?

@busunkim96
Copy link
Contributor

@tseaver Nope, just the microgenerator is fine. The monolith is almost gone (we're down to 2 libraries). :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants