diff --git a/src/middlewared/middlewared/api/base/types/__init__.py b/src/middlewared/middlewared/api/base/types/__init__.py index a7d4f619d98fe..6c96e6b37546e 100644 --- a/src/middlewared/middlewared/api/base/types/__init__.py +++ b/src/middlewared/middlewared/api/base/types/__init__.py @@ -1,5 +1,6 @@ from .fc import * # noqa from .filesystem import * # noqa from .iscsi import * # noqa +from .network import * # noqa from .string import * # noqa from .user import * # noqa diff --git a/src/middlewared/middlewared/api/base/types/filesystem.py b/src/middlewared/middlewared/api/base/types/filesystem.py index 394b6f086fcd0..865ec190cd074 100644 --- a/src/middlewared/middlewared/api/base/types/filesystem.py +++ b/src/middlewared/middlewared/api/base/types/filesystem.py @@ -1,8 +1,10 @@ +import os from typing import Annotated +from pydantic import Field from pydantic.functional_validators import AfterValidator -__all__ = ["UnixPerm"] +__all__ = ["UnixPerm", "Dir"] def validate_unix_perm(value: str) -> str: @@ -18,3 +20,22 @@ def validate_unix_perm(value: str) -> str: UnixPerm = Annotated[str, AfterValidator(validate_unix_perm)] + + +def validate_host_path(value: str) -> str: + if value is None: + return + + if value: + if not os.path.exists(value): + raise ValueError( + 'Path does not exist (underlying dataset may be locked or the path is just missing).', + ) + else: + if not os.path.isdir(value): + raise ValueError('This path is not a directory.') + + return value + + +Dir = Annotated[str, Field(min_length=1), AfterValidator(validate_host_path)] diff --git a/src/middlewared/middlewared/api/base/types/network.py b/src/middlewared/middlewared/api/base/types/network.py new file mode 100644 index 0000000000000..e6922f1665d59 --- /dev/null +++ b/src/middlewared/middlewared/api/base/types/network.py @@ -0,0 +1,20 @@ +import functools +from typing import Annotated +from pydantic import Field + +__all__ = ["TcpPort", "exclude_tcp_ports"] + + +def _exclude_port_validation(value: int, *, ports: list[int]) -> int: + if value in ports: + raise ValueError( + f'{value} is a reserved for internal use. Please select another value.' + ) + return value + + +def exclude_tcp_ports(ports: list[int]): + return functools.partial(_exclude_port_validation, ports=ports or []) + + +TcpPort = Annotated[int, Field(ge=1, le=65535)] diff --git a/src/middlewared/middlewared/api/v25_04_0/__init__.py b/src/middlewared/middlewared/api/v25_04_0/__init__.py index dcc77108f67dc..402d178845ad7 100644 --- a/src/middlewared/middlewared/api/v25_04_0/__init__.py +++ b/src/middlewared/middlewared/api/v25_04_0/__init__.py @@ -37,6 +37,7 @@ from .keychain import * # noqa from .k8s_to_docker import * # noqa from .netdata import * # noqa +from .nfs import * # noqa from .pool import * # noqa from .pool_resilver import * # noqa from .pool_scrub import * # noqa diff --git a/src/middlewared/middlewared/api/v25_04_0/nfs.py b/src/middlewared/middlewared/api/v25_04_0/nfs.py new file mode 100644 index 0000000000000..159a194a203dc --- /dev/null +++ b/src/middlewared/middlewared/api/v25_04_0/nfs.py @@ -0,0 +1,164 @@ +import re +from typing import Annotated, Literal + +from pydantic import ( + ConfigDict, Field, IPvAnyAddress, IPvAnyNetwork, + AfterValidator, field_validator +) + +from middlewared.api.base import ( + BaseModel, Excluded, excluded_field, ForUpdateMetaclass, NonEmptyString, + single_argument_args, single_argument_result, + TcpPort, exclude_tcp_ports, Dir +) + +__all__ = ["NfsEntry", + "NfsUpdateArgs", "NfsUpdateResult", + "NfsBindipChoicesArgs", "NfsBindipChoicesResult", + "NfsShareEntry", + "NfsShareCreateArgs", "NfsShareCreateResult", + "NfsShareUpdateArgs", "NfsShareUpdateResult", + "NfsShareDeleteArgs", "NfsShareDeleteResult"] + +NFS_protocols = Literal["NFSV3", "NFSV4"] +NFS_RDMA_DEFAULT_PORT = 20049 +EXCLUDED_PORTS = [NFS_RDMA_DEFAULT_PORT] + + +class NfsEntry(BaseModel): + id: int + servers: Annotated[int | None, Field(ge=1, le=256)] + """ Specify the number of nfsd. Default: Number of nfsd is equal number of CPU. """ + allow_nonroot: bool + """ Allow non-root mount requests. This equates to 'insecure' share option. """ + protocols: list[NFS_protocols] + """ Specify supported NFS protocols: NFSv3, NFSv4 or both can be listed. """ + v4_krb: bool + """ Force Kerberos authentication on NFS shares. """ + v4_domain: str + """ Specify a DNS domain (NFSv4 only) """ + # NOTE: Using IPvAnyAddress in bindip will generate a JSON serialization error + bindip: list[NonEmptyString] = Field(default_factory=[]) + """ Limit the server IP addresses available for NFS """ + mountd_port: Annotated[TcpPort | None, AfterValidator(exclude_tcp_ports(EXCLUDED_PORTS))] + """ Specify the mountd port binding """ + rpcstatd_port: Annotated[TcpPort | None, AfterValidator(exclude_tcp_ports(EXCLUDED_PORTS))] + """ Specify the rpc.statd port binding """ + rpclockd_port: Annotated[TcpPort | None, AfterValidator(exclude_tcp_ports(EXCLUDED_PORTS))] + """ Specify the rpc.lockd port binding """ + mountd_log: bool + """ Enable or disable mountd logging """ + statd_lockd_log: bool + """ Enable or disable statd and lockd logging """ + v4_krb_enabled: bool + """ Status of NFSv4 authentication requirement (status only) """ + userd_manage_gids: bool + """ Enable to allow server to manage gids """ + keytab_has_nfs_spn: bool + """ Report status of NFS Principal Name in keytab (status only)""" + managed_nfsd: bool + """ Report status of 'servers' field. + If True the number of nfsd are managed by the server. (status only)""" + rdma: bool + """ Enable or disable NFS over RDMA. Requires RDMA capable NIC """ + + @field_validator('bindip') + @classmethod + def check_bind_ip(cls, field_value: list): + """ Custom validator for IP addresses to avoid JSON serialization errors """ + all(isinstance(v, IPvAnyAddress) for v in field_value) + return field_value + + +@single_argument_args('nfs_update') +class NfsUpdateArgs(NfsEntry, metaclass=ForUpdateMetaclass): + id: Excluded = excluded_field() + managed_nfsd: Excluded = excluded_field() + v4_krb_enabled: Excluded = excluded_field() + keytab_has_nfs_spn: Excluded = excluded_field() + + +class NfsUpdateResult(BaseModel): + result: NfsEntry + + +class NfsBindipChoicesArgs(BaseModel): + pass + + +@single_argument_result +class NfsBindipChoicesResult(BaseModel): + """ Return a dictionary of IP addresses """ + model_config = ConfigDict(extra='allow') + + +class NfsShareCreate(BaseModel): + path: Dir + aliases: list[Dir] = [] + comment: str = "" + networks: list[IPvAnyNetwork] = [] + hosts: list[str] = [] + ro: bool = False + maproot_user: str | None = None + maproot_group: str | None = None + mapall_user: str | None = None + mapall_group: str | None = None + security: list[Literal["SYS", "KRB5", "KRB5I", "KRB5P"]] = [] + enabled: bool = False + + @field_validator('networks', 'hosts') + @classmethod + def check_unique(cls, field_value: list): + """ Custom validator to confirm unique entries """ + s = set() + not_unique = [] + for i, v in enumerate(field_value): + if v in s: + # verrors.add(f"{self.name}.{i}", "This value is not unique.") + not_unique.append(v) + s.add(v) + if not_unique: + raise ValueError(f"Entries must be unique, the following are not: {not_unique}") + + return field_value + + @field_validator('hosts') + @classmethod + def no_spaces_or_quotes(cls, field_value: list): + """ Custom validator for host field: No spaces or quotes """ + regex = re.compile(r'.*[\s"]') + not_valid = [] + for v in field_value: + if v is not None and regex.match(v): + not_valid.append(v) + if not_valid: + raise ValueError(f"Cannot contain spaces or quotes: {not_valid}") + + +class NfsShareEntry(NfsShareCreate): + id: int + locked: bool + + +class NfsShareCreateArgs(BaseModel): + data: NfsShareCreate + + +class NfsShareCreateResult(BaseModel): + result: NfsShareEntry + + +class NfsShareUpdateArgs(NfsShareCreate, metaclass=ForUpdateMetaclass): + id: int + + +class NfsShareUpdateResult(BaseModel): + result: NfsShareEntry + + +class NfsShareDeleteArgs(BaseModel): + id: int + + +class NfsShareDeleteResult(BaseModel): + result: Literal[True] diff --git a/src/middlewared/middlewared/plugins/nfs.py b/src/middlewared/middlewared/plugins/nfs.py index 04ce659377e03..d4c6fbaf95b47 100644 --- a/src/middlewared/middlewared/plugins/nfs.py +++ b/src/middlewared/middlewared/plugins/nfs.py @@ -5,10 +5,21 @@ import os import shutil +from middlewared.api import api_method +from middlewared.api.current import ( + NfsEntry, + NfsUpdateArgs, NfsUpdateResult, + NfsBindipChoicesArgs, NfsBindipChoicesResult, + NfsShareEntry, + NfsShareCreateArgs, NfsShareCreateResult, + NfsShareUpdateArgs, NfsShareUpdateResult, + NfsShareDeleteArgs, NfsShareDeleteResult +) from middlewared.common.listen import SystemServiceListenMultipleDelegate -from middlewared.schema import accepts, Bool, Dict, Dir, Int, IPAddr, List, Patch, returns, Str +# from middlewared.schema import accepts, Bool, Dict, Dir, Int, IPAddr, List, Patch, returns, Str from middlewared.async_validators import check_path_resides_within_volume, validate_port -from middlewared.validators import Match, NotMatch, Port, Range, IpAddress +# from middlewared.validators import Match, NotMatch, IpAddress +from middlewared.validators import IpAddress from middlewared.service import private, SharingService, SystemServiceService from middlewared.service import CallError, ValidationError, ValidationErrors import middlewared.sqlalchemy as sa @@ -16,8 +27,6 @@ from middlewared.plugins.nfs_.utils import get_domain, leftmost_has_wildcards, get_wildcard_domain from middlewared.plugins.system_dataset.utils import SYSDATASET_PATH -NFS_RDMA_DEFAULT_PORT = 20049 - # Support the nfsv4recoverydir procfs entry. This may deprecate. NFSV4_RECOVERY_DIR_PROCFS_PATH = '/proc/fs/nfsd/nfsv4recoverydir' @@ -83,26 +92,7 @@ class Config: cli_namespace = "service.nfs" role_prefix = "SHARING_NFS" - ENTRY = Dict( - 'nfs_entry', - Int('id', required=True), - Int('servers', null=True, validators=[Range(min_=1, max_=256)], required=True), - Bool('allow_nonroot', required=True), - List('protocols', items=[Str('protocol', enum=NFSProtocol.choices())], required=True), - Bool('v4_krb', required=True), - Str('v4_domain', required=True), - List('bindip', items=[IPAddr('ip')], required=True), - Int('mountd_port', null=True, validators=[Port(exclude=[NFS_RDMA_DEFAULT_PORT])], required=True), - Int('rpcstatd_port', null=True, validators=[Port(exclude=[NFS_RDMA_DEFAULT_PORT])], required=True), - Int('rpclockd_port', null=True, validators=[Port(exclude=[NFS_RDMA_DEFAULT_PORT])], required=True), - Bool('mountd_log', required=True), - Bool('statd_lockd_log', required=True), - Bool('v4_krb_enabled', required=True), - Bool('userd_manage_gids', required=True), - Bool('keytab_has_nfs_spn', required=True), - Bool('managed_nfsd', default=True), - Bool('rdma', default=False), - ) + entry = NfsEntry @private def name_to_id_conversion(self, name, name_type='user'): @@ -222,8 +212,7 @@ async def nfs_compress(self, nfs): return nfs - @accepts() - @returns(Dict(additional_attrs=True)) + @api_method(NfsBindipChoicesArgs, NfsBindipChoicesResult) async def bindip_choices(self): """ Returns ip choices for NFS service to use @@ -259,17 +248,7 @@ async def bindip(self, config): return [] - @accepts( - Patch( - 'nfs_entry', 'nfs_update', - ('rm', {'name': 'id'}), - ('rm', {'name': 'v4_krb_enabled'}), - ('rm', {'name': 'keytab_has_nfs_spn'}), - ('rm', {'name': 'managed_nfsd'}), - ('attr', {'update': True}), - ), - audit='Update NFS configuration', - ) + @api_method(NfsUpdateArgs, NfsUpdateResult, audit='Update NFS configuration') async def do_update(self, data): """ Update NFS Service Configuration. @@ -383,7 +362,10 @@ async def do_update(self, data): bindip_choices = await self.bindip_choices() for i, bindip in enumerate(new['bindip']): if bindip not in bindip_choices: - verrors.add(f'nfs_update.bindip.{i}', 'Please provide a valid ip address') + verrors.add( + f"nfs_update.bindip.{i}", + f"Cannot use {bindip}. Please provide a valid ip address." + ) if NFSProtocol.NFSv4 in new["protocols"] and new_v4_krb_enabled: """ @@ -453,39 +435,45 @@ class Config: cli_namespace = "sharing.nfs" role_prefix = "SHARING_NFS" - ENTRY = Patch( - 'sharingnfs_create', 'sharing_nfs_entry', - ('add', Int('id')), - ('add', Bool('locked')), - register=True, - ) - - @accepts( - Dict( - "sharingnfs_create", - Dir("path", required=True), - List("aliases", items=[Str("path", validators=[Match(r"^/.*")])]), - Str("comment", default=""), - List("networks", items=[IPAddr("network", network=True)], unique=True), - List( - "hosts", items=[Str("host", validators=[NotMatch( - r'.*[\s"]', explanation='Name cannot contain spaces or quotes')] - )], - unique=True - ), - Bool("ro", default=False), - Str("maproot_user", required=False, default=None, null=True), - Str("maproot_group", required=False, default=None, null=True), - Str("mapall_user", required=False, default=None, null=True), - Str("mapall_group", required=False, default=None, null=True), - List( - "security", - items=[Str("provider", enum=["SYS", "KRB5", "KRB5I", "KRB5P"])], - ), - Bool("enabled", default=True), - register=True, - strict=True, - ), + entry = NfsShareEntry + + # ENTRY = Patch( + # 'sharingnfs_create', 'sharing_nfs_entry', + # ('add', Int('id')), + # ('add', Bool('locked')), + # register=True, + # ) + + # @accepts( + # Dict( + # "sharingnfs_create", + # Dir("path", required=True), + # List("aliases", items=[Str("path", validators=[Match(r"^/.*")])]), + # Str("comment", default=""), + # List("networks", items=[IPAddr("network", network=True)], unique=True), + # List( + # "hosts", items=[Str("host", validators=[NotMatch( + # r'.*[\s"]', explanation='Name cannot contain spaces or quotes')] + # )], + # unique=True + # ), + # Bool("ro", default=False), + # Str("maproot_user", required=False, default=None, null=True), + # Str("maproot_group", required=False, default=None, null=True), + # Str("mapall_user", required=False, default=None, null=True), + # Str("mapall_group", required=False, default=None, null=True), + # List( + # "security", + # items=[Str("provider", enum=["SYS", "KRB5", "KRB5I", "KRB5P"])], + # ), + # Bool("enabled", default=True), + # register=True, + # strict=True, + # ), + # audit='NFS share create', audit_extended=lambda data: data["path"] + # ) + @api_method( + NfsShareCreateArgs, NfsShareCreateResult, audit='NFS share create', audit_extended=lambda data: data["path"] ) async def do_create(self, data): @@ -522,15 +510,19 @@ async def do_create(self, data): return await self.get_instance(data["id"]) - @accepts( - Int("id"), - Patch( - "sharingnfs_create", - "sharingnfs_update", - ("attr", {"update": True}) - ), - audit='NFS share update', - audit_callback=True, + # @accepts( + # Int("id"), + # Patch( + # "sharingnfs_create", + # "sharingnfs_update", + # ("attr", {"update": True}) + # ), + # audit='NFS share update', + # audit_callback=True, + # ) + @api_method( + NfsShareUpdateArgs, NfsShareUpdateResult, + audit='NFS share update', audit_callback=True ) async def do_update(self, audit_callback, id_, data): """ @@ -559,20 +551,25 @@ async def do_update(self, audit_callback, id_, data): return await self.get_instance(id_) - @accepts( - Int("id"), - audit='NFS share delete', - audit_callback=True, + # @accepts( + # Int("id"), + # audit='NFS share delete', + # audit_callback=True, + # ) + # @returns() + @api_method( + NfsShareDeleteArgs, NfsShareDeleteResult, + audit='NFS share delete', audit_callback=True ) - @returns() async def do_delete(self, audit_callback, id_): """ Delete NFS Share of `id`. """ nfs_share = await self.get_instance(id_) audit_callback(nfs_share['path']) - await self.middleware.call("datastore.delete", self._config.datastore, id_) + res = await self.middleware.call("datastore.delete", self._config.datastore, id_) await self._service_change("nfs", "reload") + return res @private async def validate(self, data, schema_name, verrors, old=None): diff --git a/tests/api2/test_300_nfs.py b/tests/api2/test_300_nfs.py index a98dde34b78e6..f2e227d93bff2 100644 --- a/tests/api2/test_300_nfs.py +++ b/tests/api2/test_300_nfs.py @@ -631,12 +631,10 @@ def test_service_update(self, start_nfs, nfsd, cores, expected): # Test making change to non-'server' setting does not change managed_nfsd assert call("nfs.config")['managed_nfsd'] == expected['managed'] else: - with pytest.raises(ValidationErrors) as ve: + with pytest.raises(ValidationErrors, match="Input should be"): assert call("nfs.config")['managed_nfsd'] == expected['managed'] call("nfs.update", {"servers": nfsd}) - assert ve.value.errors == [ValidationError('nfs_update.servers', 'Should be between 1 and 256', 22)] - def test_share_update(self, start_nfs, nfs_dataset_and_share): """ Test changing the security and enabled fields @@ -1305,9 +1303,24 @@ def test_service_udp(self, start_nfs): assert s.get('nfsd', {}).get('udp') is None, s @pytest.mark.parametrize('test_port', [ - pp([["mountd", 618, None], ["rpcstatd", 871, None], ["rpclockd", 32803, None]], id="valid ports"), - pp([["rpcstatd", -21, 0], ["rpclockd", 328031, 0]], id="invalid ports"), - pp([["mountd", 20049, 1]], id="excluded ports"), + pp( + [ + ["mountd", 618, None], + ["rpcstatd", 871, None], + ["rpclockd", 32803, None] + ], id="valid ports" + ), + pp( + [ + ["rpcstatd", -21, "Input should be greater than"], + ["rpclockd", 328031, "Input should be less than"] + ], id="invalid ports" + ), + pp( + [ + ["mountd", 20049, "reserved for internal use"] + ], id="excluded ports" + ), ]) def test_service_ports(self, start_nfs, test_port): """ @@ -1324,9 +1337,6 @@ def test_service_ports(self, start_nfs, test_port): value = 1 err = 2 - # Error message snippets - errmsg = ["Should be between", "reserved for internal use"] - # Test ports for port in test_port: port_name = port[name] + "_port" @@ -1334,10 +1344,8 @@ def test_service_ports(self, start_nfs, test_port): nfs_conf = call("nfs.update", {port_name: port[value]}) assert nfs_conf[port_name] == port[value] else: - with pytest.raises(ValidationErrors) as ve: + with pytest.raises(ValidationErrors, match=port[err]): nfs_conf = call("nfs.update", {port_name: port[value]}) - errStr = str(ve.value.errors[0]) - assert errmsg[port[err]] in errStr # Compare DB with setting in /etc/nfs.conf.d/local.conf with nfs_config() as config_db: @@ -1383,44 +1391,69 @@ def test_runtime_debug(self, start_nfs): res = call('nfs.get_debug') assert res == disabled - def test_bind_ip(self, start_nfs): + @pytest.mark.parametrize("TestType,param,errmsg", [ + pp("basic", [], None, id="basic settings"), + pp("NotInChoice", ["1.2.3.4"], "Cannot use", id="Not in choices"), + pp("NotValidIP", ["a.b.c.d"], "does not appear", id="Not a valid IP"), + pp("HalfValid-1", ["", "8.8.8.8"], "Cannot use", id="2nd entry not in choices"), + pp("HalfValid-2", ["", "ixsystems.com"], "does not appear", id="2nd entry not valid IP"), + pp("HalfValid-3", ["", None], "Input should be", id="2nd entry is None") + ]) + def test_bind_ip(self, start_nfs, TestType, param, errmsg): ''' This test requires a static IP address - * Test the private nfs.bindip call - * Test the actual bindip config setting - - Confirm setting in conf files - - Confirm service on IP address + * Success testing: + - Test the private nfs.bindip call + - Test the actual bindip config setting + o Confirm setting in conf files + o Confirm service on IP address + * Failure testing: + - Valid IP, but does not match choices + - Invalid IP + - Two entries, one valid the other not ''' assert start_nfs is True - # Multiple restarts cause systemd failures. Reset the systemd counters. - reset_svcs("nfs-idmapd nfs-mountd nfs-server rpcbind rpc-statd") + if TestType == "basic": + # Multiple restarts cause systemd failures. Reset the systemd counters. + reset_svcs("nfs-idmapd nfs-mountd nfs-server rpcbind rpc-statd") - choices = call("nfs.bindip_choices") - assert truenas_server.ip in choices + choices = call("nfs.bindip_choices") + assert truenas_server.ip in choices - call("nfs.bindip", {"bindip": [truenas_server.ip]}) - call("nfs.bindip", {"bindip": []}) + call("nfs.bindip", {"bindip": [truenas_server.ip]}) + call("nfs.bindip", {"bindip": []}) - # Test config with bindip. Use choices from above - # TODO: check with 'nmap -sT ' from the runner. - with nfs_config() as db_conf: + # Test config with bindip. Use choices from above + # TODO: check with 'nmap -sT ' from the runner. + with nfs_config() as db_conf: - # Should have no bindip setting - nfs_conf = parse_server_config() - rpc_conf = parse_rpcbind_config() - assert db_conf['bindip'] == [] - assert nfs_conf['nfsd'].get('host') is None - assert rpc_conf.get('-h') is None + # Should have no bindip setting + nfs_conf = parse_server_config() + rpc_conf = parse_rpcbind_config() + assert db_conf['bindip'] == [] + assert nfs_conf['nfsd'].get('host') is None + assert rpc_conf.get('-h') is None - # Set bindip - call("nfs.update", {"bindip": [truenas_server.ip]}) + # Set bindip + call("nfs.update", {"bindip": [truenas_server.ip]}) - # Confirm we see it in the nfs and rpc conf files - nfs_conf = parse_server_config() - rpc_conf = parse_rpcbind_config() - assert truenas_server.ip in nfs_conf['nfsd'].get('host'), f"nfs_conf = {nfs_conf}" - assert truenas_server.ip in rpc_conf.get('-h'), f"rpc_conf = {rpc_conf}" + # Confirm we see it in the nfs and rpc conf files + nfs_conf = parse_server_config() + rpc_conf = parse_rpcbind_config() + assert truenas_server.ip in nfs_conf['nfsd'].get('host'), f"nfs_conf = {nfs_conf}" + assert truenas_server.ip in rpc_conf.get('-h'), f"rpc_conf = {rpc_conf}" + else: + # None of these should make it to the config + if TestType.startswith("HalfValid"): + param[0] = truenas_server.ip + + with pytest.raises((ValueError, ValidationErrors)) as ve: + call("nfs.update", {"bindip": param}) + if ve.typename == "ValueError": + assert errmsg in str(ve.value) + else: + assert errmsg in str(ve.value.errors[0]) def test_v4_domain(self, start_nfs): '''