Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix impersonation issue with LocalTaskJob #16852

Merged
merged 1 commit into from Jul 7, 2021

Conversation

ephraimbuddy
Copy link
Contributor

@ephraimbuddy ephraimbuddy commented Jul 7, 2021

Fix impersonation issue with LocalTaskJob

Running a task with run_as_user fails because PIDs are not matched
correctly.

This change fixes it by matching the parent process ID of the task instance
to the current process ID of the task_runner process when we use impersonation

Fixes: #15537 (comment)


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
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.

@boring-cyborg boring-cyborg bot added the area:Scheduler Scheduler or dag parsing Issues label Jul 7, 2021
@ashb
Copy link
Member

ashb commented Jul 7, 2021

Tasks with impersonation (run_as_user) have been broken since 2.1.0 it turns out, so I'm also included this change in 2.1.2 @kaxil @jhtimmins .

@ashb ashb added this to the Airflow 2.1.2 milestone Jul 7, 2021
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test could also do with a few inline comments to help us work out what behaviour it is testing.

tests/jobs/test_local_task_job.py Outdated Show resolved Hide resolved
tests/jobs/test_local_task_job.py Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jul 7, 2021

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2021
Running a task with run_as_user fails because PIDs are not matched
correctly.

This change fixes it by matching the parent process ID of the task instance
to the current process ID of the task_runner process when we use impersonation

Update tests/jobs/test_local_task_job.py

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

fixup! Update tests/jobs/test_local_task_job.py

fixup! Fix impersonation issue with LocalTaskJob
@ephraimbuddy ephraimbuddy reopened this Jul 7, 2021
@ashb ashb merged commit feea380 into apache:main Jul 7, 2021
@ashb ashb deleted the fix-run-as-user-pid branch July 7, 2021 14:06
ashb pushed a commit that referenced this pull request Jul 7, 2021
Running a task with run_as_user fails because PIDs are not matched
correctly.

This change fixes it by matching the parent process ID (the `sudo`
process) of the task instance to the current process ID of the task_runner
process when we use impersonation

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
(cherry picked from commit feea380)
jhtimmins pushed a commit to astronomer/airflow that referenced this pull request Jul 9, 2021
Running a task with run_as_user fails because PIDs are not matched
correctly.

This change fixes it by matching the parent process ID (the `sudo`
process) of the task instance to the current process ID of the task_runner
process when we use impersonation

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
(cherry picked from commit feea380)
jhtimmins pushed a commit that referenced this pull request Jul 9, 2021
Running a task with run_as_user fails because PIDs are not matched
correctly.

This change fixes it by matching the parent process ID (the `sudo`
process) of the task instance to the current process ID of the task_runner
process when we use impersonation

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
(cherry picked from commit feea380)
kaxil pushed a commit to astronomer/airflow that referenced this pull request Jul 13, 2021
Running a task with run_as_user fails because PIDs are not matched
correctly.

This change fixes it by matching the parent process ID (the `sudo`
process) of the task instance to the current process ID of the task_runner
process when we use impersonation

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>
(cherry picked from commit feea380)
(cherry picked from commit 26a2beb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Scheduler Scheduler or dag parsing Issues full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants