Skip to content

Commit 46a2181

Browse files
shaun0927haranrk
authored andcommitted
fix: restore auth_token initialization for secret and parameter manager clients
Merge #5371 **Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: #5370 **2. Or, if no issue exists, describe the change:** **Problem:** Both `SecretManagerClient` and `ParameterManagerClient` advertise an `auth_token` constructor path, but the implementation instantiates `google.auth.credentials.Credentials(...)` directly. That base class is abstract, so callers hit a `TypeError` before the underlying Google Cloud client is constructed. `SecretManagerClient` also documents that `service_account_json` and `auth_token` are mutually exclusive, but the current implementation silently accepts both inputs and ignores the token. **Solution:** - Switch both helpers to `google.oauth2.credentials.Credentials(token=auth_token)` for the `auth_token` path. - Keep the change narrow to the validated runtime regression and its adjacent contract mismatch. - Replace the mocked-constructor token-path tests with regression coverage that exercises the real credentials constructor path. - Add a `SecretManagerClient` test that confirms conflicting credential inputs raise `ValueError`. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. Passed locally: ```text .venv/bin/pytest tests/unittests/integrations/secret_manager/test_secret_client.py tests/unittests/integrations/parameter_manager/test_parameter_client.py -q 17 passed, 1 warning in 24.43s ``` **Manual End-to-End (E2E) Tests:** I also re-ran the original local reproductions with patched cloud clients to confirm the behavior change: ```python from unittest.mock import MagicMock, patch from google.adk.integrations.secret_manager.secret_client import SecretManagerClient from google.adk.integrations.parameter_manager.parameter_client import ParameterManagerClient with patch("google.cloud.secretmanager.SecretManagerServiceClient", return_value=MagicMock()): client = SecretManagerClient(auth_token="test-token") assert client._credentials.token == "test-token" with patch("google.cloud.parametermanager_v1.ParameterManagerClient", return_value=MagicMock()): client = ParameterManagerClient(auth_token="test-token") assert client._credentials.token == "test-token" ``` And for the conflicting-input case: ```python import json from google.adk.integrations.secret_manager.secret_client import SecretManagerClient try: SecretManagerClient( service_account_json=json.dumps({"type": "service_account"}), auth_token="test-token", ) except ValueError: pass else: raise AssertionError("Expected conflicting credentials to raise ValueError") ``` ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [x] Any dependent changes have been merged and published in downstream modules. ### Additional context I intentionally kept this PR focused on the reproducible auth helper regressions that are still present in the latest stable release and current `main`. Co-authored-by: Haran Rajkumar <haranrk@google.com> COPYBARA_INTEGRATE_REVIEW=#5371 from shaun0927:fix/token-credentials-for-integrations 3567a25 PiperOrigin-RevId: 939989046
1 parent 798207a commit 46a2181

4 files changed

Lines changed: 72 additions & 69 deletions

File tree

src/google/adk/integrations/parameter_manager/parameter_client.py

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@
1919
from typing import Optional
2020

2121
from google.api_core.gapic_v1 import client_info
22-
import google.auth
2322
from google.auth import default as default_service_credential
24-
import google.auth.transport.requests
2523
from google.cloud import parametermanager_v1
24+
from google.oauth2 import credentials as user_credentials
2625
from google.oauth2 import service_account
2726

