You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/11/10 13:41:42 UTC
[GitHub] [airflow] lwyszomi opened a new pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
lwyszomi opened a new pull request #19518:
URL: https://github.com/apache/airflow/pull/19518
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749660793
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
We should use the same credentials in Beam operators and Kubernetes Engine Operator for submiting and monitoring tasks. I do not see the reason why the pod/beam job should be created with the use of one credential and then monitored with the use of another credentials. This likely won't work as each credential has a different permission scope and a task created by one account with one credential doesn't need to access the other account's data.
If we do not have any warnings and notifications, this is even more problematic because the user is not aware that we have not used the credentials selected by the user.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749111779
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
[GoogleBaseHook.provide_authorized_gcloud](https://github.com/apache/airflow/blob/6e43137cac4f9adedcb1622c7178862a051ecd30/airflow/providers/google/common/hooks/base_google.py#L484) is used to pass credentials from Apache Airflow to Apache Beam process. And this method does not consider impersonation chain as each application have a different way to pass the impersonation chain
I think this option was added by mistake in [this PR](https://github.com/apache/airflow/pull/12814/files) and has never been tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r750100111
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
Yes. Exactly. Therefore, we cannot recommend new operators in the documentation, because this impersonation does not fully work. The fact that this parameter was added is a mistake as it does not work and It has also never been tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi edited a comment on pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi edited a comment on pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#issuecomment-970154786
@mik-laj I updated the PR, I removed recommendation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r750100111
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
Yes. Exactly. Therefore, we cannot recommend new operators in the documentation, because this impersonation does not fully work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749111779
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
[GoogleBaseHook.provide_authorized_gcloud](https://github.com/apache/airflow/blob/6e43137cac4f9adedcb1622c7178862a051ecd30/airflow/providers/google/common/hooks/base_google.py#L484) is used to pass credentials from to Apache Beam process. And this method does not consider impersonation chain as each application have a different way to pass the impersonation chain
I think this option was added by mistake in [this PR](https://github.com/apache/airflow/pull/12814/files) and has never been tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749051640
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
@mik-laj based on the documentation: to the both operators we can pass [DataflowConfiguration](https://airflow.apache.org/docs/apache-airflow-providers-google/stable/_api/airflow/providers/google/cloud/operators/dataflow/index.html) where we can specify `impersonation_chain` parameter and we don't have any information that we support only monitoring process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749051741
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,13 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
Review comment:
Make sense, Thanks. 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r752049070
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
Review comment:
The previous use of the impersonation chain looked fine for me. I just had my doubts about the name of the local variable. A local variable has never stored multiple accounts, but always only one account.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r752067818
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
Review comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749314776
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
@mik-laj I assumed that if we using that configuration and we don't have any warnings or information then 'impersonation_chain' is used inside the operators and not only this but all other parameters from DataflowConfiguration. I didn't run beam operators to verify that works or not. I only verified GKEStartPodOperator.
in the `provide_authorization_gcloud` we only set up the gcloud connection and we don't need there `--impersonate-service-account`, this parameter is only needed in the execution process. Authorization process still should use the orginal SA, please let me know if I'm wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#issuecomment-972684174
CI are sad. Can you do aa rebase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r752189374
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
Review comment:
@mik-laj I adjust my solution
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#issuecomment-972817548
@mik-laj Rebased
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r748633431
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -285,6 +285,15 @@ class GKEStartPodOperator(KubernetesPodOperator):
:param gcp_conn_id: The google cloud connection id to use. This allows for
users to specify a service account.
:type gcp_conn_id: str
+ :param impersonation_chain: Optional service account to impersonate using short-term
+ credentials, or chained list of accounts required to get the access_token
Review comment:
the chained list of multiple accounts is not supported. We only support one element listt.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749970729
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
@mik-laj So if now we support impersonation only in the monitoring tasks we should adjust the submitting tasks, to use impersonation, because from my perspective now this is confused for users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r748632907
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
Did you check it? I think we are still missing the piece of code that will pass impersonation details to the Apache Beam subprocess. For now, only the monitoring process supports, but that might not be enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749314776
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
@mik-laj I assumed that if we using that configuration and we don't have any warnings or information then it used inside the operators. I didn't run beam operators to verify that works or not. I only verified GKEStartPodOperator.
in the `provide_authorization_gcloud` we only set up the gcloud connection and we don't need there `--impersonate-service-account`, this parameter is only needed in the execution process. Authorization process still should use the orginal SA, please let me know if I'm wrong.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r748633274
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,13 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
Review comment:
Since we only support one element, we should throw in an exception when someone passes several elements in an array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r749051814
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -285,6 +285,15 @@ class GKEStartPodOperator(KubernetesPodOperator):
:param gcp_conn_id: The google cloud connection id to use. This allows for
users to specify a service account.
:type gcp_conn_id: str
+ :param impersonation_chain: Optional service account to impersonate using short-term
+ credentials, or chained list of accounts required to get the access_token
Review comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r750137130
##########
File path: docs/apache-airflow-providers-google/connections/gcp.rst
##########
@@ -160,10 +160,13 @@ access token, which will allow to act on its behalf using its permissions. ``imp
does not even need to have a generated key.
.. warning::
- :class:`~airflow.providers.google.cloud.operators.kubernetes_engine.GKEStartPodOperator`,
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreateJavaJobOperator` and
:class:`~airflow.providers.google.cloud.operators.dataflow.DataflowCreatePythonJobOperator`
- do not support direct impersonation as of now.
+ do not support direct impersonation as of now. Both are deprecated and new operators
+ :class:`~airflow.providers.apache.beam.operators.beam.BeamRunJavaPipelineOperator` and
Review comment:
@mik-laj great that we are on the same page, I will remove recommendation in the documentation and I will prepare new PR to support impersonation in the Beam operators for Data Flow execution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi closed pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi closed pull request #19518:
URL: https://github.com/apache/airflow/pull/19518
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#issuecomment-970154786
@mik-laj tr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r750334691
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
+ elif len(self.impersonation_chain) == 1:
+ impersonation_chain = self.impersonation_chain[:-1]
+ else:
+ raise AirflowException(
+ "Chained list of accounts is not supported, please specify only one service account"
+ )
+
+ cmd.extend(
Review comment:
Can you add any tests for tthis change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r750337704
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
Review comment:
This variable name is confusing. The chain suggests it will be. multiple accounts, but you have one account. This variable (`impersonation_chain`) can also be confused with the `self.impersonation_chain` field. What do you think to rename it to ``impersonation_account``?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r750337099
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
+ elif len(self.impersonation_chain) == 1:
+ impersonation_chain = self.impersonation_chain[:-1]
+ else:
+ raise AirflowException(
+ "Chained list of accounts is not supported, please specify only one service account"
+ )
+
+ cmd.extend(
Review comment:
@mik-laj I will add unit tests for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r750345438
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
Review comment:
@mik-laj I wanted to change this to `impersonation_service_account` but then we don't need to check if we have more that one element in it, also I wanted to be consistent between operators that why I used `impersonation_chain`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj edited a comment on pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#issuecomment-972684174
CI is sad. Can you do aa rebase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19518:
URL: https://github.com/apache/airflow/pull/19518#discussion_r751090413
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
+ elif len(self.impersonation_chain) == 1:
+ impersonation_chain = self.impersonation_chain[:-1]
+ else:
+ raise AirflowException(
+ "Chained list of accounts is not supported, please specify only one service account"
+ )
+
+ cmd.extend(
Review comment:
@mik-laj Done
##########
File path: airflow/providers/google/cloud/operators/kubernetes_engine.py
##########
@@ -350,6 +361,22 @@ def execute(self, context) -> Optional[str]:
"--project",
self.project_id,
]
+ if self.impersonation_chain:
+ if isinstance(self.impersonation_chain, str):
+ impersonation_chain = self.impersonation_chain
Review comment:
@mik-laj Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi closed pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
lwyszomi closed pull request #19518:
URL: https://github.com/apache/airflow/pull/19518
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj merged pull request #19518: Support impersonation_chain parameter in the GKEStartPodOperator
Posted by GitBox <gi...@apache.org>.
mik-laj merged pull request #19518:
URL: https://github.com/apache/airflow/pull/19518
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org