Skip to content

EKS hook: Update auth. API version to v1 and replace non-POSIX 'source' with '.' - #61441

Merged
shahar1 merged 11 commits into
apache:mainfrom
korex-f:fix-60269-eks-posix-shell
Feb 10, 2026
Merged

EKS hook: Update auth. API version to v1 and replace non-POSIX 'source' with '.'#61441
shahar1 merged 11 commits into
apache:mainfrom
korex-f:fix-60269-eks-posix-shell

Conversation

@korex-f

Copy link
Copy Markdown
Contributor

Fix#60269: Replace non-POSIX 'source' with '.' in EKS hook

Problem

EksPodOperator fails with 401 Unauthorized errors when running on Debian/Ubuntu-based containers (Astronomer Runtime, official Airflow Docker images, MWAA, etc.).

Root Cause

The issue occurs in airflow/providers/amazon/aws/hooks/eks.py line 83, where the COMMAND template uses source:

source {credentials_file}

The problem:source is a bash-specific builtin command, not a POSIX standard command. On Debian/Ubuntu systems, /bin/sh is symlinked to dash (not bash), which doesn't recognize source:

$ sh -c 'source /dev/null'
sh: 1: source: not found

This causes the credential loading to fail silently, resulting in 401 Unauthorized errors when the EKS token generation falls back to an empty credential chain.

Why This Is Hard to Detect

The bug is masked during local development when developers have ~/.aws/credentials mounted in containers:

  1. source {credentials_file} fails silently (stderr not checked)
  2. eks_get_token.py falls back to boto3's default credential chain
  3. Finds credentials in ~/.aws/credentials → token generation succeeds ✅

In production/cloud environments without ~/.aws/ directory, credentials are only available via the temp file that failed to source, causing 401 errors ❌

Solution

This PR implements two fixes:

1. Use POSIX-Compliant Dot Operator

Replace source with . (dot operator), which is POSIX-compliant and works in all shells (bash, dash, sh):

# Before (bash-specific)source {credentials_file}
# After (POSIX-compliant). {credentials_file}

2. Update Deprecated Kubernetes API Version

Update the authentication API version from deprecated v1alpha1 to v1beta1:

# BeforeAUTHENTICATION_API_VERSION="client.authentication.k8s.io/v1alpha1"# AfterAUTHENTICATION_API_VERSION="client.authentication.k8s.io/v1beta1"

Note:v1alpha1 was deprecated in Kubernetes 1.24 and removed in 1.28.

Changes Made

  • Replace 'source' with POSIX-compliant '.' operator in COMMAND template
  • Update deprecated v1alpha1 to v1beta1 Kubernetes API version
  • Add shell compatibility tests for dash/POSIX shells
  • Update comment to reflect POSIX compliance

Testing

Added comprehensive test coverage in test_eks.py:

New Test Classes

  1. TestEksHookShellCompatibility

    • test_command_template_is_posix_compliant: Verifies the template uses . not source
    • test_credential_loading_works_with_dash: Confirms credentials load correctly with dash shell
    • test_source_command_fails_with_dash: Documents the original bug
  2. TestEksHookKubernetesVersion

    • test_uses_stable_kubernetes_api_version: Ensures we're not using deprecated v1alpha1

Manual Testing

Verified in Breeze (Debian-based container):

$ ls -la /bin/sh
/bin/sh -> dash
$ sh -c '. /tmp/test.sh && echo $AWS_ACCESS_KEY_ID'
test_key # ✅ Works
$ sh -c 'source /tmp/test.sh && echo $AWS_ACCESS_KEY_ID'
sh: 1: source: not found # ❌ Fails

Impact

This fix resolves 401 Unauthorized errors for all Debian/Ubuntu-based Airflow deployments:

  • ✅ Astronomer Runtime
  • ✅ Official Apache Airflow Docker images
  • ✅ Amazon MWAA (Managed Workflows for Apache Airflow)
  • ✅ Any deployment where /bin/sh is dash

The change is backward-compatible as the . operator works in both bash and dash shells.


Fixes#60269


Was generative AI tooling used to co-author this PR?
  • Yes (Claude AI was used for documentation)

@boring-cyborg

Copy link
Copy Markdown

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 Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide 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.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    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://s.apache.org/airflow-slack

@boring-cyborgboring-cyborgBot added area:providers provider:amazon AWS/Amazon - related issues labels Feb 4, 2026
@korex-f
korex-fforce-pushed the fix-60269-eks-posix-shell branch from f982627 to 3adc5a2CompareFebruary 4, 2026 10:00
Comment threadproviders/amazon/tests/unit/amazon/aws/hooks/test_eks.py Outdated
@vincbeck

Copy link
Copy Markdown
Contributor

I dont think I talk to a human but the code looks good

@korex-f

Copy link
Copy Markdown
ContributorAuthor

I dont think I talk to a human but the code looks good

Thanks. But i don't get the human reference

- Replace 'source' with POSIX-compliant '.' operator in COMMAND template
- Update deprecated v1alpha1 to v1beta1 Kubernetes API version
- Add shell compatibility tests for dash/POSIX shells
- Update comment to reflect POSIX compliance
Fixesapache#60269
Reduced to single assertion test as suggested by @vincbeck
@korex-f
korex-fforce-pushed the fix-60269-eks-posix-shell branch from f89341c to cbc5481CompareFebruary 4, 2026 20:02
@vincbeck

Copy link
Copy Markdown
Contributor

