[AIRFLOW-6585] Fixed Timestamp bug in RefreshKubeConfigLoader

[Bruschkov]

JIRA

https://issues.apache.org/jira/browse/AIRFLOW-6585

Description

When using the KubernetesPodOperator on an aws kubernetes cluster, the aws-iam-authenticator is used to obtain kubernetes authentication tokens. The aws tokens contain ISO-8601 formatted timestamps, which couldn't be parsed in case of a "Z" (Zulu Time) timezone. This PR fixes this problem by converting the "Z" timezone into a regular "+0000" format.

Upon further review this is only a problem with python version <= 3.6. But that should not keep the issue from being fixed.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community!
If you have any issues or are unsure about any anything please check our
Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)

Here are some useful points:

• Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits
  will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory).
  Adding a new operator? Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing
  locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from
  Committers.

Apache Airflow is a community-driven project and together we are making it better 🚀.

In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://apache-airflow-slack.herokuapp.com/

[potiuk]

I'd still prefer to have a test for that one. This should b e rather easy to add such test.

There is already the tests/kubernetes/test_client.py with two tests and adding a test there would be just great (we need to improve coverage on Kubernetes part and even really small tests like that gradually added help a lot)

Would you be so nice to add it please?

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@e7bf8ec). Click here to learn what that means.
The diff coverage is 83.33%.

@@ Coverage Diff @@## master #7153 +/- ##
========================================
Coverage ? 85.4% ========================================
Files ? 710 Lines ? 39484 Branches ? 0 ========================================
Hits ? 33723 Misses ? 5761 Partials ? 0

Impacted Files	Coverage Δ
airflow/kubernetes/refresh_config.py	76.78% <83.33%> (ø)

---

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 e7bf8ec...ce626e1. Read the comment docs.

[Bruschkov]

I'd still prefer to have a test for that one. This should b e rather easy to add such test.

There is already the tests/kubernetes/test_client.py with two tests and adding a test there would be just great (we need to improve coverage on Kubernetes part and even really small tests like that gradually added help a lot)

Would you be so nice to add it please?

Hi, thanks for your quick feedback. Added the test and amended the PR.

[Bruschkov]

@potiuk

Everything solved and build passes now.

Best regards

[ashb]

Change looks good now, but as this JIRA was already released we need a new JIRA please. (We use the Fix Version to know what to pull back to our releases, and as AIRFLOW-5117 is already released we would otherwise miss this change when building 1.10.8)

[ashb]

Please create new Jira ticket for this.

[Bruschkov]

@ashb New JIRA created and linked to this PR.

[mik-laj]

Why are these changes being made in this project and not in the Kubernetes library? I can't find the ticket in the library repository.

[Bruschkov]

Why are these changes being made in this project and not in the Kubernetes library? I can't find the ticket in the library repository.

#5731 (comment)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ashb]

Using pendulum (which we already use) might be a better fix than this.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[benjaminsky]

This is still an issue with EKS and Airflow v1.10.10. This PR fixes it. Can we reopen this PR?

[potiuk]

@benjaminsky -> sure, would you mind picking this up? I think this one neds rebase and there is a change requested. Unnless (@Bruschkov wants to do it).

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ramsesrm]

Hi, this is still an issue on Airflow v1.10.12. Can we reopen the PR?