[airflow] branch main updated: SFTP Provider: Fix default folder permissions (#26593)

[po...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new 662a8aaac3 SFTP Provider: Fix default folder permissions  (#26593)
662a8aaac3 is described below

commit 662a8aaac302aae9e1768c7c55e15aa8c539208e
Author: Matthieu Blais <bl...@ece.fr>
AuthorDate: Tue Oct 25 23:33:03 2022 +0800

    SFTP Provider: Fix default folder permissions  (#26593)
    
    * mode must be octal in paramiko function mkdir
    
    * including umask in to test permissions
    
    * Fixing the test case for checking sftp folder permissions after creation by the hook.
    
    * using octal mode by default for directory permissions
    
    * Update docstring to pass spellcheck during build-docs
    
    Co-authored-by: Matthieu Blais <ma...@tech.jago.com>
---
 airflow/providers/sftp/hooks/sftp.py    | 12 +++++++-----
 tests/providers/sftp/hooks/test_sftp.py |  8 ++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/airflow/providers/sftp/hooks/sftp.py b/airflow/providers/sftp/hooks/sftp.py
index 93eb3d52d4..85edeee46e 100644
--- a/airflow/providers/sftp/hooks/sftp.py
+++ b/airflow/providers/sftp/hooks/sftp.py
@@ -159,15 +159,16 @@ class SFTPHook(SSHHook):
         files = sorted(conn.listdir(path))
         return files
 
-    def mkdir(self, path: str, mode: int = 777) -> None:
+    def mkdir(self, path: str, mode: int = 0o777) -> None:
         """
         Creates a directory on the remote system.
+        The default mode is 0777, but on some systems, the current umask value is first masked out.
 
         :param path: full path to the remote directory to create
-        :param mode: permissions to set the directory with
+        :param mode: int permissions of octal mode for directory
         """
         conn = self.get_conn()
-        conn.mkdir(path, mode=int(str(mode), 8))
+        conn.mkdir(path, mode=mode)
 
     def isdir(self, path: str) -> bool:
         """
@@ -195,12 +196,13 @@ class SFTPHook(SSHHook):
             result = False
         return result
 
-    def create_directory(self, path: str, mode: int = 777) -> None:
+    def create_directory(self, path: str, mode: int = 0o777) -> None:
         """
         Creates a directory on the remote system.
+        The default mode is 0777, but on some systems, the current umask value is first masked out.
 
         :param path: full path to the remote directory to create
-        :param mode: int representation of octal mode for directory
+        :param mode: int permissions of octal mode for directory
         """
         conn = self.get_conn()
         if self.isdir(path):
diff --git a/tests/providers/sftp/hooks/test_sftp.py b/tests/providers/sftp/hooks/test_sftp.py
index f3af70cc16..b471dc88b0 100644
--- a/tests/providers/sftp/hooks/test_sftp.py
+++ b/tests/providers/sftp/hooks/test_sftp.py
@@ -107,12 +107,20 @@ class TestSFTPHook(unittest.TestCase):
         self.hook.mkdir(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS, new_dir_name))
         output = self.hook.describe_directory(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS))
         assert new_dir_name in output
+        # test the directory has default permissions to 777 - umask
+        umask = 0o022
+        output = self.hook.get_conn().lstat(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS, new_dir_name))
+        assert output.st_mode & 0o777 == 0o777 - umask
 
     def test_create_and_delete_directory(self):
         new_dir_name = "new_dir"
         self.hook.create_directory(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS, new_dir_name))
         output = self.hook.describe_directory(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS))
         assert new_dir_name in output
+        # test the directory has default permissions to 777
+        umask = 0o022
+        output = self.hook.get_conn().lstat(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS, new_dir_name))
+        assert output.st_mode & 0o777 == 0o777 - umask
         # test directory already exists for code coverage, should not raise an exception
         self.hook.create_directory(os.path.join(TMP_PATH, TMP_DIR_FOR_TESTS, new_dir_name))
         # test path already exists and is a file, should raise an exception