Skip to content

Commit d8816e1

Browse files
refactor: apply code review fixes to GCP OIDC detector
- Cache ADC credentials resolved during detect() and reuse them in get_token(), avoiding a second metadata-server probe per token mint - Hoist DEFAULT_AUDIENCE into detectors.base and import it in the AWS, GCP, and GitHub Actions detectors - Replace the file-wide pylint import-error disable with per-line suppressions, matching the AWS detector convention - Reuse a single transport Request for all calls within a token mint - Log a debug message when a custom oidc_audience is ignored on the user-credentials path - Install the gcp extra in the CI test workflow so the detector tests run instead of silently skipping via importorskip - Add the copyright header and extend the README optional-extras note to cover the GCP detector Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0d669f2 commit d8816e1

7 files changed

Lines changed: 71 additions & 45 deletions

File tree

.github/workflows/test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ jobs:
3939
pip install -r requirements.txt
4040
4141
- name: Install package
42-
run: pip install -e .
42+
run: pip install -e ".[gcp]"
4343

4444
- name: Run pytest
4545
env:

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ To install all optional dependencies:
181181
pip install cloudsmith-cli[all]
182182
```
183183

184-
**Note:** If you don't install the AWS extra, the AWS OIDC detector will gracefully skip itself with no errors.
184+
**Note:** If you don't install the AWS or GCP extras, the corresponding OIDC detectors will gracefully skip themselves with no errors.
185185

186186
#### Azure DevOps OIDC Support
187187

cloudsmith_cli/core/credentials/oidc/detectors/aws.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,10 @@
1313

1414
import logging
1515

16-
from .base import EnvironmentDetector
16+
from .base import DEFAULT_AUDIENCE, EnvironmentDetector
1717

1818
logger = logging.getLogger(__name__)
1919

20-
DEFAULT_AUDIENCE = "cloudsmith"
21-
2220

2321
class AWSDetector(EnvironmentDetector):
2422
"""Detects AWS environments and obtains a JWT via STS GetWebIdentityToken."""

cloudsmith_cli/core/credentials/oidc/detectors/base.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
if TYPE_CHECKING:
88
from ... import CredentialContext
99

10+
DEFAULT_AUDIENCE = "cloudsmith"
11+
1012

1113
class EnvironmentDetector:
1214
"""Base class for OIDC environment detectors."""

cloudsmith_cli/core/credentials/oidc/detectors/gcp.py

Lines changed: 55 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
# Copyright 2026 Cloudsmith Ltd
12
"""Google Cloud OIDC detector.
23
34
Discovers the ambient Google identity via google-auth's Application Default
@@ -12,19 +13,14 @@
1213
https://github.com/googleapis/google-cloud-python/tree/main/packages/google-auth
1314
"""
1415

15-
# google-auth is an optional dependency; its imports are unavailable at lint time.
16-
# pylint: disable=import-error
17-
1816
from __future__ import annotations
1917

2018
import logging
2119

22-
from .base import EnvironmentDetector
20+
from .base import DEFAULT_AUDIENCE, EnvironmentDetector
2321

2422
logger = logging.getLogger(__name__)
2523

26-
DEFAULT_AUDIENCE = "cloudsmith"
27-
2824
METADATA_IDENTITY_PATH = "instance/service-accounts/default/identity"
2925

3026

@@ -33,6 +29,10 @@ class GCPDetector(EnvironmentDetector):
3329

3430
name = "Google Cloud"
3531

32+
def __init__(self, context):
33+
super().__init__(context)
34+
self._credentials = None
35+
3636
def detect(self) -> bool:
3737
try:
3838
import google.auth
@@ -42,7 +42,7 @@ def detect(self) -> bool:
4242
return False
4343

4444
try:
45-
google.auth.default()
45+
self._credentials, _ = google.auth.default()
4646
return True
4747
except exceptions.DefaultCredentialsError:
4848
return self._on_gce()
@@ -53,27 +53,41 @@ def detect(self) -> bool:
5353
def get_token(self) -> str:
5454
audience = self.context.oidc_audience or DEFAULT_AUDIENCE
5555

56-
import google.auth
57-
from google.auth import compute_engine, exceptions
58-
from google.auth.transport.requests import Request
59-
from google.oauth2 import credentials as user_creds
56+
import google.auth # pylint: disable=import-error
57+
from google.auth import ( # pylint: disable=import-error
58+
compute_engine,
59+
exceptions,
60+
)
61+
from google.auth.transport.requests import ( # pylint: disable=import-error
62+
Request,
63+
)
64+
from google.oauth2 import ( # pylint: disable=import-error
65+
credentials as user_creds,
66+
)
6067

6168
request = Request()
62-
try:
63-
credentials, _ = google.auth.default()
64-
except exceptions.DefaultCredentialsError:
65-
token = self._token_from_metadata(audience)
69+
credentials = self._credentials
70+
if credentials is None:
71+
try:
72+
credentials, _ = google.auth.default()
73+
except exceptions.DefaultCredentialsError:
74+
credentials = None
75+
76+
if credentials is None:
77+
token = self._token_from_metadata(audience, request)
78+
elif isinstance(credentials, compute_engine.Credentials):
79+
token = self._token_from_metadata(audience, request)
80+
if not token:
81+
token = self._token_from_compute_id_token_credentials(audience, request)
82+
elif isinstance(credentials, user_creds.Credentials):
83+
if self.context.oidc_audience:
84+
logger.debug(
85+
"GCPDetector: user credentials cannot mint a token for a "
86+
"custom audience; the configured oidc_audience is ignored"
87+
)
88+
token = self._token_from_user_credentials(credentials, request)
6689
else:
67-
if isinstance(credentials, compute_engine.Credentials):
68-
token = self._token_from_metadata(audience)
69-
if not token:
70-
token = self._token_from_compute_id_token_credentials(
71-
audience, request
72-
)
73-
elif isinstance(credentials, user_creds.Credentials):
74-
token = self._token_from_user_credentials(credentials, request)
75-
else:
76-
token = self._token_from_id_token_credentials(audience, request)
90+
token = self._token_from_id_token_credentials(audience, request)
7791

7892
if not token:
7993
raise ValueError(
@@ -84,23 +98,24 @@ def get_token(self) -> str:
8498
return token
8599

86100
def _on_gce(self) -> bool:
87-
from google.auth import exceptions
88-
from google.auth.compute_engine import _metadata
89-
from google.auth.transport.requests import Request
101+
from google.auth import exceptions # pylint: disable=import-error
102+
from google.auth.compute_engine import _metadata # pylint: disable=import-error
103+
from google.auth.transport.requests import ( # pylint: disable=import-error
104+
Request,
105+
)
90106

91107
try:
92108
return bool(_metadata.is_on_gce(Request()))
93109
except exceptions.GoogleAuthError:
94110
return False
95111

96-
def _token_from_metadata(self, audience: str) -> str | None:
97-
from google.auth import exceptions
98-
from google.auth.compute_engine import _metadata
99-
from google.auth.transport.requests import Request
112+
def _token_from_metadata(self, audience: str, request) -> str | None:
113+
from google.auth import exceptions # pylint: disable=import-error
114+
from google.auth.compute_engine import _metadata # pylint: disable=import-error
100115

101116
try:
102117
token = _metadata.get(
103-
Request(),
118+
request,
104119
METADATA_IDENTITY_PATH,
105120
params={"audience": audience, "format": "full"},
106121
)
@@ -118,7 +133,10 @@ def _token_from_compute_id_token_credentials(
118133
# _token_from_metadata returns nothing there. Fall back to the IAM
119134
# Credentials API (generateIdToken), which the workload's service
120135
# account can call for itself when granted roles/iam.serviceAccountTokenCreator.
121-
from google.auth import compute_engine, exceptions
136+
from google.auth import ( # pylint: disable=import-error
137+
compute_engine,
138+
exceptions,
139+
)
122140

123141
try:
124142
id_credentials = compute_engine.IDTokenCredentials(
@@ -136,7 +154,7 @@ def _token_from_compute_id_token_credentials(
136154
return id_credentials.token or None
137155

138156
def _token_from_user_credentials(self, credentials, request) -> str | None:
139-
from google.auth import exceptions
157+
from google.auth import exceptions # pylint: disable=import-error
140158

141159
try:
142160
credentials.refresh(request)
@@ -146,8 +164,8 @@ def _token_from_user_credentials(self, credentials, request) -> str | None:
146164
return getattr(credentials, "id_token", None) or None
147165

148166
def _token_from_id_token_credentials(self, audience: str, request) -> str | None:
149-
from google.auth import exceptions
150-
from google.oauth2 import id_token
167+
from google.auth import exceptions # pylint: disable=import-error
168+
from google.oauth2 import id_token # pylint: disable=import-error
151169

152170
try:
153171
id_credentials = id_token.fetch_id_token_credentials(audience, request)

cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616
from urllib.parse import quote
1717

1818
from ....rest import create_requests_session as create_session
19-
from .base import EnvironmentDetector
20-
21-
DEFAULT_AUDIENCE = "cloudsmith"
19+
from .base import DEFAULT_AUDIENCE, EnvironmentDetector
2220

2321

2422
class GitHubActionsDetector(EnvironmentDetector):

cloudsmith_cli/core/tests/test_gcp_detector.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,16 @@ def test_uses_configured_audience(self):
7373
_, kwargs = get.call_args
7474
assert kwargs["params"]["audience"] == "custom-aud"
7575

76+
def test_get_token_reuses_credentials_resolved_during_detect(self):
77+
compute_creds = mock.Mock(spec=compute_engine.Credentials)
78+
detector = make_detector()
79+
with mock.patch(
80+
"google.auth.default", return_value=(compute_creds, "proj")
81+
) as default, mock.patch(METADATA_GET, return_value="meta-jwt"):
82+
assert detector.detect() is True
83+
assert detector.get_token() == "meta-jwt"
84+
default.assert_called_once()
85+
7686
def test_falls_back_to_metadata_when_default_raises(self):
7787
with mock.patch(
7888
"google.auth.default",

0 commit comments

Comments
 (0)