Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix impersonation failure on GKE start pod operator async #1274

Merged
merged 9 commits into from
Aug 16, 2023

Conversation

Lee-W
Copy link
Contributor

@Lee-W Lee-W commented Aug 1, 2023

What's the issue?

The argument impersonation_chain does not work. The operator still uses the gcp_conn to start the pod.

What's changed
We suspect the issue is due to that the token is not correctly generated. Thus, we try to write this token into kube_config

@Lee-W Lee-W force-pushed the fix-impersonation-failure-on-GKEStartPodOperatorAsync branch from e40bccb to 9b9ea6a Compare August 1, 2023 10:02
@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e2380a4) 98.57% compared to head (6278686) 98.58%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1274   +/-   ##
=======================================
  Coverage   98.57%   98.58%           
=======================================
  Files          91       91           
  Lines        5404     5428   +24     
=======================================
+ Hits         5327     5351   +24     
  Misses         77       77           
Files Changed Coverage Δ
astronomer/providers/google/cloud/gke_utils.py 100.00% <100.00%> (ø)
...viders/google/cloud/operators/kubernetes_engine.py 100.00% <100.00%> (ø)
...oviders/google/cloud/triggers/kubernetes_engine.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Lee-W Lee-W force-pushed the fix-impersonation-failure-on-GKEStartPodOperatorAsync branch 4 times, most recently from 3d53c13 to 7c7181c Compare August 2, 2023 11:01
@Lee-W
Copy link
Contributor Author

Lee-W commented Aug 2, 2023

圖片

It works fine on my side. But would like to mark it as "Ready for review" after testing by CRE.

Copy link
Collaborator

@phanikumv phanikumv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add more context on why this code change is necessary in the description?

@Lee-W Lee-W force-pushed the fix-impersonation-failure-on-GKEStartPodOperatorAsync branch 6 times, most recently from d2986d5 to dc1bdb2 Compare August 4, 2023 14:28
@Lee-W Lee-W marked this pull request as ready for review August 4, 2023 16:05
@Lee-W Lee-W force-pushed the fix-impersonation-failure-on-GKEStartPodOperatorAsync branch 2 times, most recently from d55a242 to 84f2308 Compare August 6, 2023 07:00
@Lee-W
Copy link
Contributor Author

Lee-W commented Aug 7, 2023

Screenshot 2023-08-07 at 2 45 46 PM

@@ -13,7 +13,7 @@ RUN apt-get install -y --no-install-recommends \
echo "deb [signed-by=/usr/share/keyrings/cloud.google.gpg] http://packages.cloud.google.com/apt cloud-sdk main" | tee -a /etc/apt/sources.list.d/google-cloud-sdk.list && \
curl https://packages.cloud.google.com/apt/doc/apt-key.gpg | apt-key --keyring /usr/share/keyrings/cloud.google.gpg add - && \
apt-get update -y && \
apt-get install google-cloud-sdk -y && \
apt-get install python3 google-cloud-sdk -y && \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we need python3 here? Does it not come with the base image?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the base image, but without this. It doesn't work on my local machine

astronomer/providers/google/cloud/gke_utils.py Outdated Show resolved Hide resolved
astronomer/providers/google/cloud/gke_utils.py Outdated Show resolved Hide resolved
astronomer/providers/google/cloud/gke_utils.py Outdated Show resolved Hide resolved
astronomer/providers/google/cloud/gke_utils.py Outdated Show resolved Hide resolved
@Lee-W Lee-W force-pushed the fix-impersonation-failure-on-GKEStartPodOperatorAsync branch 4 times, most recently from f4e6a0d to df411d6 Compare August 11, 2023 10:45
@Lee-W Lee-W changed the title Fix impersonation failure on gke start pod operator async Fix impersonation failure on GKE start pod operator async Aug 11, 2023
@Lee-W Lee-W force-pushed the fix-impersonation-failure-on-GKEStartPodOperatorAsync branch from df411d6 to 6278686 Compare August 14, 2023 09:28
@phanikumv phanikumv merged commit 9bf9d73 into main Aug 16, 2023
11 checks passed
@phanikumv phanikumv deleted the fix-impersonation-failure-on-GKEStartPodOperatorAsync branch August 16, 2023 14:07
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.

None yet

4 participants