Some tests are failing, they need to be updated. It should be fairly easy

The test was checking for 'source' but the fix changed it to '.' (POSIX-compliant).
Also updated API version check from v1alpha1 to v1beta1.
@korex-f

Copy link
Copy Markdown
ContributorAuthor

Some tests are failing, they need to be updated. It should be fairly easy

My bad, i am somewhat relatively new to open source, so i didn't go through the existing test script thoroughly to check if there's a test that checks for the changes i made. With what you said about my tests being excessive should have been a cue to that effect, but i was slow to understanding what you truly mean.

I have updated the tests, so all should be good now.

Thanks for the guidance.

@andrewhharmon

Copy link
Copy Markdown
Contributor

should we use v1 instead of v1beta1?

@shahar1shahar1 changed the title Fix #60269: Replace non-POSIX 'source' with '.' in EKS hookReplace non-POSIX 'source' with '.' in EKS hookFeb 10, 2026
@shahar1shahar1 changed the title Replace non-POSIX 'source' with '.' in EKS hookEKS hook: Update auth. API version to v1beta1 and replace non-POSIX 'source' with '.'Feb 10, 2026
@shahar1shahar1 changed the title EKS hook: Update auth. API version to v1beta1 and replace non-POSIX 'source' with '.'EKS hook: Update auth. API version to v1 and replace non-POSIX 'source' with '.'Feb 10, 2026
@shahar1

shahar1 commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

should we use v1 instead of v1beta1?

Good call!
I think we should, I updated the reference and updated the PR's title (as it might be even more important than just the POSIX).
If during the RC1 tests we'll see a regression because of that, we could postpone the release.

@andrewhharmon

Copy link
Copy Markdown
Contributor

should we use v1 instead of v1beta1?

Good call! I think we should, I updated the reference and updated the PR's title (as it might be even more important than just the POSIX). If during the RC1 tests we'll see a regression because of that, we could postpone the release.

oops, missed the unit test

@shahar1

Copy link
Copy Markdown
Contributor

should we use v1 instead of v1beta1?

Good call! I think we should, I updated the reference and updated the PR's title (as it might be even more important than just the POSIX). If during the RC1 tests we'll see a regression because of that, we could postpone the release.

oops, missed the unit test

You got me, thanks!
Trying again

@korex-f

Copy link
Copy Markdown
ContributorAuthor

should we use v1 instead of v1beta1?

i actually thought of this, but just felt i should make the changes based on what was in the script

@shahar1

shahar1 commented Feb 10, 2026

Copy link
Copy Markdown
Contributor

@idrisakorede please do not merge from main and let the CI run without interruptions, otherwise I need to re-approve and rerun it each time. I want to start working on the release in an hour or so, so it's important if you want this PR to be included there :)
Thank you!

@korex-f

korex-f commented Feb 10, 2026

Copy link
Copy Markdown
ContributorAuthor

@idrisakorede please do not merge from main and let the CI run without interruptions, otherwise I need to re-approve and rerun it each time. I want to start working on the release in an hour or so, so it's important if you want this PR to be included there :) Thank you!

@shahar1 Understood! I apologize for the interruptions, I’ll stop clicking the update button and let the CI run completely. I didn’t realize each update was restarting the process.
Looking forward to seeing this included in the release. Thank you for your patience and guidance!

@shahar1

Copy link
Copy Markdown
Contributor

@idrisakorede please do not merge from main and let the CI run without interruptions, otherwise I need to re-approve and rerun it each time. I want to start working on the release in an hour or so, so it's important if you want this PR to be included there :) Thank you!

@shahar1 Understood! I apologize for the interruptions, I’ll stop clicking the update button and let the CI run completely. I didn’t realize each update was restarting the process. Looking forward to seeing this included in the release. Thank you for your patience and guidance!

No problem at all! it's almost done and ready to go :)

@shahar1
shahar1 merged commit 2b6ef4a into apache:mainFeb 10, 2026
90 checks passed
@boring-cyborg

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@ferruzzi

Copy link
Copy Markdown
Contributor

@shahar1 - This PR introduced a bug that breaks EKS operators. #61891 should be the fix. Any chance you can squeeze that one in still so we don't push broken code?

@shahar1

Copy link
Copy Markdown
Contributor

@shahar1 - This PR introduced a bug that breaks EKS operators. #61891 should be the fix. Any chance you can squeeze that one in still so we don't push broken code?

Sure, thanks for testing - I'll exclude it from upcoming release.

Ratasa143 pushed a commit to Ratasa143/airflow that referenced this pull request Feb 15, 2026
choo121600 pushed a commit to choo121600/airflow that referenced this pull request Feb 22, 2026
AkshayArali pushed a commit to AkshayArali/airflow_630 that referenced this pull request Feb 27, 2026
AkshayArali pushed a commit to AkshayArali/airflow_630 that referenced this pull request Feb 27, 2026
Subham-KRLX pushed a commit to Subham-KRLX/airflow that referenced this pull request Mar 4, 2026
dominikhei pushed a commit to dominikhei/airflow that referenced this pull request Mar 11, 2026
Ankurdeewan pushed a commit to Ankurdeewan/airflow that referenced this pull request Mar 15, 2026
radhwene pushed a commit to radhwene/airflow that referenced this pull request Mar 21, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providersprovider:amazonAWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EksPodOperator fails on POSIX shells (dash) due to non-portable 'source' command

5 participants

@korex-f@vincbeck@andrewhharmon@shahar1@ferruzzi