Skip to content

Add impersonation to Google operators#10052

Merged
mik-laj merged 1 commit intoapache:masterfrom
PolideaInternal:add-impersonation-to-google-operators
Aug 24, 2020
Merged

Add impersonation to Google operators#10052
mik-laj merged 1 commit intoapache:masterfrom
PolideaInternal:add-impersonation-to-google-operators

Conversation

@olchas
Copy link
Contributor

@olchas olchas commented Jul 29, 2020

This PR adds direct impersonation of a service account in Google services. Part-of #8803

A few notes summarizing the changes:

  • no changes in GKEStartPodOperator, DataflowCreateJavaJobOperator and DataflowCreatePythonJobOperator - as these operators do not use Credentials class for authentication
  • in case of operators that connect to multiple Google services, all hooks use the same value of impersonation_chain (if applicable)
  • in case of operators that also communicate with services of other cloud providers, the argument is named google_impersonation_chain
  • changed get_credentials_and_project method of _CredentialProvider class so that if impersonation_chain argument is used then project_id returned is extracted from the e-mail of the impersonated account (target_principal). This is for the scenario when impersonated service account is from different project than the account from Connection.
    project_id will still be overridden if specified in Connection's extras or explicitly in operator's arguments.
  • GCSToS3Operator was modified so that it no longer derivatives from GCSListObjectsOperator
  • impersonation_chain was moved to be the last argument of hooks (if it was not already) to avoid potentially breaking changes when initializing these hooks with positional arguments

Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, 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 UPDATING.md.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg bot added k8s provider:amazon AWS/Amazon - related issues provider:google Google (including GCP) related issues labels Jul 29, 2020
@mik-laj
Copy link
Member

mik-laj commented Jul 30, 2020

@amithmathew Can you look at it?

@olchas
Copy link
Contributor Author

olchas commented Jul 31, 2020

Changed get_credentials_and_project method of _CredentialProvider class so that project_id returned is extracted from target_principal (e-mail of final impersonated service account) - this is for the purpose of impersonating service account from different project than the one specified in Connection. project_id can still be overwritten in Connection's extras or in operator's attributes.

@olchas olchas force-pushed the add-impersonation-to-google-operators branch 3 times, most recently from e339d6b to d664dfc Compare August 3, 2020 11:11
@olchas
Copy link
Contributor Author

olchas commented Aug 3, 2020