2827
from ... import version
@@ -89,15 +88,7 @@ def __init__(
8988
except json.JSONDecodeError as e:
9089
raise ValueError(f"Invalid service account JSON: {e}") from e
9190
elif auth_token:
92-
credentials = google.auth.credentials.Credentials(
93-
token=auth_token,
94-
refresh_token=None,
95-
token_uri=None,
96-
client_id=None,
97-
client_secret=None,
98-
)
99-
request = google.auth.transport.requests.Request()
100-
credentials.refresh(request)
91+
credentials = user_credentials.Credentials(token=auth_token)
10192
else:
10293
try:
10394
credentials, _ = default_service_credential(

src/google/adk/integrations/secret_manager/secret_client.py

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@
1919
from typing import Optional
2020

2121
from google.api_core.gapic_v1 import client_info
22-
import google.auth
2322
from google.auth import default as default_service_credential
24-
import google.auth.transport.requests
2523
from google.cloud import secretmanager
24+
from google.oauth2 import credentials as user_credentials
2625
from google.oauth2 import service_account
2726

2827
from ... import version
@@ -42,8 +41,9 @@ class SecretManagerClient:
4241
"""A client for interacting with Google Cloud Secret Manager.
4342
4443
This class provides a simplified interface for retrieving secrets from
45-
Secret Manager, handling authentication using either a service account
46-
JSON keyfile (passed as a string) or a preexisting authorization token.
44+
Secret Manager, handling authentication using a service account JSON
45+
keyfile (passed as a string) or a preexisting authorization token. If
46+
neither is provided, it falls back to Application Default Credentials.
4747
4848
Attributes:
4949
_credentials: Google Cloud credentials object (ServiceAccountCredentials
@@ -59,6 +59,10 @@ def __init__(
5959
):
6060
"""Initializes the SecretManagerClient.
6161
62+
Credentials are resolved in priority order: `service_account_json`, then
63+
`auth_token`, then Application Default Credentials when neither is
64+
provided.
65+
6266
Args:
6367
service_account_json: The content of a service account JSON keyfile (as
6468
a string), not the file path. Must be valid JSON.
@@ -67,12 +71,18 @@ def __init__(
6771
Manager service. If not provided, the global endpoint is used.
6872
6973
Raises:
70-
ValueError: If neither `service_account_json` nor `auth_token` is
71-
provided,
72-
or if both are provided. Also raised if the service_account_json
73-
is not valid JSON.
74+
ValueError: If both `service_account_json` and `auth_token` are
75+
provided, if `service_account_json` is not valid JSON, or if
76+
neither is provided and Application Default Credentials cannot be
77+
resolved.
7478
google.auth.exceptions.GoogleAuthError: If authentication fails.
7579
"""
80+
if service_account_json and auth_token:
81+
raise ValueError(
82+
"Must provide either 'service_account_json' or 'auth_token', not"
83+
" both."
84+
)
85+
7686
if service_account_json:
7787
try:
7888
credentials = service_account.Credentials.from_service_account_info(
@@ -81,15 +91,7 @@ def __init__(
8191
except json.JSONDecodeError as e:
8292
raise ValueError(f"Invalid service account JSON: {e}") from e
8393
elif auth_token:
84-
credentials = google.auth.credentials.Credentials(
85-
token=auth_token,
86-
refresh_token=None,
87-
token_uri=None,
88-
client_id=None,
89-
client_secret=None,
90-
)
91-
request = google.auth.transport.requests.Request()
92-
credentials.refresh(request)
94+
credentials = user_credentials.Credentials(token=auth_token)
9395
else:
9496
try:
9597
credentials, _ = default_service_credential(

tests/unittests/integrations/parameter_manager/test_parameter_client.py

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from unittest.mock import patch
2020

2121
from google.api_core.gapic_v1 import client_info
22+
from google.oauth2.credentials import Credentials
2223
import pytest
2324

2425
pytest.importorskip("google.cloud.parametermanager_v1")
@@ -95,28 +96,19 @@ def test_init_with_service_account_json(
9596
@patch("google.cloud.parametermanager_v1.ParameterManagerClient")
9697
def test_init_with_auth_token(self, mock_pm_client_class):
9798
"""Test initialization with auth token."""
98-
# Setup
9999
auth_token = "test-token"
100-
mock_credentials = MagicMock()
101-
102-
with (
103-
patch("google.auth.credentials.Credentials") as mock_credentials_class,
104-
patch("google.auth.transport.requests.Request") as mock_request,
105-
):
106-
mock_credentials_class.return_value = mock_credentials
107100

108-
# Execute
109-
client = ParameterManagerClient(auth_token=auth_token)
101+
client = ParameterManagerClient(auth_token=auth_token)
110102

111-
# Verify
112-
mock_credentials.refresh.assert_called_once()
113-
mock_pm_client_class.assert_called_once()
114-
call_kwargs = mock_pm_client_class.call_args.kwargs
115-
assert call_kwargs["credentials"] == mock_credentials
116-
assert call_kwargs["client_options"] is None
117-
assert call_kwargs["client_info"].user_agent == USER_AGENT
118-
assert client._credentials == mock_credentials
119-
assert client._client == mock_pm_client_class.return_value
103+
mock_pm_client_class.assert_called_once()
104+
call_kwargs = mock_pm_client_class.call_args.kwargs
105+
assert isinstance(call_kwargs["credentials"], Credentials)
106+
assert call_kwargs["credentials"].token == auth_token
107+
assert call_kwargs["client_options"] is None
108+
assert call_kwargs["client_info"].user_agent == USER_AGENT
109+
assert isinstance(client._credentials, Credentials)
110+
assert client._credentials.token == auth_token
111+
assert client._client == mock_pm_client_class.return_value
120112

121113
@patch("google.cloud.parametermanager_v1.ParameterManagerClient")
122114
@patch(
@@ -181,6 +173,20 @@ def test_init_with_invalid_service_account_json(self):
181173
with pytest.raises(ValueError, match="Invalid service account JSON"):
182174
ParameterManagerClient(service_account_json="invalid-json")
183175

176+
def test_init_with_both_service_account_json_and_auth_token(self):
177+
"""Test initialization rejects conflicting credential inputs."""
178+
with pytest.raises(
179+
ValueError,
180+
match=(
181+
"Must provide either 'service_account_json' or 'auth_token', not"
182+
" both."
183+
),
184+
):
185+
ParameterManagerClient(
186+
service_account_json=json.dumps({"type": "service_account"}),
187+
auth_token="test-token",
188+
)
189+
184190
@patch("google.cloud.parametermanager_v1.ParameterManagerClient")
185191
@patch(
186192
"google.adk.integrations.parameter_manager.parameter_client.default_service_credential"

tests/unittests/integrations/secret_manager/test_secret_client.py

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from google.adk.integrations.secret_manager.secret_client import SecretManagerClient
2222
from google.adk.integrations.secret_manager.secret_client import USER_AGENT
2323
from google.api_core.gapic_v1 import client_info
24+
from google.oauth2.credentials import Credentials
2425
import pytest
2526

2627
import google
@@ -94,30 +95,19 @@ def test_init_with_service_account_json(
9495
@patch("google.cloud.secretmanager.SecretManagerServiceClient")
9596
def test_init_with_auth_token(self, mock_secret_manager_client):
9697
"""Test initialization with auth token."""
97-
# Setup
9898
auth_token = "test-token"
99-
mock_credentials = MagicMock()
10099

101-
# Mock the entire credentials creation process
102-
with (
103-
patch("google.auth.credentials.Credentials") as mock_credentials_class,
104-
patch("google.auth.transport.requests.Request") as mock_request,
105-
):
106-
# Configure the mock to return our mock_credentials when instantiated
107-
mock_credentials_class.return_value = mock_credentials
108-
109-
# Execute
110-
client = SecretManagerClient(auth_token=auth_token)
111-
112-
# Verify
113-
mock_credentials.refresh.assert_called_once()
114-
mock_secret_manager_client.assert_called_once()
115-
call_kwargs = mock_secret_manager_client.call_args.kwargs
116-
assert call_kwargs["credentials"] == mock_credentials
117-
assert call_kwargs["client_options"] is None
118-
assert call_kwargs["client_info"].user_agent == USER_AGENT
119-
assert client._credentials == mock_credentials
120-
assert client._client == mock_secret_manager_client.return_value
100+
client = SecretManagerClient(auth_token=auth_token)
101+
102+
mock_secret_manager_client.assert_called_once()
103+
call_kwargs = mock_secret_manager_client.call_args.kwargs
104+
assert isinstance(call_kwargs["credentials"], Credentials)
105+
assert call_kwargs["credentials"].token == auth_token
106+
assert call_kwargs["client_options"] is None
107+
assert call_kwargs["client_info"].user_agent == USER_AGENT
108+
assert isinstance(client._credentials, Credentials)
109+
assert client._credentials.token == auth_token
110+
assert client._client == mock_secret_manager_client.return_value
121111

122112
@patch("google.cloud.secretmanager.SecretManagerServiceClient")
123113
@patch(
@@ -182,6 +172,20 @@ def test_init_with_invalid_service_account_json(self):
182172
with pytest.raises(ValueError, match="Invalid service account JSON"):
183173
SecretManagerClient(service_account_json="invalid-json")
184174

175+
def test_init_with_both_service_account_json_and_auth_token(self):
176+
"""Test initialization rejects conflicting credential inputs."""
177+
with pytest.raises(
178+
ValueError,
179+
match=(
180+
"Must provide either 'service_account_json' or 'auth_token', not"
181+
" both."
182+
),
183+
):
184+
SecretManagerClient(
185+
service_account_json=json.dumps({"type": "service_account"}),
186+
auth_token="test-token",
187+
)
188+
185189
@patch("google.cloud.secretmanager.SecretManagerServiceClient")
186190
@patch(
187191
"google.adk.integrations.secret_manager.secret_client.default_service_credential"

0 commit comments

Comments
 (0)