refactor(cli): migrate BigQuery integration tests from KMS-encrypted test key to ADC by mbiernacik · Pull Request #2362 · dataform-co/dataform

mbiernacik · 2026-10-05T15:30:07Z

Summary

BigQuery integration and e2e tests now authenticate with Application Default Credentials (ADC) instead of the KMS-encrypted service account key test_credentials/bigquery.json.enc. They run as dataform-testing@dataform-open-source.iam.gserviceaccount.com through service account impersonation, while CI itself keeps running as the Cloud Build service account. The repository no longer stores or decrypts any private key.

Changes

impersonateServiceAccount credentials field (protos/profiles.proto): optional field in .df-credentials.json. When set, the CLI uses the key from credentials, or ADC, only as source credentials to impersonate that service account (google-auth-library Impersonated, scopes cloud-platform and drive), for both BigQuery and lineage emission. The source identity needs roles/iam.serviceAccountTokenCreator on the target account. The adapter is still created synchronously: the source client is resolved on first use.
dataform init-creds (cli/credentials.ts): asks for an optional service account to impersonate as its last step (Enter skips it) and saves it as impersonateServiceAccount. The connection test then already runs as that account, so missing permissions are reported before the credentials file is written.
google-auth-library is now a direct dependency (~10.5.0, the version @google-cloud/bigquery and google-gax already use) and is external in the CLI bundle.
test_credentials/: the committed bigquery.default.json sets the project, location and service account to impersonate (dataform-open-source, US, dataform-testing@…). A git-ignored bigquery.json overrides it locally, with the same name and behaviour as before.
testing/credentials.ts: integrationSuite() works like suite(), but first registers a set-up hook that fails the whole suite within about a second if no credentials are found. The check looks, in order, for a key in the credentials file, GOOGLE_APPLICATION_CREDENTIALS, the gcloud ADC file and the metadata server; it doesn't validate what it finds. All 11 integration suites use it.
.bazelrc: ADC-related environment variables and the metadata server settings (GCE_METADATA_HOST, GCE_METADATA_IP, METADATA_SERVER_DETECTION) are forwarded only with --config=integration, which scripts/run_integration_tests uses, so unit tests stay hermetic.
KMS decryption removed: gcloud_secret_or_local, tools/gcloud/, scripts/decrypt_secret, scripts/update_test_credentials and gcloud_sdk_ext in MODULE.bazel.
--credentials key file: integration tests no longer exercise it, so it's covered by unit tests of getBigQueryClientOptions() and by tests/api/credentials.spec.ts.
Docs: contributing.md (ADC setup, impersonation requirements, local override), cli/README.md (the new credentials field and the init-creds prompt).

Testing

./scripts/run_tests and ./scripts/lint pass.
New unit tests: client options, a shared auth client per provider and the token exchange against a fake source client (bigquery_test.ts), the lineage client (emitter_test.ts), reading the new field (credentials.spec.ts), the ADC check (testing/credentials_test.ts) and the init-creds prompts (cli/credentials_test.ts).
Integration tests pass in CI with the default configuration, i.e. impersonating dataform-testing@ (Cloud Build 9bfd566a on 4548f9c), and locally with a personal GCP project override, without impersonation.
A new integration test checks that SESSION_USER() equals the configured service account.
Without any credentials, an integration suite fails in about a second on the ADC check, before its own set-up hooks.

Notes

Locally, ADC impersonation calls the IAM Service Account Credentials API on behalf of the ADC quota project, so that API must be enabled there (documented in contributing.md).
Without impersonation, createBigQueryClientProvider() still passes a key through google-auth-library's credentials option, which newer typings mark as deprecated. It's unchanged here and left for a follow-up.

GCP admin actions (`dataform-open-source`)

Before merging

Nothing. The Cloud Build service account already has roles/iam.serviceAccountTokenCreator on dataform-testing@, iamcredentials.googleapis.com is enabled, and dataform-testing@ has roles/bigquery.admin in the project.

After merging (once CI is green on `main`)

Delete the user-managed keys of dataform-testing@dataform-open-source.iam.gserviceaccount.com. Removing bigquery.json.enc from the repository doesn't revoke the key inside it. Keep the service account itself: CI now impersonates it.
Remove the project-level roles/cloudkms.cryptoKeyEncrypterDecrypter from the Cloud Build service account and from dataform-testing@. It was only needed to decrypt the test key with dataform-builder-key (key ring dataform-builder-keyring, global). Nothing else in this repository uses that key, but check that nothing outside it does first.

Open PRs that branched off before this one still decrypt the old key in CI. After steps 1 and 2, their integration tests fail until they're rebased onto main.

rafal-hawrylak · 2026-10-08T07:58:52Z

