Skip to content

fix: restore auth_token initialization for secret and parameter manager clients#5371

Open
shaun0927 wants to merge 2 commits intogoogle:mainfrom
shaun0927:fix/token-credentials-for-integrations
Open

fix: restore auth_token initialization for secret and parameter manager clients#5371
shaun0927 wants to merge 2 commits intogoogle:mainfrom
shaun0927:fix/token-credentials-for-integrations

Conversation

@shaun0927
Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

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:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Passed locally:

.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:

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:

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

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • 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.

Both integration helpers advertised an auth_token constructor path, but the
implementation instantiated google.auth.credentials.Credentials directly.
That base class is abstract, so callers hit a TypeError before any request
could be made. This patch switches both helpers to a concrete OAuth2
Credentials object and adds regression coverage that exercises the real
constructor path.

The SecretManager helper also now rejects service_account_json + auth_token
up front so its behavior matches its own documented contract.

Constraint: Keep the change narrow to the validated auth helper regressions
Rejected: Leave the existing mocked constructor tests in place | they masked the real runtime failure
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep token-path tests using the real credentials class so abstract-constructor regressions do not reappear
Tested: .venv/bin/pytest tests/unittests/integrations/secret_manager/test_secret_client.py tests/unittests/integrations/parameter_manager/test_parameter_client.py -q
Not-tested: Full repo test suite; lint tooling was not installed in the local venv
@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Apr 17, 2026
@rohityan rohityan self-assigned this Apr 20, 2026
@rohityan rohityan added the request clarification [Status] The maintainer need clarification or more information from the author label Apr 20, 2026
@rohityan
Copy link
Copy Markdown
Collaborator

Hi @shaun0927 , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix formatting errors by running autoformat.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

request clarification [Status] The maintainer need clarification or more information from the author services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SecretManagerClient and ParameterManagerClient auth_token paths fail at runtime

3 participants