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.

@ashb
Copy link
Member

@ashb 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

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
@ephraimbuddy ephraimbuddy force-pushed the fix-run-as-user-pid branch from 13b172a to 80421cb Jul 7, 2021
tests/jobs/test_local_task_job.py Outdated Show resolved Hide resolved
ashb
ashb approved these changes Jul 7, 2021
@github-actions
Copy link

@github-actions 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.

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 force-pushed the fix-run-as-user-pid branch from d964204 to f0e6ede Jul 7, 2021
@ephraimbuddy ephraimbuddy reopened this Jul 7, 2021
@ashb ashb merged commit feea380 into apache:main Jul 7, 2021
56 of 60 checks passed
@ashb ashb deleted the fix-run-as-user-pid branch Jul 7, 2021
ashb added a commit that referenced this issue 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 added a commit to astronomer/airflow that referenced this issue 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)
(cherry picked from commit 72f0ce4)
jhtimmins added a commit to astronomer/airflow that referenced this issue 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 added a commit that referenced this issue 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 added a commit to astronomer/airflow that referenced this issue 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)
kaxil added a commit to astronomer/airflow that referenced this issue 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)
ferruzzi added a commit to ferruzzi/airflow that referenced this issue Jul 19, 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants