Skip to content

Fix BigQuery test credential selection and cleanup - #127

Closed
danielmawhirter wants to merge 1 commit into
mainfrom
dm-test-credential-isolation
Closed

danielmawhirter wants to merge 1 commit into
mainfrom
dm-test-credential-isolation

Conversation

@danielmawhirter

@danielmawhirter danielmawhirter commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Related investigation: #104

Summary

Fix restricted BigQuery tests treating a stored service-account key as federated ADC and demanding impersonation targets. Credential selection now checks the actual JSON credential type: key-based tests retain their existing per-identity secret lookup, while federated/user ADC uses the configured restricted service account.

Scope credential environment changes and generated files to each decorated test. Temporary keys are removed and the original environment is restored after success, failure, skip, or setup failure, including async tests. Existing credential input files remain unchanged.

This first repair deliberately retains the stored-key fallback and current decorator setup timing. Removing the fallback and moving authentication before dependent fixtures belong to the subsequent environment migration. Workflow configuration and strict WIF enforcement are unchanged.

Change map

File-by-file change map.

Testing Done

  • Focused BigQuery credential suite: 33 passed, independently rerun.
  • flake8 -j 1, isort --check-only, and git diff --check: passed.
  • Used the existing Python 3.14 environment with this checkout's package directories on PYTHONPATH; this worktree has no .venv. Dependency warnings remain.
  • CI connector/config shard now passes: 1,877 passed, 41 skipped, 1 xfailed, resolving the two observed failures (job).
  • GCS integration CI still rejects stored-key deployment credentials while the old workflow requests workload exchange. The credential-mode/workflow mismatch is addressed in dependent #129; successful live federation remains required before merging the migration.

Checklist

  • Listed behavior/API changes: setup_bigquery_credentials now scopes setup through a context manager; its only in-tree consumers are the decorator and focused tests. Existing decorator callsites are unchanged.
  • Reviewed documentation requirements; no external setup contract changes in this repair.
  • Added regressions for credential-type selection, restricted identities, environment restoration, temporary-file cleanup, and setup/test failures.

@copy-pr-bot

copy-pr-bot Bot commented Oct 27, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@danielmawhirter
danielmawhirter temporarily deployed to gha-kumo-workflow-tests October 27, 2026 20:11 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@danielmawhirter

Copy link
Copy Markdown
Collaborator Author

Change map

File Function / scope What changed Why
kumo/testing/bigquery.py CredentialKind, credential_kind, setup_bigquery_credentials, withBigQueryCredentials Classify explicit ADC by its JSON type; keep transitional per-identity key lookup for restricted key-based tests; use unique temporary credential files and restore all modified environment variables through a context manager in sync/async decorators. A prior stored-key setup exported GOOGLE_APPLICATION_CREDENTIALS; later restricted tests treated any existing file as federated ADC and required missing impersonation targets. Shared credential files and leaked environment state also coupled successive tests.
test/testing/test_bigquery_credentials.py Credential selection/lifetime regressions and existing connector/REST identity checks Exercise default/restricted/default sequences for key and ADC modes; assert missing targets and malformed/missing files fail; verify no secret lookup for valid ADC, temporary-file cleanup, and environment restoration after success, failure, skip, and setup errors. Reproduce the observed credential-mode confusion and verify isolation at the helper's actual lifetime boundary rather than only testing the isolation fixture itself.

Observed failure: the two restricted BigQuery permission tests required GCP_BIGQUERY_NO_TABLE_SERVICE_ACCOUNT / GCP_BIGQUERY_NO_JOB_SERVICE_ACCOUNT after a stored service-account key had been exported as ADC. Existing selection used file presence rather than credential type.

Smallest sufficient repair: classify the input credential file, preserve the existing per-identity selection for stored keys, and scope all setup side effects to the decorated test. No workflow, fixture-selection, or production connector changes are included.

Focused verification: 33 tests passed from the fresh-main worktree using the installed Python 3.14 environment and explicit current-checkout package paths. Independent rerun, flake8, isort, and diff whitespace checks passed. Live provider behavior remains unverified.

Stopping condition: credential-type selection and state/file isolation pass their focused regressions while workflow behavior and decorator callsites remain unchanged. Transitional stored-key fallback, broad missing-secret skips, and authentication occurring after dependent fixture setup remain explicit follow-ups for the environment migration; this repair does not claim to solve federation provisioning or every hosted test failure.

@danielmawhirter

Copy link
Copy Markdown
Collaborator Author

Superseded by #224 and #225. The consolidated credential cleanup in #225 retains credential-type correctness and scoped restoration, while removing the AWS stored-key fallback instead of improving it in an intermediate PR. Closing this superseded phase; no separate follow-up is needed.

This branch was previously deployed

1 inactive deployment
gha-kumo-workflow-tests — e48bd3eb Deployed Oct 27, 2026 by danielmawhirter via import #873
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant