Skip to content

feat(gke): support IAM credentials via Application Default Credentials#6610

Open
aditya-786 wants to merge 3 commits into
keephq:mainfrom
aditya-786:feat/gke-adc-credentials
Open

feat(gke): support IAM credentials via Application Default Credentials#6610
aditya-786 wants to merge 3 commits into
keephq:mainfrom
aditya-786:feat/gke-adc-credentials

Conversation

@aditya-786

Copy link
Copy Markdown
Contributor

What

Makes the GKE provider usable without uploading a service account JSON. When service_account_json is left empty, the provider authenticates with Application Default Credentials (for example a GKE Workload Identity service account) through google.auth.default(). When a JSON is supplied the behavior is unchanged.

service_account_json is now optional and a new optional project_id field was added. The project is resolved from project_id, then the service account JSON, then the Application Default Credentials.

Credential selection is centralized in a small helper (build_gke_credentials) used by both validate_scopes and the Kubernetes client builder.

Why

Closes #3721. Workloads running in GKE usually have an IAM service account attached via Workload Identity, so requiring a generated JSON key is unnecessary friction and an extra long-lived secret to manage.

Testing

tests/test_gke_credentials.py covers credential selection: the JSON path, the ADC fallback when no JSON is given, an explicit project_id override, and project resolution from the JSON. The helper has no app dependencies, so it runs in isolation:

.venv/bin/python -m pytest --noconftest tests/test_gke_credentials.py

The provider docs snippet was regenerated; python scripts/docs_render_provider_snippets.py --validate passes.

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. Enhancement New feature or request Provider Providers related issues labels Jun 29, 2026

@shahargl shahargl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice feature — removing the service-account-JSON requirement for GKE Workload Identity users is a real friction win, and extracting build_gke_credentials into a small, well-tested helper is clean. A few things I'd like addressed before merge.

Blocking

1. Silent ADC fallback on malformed JSON (gke_provider.py, __init__)

When service_account_json is provided but json.loads fails, the except block now sets self._service_account_data = None but no longer resets self._project_id. As a result, a user who supplies a malformed SA JSON (and no explicit project_id) will silently fall back to Application Default Credentials instead of getting an error. Previously both were cleared together, so this is a behavior change that can mask a real misconfiguration — the user thinks they're authenticating with their SA, but they're actually using the node's identity.

Please at least log a warning so the fallback is observable, e.g.:

except Exception:
    self.logger.warning(
        "Invalid service_account_json provided, falling back to Application Default Credentials"
    )
    self._service_account_data = None

Important

2. No test for the changed __init__ credential-selection branch

The helper has good coverage (JSON path, ADC fallback, explicit project_id override, SA project resolution), but the riskiest change in this PR is the provider's __init__ logic — and it's currently untested. Please add a test that covers the malformed-JSON → ADC fallback behavior, since that's where the actual behavior change lives.

3. DefaultCredentialsError not handled with a clear message

google.auth.default() raises google.auth.exceptions.DefaultCredentialsError when neither a SA JSON nor ADC is available (common when running locally without gcloud auth). It propagates raw out of build_gke_credentials. In validate_scopes it gets stringified per-scope; in __generate_client it's wrapped generically. A clearer message ("No service account JSON provided and no Application Default Credentials found") would make this much easier to diagnose. A test for this path would be good too.

Nits

  • build_gke_credentials has no type hints — since the file is new, consider service_account_data: dict | None = None, project_id: str | None = None and a return annotation.
  • In validate_scopes, the project_id resolution check runs after credentials.refresh(). You could move it before the refresh to fail faster, since refreshing ADC isn't needed to know the project is missing.

Happy to approve once the silent-fallback warning + a test for the __init__ branch are in.

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 30, 2026
@aditya-786

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. All addressed in the latest push.

  1. Moved the JSON parsing and project resolution out of __init__ into resolve_service_account, which logs the warning you suggested when the JSON is malformed. The fallback to ADC is now observable instead of silent, and an explicit project_id is still kept on that path.

  2. The behavior now lives in that helper, so the previously-untested branch is covered: test_resolve_malformed_json_warns_and_falls_back asserts both the warning and the fallback, plus empty / explicit-project / valid-JSON cases. __init__ is now just the delegation call.

  3. build_gke_credentials catches DefaultCredentialsError from google.auth.default() and re-raises it as "No service account JSON provided and no Application Default Credentials found". Covered by test_build_gke_credentials_clear_error_when_no_adc.

Nits: added type hints to both helpers, and moved the project_id check ahead of credentials.refresh() in validate_scopes so it fails fast.

All 10 tests pass with pytest --noconftest tests/test_gke_credentials.py.

@aditya-786 aditya-786 force-pushed the feat/gke-adc-credentials branch from 93b8add to 34c9140 Compare June 30, 2026 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request Provider Providers related issues size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[➕ Feature]: GKE provider - use IAM credentials

2 participants