Skip to content

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

Open
shaun0927 wants to merge 1 commit 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 1 commit 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.

@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

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
@shaun0927 shaun0927 force-pushed the fix/token-credentials-for-integrations branch from 2aeb222 to f91cb17 Compare April 22, 2026 17:29
@shaun0927
Copy link
Copy Markdown
Author

shaun0927 commented Apr 22, 2026

@rohityan @surajksharma07 Thanks for the reviews. I've rebased onto main and run autoformat.sh — the only diff was a one-line regex wrap in tests/unittests/integrations/secret_manager/test_secret_client.py. pyink --check now passes locally, and the 17 existing unit tests still pass.

PTAL when you get a chance.

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