From e2feb29760a35c98f8696643145f71b55251304f Mon Sep 17 00:00:00 2001 From: Kevin Barranco <63886802+kevinbsilva@users.noreply.github.com> Date: Wed, 19 May 2021 11:53:06 -0300 Subject: [PATCH 1/8] Applied permissions to self._error_file Used os.chmod with 0o0777 umask to avoid Permission Denied errors when accessing the temporary error file. --- airflow/task/task_runner/base_task_runner.py | 1 + 1 file changed, 1 insertion(+) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index d763dab4325c1..d731e508d7446 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -85,6 +85,7 @@ def __init__(self, local_task_job): cfg_path = tmp_configuration_copy(chmod=0o600) self._error_file = NamedTemporaryFile(delete=True) + os.chmod(self._error_file.name, 0o0777) self._cfg_path = cfg_path self._command = ( popen_prepend From 39a2745cfb8d50862f3d991c30ba99e6aa7d954a Mon Sep 17 00:00:00 2001 From: Kevin Barranco <63886802+kevinbsilva@users.noreply.github.com> Date: Mon, 7 Jun 2021 12:12:06 -0300 Subject: [PATCH 2/8] Changed os.chmod to os.chown Instead of using os.chmod and applying permissions to the error file, we are setting the file owner according to the variable self.run_as_user. This would avoid setting hardcoded permissions for the log file while maintaining the correct permissions to the user responsible for the DAG to manipulate the log file. --- airflow/task/task_runner/base_task_runner.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index d731e508d7446..c2198da7a4724 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -21,6 +21,7 @@ import threading from tempfile import NamedTemporaryFile from typing import Optional, Union +from pwd import getpwnam from airflow.configuration import conf from airflow.exceptions import AirflowConfigException @@ -85,7 +86,11 @@ def __init__(self, local_task_job): cfg_path = tmp_configuration_copy(chmod=0o600) self._error_file = NamedTemporaryFile(delete=True) - os.chmod(self._error_file.name, 0o0777) + + # Avoid executing chown when run_as_user is set to None + if self.run_as_user: + os.chown(self._error_file.name, getpwnam(self.run_as_user).pw_uid, -1) + self._cfg_path = cfg_path self._command = ( popen_prepend From 99462d287add4e01e3d6ce38d3579e91ad5d6b7a Mon Sep 17 00:00:00 2001 From: Kevin Silva Date: Thu, 22 Jul 2021 20:17:41 -0300 Subject: [PATCH 3/8] Added exception to avoid erros at the os.chown, if receive something other than a string --- airflow/task/task_runner/base_task_runner.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index c2198da7a4724..708b30742334b 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -89,7 +89,10 @@ def __init__(self, local_task_job): # Avoid executing chown when run_as_user is set to None if self.run_as_user: - os.chown(self._error_file.name, getpwnam(self.run_as_user).pw_uid, -1) + try: + os.chown(self._error_file.name, getpwnam(self.run_as_user).pw_uid, -1) + except Exception: + pass self._cfg_path = cfg_path self._command = ( From 30fd2e6b54bfc0630fe4f9df99dce28530f9ded8 Mon Sep 17 00:00:00 2001 From: Kevin Silva Date: Thu, 22 Jul 2021 21:18:12 -0300 Subject: [PATCH 4/8] Ordered imports in alphabetical order --- airflow/task/task_runner/base_task_runner.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index 708b30742334b..26778663b0ec1 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -17,11 +17,11 @@ # under the License. """Base task runner""" import os +from pwd import getpwnam import subprocess import threading from tempfile import NamedTemporaryFile from typing import Optional, Union -from pwd import getpwnam from airflow.configuration import conf from airflow.exceptions import AirflowConfigException @@ -86,14 +86,14 @@ def __init__(self, local_task_job): cfg_path = tmp_configuration_copy(chmod=0o600) self._error_file = NamedTemporaryFile(delete=True) - + # Avoid executing chown when run_as_user is set to None if self.run_as_user: try: os.chown(self._error_file.name, getpwnam(self.run_as_user).pw_uid, -1) except Exception: pass - + self._cfg_path = cfg_path self._command = ( popen_prepend From 53cb6dc3b5179f968bcbec45ba89c4e970c8cbe9 Mon Sep 17 00:00:00 2001 From: Kevin Silva Date: Fri, 23 Jul 2021 09:25:27 -0300 Subject: [PATCH 5/8] Adjusted code format to black --- airflow/task/task_runner/base_task_runner.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index 26778663b0ec1..8e6eac33ef716 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -86,8 +86,6 @@ def __init__(self, local_task_job): cfg_path = tmp_configuration_copy(chmod=0o600) self._error_file = NamedTemporaryFile(delete=True) - - # Avoid executing chown when run_as_user is set to None if self.run_as_user: try: os.chown(self._error_file.name, getpwnam(self.run_as_user).pw_uid, -1) From aa129c0efadf108cfd1619cb4ab46b323449b2a6 Mon Sep 17 00:00:00 2001 From: Kevin Silva Date: Fri, 23 Jul 2021 10:30:55 -0300 Subject: [PATCH 6/8] Fixed ordering group of imports --- airflow/task/task_runner/base_task_runner.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index 8e6eac33ef716..542d61b9c4f28 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -17,9 +17,10 @@ # under the License. """Base task runner""" import os -from pwd import getpwnam import subprocess import threading + +from pwd import getpwnam from tempfile import NamedTemporaryFile from typing import Optional, Union From 90a279757d0b79d3f628425af6525aa8099ab0ed Mon Sep 17 00:00:00 2001 From: Kevin Silva Date: Fri, 23 Jul 2021 10:52:00 -0300 Subject: [PATCH 7/8] Removed blank line from import. Applied isort --- airflow/task/task_runner/base_task_runner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index 542d61b9c4f28..f133de9f52c6e 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -19,7 +19,6 @@ import os import subprocess import threading - from pwd import getpwnam from tempfile import NamedTemporaryFile from typing import Optional, Union From 8f20361ec3a79dbfbac5293a8d35ca5f69a52f3f Mon Sep 17 00:00:00 2001 From: Ash Berlin-Taylor Date: Fri, 3 Sep 2021 12:28:42 +0100 Subject: [PATCH 8/8] Update airflow/task/task_runner/base_task_runner.py --- airflow/task/task_runner/base_task_runner.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/airflow/task/task_runner/base_task_runner.py b/airflow/task/task_runner/base_task_runner.py index f133de9f52c6e..b247b32773f39 100644 --- a/airflow/task/task_runner/base_task_runner.py +++ b/airflow/task/task_runner/base_task_runner.py @@ -87,10 +87,7 @@ def __init__(self, local_task_job): self._error_file = NamedTemporaryFile(delete=True) if self.run_as_user: - try: - os.chown(self._error_file.name, getpwnam(self.run_as_user).pw_uid, -1) - except Exception: - pass + os.chown(self._error_file.name, getpwnam(self.run_as_user).pw_uid, -1) self._cfg_path = cfg_path self._command = (