Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces credentials scoping for BigQuery Read and Write clients in BigQueryConnection to support Trusted Partner Cloud (TPC) environments, and updates several integration tests by removing the @Tag("disable_tpc") annotation and asserting specific result set types. The review feedback suggests improving test robustness by adding null and presence checks for the Authorization header in the test helper extractBearerJwtPayload to avoid potential NullPointerException or IndexOutOfBoundsException, and using try-with-resources for jsonResultSet in ITBigQueryJDBCTest to prevent resource leaks if assertions fail.
…c/it/ITBigQueryJDBCTest.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| assertThrows(BigQueryJdbcException.class, () -> new BigQueryConnection(url)); | ||
| assertTrue(ex.getMessage().contains("multiple 'session_id' entries")); | ||
| } | ||
|
|
There was a problem hiding this comment.
A few nits on the tests, mainly to make them more readable and to reuse the setup and helpers we already have. Could you do a quick cleanup pass along these lines? Something like this prompt should cover it:
Refactor the tests added in this PR for readability without changing what they cover: reuse the existing fixtures in
BigQueryJdbcBaseTest(fake_pkcs8_key,getUriOAuthServiceAccount()) instead of generating a key and building the PEM and URL by hand, extract the duplicated Read/Write credential assertions into one helper, and add cases for a non-service-account credential and for the default universe. In the integration tests, move the sharedGENERATE_ARRAYquery and the Arrow result set class check intoITBaseas a constant and a helper, and run the formatter.
Summary
Queries routed through the BigQuery Storage Read API (HTAPI / Arrow) or Storage Write API fail with
UNAUTHENTICATEDwhen connecting to non-default universe domains (TPC / customuniverseDomain) using service account or Application Default Credentials.Root Cause
BigQueryJdbcOAuthUtility.getCredentials(...)returns unscopedGoogleCredentials(createScopedRequired() == true) unlessRequestGoogleDriveScope=1or service account impersonation is configured.BigQueryclient,BigQueryOptions(ServiceOptions.getScopedCredentials()) automatically scopes credentials, causingServiceAccountCredentialsin non-default universes to mint a scope-based self-signed JWT (scopeclaim, noaudclaim).BigQueryReadClientandBigQueryWriteClient,BigQueryConnectionbypassedBigQueryReadSettings.defaultCredentialsProviderBuilder()/BigQueryWriteSettings.defaultCredentialsProviderBuilder()and wrapped the unscoped credential directly inFixedCredentialsProvider.create(...). BecausecreateScopedRequired()remainedtrue,ServiceAccountCredentialsfell back to minting an audience-based self-signed JWT (audset to the custom universe Storage endpoint, noscopeclaim), which is rejected withUNAUTHENTICATED.Fix
BigQueryConnection.scopeStorageCredentials(Credentials, List<String>)mirroringGoogleCredentialsProviderinBigQueryReadSettingsandBigQueryWriteSettings:BigQueryReadSettings.getDefaultServiceScopes()/BigQueryWriteSettings.getDefaultServiceScopes()) whencreateScopedRequired()istrue, while preserving explicit scopes when already configured (such asRequestGoogleDriveScope=1).createWithUseJwtAccessWithScope(true)onServiceAccountCredentialsso self-signed JWTs include thescopeclaim without requiring extra OAuth token endpoint round-trips in GDU.@Tag("disable_tpc")) that were previously disabled due to this authentication failure.Tests
BigQueryConnectionTest: verifiesBigQueryReadClientandBigQueryWriteClientpreserve customuniverseDomainsettings and mint scope-based self-signed JWTs (scopeclaim present,audabsent), including when explicit scopes are pre-applied.ITBigQueryJDBCTest: re-enabledtestReadAPIPathLarge,testReadAPIPathLargeWithThresholdParameters, andtestReadAPIPathLargeWithThresholdNotMetfor TPC using self-containedUNNEST(GENERATE_ARRAY(...))queries and explicit result set type assertions.ITNightlyBigQueryTest: re-enabledtestHTAPIWithValidDestinationTableSavesQueriesWithStandardSQL,testReadAPIPathLarge, andtestReadAPIPathLargeWithThresholdParametersfor TPC.