Implemented impersonation in newly added Datastore operators (#10032)

@olchas olchas force-pushed the add-impersonation-to-google-operators branch from d664dfc to 25f3d3f Compare August 3, 2020 11:18
@turbaszek
Copy link
Member

@olchas would you mind adding an example DAG to show how users can use impersonation?

@olchas olchas force-pushed the add-impersonation-to-google-operators branch from 25f3d3f to 34fadb3 Compare August 5, 2020 10:47
@codecov-commenter
Copy link

Codecov Report

Merging #10052 into master will decrease coverage by 53.95%.
The diff coverage is 44.38%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #10052       +/-   ##
===========================================
- Coverage   89.06%   35.10%   -53.96%     
===========================================
  Files        1037     1037               
  Lines       49923    50230      +307     
===========================================
- Hits        44464    17634    -26830     
- Misses       5459    32596    +27137     
Flag Coverage Δ
#kubernetes-tests-image-3.6-v1.16.9 ?
#kubernetes-tests-image-3.6-v1.17.5 ?
#kubernetes-tests-image-3.6-v1.18.6 ?
#kubernetes-tests-image-3.7-v1.16.9 ?
#kubernetes-tests-image-3.7-v1.17.5 ?
#kubernetes-tests-image-3.7-v1.18.6 ?
#mysql-tests-Core-3.7-5.7 ?
#mysql-tests-Core-3.8-5.7 ?
#mysql-tests-Integration-3.7-5.7 34.74% <44.38%> (-0.04%) ⬇️
#mysql-tests-Integration-3.8-5.7 ?
#postgres-tests-Core-3.6-10 ?
#postgres-tests-Core-3.6-9.6 ?
#postgres-tests-Core-3.7-10 ?
#postgres-tests-Core-3.7-9.6 ?
#postgres-tests-Integration-3.6-10 34.72% <44.38%> (-0.04%) ⬇️
#postgres-tests-Integration-3.6-9.6 34.72% <44.38%> (-0.04%) ⬇️
#postgres-tests-Integration-3.7-10 34.72% <44.38%> (-0.04%) ⬇️
#postgres-tests-Integration-3.7-9.6 ?
#sqlite-tests-Core-3.6 ?
#sqlite-tests-Core-3.8 ?
#sqlite-tests-Integration-3.6 34.17% <44.38%> (-0.04%) ⬇️
#sqlite-tests-Integration-3.8 34.43% <44.38%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
airflow/providers/google/cloud/hooks/bigquery.py 19.25% <ø> (-73.53%) ⬇️
airflow/providers/google/cloud/hooks/dataflow.py 30.71% <ø> (-58.93%) ⬇️
airflow/providers/google/cloud/hooks/datastore.py 30.76% <ø> (-67.04%) ⬇️
airflow/providers/google/cloud/hooks/gcs.py 16.43% <ø> (-70.48%) ⬇️
.../providers/google/cloud/hooks/kubernetes_engine.py 39.18% <ø> (-56.76%) ⬇️
...ow/providers/google/cloud/operators/datacatalog.py 25.56% <2.32%> (-67.86%) ⬇️
...viders/google/cloud/operators/cloud_memorystore.py 31.84% <12.00%> (-66.59%) ⬇️
...oviders/google/cloud/utils/credentials_provider.py 28.90% <16.66%> (-67.82%) ⬇️
...irflow/providers/google/cloud/operators/spanner.py 27.97% <20.00%> (-70.79%) ⬇️
airflow/providers/google/cloud/operators/tasks.py 30.76% <21.21%> (-68.38%) ⬇️
... and 973 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95328c4...34fadb3. Read the comment docs.

@olchas olchas force-pushed the add-impersonation-to-google-operators branch from 34fadb3 to 9f41190 Compare August 6, 2020 08:00
@codecov-commenter
Copy link

Codecov Report

Merging #10052 into master will decrease coverage by 54.33%.
The diff coverage is 44.38%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #10052       +/-   ##
===========================================
- Coverage   89.44%   35.11%   -54.34%     
===========================================
  Files        1037     1037               
  Lines       49927    50234      +307     
===========================================
- Hits        44658    17639    -27019     
- Misses       5269    32595    +27326     
Flag Coverage Δ
#kubernetes-tests-3.6-9.6 ?
#kubernetes-tests-image-3.6-v1.16.9 ?
#kubernetes-tests-image-3.6-v1.17.5 ?
#kubernetes-tests-image-3.6-v1.18.6 ?
#kubernetes-tests-image-3.7-v1.16.9 ?
#kubernetes-tests-image-3.7-v1.17.5 ?
#kubernetes-tests-image-3.7-v1.18.6 ?
#mysql-tests-Core-3.8-5.7 ?
#mysql-tests-Integration-3.7-5.7 ?
#mysql-tests-Integration-3.8-5.7 35.01% <44.38%> (-0.04%) ⬇️
#postgres-tests-Core-3.6-10 ?
#postgres-tests-Core-3.6-9.6 ?
#postgres-tests-Core-3.7-10 ?
#postgres-tests-Core-3.7-9.6 ?
#postgres-tests-Integration-3.6-10 34.73% <44.38%> (-0.04%) ⬇️
#postgres-tests-Integration-3.6-9.6 ?
#postgres-tests-Integration-3.7-10 34.73% <44.38%> (-0.04%) ⬇️
#postgres-tests-Integration-3.7-9.6 34.73% <44.38%> (?)
#sqlite-tests-Core-3.6 ?
#sqlite-tests-Core-3.8 ?
#sqlite-tests-Integration-3.6 34.18% <44.38%> (-0.04%) ⬇️
#sqlite-tests-Integration-3.8 34.44% <44.38%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
airflow/providers/google/cloud/hooks/bigquery.py 19.25% <ø> (-73.53%) ⬇️
airflow/providers/google/cloud/hooks/dataflow.py 30.71% <ø> (-58.93%) ⬇️
airflow/providers/google/cloud/hooks/datastore.py 30.76% <ø> (-67.04%) ⬇️
airflow/providers/google/cloud/hooks/gcs.py 16.43% <ø> (-70.48%) ⬇️
.../providers/google/cloud/hooks/kubernetes_engine.py 39.18% <ø> (-56.76%) ⬇️
...ow/providers/google/cloud/operators/datacatalog.py 25.56% <2.32%> (-67.86%) ⬇️
...viders/google/cloud/operators/cloud_memorystore.py 31.84% <12.00%> (-66.59%) ⬇️
...oviders/google/cloud/utils/credentials_provider.py 28.90% <16.66%> (-67.82%) ⬇️
...irflow/providers/google/cloud/operators/spanner.py 27.97% <20.00%> (-70.79%) ⬇️
airflow/providers/google/cloud/operators/tasks.py 30.76% <21.21%> (-68.38%) ⬇️
... and 973 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b3287d...9f41190. Read the comment docs.

@olchas olchas force-pushed the add-impersonation-to-google-operators branch 4 times, most recently from a411d19 to 97353d8 Compare August 10, 2020 08:16
@olchas olchas force-pushed the add-impersonation-to-google-operators branch from 97353d8 to 53cd3df Compare August 18, 2020 10:32
@olchas
Copy link
Contributor Author

olchas commented Aug 19, 2020

Implemented impersonation in newly added Bigtable operator (#10340)

@olchas olchas force-pushed the add-impersonation-to-google-operators branch from 53cd3df to 06b99f7 Compare August 21, 2020 09:55
@olchas
Copy link
Contributor Author

olchas commented Aug 21, 2020

I have added instructions of setting up and using direct impersonation in docs/howto/connection/gcp.rst.

@mik-laj, could you take a look?

@turbaszek, do you think it will suffice for usage example? I am not sure if there is much point building a whole example dag around impersonation - usage for every Google operator is the same.

@turbaszek
Copy link
Member

That's huge. The docs look good and I hope that the auto refactors worked as expected. Regarding the example, I think we should add it but we can create an issue for that so no need to do this now.

@mik-laj mik-laj self-requested a review August 21, 2020 17:33
@mik-laj
Copy link
Member

mik-laj commented Aug 21, 2020

I would like to do a review. Please wait a moment.

@mik-laj
Copy link
Member

mik-laj commented Aug 21, 2020

Are you planning to add impersonations for DataflowCreatePythonJobOperator and DataflowCreateJavaJobOperator, GKEStartPodOperator and BigQueryTablePartitionExistenceSensor?

@olchas olchas force-pushed the add-impersonation-to-google-operators branch from 06b99f7 to e7549f6 Compare August 24, 2020 09:18
@olchas olchas force-pushed the add-impersonation-to-google-operators branch from e7549f6 to 3fca76f Compare August 24, 2020 09:24
@olchas
Copy link
Contributor Author

olchas commented Aug 24, 2020

@mik-laj I have added impersonation to BigQueryTablePartitionExistenceSensor. As I mentioned in PR's description, DataflowCreatePythonJobOperator, DataflowCreateJavaJobOperator and GKEStartPodOperator use different form of authentication (they do not use Credentials class) and I would like to leave them for another PR.

@mik-laj mik-laj changed the title Add impersonation to google operators Add impersonation to Google operators Aug 24, 2020
@mik-laj mik-laj merged commit 3734876 into apache:master Aug 24, 2020
@mik-laj mik-laj deleted the add-impersonation-to-google-operators branch August 24, 2020 11:48
@ashb ashb added the provider:cncf-kubernetes Kubernetes (k8s) provider related issues label Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

provider:amazon AWS/Amazon - related issues provider:cncf-kubernetes Kubernetes (k8s) provider related issues provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments