NAS-132812 / 25.04 / Convert listdir and mkdir to api_method (#15066)

This commit converts the two filesystem API endpoints
`filesystem.listdir` and `filesystem.mkdir` to new api_method.

As part of this change, the deprecated way of calling
filesystem.mkdir using positional arguments was removed.

src/middlewared/middlewared/api/v25_04_0/filesystem.py

@@ -3,17 +3,24 @@
     NonEmptyString,
     UnixPerm,
     single_argument_args,
+    query_result
 )
 from pydantic import Field, model_validator
 from typing import Literal, Self
 from middlewared.utils.filesystem.acl import (
     ACL_UNDEFINED_ID,
 )
+from middlewared.utils.filesystem.stat_x import (
+    StatxEtype,
+)
 from .acl import AceWhoId
+from .common import QueryFilters, QueryOptions
 
 __all__ = [
     'FilesystemChownArgs', 'FilesystemChownResult',
     'FilesystemSetPermArgs', 'FilesystemSetPermResult',
+    'FilesystemListdirArgs', 'FilesystemListdirResult',
+    'FilesystemMkdirArgs', 'FilesystemMkdirResult',
 ]
 
 
@@ -79,3 +86,98 @@ def payload_is_actionable(self) -> Self:
 
 class FilesystemSetPermResult(BaseModel):
     result: Literal[None]
+
+
+FILESYSTEM_STATX_ATTRS = Literal[
+    'COMPRESSED',
+    'APPEND',
+    'NODUMP',
+    'IMMUTABLE',
+    'AUTOMOUNT',
+    'MOUNT_ROOT',
+    'VERIFY',
+    'DAX'
+]
+
+
+FILESYSTEM_ZFS_ATTRS = Literal[
+    'READONLY',
+    'HIDDEN',
+    'SYSTEM',
+    'ARCHIVE',
+    'IMMUTABLE',
+    'NOUNLINK',
+    'APPENDONLY',
+    'NODUMP',
+    'OPAQUE',
+    'AV_QUARANTINED',
+    'AV_MODIFIED',
+    'REPARSE',
+    'OFFLINE',
+    'SPARSE'
+]
+
+
+class FilesystemDirEntry(BaseModel):
+    name: NonEmptyString
+    """ Entry's base name. """
+    path: NonEmptyString
+    """ Entry's full path. """
+    realpath: NonEmptyString
+    """ Canonical path of the entry, eliminating any symbolic links"""
+    type: Literal[
+        StatxEtype.DIRECTORY,
+        StatxEtype.FILE,
+        StatxEtype.SYMLINK,
+        StatxEtype.OTHER,
+    ]
+    size: int
+    """ Size in bytes of a plain file. This corresonds with stx_size. """
+    allocation_size: int
+    """ Allocated size of file. Calculated by multiplying stx_blocks by 512. """
+    mode: int
+    """ Entry's mode including file type information and file permission bits. This corresponds with stx_mode. """
+    mount_id: int
+    """ The mount ID of the mount containing the entry. This corresponds to the number in first
+    field of /proc/self/mountinfo and stx_mnt_id. """
+    acl: bool
+    """ Specifies whether ACL is present on the entry. If this is the case then file permission
+    bits as reported in `mode` may not be representative of the actual permissions. """
+    uid: int
+    """ User ID of the entry's owner. This corresponds with stx_uid. """
+    gid: int
+    """ Group ID of the entry's owner. This corresponds with stx_gid. """
+    is_mountpoint: bool
+    """ Specifies whether the entry is also the mountpoint of a filesystem. """
+    is_ctldir: bool
+    """ Specifies whether the entry is located within the ZFS ctldir (for example a snapshot). """
+    attributes: list[FILESYSTEM_STATX_ATTRS]
+    """ Extra file attribute indicators for entry as returned by statx. Expanded from stx_attributes. """
+    xattrs: list[NonEmptyString]
+    """ List of xattr names of extended attributes on file. """
+    zfs_attrs: list[FILESYSTEM_ZFS_ATTRS] | None
+    """ List of extra ZFS-related file attribute indicators on file. Will be None type if filesystem is not ZFS. """
+
+
+class FilesystemListdirArgs(BaseModel):
+    path: NonEmptyString
+    query_filters: QueryFilters = []
+    query_options: QueryOptions = QueryOptions()
+
+
+FilesystemListdirResult = query_result(FilesystemDirEntry)
+
+
+class FilesystemMkdirOptions(BaseModel):
+    mode: UnixPerm = '755'
+    raise_chmod_error: bool = True
+
+
+@single_argument_args('filesystem_mkdir')
+class FilesystemMkdirArgs(BaseModel):
+    path: NonEmptyString
+    options: FilesystemMkdirOptions = Field(default=FilesystemMkdirOptions())
+
+
+class FilesystemMkdirResult(BaseModel):
+    result: FilesystemDirEntry

src/middlewared/middlewared/plugins/filesystem.py

@@ -10,11 +10,16 @@
 import pyinotify
 
 from itertools import product
+from middlewared.api import api_method
+from middlewared.api.current import (
+    FilesystemListdirArgs, FilesystemListdirResult,
+    FilesystemMkdirArgs, FilesystemMkdirResult,
+)
 from middlewared.event import EventSource
 from middlewared.plugins.pwenc import PWENC_FILE_SECRET, PWENC_FILE_SECRET_MODE
 from middlewared.plugins.docker.state_utils import IX_APPS_DIR_NAME
-from middlewared.schema import accepts, Bool, Dict, Float, Int, List, Ref, returns, Path, Str, UnixPerm
-from middlewared.service import private, CallError, filterable_returns, filterable, Service, job
+from middlewared.schema import accepts, Bool, Dict, Float, Int, List, Ref, returns, Path, Str
+from middlewared.service import private, CallError, filterable, Service, job
 from middlewared.utils import filter_list
 from middlewared.utils.filesystem import attrs, stat_x
 from middlewared.utils.filesystem.acl import acl_is_present
@@ -116,21 +121,7 @@ def mount_info(self, filters, options):
         mntinfo = getmntinfo()
         return filter_list(list(mntinfo.values()), filters, options)
 
-    @accepts(Dict(
-        'filesystem_mkdir',
-        Str('path'),
-        Dict(
-            'options',
-            UnixPerm('mode', default='755'),
-            Bool('raise_chmod_error', default=True)
-        ),
-    ), deprecated=[(
-        lambda args: len(args) == 1 and isinstance(args[0], str),
-        lambda mkdir_path: [{
-            'path': mkdir_path
-        }]
-    )], roles=['FILESYSTEM_DATA_WRITE'])
-    @returns(Ref('path_entry'))
+    @api_method(FilesystemMkdirArgs, FilesystemMkdirResult, roles=['FILESYSTEM_DATA_WRITE'])
     def mkdir(self, data):
         """
         Create a directory at the specified path.
@@ -162,8 +153,10 @@ def mkdir(self, data):
             raise CallError(f'{path}: path not permitted', errno.EPERM)
 
         os.mkdir(path, mode=mode)
-        stat = p.stat()
-        if statlib.S_IMODE(stat.st_mode) != mode:
+        st = stat_x.statx_entry_impl(p, None)
+        stat = st['st']
+
+        if statlib.S_IMODE(stat.stx_mode) != mode:
             # This may happen if requested mode is greater than umask
             # or if underlying dataset has restricted aclmode and ACL is present
             try:
@@ -183,13 +176,16 @@ def mkdir(self, data):
             'path': path,
             'realpath': realpath,
             'type': 'DIRECTORY',
-            'size': stat.st_size,
-            'mode': stat.st_mode,
+            'size': stat.stx_size,
+            'allocation_size': stat.stx_blocks * 512,
+            'mode': stat.stx_mode,
             'acl': acl_is_present(os.listxattr(path)),
-            'uid': stat.st_uid,
-            'gid': stat.st_gid,
+            'uid': stat.stx_uid,
+            'gid': stat.stx_gid,
             'is_mountpoint': False,
             'is_ctldir': False,
+            'mount_id': st['st'].stx_mnt_id,
+            'attributes': st['attributes'],
             'xattrs': [],
             'zfs_attrs': ['ARCHIVE']
         }
@@ -221,36 +217,7 @@ def listdir_request_mask(self, select):
 
         return request_mask
 
-    @accepts(
-        Str('path', required=True),
-        Ref('query-filters'),
-        Ref('query-options'),
-        roles=['FILESYSTEM_ATTRS_READ']
-    )
-    @filterable_returns(Dict(
-        'path_entry',
-        Str('name', required=True),
-        Path('path', required=True),
-        Path('realpath', required=True),
-        Str('type', required=True, enum=['DIRECTORY', 'FILE', 'SYMLINK', 'OTHER']),
-        Int('size', required=True, null=True),
-        Int('allocation_size', required=True, null=True),
-        Int('mode', required=True, null=True),
-        Int('mount_id', required=True, null=True),
-        Bool('acl', required=True, null=True),
-        Int('uid', required=True, null=True),
-        Int('gid', required=True, null=True),
-        Bool('is_mountpoint', required=True),
-        Bool('is_ctldir', required=True),
-        List(
-            'attributes',
-            required=True,
-            items=[Str('statx_attribute', enum=[attr.name for attr in stat_x.StatxAttr])]
-        ),
-        List('xattrs', required=True, null=True),
-        List('zfs_attrs', required=True, null=True),
-        register=True
-    ))
+    @api_method(FilesystemListdirArgs, FilesystemListdirResult, roles=['FILESYSTEM_ATTRS_READ'])
     def listdir(self, path, filters, options):
         """
         Get the contents of a directory.

src/middlewared/middlewared/utils/filesystem/stat_x.py

@@ -8,16 +8,16 @@
 import os
 import ctypes
 import stat as statlib
-from enum import auto, Enum, IntFlag
+from enum import IntFlag, StrEnum
 from .constants import AT_FDCWD
 from .utils import path_in_ctldir
 
 
-class StatxEtype(Enum):
-    DIRECTORY = auto()
-    FILE = auto()
-    SYMLINK = auto()
-    OTHER = auto()
+class StatxEtype(StrEnum):
+    DIRECTORY = 'DIRECTORY'
+    FILE = 'FILE'
+    SYMLINK = 'SYMLINK'
+    OTHER = 'OTHER'
 
 
 class ATFlags(IntFlag):
@@ -177,8 +177,8 @@ def statx_entry_impl(entry, dir_fd=None, get_ctldir=True):
         # This is equivalent to lstat() call
         out['st'] = statx(
             path,
-            dir_fd = dir_fd,
-            flags = __statx_lstat_flags
+            dir_fd=dir_fd,
+            flags=__statx_lstat_flags
         )
     except FileNotFoundError:
         return None

tests/api2/test_190_filesystem.py

@@ -75,9 +75,9 @@ def test_immutable_flag():
             # 2) "is_immutable_set" returns sane response
             if flag_set:
                 with pytest.raises(PermissionError):
-                    call("filesystem.mkdir", f"{t_child_path}_{flag_set}")
+                    call('filesystem.mkdir', {'path': f"{t_child_path}_{flag_set}"})
             else:
-                call("filesystem.mkdir", f"{t_child_path}_{flag_set}")
+                call('filesystem.mkdir', {'path': f"{t_child_path}_{flag_set}"})
 
             is_immutable = 'IMMUTABLE' in call('filesystem.stat', t_path)['attributes']
             err = "Immutable flag is still not set"

tests/api2/test_435_smb_registry.py

@@ -70,7 +70,7 @@ def create_smb_share(path, share_name, mkdir=False, options=None):
     cr_opts = options or {}
 
     if mkdir:
-        call('filesystem.mkdir', path)
+        call('filesystem.mkdir', {'path': path, 'options': {'raise_chmod_error': False}})
 
     with smb_share(path, share_name, cr_opts) as share:
         yield share
@@ -82,7 +82,7 @@ def setup_smb_shares(mountpoint):
 
     for share in SHARES:
         share_path = os.path.join(mountpoint, share)
-        call('filesystem.mkdir', share_path)
+        call('filesystem.mkdir', {'path': share_path, 'options': {'raise_chmod_error': False}})
         new_share = call('sharing.smb.create', {
             'comment': 'My Test SMB Share',
             'name': share,
@@ -342,7 +342,7 @@ def test__delete_shares(setup_for_tests):
 def test__create_homes_share(setup_for_tests):
     mp, ds, share_dict = setup_for_tests
     home_path = os.path.join(mp, 'HOME_SHARE')
-    call('filesystem.mkdir', home_path)
+    call('filesystem.mkdir', {'path': home_path, 'options': {'raise_chmod_error': False}})
 
     new_share = call('sharing.smb.create', {
         "comment": "My Test SMB Share",

tests/api2/test_account_privilege_role.py

@@ -113,7 +113,7 @@ def test_readonly_can_not_call_method():
 
         with pytest.raises(CallError) as ve:
             # fails with EPERM if API access granted
-            c.call("filesystem.mkdir", "/foo")
+            c.call("filesystem.mkdir", {"path": "/foo"})
 
         assert ve.value.errno == errno.EACCES
 

tests/api2/test_large_message.py

@@ -13,7 +13,7 @@ def test_large_message_default():
 
     with pytest.raises(ClientException) as ce:
         with client() as c:
-            c.call('filesystem.mkdir', LARGE_PAYLOAD_1)
+            c.call('filesystem.mkdir', {'path': LARGE_PAYLOAD_1})
 
     assert MSG_TOO_BIG_ERR in ce.value.error