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/08/31 18:09:31 UTC
[GitHub] [airflow] mik-laj opened a new pull request #17951: Refresh credentials for long-running pod on EKS
mik-laj opened a new pull request #17951:
URL: https://github.com/apache/airflow/pull/17951
It is still a draft. I need to update the tests.
The currently generated token is generated only once and is valid for one hour, so when the task is longer, the operation will fail.
I added support for refreshing credentials with [client-go credential plugins](https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins), so now a new token is fetched every time it expires. I wrote an `airflow/providers/amazon/aws/utils/eks_get_token.py` script for this, which is similar to `aws eks get-token`, but all supports authentication protocols natively supported by Airflow including Secret backend, Google Credential using Web Identity Federation, HTTP SPEGO, SAML.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ 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] potiuk commented on a change in pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r718857201
##########
File path: airflow/providers/amazon/aws/example_dags/example_eks_using_defaults.py
##########
@@ -73,7 +72,7 @@
task_id="run_pod",
cluster_name=CLUSTER_NAME,
image="amazon/aws-cli:latest",
- cmds=["sh", "-c", "ls"],
+ cmds=["sh", "-c", "echo Test Airflow; date"],
Review comment:
Nice change indeed.
--
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] ferruzzi commented on a change in pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r712337062
##########
File path: airflow/providers/amazon/aws/example_dags/example_eks_using_defaults.py
##########
@@ -73,7 +72,7 @@
task_id="run_pod",
cluster_name=CLUSTER_NAME,
image="amazon/aws-cli:latest",
- cmds=["sh", "-c", "ls"],
+ cmds=["sh", "-c", "echo Test Airflow; date"],
Review comment:
Purely out of curiosity and so I know for he future, is there a reason for this change or just preference??
--
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] potiuk commented on a change in pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r718857582
##########
File path: airflow/providers/amazon/aws/hooks/eks.py
##########
@@ -35,6 +35,7 @@
DEFAULT_PAGINATION_TOKEN = ''
STS_TOKEN_EXPIRES_IN = 60
+AUTHENTICATION_API_VERSION = "client.authentication.k8s.io/v1alpha1"
Review comment:
Sounds good!
--
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] potiuk merged pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #17951:
URL: https://github.com/apache/airflow/pull/17951
--
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 #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#issuecomment-913106841
@potiuk I am awaiting a review from AWS Team, but the person is on vacation.
--
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] ferruzzi commented on a change in pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r712380866
##########
File path: airflow/providers/amazon/aws/hooks/eks.py
##########
@@ -35,6 +35,7 @@
DEFAULT_PAGINATION_TOKEN = ''
STS_TOKEN_EXPIRES_IN = 60
+AUTHENTICATION_API_VERSION = "client.authentication.k8s.io/v1alpha1"
Review comment:
Niko is correct, I'm not sure what the functional difference is, but the docs said (and still say) alpha so that is what I went with in the initial contribution. That doesn't mean we can't go beta, just explaining why I went with what I did.
--
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] ferruzzi commented on a change in pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r712388033
##########
File path: airflow/providers/amazon/aws/example_dags/example_eks_using_defaults.py
##########
@@ -73,7 +72,7 @@
task_id="run_pod",
cluster_name=CLUSTER_NAME,
image="amazon/aws-cli:latest",
- cmds=["sh", "-c", "ls"],
+ cmds=["sh", "-c", "echo Test Airflow; date"],
Review comment:
Got it, I'll keep that in mind for the future, 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 #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r703667973
##########
File path: airflow/providers/amazon/aws/hooks/eks.py
##########
@@ -35,6 +35,7 @@
DEFAULT_PAGINATION_TOKEN = ''
STS_TOKEN_EXPIRES_IN = 60
+AUTHENTICATION_API_VERSION = "client.authentication.k8s.io/v1alpha1"
Review comment:
Should we use `client.authentication.k8s.io/v1beta1`?
--
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] potiuk merged pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #17951:
URL: https://github.com/apache/airflow/pull/17951
--
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 #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#issuecomment-928175875
@potiuk Can you look at it? It is ready for review now.
--
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] o-nikolas commented on pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
o-nikolas commented on pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#issuecomment-918684577
> @potiuk I am awaiting a review from AWS Team, but the person is on vacation.
Yes, sorry folks, both Dennis and I have been on vacation for the past 3 weeks. One of us will get to reviewing this PR in the next few days as we catch back up :)
--
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] o-nikolas commented on a change in pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
o-nikolas commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r709451000
##########
File path: airflow/providers/amazon/aws/hooks/eks.py
##########
@@ -35,6 +35,7 @@
DEFAULT_PAGINATION_TOKEN = ''
STS_TOKEN_EXPIRES_IN = 60
+AUTHENTICATION_API_VERSION = "client.authentication.k8s.io/v1alpha1"
Review comment:
I believe we went with alpha since that's what the [EKS documentation](https://docs.aws.amazon.com/eks/latest/userguide/create-kubeconfig.html) still shows, but I reckon beta will work just fine.
Whichever you go with should probably be the same as what you use in `eks_get_token.py `
--
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 #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r712379132
##########
File path: airflow/providers/amazon/aws/example_dags/example_eks_using_defaults.py
##########
@@ -73,7 +72,7 @@
task_id="run_pod",
cluster_name=CLUSTER_NAME,
image="amazon/aws-cli:latest",
- cmds=["sh", "-c", "ls"],
+ cmds=["sh", "-c", "echo Test Airflow; date"],
Review comment:
`ls` was returning empty output, which could look like a problem. Now we have a user-friendly message.
--
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] potiuk commented on pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#issuecomment-913030381
Some tests still failing
--
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 #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#issuecomment-928175875
@potiuk Can you look at it? It is ready for review now.
--
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] potiuk commented on a change in pull request #17951: Refresh credentials for long-running pods on EKS
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17951:
URL: https://github.com/apache/airflow/pull/17951#discussion_r718857201
##########
File path: airflow/providers/amazon/aws/example_dags/example_eks_using_defaults.py
##########
@@ -73,7 +72,7 @@
task_id="run_pod",
cluster_name=CLUSTER_NAME,
image="amazon/aws-cli:latest",
- cmds=["sh", "-c", "ls"],
+ cmds=["sh", "-c", "echo Test Airflow; date"],
Review comment:
Nice change indeed.
##########
File path: airflow/providers/amazon/aws/hooks/eks.py
##########
@@ -35,6 +35,7 @@
DEFAULT_PAGINATION_TOKEN = ''
STS_TOKEN_EXPIRES_IN = 60
+AUTHENTICATION_API_VERSION = "client.authentication.k8s.io/v1alpha1"
Review comment:
Sounds good!
--
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