add service_file support to GKEPodAsyncHook #37081
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
area:providers
provider:cncf-kubernetes
Lee-W
changed the title
15 tasks
Lee-W
force-pushed
the
add-service-file-support-to-GKEPodAsyncHook
branch
from
957ad69 to
3339adc
Compare
dirrao
reviewed
Jan 30, 2024
Lee-W
force-pushed
the
add-service-file-support-to-GKEPodAsyncHook
branch
from
d16f2b1 to
b5acaff
Compare
Lee-W
force-pushed
the
add-service-file-support-to-GKEPodAsyncHook
branch
5 times, most recently
from
e5c6eb1 to
1867070
Compare
Lee-W
force-pushed
the
add-service-file-support-to-GKEPodAsyncHook
branch
2 times, most recently
from
97615f2 to
16a0be4
Compare
phanikumv
force-pushed
the
add-service-file-support-to-GKEPodAsyncHook
branch
from
16a0be4 to
a102a08
Compare
pankajkoti
approved these changes
Feb 5, 2024
|
I am merging this one. But please let us know here if there's some feedback and we can address in a subsequent PR cc: @VladaZakharova |
|
I think this commit is problematic when using application default credentials for auth: Sample DAG: from airflow import DAG
from airflow.providers.google.cloud.operators.kubernetes_engine import (
GKEStartPodOperator,
)
DEFAULT_TASK_ARGS = {
"owner": "gcp-data-platform",
"start_date": "2021-04-20",
"retries": 0,
"retry_delay": 60,
}
with DAG(
dag_id="test_gke_op",
schedule_interval="@daily",
max_active_runs=1,
max_active_tasks=5,
catchup=False,
default_args=DEFAULT_TASK_ARGS,
) as dag:
_ = GKEStartPodOperator(
task_id="whoami",
name="whoami",
cmds=["gcloud"],
arguments=["auth", "list"],
image="gcr.io/google.com/cloudsdktool/cloud-sdk:slim",
project_id="redacted-project-id",
namespace="airflow-default",
location="us-central1",
cluster_name="airflow-gke-cluster",
service_account_name="default",
deferrable=True,
do_xcom_push=True,
)
_ = GKEStartPodOperator(
task_id="fail",
name="fail",
cmds=["bash"],
arguments=["-xc", "sleep 2 && exit 1"],
image="gcr.io/google.com/cloudsdktool/cloud-sdk:slim",
project_id="redacted-project-id",
namespace="airflow-default",
location="us-central1",
cluster_name="airflow-gke-cluster",
service_account_name="default",
deferrable=True,
do_xcom_push=True,
)Logs |
This was referenced Feb 10, 2024
81 tasks
m1racoli
pushed a commit
to king/airflow
that referenced
this pull request
Feb 16, 2024
We utilize the existing implementation of `_CredentialsToken` by using the async hook's `get_token` method. This implementation allows us to leverage several features of the Google connection from `Keyfile Path` or `Keyfile JSON` (see apache#37081) to impersonation chain on hook or connection level. We therefore do not need to rely on the async hook's `service_file_as_context` method, which does not support impersonation chain. With this change we effectively gain support for impersonation chain in GKEStartPodOperator in deferrable mode.
potiuk
pushed a commit
that referenced
this pull request
Feb 22, 2024
…7486) We utilize the existing implementation of `_CredentialsToken` by using the async hook's `get_token` method. This implementation allows us to leverage several features of the Google connection from `Keyfile Path` or `Keyfile JSON` (see #37081) to impersonation chain on hook or connection level. We therefore do not need to rely on the async hook's `service_file_as_context` method, which does not support impersonation chain. With this change we effectively gain support for impersonation chain in GKEStartPodOperator in deferrable mode.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, GKEPodAsyncHook does not support service_file. Thus, passing credentials through " Keyfile Path " or " Keyfile JSON " will be ignored. This PR intends to fix this issue. As the default value of
service_fileinTokenis None (https://github.com/talkiq/gcloud-aio/blob/8c8e1b39ec2e40b42212c270acb98c039267fbc5/auth/gcloud/aio/auth/token.py#L157) and the return value ofservice_file_as_contextwhen both key file path and key file json are not provided is None. This change won't affect existing behaviorairflow/airflow/providers/google/common/hooks/base_google.py
Line 537 in 8914e49
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.