[ADAP-621] [Bug] impersonate_service_account and job_execution_timeout_seconds

[cmc333333]

Is this a new bug in dbt-bigquery?

• I believe this is a new bug in dbt-bigquery
• I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

When setting impersonate_service_account along with a high (>1 hr) job_execution_timeout_seconds, the BQ API gives us an error:

Access tokens for service account [service account email here] can not have lifetimes of more than 1 hour. Use the Org Policy Constraint constraints/iam.allowServiceAccountCredentialLifetimeExtension to allow up to 12 hours.

The credentials shouldn't need to live the whole hour+, however, since they can be re-generated whenever making a new BQ call.

Expected Behavior

A high job_execution_timeout_seconds would be compatible with impersonate_service_account without needing to adjust org policy constraints.

Steps To Reproduce

1. Set up service account impersonation per this doc
2. Add a three hour timeout (using timeout_seconds rather than job_execution_timeout_seconds to match our actual config)

my-profile:
  target: airflow
  outputs:
    airflow:
      type: bigquery
      method: oauth
      impersonate_service_account: "{{ env_var('DBT_SA_EMAIL') }}"
      project: "{{ env_var('DBT_PROJECT') }}"
      dataset: "dbt"
      retries: 3
      threads: 10
      timeout_seconds: 10800 # 3 hours
      location: US
      priority: interactive

3. Try to run that target. See an error

Relevant log output

No response

Environment

- OS: Debian GNU/Linux 11 (bullseye)
- Python: Python 3.9.16
- dbt-core: 1.5.1
- dbt-bigquery: 1.5.1

Additional Context

We're trying to switch away from keyfile_json, where we first specified the three hour limit.

[Fleid]

Hi @cmc333333,

I've just given a quick read to what we're doing with that timeout settings.
We're using it when fetching results from the query, as expected:

dbt-bigquery/dbt/adapters/bigquery/connections.py

Line 596 in ab7ed5d

iterator = query_job.result(timeout=job_execution_timeout)

But we're also injecting it as lifetime of the impersonated_credentials:

Number of seconds the delegated credential should be valid for

dbt-bigquery/dbt/adapters/bigquery/connections.py

Line 304 in ab7ed5d

lifetime=(profile_credentials.job_execution_timeout_seconds or 300),

I don't know why we are doing that to be honest. The parameter is not mandatory, and the default value is its max anyway (3600 sec = 1h).
I did some digging and the initial commit just wires it that way with no explanation that I can find.

Does anything come to mind on your side?

So we could just remove the wiring, or replace it with a new parameter - but I'm wondering if that's really necessary since it's already defaulting to its max.

[cmc333333]

Hiya @Fleid ! I'll try to give you my understanding of the world and we can see how that matches up.

• Impersonation is also known as "short lived access tokens" -- more or less allowing one Google account to do everything a particular service account can do for a limited period of time.
• If the necessary permission is revoked the token is still valid, so that "short lived" part is pretty important. In fact, Google offers a mechanism for you to reduce the token lifetime when requesting it (from the default 1 hour).
• Google reused this mechanism to allow the tokens to expire after up to twelve hours, if requested. The catch is that you need to set an "org policy" at the organization, folder, or project level. See "constraints/iam.allowServiceAccountCredentialLifetimeExtension" here for more details. My understanding is that this is for "weird" cases where you can't easily regenerate the auth token.
• For most cases, we can instead rely on the google-auth Python library (which is used by all of the Google-provided Python SDKs). It knows how to fetch a new token when the earlier one expires. Notably: if the "hey is that BQ job done?" request uses an expired token, the google libraries should retry it for us on their own.

I think that all points to dbt-bigquery's use of "lifetime" as not quite right. I think the simplest fix would be to stop setting the value. If we really wanted, we could have a separate config, but I don't see a use case -- we're relying on Google's SDK throughout, yeah?

[Fleid]

I'm aligned @cmc333333
Are you interested in opening a PR for that?

[cmc333333]

Will give it a go soonish, @Fleid , thanks!

[dbeatty10]

Thanks again for raising this and providing all your keen insights @cmc333333 🧠

Are you still interested in putting up a PR for this, by any chance?

[cmc333333]

Thanks for the nudge @dbeatty10 ; I need to get a local setup going, but I created a PR and signed the CLA.

[dbeatty10]

Awesome @cmc333333 !

@Fleid I marked the PR (#882) as ready_for_review ✅