Skip to content

SFTP Provider: Fix default folder permissions #26593

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.

Sign up for GitHub

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

Merged
potiuk merged 5 commits into from
Oct 25, 2022
Merged

SFTP Provider: Fix default folder permissions #26593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8e8de8a
mode must be octal in paramiko function mkdir
Sep 9, 2022
0664b2c
including umask in to test permissions
Sep 22, 2022
19ee113
Fixing the test case for checking sftp folder permissions after creat…
Sep 23, 2022
c7f571e
using octal mode by default for directory permissions
Oct 8, 2022
bf20f85
Update docstring to pass spellcheck during build-docs
Oct 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions airflow/providers/sftp/hooks/sftp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,16 @@ def list_directory(self, path: str) -> list[str]:
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:
"""
Expand Down Expand Up @@ -195,12 +196,13 @@ def isfile(self, path: str) -> bool:
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):
Expand Down
8 changes: 8 additions & 0 deletions tests/providers/sftp/hooks/test_sftp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,20 @@ def test_mkdir(self):
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
Expand Down