+    return Promise.resolve(false);
+  }
+  const metadataHost = env.GCE_METADATA_IP || env.GCE_METADATA_HOST || DEFAULT_METADATA_HOST;
+  const cleanHost = metadataHost.replace(/^https?:\/\//, "");


The fast credentials check gets a wrong result when GCE_METADATA_HOST includes a port (e.g. localhost:8080 for a metadata emulator). http.get({ host: "127.0.0.1:PORT" }) fails with ENOTFOUND, while the same address as a URL returns 200 (verified). Then ensureAdcAvailable says credentials are missing when they aren't. The Google auth library and the bash version in scripts/run_integration_tests both build a URL, so they work. Use a URL here: http.get(\http://cleanHost{METADATA_TOKEN_PATH}`, {...})`.

Good catch, fixed. The probe now builds the URL the way gcp-metadata does, so host:port works and an explicit http:///https:// scheme is kept. Added regression tests with a fake metadata server.

rafal-hawrylak · 2026-10-08T08:00:52Z

+  --test_env=GOOGLE_APPLICATION_CREDENTIALS \
+  --test_env=CLOUDSDK_CONFIG \
+  --test_env=GOOGLE_CLOUD_QUOTA_PROJECT \
+  --test_env=DATAFORM_TEST_CREDENTIALS \


A relative DATAFORM_TEST_CREDENTIALS passes the bash check, because it's resolved from the repo root there. Bazel then passes the same relative value into the test sandbox - will it work there?

Good point, it wouldn't: tests run from the runfiles tree, so a relative path only worked if the file happened to be in runfiles. Both the bash check and DATAFORM_TEST_CREDENTIALS are gone now. The only override, test_credentials/bigquery.json, comes through the :bigquery_credentials filegroup.

rafal-hawrylak · 2026-10-08T08:01:28Z

+    expect(envWithExistingCreds.GOOGLE_APPLICATION_CREDENTIALS).to.equal("/existing/creds.json");
+  });
+
+  test("ensureAdcAvailable succeeds for explicit key, env var, or gcloud ADC and fails fast when missing", async () => {


One test covers 5 scenarios, and two of them use try/catch with ?.message, so a missing throw only shows up as a vague failure. Split it into separate tests and use expect(...).to.be.rejectedWith(...).

Done, split into separate tests (now in testing/credentials_test.ts). For rejections I used Node's assert.rejects instead of rejectedWith, which would need chai-as-promised as a new dependency. It also fails explicitly when nothing is thrown.

rafal-hawrylak · 2026-10-08T08:01:48Z

+suite("BigQueryDbAdapter", ({ afterEach }) => {
+  const tmpDirFixture = new TmpDirFixture(afterEach);
+
+  test("createBigQueryClientProvider parses service account key from --credentials file", async () => {


Its credentials.read() checks repeat the new tests in tests/api/credentials.spec.ts. Keep them in one place only.

Done, these checks are now only in tests/api/credentials.spec.ts.

rafal-hawrylak · 2026-10-08T08:02:34Z

+# To override the target GCP project or location locally, create a git-ignored
+# `test_credentials/bigquery.local.json` file or set the `DATAFORM_TEST_CREDENTIALS`
+# environment variable. See contributing.md for details.
+exports_files(["bigquery.json"])


exports_files(["bigquery.json"]) is now unused, since everything goes through :bigquery_credentials. Remove it.

apilaskowski · 2026-10-13T11:38:17Z

 )
-
-# Custom gcloud SDK extension
-gcloud_sdk_ext = use_extension("//tools/gcloud:extensions.bzl", "gcloud_sdk_extension")


The description says tools/gcloud/ was removed, but this PR doesn't touch it. tools/gcloud/secrets.bzl, extensions.bzl and repository_rules.bzl are still there, and with this extension usage gone they're dead code. Could you delete them here (or update the description)?

This PR does delete tools/gcloud/: all five files (BUILD, extensions.bzl, gcloud_function.bzl, repository_rules.bzl and secrets.bzl) are in the diff as deleted. GitHub just collapses deleted files in "Files changed". On the branch head there's no tools/gcloud/ directory, and nothing references it anymore (git grep -E "tools/gcloud|gcloud_secret|gcloud_sdk" finds nothing).

apilaskowski · 2026-10-13T11:38:17Z

+    srcs = [
+        "bigquery.default.json",
+    ] + glob(
+        ["bigquery.json"],


This file (and contributing.md) call test_credentials/bigquery.json git-ignored, but I can't find it in .gitignore. The only credentials entry there is .df-credentials.json. The docs still suggest putting a service account key in this file, so it's easy to commit a secret by accident. Please add test_credentials/bigquery.json to .gitignore in this PR.

It's already ignored by the existing bigquery.json entry in .gitignore (line 17). That entry was already on main, so I didn't add a separate one.

apilaskowski · 2026-10-13T11:38:17Z

-# * Run the "scripts/update_test_credentials" script.
-gcloud_secret_or_local(
-    name = "bigquery.json",
+# Integration tests authenticate with Application Default Credentials (ADC); no secret key is


Deleting bigquery.json.enc doesn't revoke the key inside it. Could we track deleting the dataform-testing@dataform-open-source SA key in IAM, and cleaning up dataform-builder-key/keyring access if nothing else uses it, as a follow-up after merge?

Agreed, deleting the file doesn't revoke the key. After merge, once CI is green on main:

Delete the user-managed keys of dataform-testing@dataform-open-source.iam.gserviceaccount.com. The service account itself has to stay, because CI now impersonates it.

Remove roles/cloudkms.cryptoKeyEncrypterDecrypter from the Cloud Build service account and from dataform-testing@. Nothing else in this repo uses dataform-builder-key (the publish and version builds read their tokens from Secret Manager), but something outside the repo might, so that's worth checking first.

I'll list both steps in the PR description, so they're tracked with the merge.

apilaskowski · 2026-10-13T11:38:17Z

-# Run integration tests
+# Run integration tests (--config=integration forwards the ADC environment, see .bazelrc).
 bazel test ... \
+  --config=integration \


How does CI authenticate now? Before, the Bazel gcloud_secret rule decrypted the key. Now it depends on the build SA reaching the metadata server from inside the test sandbox. Could you confirm that the CI SA has roles/bigquery.jobUser and roles/bigquery.dataEditor on dataform-open-source, and that the metadata server is reachable with USE_CLOUD_BUILD_NETWORK? A link to a green integration run with this change would be great.

Since your review I've added service account impersonation, so in CI it works like this:

Cloud Build's bazel step runs scripts/run_tests_on_cloudbuild, which runs scripts/run_integration_tests (bazel test --config=integration).

The tests get the Cloud Build service account's token from the metadata server. Build step containers can reach it (the first step of cloudbuild-test.yaml already relies on it for gcloud auth print-access-token), and Bazel's sandbox doesn't block the network.

With that token, they impersonate dataform-testing@dataform-open-source.iam.gserviceaccount.com, so all BigQuery jobs run as that account.

Permissions:

dataform-testing@ has roles/bigquery.admin on dataform-open-source, which includes roles/bigquery.jobUser and roles/bigquery.dataEditor.

The Cloud Build service account has roles/iam.serviceAccountTokenCreator on dataform-testing@, and the IAM Service Account Credentials API is enabled.

USE_CLOUD_BUILD_NETWORK=true predates this PR, and nothing in the repo reads it, so it doesn't affect access to the metadata server.

Green integration run on the current head (e2e55fa): Cloud Build 403983ac. It includes the new SESSION_USER() test, which fails if the jobs don't run as dataform-testing@. For an earlier run I also checked INFORMATION_SCHEMA.JOBS_BY_PROJECT: all 146 jobs from that build ran as dataform-testing@, and none as the Cloud Build service account.

apilaskowski · 2026-10-13T11:38:17Z

+  } else if (env.HOME) {
+    wellKnownPaths.push(path.join(env.HOME, ".config", "gcloud", ADC_FILENAME));
+  }
+  if (wellKnownPaths.some((candidate) => fs.existsSync(candidate))) {


nit: this only checks that the ADC file exists, not that it's valid or unexpired. So something like expired user credentials will pass here and fail later with a less helpful auth error. That's fine for a fast pre-check, but maybe add a comment saying so.

The ensureAdcAvailable() doc comment now says it's only a fast pre-check that looks for credentials but doesn't validate them, so e.g. expired user credentials fail later, with google-auth-library's error.

apilaskowski · 2026-10-13T11:38:17Z

+ *     ...
+ *   });
+ */
+export function requireAdc(setUp: IHookHandler, options: IAdcCheckOptions = {}): void {


nit: "must be called first in the suite" is only documented here, and the call is repeated in 11 suites. Would it be more robust to have a shared integration-suite helper (or index_test_base) register it, so new suites can't forget it or put it in the wrong order?

Good idea, done. integrationSuite() in testing/credentials.ts works like suite(), but registers the ADC check before calling the suite body, so the check always runs before the suite's own set-up hooks. All 11 suites use it now, and the "call it first" rule is gone from the requireAdc() docs.

apilaskowski · 2026-10-13T11:38:17Z

+        project_id: "key-project-id",
+        private_key_id: "key-id-123",
+        private_key:
+          "-----BEGIN PRIVATE KEY-----\nMIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC\n-----END PRIVATE KEY-----\n",


nit: the PEM-looking private_key may trigger secret scanners (e.g. GitHub push protection or internal scanners). The test only checks pass-through, so an obviously fake value like "fake-private-key" would work just as well.

Done, it's "fake-private-key" now. I changed the same pattern in tests/api/credentials.spec.ts too.

apilaskowski · 2026-10-13T11:38:18Z

+      });
+    });
+
+    test("does not downgrade an explicit https:// GCE_METADATA_HOST to plain http", async () => {


nit: this test makes a real TLS handshake to the local stub. It's quick and local, but setting a small metadataProbeTimeoutMs would keep it fast on slow CI machines.

Done, the test sets metadataProbeTimeoutMs: 200. The handshake normally fails right away, so this only caps it on slow machines. A downgrade to plain HTTP would still be caught, since the local stub answers within a few milliseconds.

apilaskowski · 2026-10-13T11:38:18Z

+test:integration --test_env=APPDATA
+test:integration --test_env=GOOGLE_APPLICATION_CREDENTIALS
+test:integration --test_env=CLOUDSDK_CONFIG
+test:integration --test_env=GOOGLE_CLOUD_QUOTA_PROJECT


nit: ensureAdcAvailable honours GCE_METADATA_HOST/GCE_METADATA_IP/METADATA_SERVER_DETECTION, but they aren't forwarded here, so in the sandbox they'd only work through an explicit --test_env. Worth forwarding them too, for people using metadata emulators?

Done. --config=integration now also forwards GCE_METADATA_HOST, GCE_METADATA_IP and METADATA_SERVER_DETECTION, which google-auth-library reads too. Bazel doesn't pass variables that aren't set on the client, so CI is unaffected.

…count

- Add integrationSuite(), a suite() wrapper that registers the ADC check before anything else in the suite, and use it in all 11 integration suites instead of calling requireAdc(before) first in each of them. - Document that ensureAdcAvailable() is only a fast pre-check: it looks for credentials but doesn't validate them. - Forward GCE_METADATA_HOST, GCE_METADATA_IP and METADATA_SERVER_DETECTION with --config=integration, so that metadata server emulators also work inside the test sandbox. - Use an obviously fake private key in tests, so that it can't trip secret scanners. - Cap the metadata probe timeout in the test that checks an https:// GCE_METADATA_HOST isn't downgraded to plain HTTP.

Keep the existing universe-domain commits, and teach the impersonated auth client from dataform-co#2362 to use the same universe so token exchange and BigQuery agree when both fields are set. Co-authored-by: ybelleguic <yohan.belleguic@gmail.com>

Keep the existing universe-domain commits, and teach the impersonated auth client from dataform-co#2362 to use the same universe so token exchange and BigQuery agree when both fields are set.

mbiernacik requested a review from a team as a code owner October 5, 2026 15:30

mbiernacik requested review from apilaskowski and removed request for a team October 5, 2026 15:30

apilaskowski reviewed Oct 7, 2026

View reviewed changes

apilaskowski requested changes Oct 7, 2026

View reviewed changes

rafal-hawrylak requested changes Oct 8, 2026

View reviewed changes

rafal-hawrylak mentioned this pull request Oct 9, 2026

Bazel toolchain rewrite #2352

Merged

mbiernacik force-pushed the chore/adc-integration-tests branch from fccffe7 to c368073 Compare October 12, 2026 13:51

apilaskowski reviewed Oct 13, 2026

View reviewed changes

mbiernacik added 2 commits October 14, 2026 12:16

Migrate BigQuery integration tests from KMS-encrypted test key to ADC

1ae69b3

Run integration tests as the impersonated dataform-testing service ac…

f12d708

…count

mbiernacik force-pushed the chore/adc-integration-tests branch from 5d1df26 to 8f9697a Compare October 14, 2026 13:54

Ask for a service account to impersonate in init-creds

e2e55fa

mbiernacik force-pushed the chore/adc-integration-tests branch 2 times, most recently from 8f9697a to e2e55fa Compare October 14, 2026 14:03

rafal-hawrylak mentioned this pull request Oct 16, 2026

Handle GCP multi-universe #2323

Open

rafal-hawrylak approved these changes Oct 16, 2026

View reviewed changes

apilaskowski approved these changes Oct 16, 2026

View reviewed changes

mbiernacik merged commit 2a913e6 into main Oct 16, 2026
14 checks passed

mbiernacik deleted the chore/adc-integration-tests branch October 16, 2026 14:36

cursor Bot mentioned this pull request Oct 20, 2026

Support setting BigQuery universeDomain ybelleguic/dataform#3

Open

Conversation

mbiernacik commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

Notes

GCP admin actions (dataform-open-source)

Before merging

After merging (once CI is green on main)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

mbiernacik commented Oct 5, 2026 •

edited

Loading

GCP admin actions (`dataform-open-source`)

After merging (once CI is green on `main`)