Skip to content

chore: add universe compatibility with older versions of api-core #2372

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

Merged
merged 7 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion googleapiclient/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,21 @@
GOOGLE_API_USE_CLIENT_CERTIFICATE = "GOOGLE_API_USE_CLIENT_CERTIFICATE"
GOOGLE_API_USE_MTLS_ENDPOINT = "GOOGLE_API_USE_MTLS_ENDPOINT"
GOOGLE_CLOUD_UNIVERSE_DOMAIN = "GOOGLE_CLOUD_UNIVERSE_DOMAIN"

DEFAULT_UNIVERSE = "googleapis.com"
# Parameters accepted by the stack, but not visible via discovery.
# TODO(dhermes): Remove 'userip' in 'v2'.
STACK_QUERY_PARAMETERS = frozenset(["trace", "pp", "userip", "strict"])
STACK_QUERY_PARAMETER_DEFAULT_VALUE = {"type": "string", "location": "query"}


class APICoreVersionError(ValueError):
def __init__(self):
message = (
"google-api-core >= 2.18.0 is required to use the universe domain feature."
)
super().__init__(message)


# Library-specific reserved words beyond Python keywords.
RESERVED_WORDS = frozenset(["body"])

Expand Down Expand Up @@ -444,6 +453,13 @@ def _retrieve_discovery_doc(
return content


def _check_api_core_compatible_with_credentials_universe(credentials):
if not HAS_UNIVERSE:
credentials_universe = getattr(credentials, "universe_domain", None)
if credentials_universe and credentials_universe != DEFAULT_UNIVERSE:
raise APICoreVersionError


@positional(1)
def build_from_document(
service,
Expand Down Expand Up @@ -559,6 +575,10 @@ def build_from_document(
client_options.universe_domain, universe_domain_env
)
base = base.replace(universe.DEFAULT_UNIVERSE, universe_domain)
else:
client_universe = getattr(client_options, "universe_domain", None)
if client_universe:
raise APICoreVersionError

audience_for_self_signed_jwt = base
if client_options.api_endpoint:
Expand Down Expand Up @@ -598,6 +618,9 @@ def build_from_document(
quota_project_id=client_options.quota_project_id,
)

# Check google-api-core >= 2.18.0 if credentials' universe != "googleapis.com".
_check_api_core_compatible_with_credentials_universe(credentials)

# The credentials need to be scoped.
# If the user provided scopes via client_options don't override them
if not client_options.scopes:
Expand Down Expand Up @@ -687,6 +710,10 @@ def build_from_document(
f"mTLS is not supported in any universe other than {universe.DEFAULT_UNIVERSE}."
)
base = mtls_endpoint
else:
# Check google-api-core >= 2.18.0 if credentials' universe != "googleapis.com".
http_credentials = getattr(http, "credentials", None)
_check_api_core_compatible_with_credentials_universe(http_credentials)

if model is None:
features = service.get("features", [])
Expand Down
77 changes: 75 additions & 2 deletions tests/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
STACK_QUERY_PARAMETERS,
V1_DISCOVERY_URI,
V2_DISCOVERY_URI,
APICoreVersionError,
ResourceMethodParameters,
_fix_up_media_path_base_url,
_fix_up_media_upload,
Expand Down Expand Up @@ -107,6 +108,11 @@
DATA_DIR = os.path.join(os.path.dirname(__file__), "data")


def _reset_universe_domain(credentials, universe_domain=None):
if hasattr(credentials, "universe_domain"):
credentials.universe_domain = universe_domain


def assertUrisEqual(testcase, expected, actual):
"""Test that URIs are the same, up to reordering of query parameters."""
expected = urllib.parse.urlparse(expected)
Expand Down Expand Up @@ -541,6 +547,7 @@ def test_credentials_and_credentials_file_mutually_exclusive(self):

class DiscoveryFromDocument(unittest.TestCase):
MOCK_CREDENTIALS = mock.Mock(spec=google.auth.credentials.Credentials)
_reset_universe_domain(MOCK_CREDENTIALS)

def test_can_build_from_local_document(self):
discovery = read_datafile("plus.json")
Expand Down Expand Up @@ -693,6 +700,7 @@ def test_scopes_from_client_options(self):
discovery = read_datafile("plus.json")

with mock.patch("googleapiclient._auth.default_credentials") as default:
_reset_universe_domain(default.return_value)
plus = build_from_document(
discovery,
client_options={"scopes": ["1", "2"]},
Expand All @@ -704,6 +712,7 @@ def test_quota_project_from_client_options(self):
discovery = read_datafile("plus.json")

with mock.patch("googleapiclient._auth.default_credentials") as default:
_reset_universe_domain(default.return_value)
plus = build_from_document(
discovery,
client_options=google.api_core.client_options.ClientOptions(
Expand All @@ -717,6 +726,7 @@ def test_credentials_file_from_client_options(self):
discovery = read_datafile("plus.json")

with mock.patch("googleapiclient._auth.credentials_from_file") as default:
_reset_universe_domain(default.return_value)
plus = build_from_document(
discovery,
client_options=google.api_core.client_options.ClientOptions(
Expand Down Expand Up @@ -772,6 +782,7 @@ def test_self_signed_jwt_disabled(self):

class DiscoveryFromDocumentMutualTLS(unittest.TestCase):
MOCK_CREDENTIALS = mock.Mock(spec=google.auth.credentials.Credentials)
_reset_universe_domain(MOCK_CREDENTIALS)
ADC_CERT_PATH = "adc_cert_path"
ADC_KEY_PATH = "adc_key_path"
ADC_PASSPHRASE = "adc_passphrase"
Expand Down Expand Up @@ -1528,6 +1539,7 @@ def test_plus_resources(self):
@unittest.skipIf(not HAS_OAUTH2CLIENT, "oauth2client unavailable.")
def test_oauth2client_credentials(self):
credentials = mock.Mock(spec=GoogleCredentials)
_reset_universe_domain(credentials)
credentials.create_scoped_required.return_value = False

discovery = read_datafile("plus.json")
Expand All @@ -1536,6 +1548,7 @@ def test_oauth2client_credentials(self):

def test_google_auth_credentials(self):
credentials = mock.Mock(spec=google.auth.credentials.Credentials)
_reset_universe_domain(credentials)
discovery = read_datafile("plus.json")
service = build_from_document(discovery, credentials=credentials)

Expand Down Expand Up @@ -2340,9 +2353,9 @@ def test_get_media(self):
self.assertEqual(b"standing in for media", response)


if HAS_UNIVERSE:
class Universe(unittest.TestCase):
if HAS_UNIVERSE:

class Universe(unittest.TestCase):
def test_validate_credentials_with_no_client_options(self):
http = build_http()
discovery = read_datafile("zoo.json")
Expand Down Expand Up @@ -2665,6 +2678,66 @@ def test_universe_env_var_configured_with_client_options_universe(self):

assert tasks._universe_domain == fake_universe

else:
if hasattr(google.api_core.client_options.ClientOptions, "universe_domain"):

def test_client_options_universe_with_older_version_of_api_core(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also add a test case for http credentials. eg. test_http_credentials_client_options_universe_with_older_version_of_api_core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setting clientOptions.universe_domain will raise an exception before we even check for credentials in build_from_document. Therefore, testing different type of credentials while also passing in clientOptions.universe_domain might not add much value here.

fake_universe = "foo.com"
credentials = mock.Mock(spec=google.auth.credentials.Credentials)
credentials.universe_domain = fake_universe
discovery = read_datafile("tasks.json")
with self.assertRaises(APICoreVersionError):
tasks = build_from_document(
discovery,
credentials=credentials,
client_options=google.api_core.client_options.ClientOptions(
universe_domain=fake_universe
),
)

def test_credentials_universe_with_older_version_of_api_core(self):
fake_universe = "foo.com"
credentials = mock.Mock(spec=google.auth.credentials.Credentials)
credentials.universe_domain = fake_universe
discovery = read_datafile("tasks.json")
with self.assertRaises(APICoreVersionError):
tasks = build_from_document(
discovery,
credentials=credentials,
)

def test_credentials_default_universe_with_older_version_of_api_core(self):
credentials = mock.Mock(spec=google.auth.credentials.Credentials)
credentials.universe_domain = "googleapis.com"
discovery = read_datafile("tasks.json")
tasks = build_from_document(
discovery,
credentials=credentials,
)

def test_http_credentials_universe_with_older_version_of_api_core(self):
fake_universe = "foo.com"
http = google_auth_httplib2.AuthorizedHttp(
credentials=mock.Mock(universe_domain=fake_universe), http=build_http()
)
discovery = read_datafile("tasks.json")
with self.assertRaises(APICoreVersionError):
tasks = build_from_document(
discovery,
http=http,
)

def test_http_credentials_default_universe_with_older_version_of_api_core(self):
http = google_auth_httplib2.AuthorizedHttp(
credentials=mock.Mock(universe_domain="googleapis.com"),
http=build_http(),
)
discovery = read_datafile("tasks.json")
tasks = build_from_document(
discovery,
http=http,
)


if __name__ == "__main__":
unittest.main()